Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error during signin when vue-router currentRoute contains a circular reference. #75

Open
leokolezhuk opened this issue Jul 18, 2022 · 4 comments

Comments

@leokolezhuk
Copy link

If the stored state ends up containing an object with a circular reference, an error occurs during the _signinStart of the oidc-client.
This results in the signin process not succeeding.

The error comes from the oidc-client trying to store the state json in localStorage here.

Error:
TypeError: Converting circular structure to JSON
--> starting at object with constructor 'Or'
| property '$options' -> object with constructor 'Object'
| property 'router' -> object with constructor 'we'
--- property 'app' closes the circle
at JSON.stringify ()
at e.toStorageString (chunk-vendors.08958c8f.js:formatted:26453:30)
at chunk-vendors.08958c8f.js:formatted:26078:73

I am not exactly sure under what circumstances the circular reference is created in the Vue router that causes it, but I wonder if it is necessary to use the entire object for the state snapshots:
https://github.com/soukoku/vue-oidc-client/blob/master/vue3/src/vue-oidc-client.ts#L332
https://github.com/soukoku/vue-oidc-client/blob/master/vue3/src/vue-oidc-client.ts#L276

I think ultimately the full path of the route should already be enough to store here, making it more robust to what the vue-router does with the currentRoute object.

@SeanLMcCullough
Copy link

I've come across a similar issue, however only as a result of signInIfNecessary which is invoked by a number of internal event
handlers. My symptom was after session expiry, pop-ups would flash on-screen momentarily before being closed immediately, with the error message Error after preparing navigator, closing navigator window.

I am trying to have a functionality to allow the user to log back in after an expired session using popup sign-in. I want the user to click a button for the pop-up to appear. The inbuilt events however call signInIfNecessary, which when on a protected route, will automatically open the pop-up window, which is not desired.

signInIfNecessary encodes the current route, but does not do anything with it (as far as I can tell) after the fact. currentRoute happens to contain a circular reference in my implementation. This throws a similar error, but is caught internally and not logged (in the catch of signInReal as part of signInIfNecessary). I had to use a local copy of the library with added logging to see the error detail about the circular reference.

I suspect this might be due to a change in vue-router, if a self-reference has been added to the route objects. I am using vue-router 3.5.3.

Since the internal event handlers call this method, I'm unsure if there's any way to remove/control the behaviour of the handlers to override them.

After testing locally by removing the currentRoute related code from signInIfNecessary, the pop-up works. However, I don't want the pop-up to show until the user decides they want to log back in again and continue their active session.

I suspect a fix/change for what I'm looking for would involved;

  • Only encoding into state the parts of the route that are needed for navigation to prevent the circular reference
  • Having some way to control the behaviour of signInIfNecessary to disable automatic re-log-in if desired
  • Being able to trigger pop-up sign-in while retaining redirect sign-in for my default log-in process

@fezebr
Copy link

fezebr commented Nov 28, 2022

@leokolezhuk @SeanLMcCullough
do you have any update for this issue?

@leokolezhuk
Copy link
Author

@fezebr, I have made a patch (for my project) that sets a different state in those two lines of code mentioned earlier.
Instead of using the entire Route object as a state, I am using the fullPath property of the Route state.

@fezebr
Copy link

fezebr commented Dec 10, 2022

@leokolezhuk tnx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants