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

Add using of solidAuth.trackSession() for check, if User is SignedIn already #263

Closed
wants to merge 3 commits into from

Conversation

smalinin
Copy link
Contributor

Recreate #256 for new sources in master branch

Sync with linkeddata/dokieli
@csarven
Copy link
Member

csarven commented Nov 19, 2018

Thanks!

@smalinin
Copy link
Contributor Author

The solid-auth-client that method .trackSession(), that could be used for get state of OIDC session, so we could check, if we LoggedIn already via OIDC(may be in another browser tab with same location.origin or etc).

@csarven
Copy link
Member

csarven commented Nov 19, 2018

It sounds like it might resolve potential issues with token expiry (see #257 (comment) and #255 (comment) ) .. What do you think?

@csarven
Copy link
Member

csarven commented Nov 28, 2018

I got a chance to look into this.. and solid-auth-client's trackSession doesn't appear to work as this PR intends to. It seems to only "invoke the callback with the current session, and notify you of any changes to the login status." ( https://github.com/solid/solid-auth-client#getting-the-current-user )

I've also noticed that the way trackSession is in place conflicts with the toggling of the user signin/out buttons.

We need to refactor a bit if we want to use trackSession, and I prefer to do that from scratch because the branches are already out of sync

@csarven csarven closed this Nov 28, 2018
@smalinin
Copy link
Contributor Author

@csarven Ok, I will rewrite code with new function solid.auth.currentSession

@csarven
Copy link
Member

csarven commented Nov 28, 2018

Hang on.. do we actually need it? The toggling works fine. As far as I can see, the session is not shared across tabs/windows.. am I overlooking something?

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

Successfully merging this pull request may close these issues.

None yet

2 participants