nodeSolidServer / solid-auth-oidc

OpenID Connect authentication support for the solid-client library
MIT License
10 stars 2 forks source link

Ability to "load" a client instance from persisted state #8

Open dan-f opened 7 years ago

dan-f commented 7 years ago

There should be a way to instantiate a client for a particular identity provider from the cache. This is useful because it prevents extra network requests to the identity provider when it's already saved ID/access tokens.

dmitrizagidulin commented 7 years ago

Thanks. Will implement this next.

dmitrizagidulin commented 7 years ago

Quick question - would you like this method to register a client if no cached copy exists? Or just resolve to null if no stored client is found?

dan-f commented 7 years ago

I'd rather there not be side effects. Since this client is designed to manage one session at a time I'm looking for something like:

const oidcClient = new Client('https://example.com', options) // loads any ID/Access tokens from storage
oidcClient.currentUser() // null if no stored user, webId otherwise
dmitrizagidulin commented 7 years ago

No prob.

So this sounds like this is orthogonal to the rp client? As in, instantiating the client will load the last webId + tokens that were stored for that provider, but doesn't concern itself with the rp client (until it's needed by login())?

dan-f commented 7 years ago

I was using the term client to refer to the interface this library exposes. I'm considering the rp client lib an implementation detail in that context.

dmitrizagidulin commented 7 years ago

Got it. Couple more questions. One, would it be ok if the loading of user was performed by a factory method rather than constructor? Something like:

const oidcClient = Client.for('https://example.com', options) // loads any ID/Access tokens from storage
oidcClient.currentUser() // null if no stored user, webId otherwise

Two, just to double check - should calling oidcClient.logout() clear the stored webId & tokens for that provider? (So that the next time currentUser() is called, it would return null).

dan-f commented 7 years ago

One, would it be ok if the loading of user was performed by a factory method rather than constructor?

Yeah totally.

Two, just to double check - should calling oidcClient.logout() clear the stored webId & tokens for that provider? (So that the next time currentUser() is called, it would return null).

Yep

dmitrizagidulin commented 7 years ago

Would it also make sense to store the last selected provider uri (in local storage)? So that the app doesn't have to remember that https://example.com was it?

This would give something like:

const oidcClient = Client.for(options)
// ^ loads the last selected provider, https://example.com if available, and loads the webId + tokens for it

And maybe make providerUri as one of the options?

dan-f commented 7 years ago

At the very least, I'd like to see the concept of "current user" for a developer-specified IDP URL. Currently it relies on the current window URL - https://github.com/solid/solid-auth-oidc/blob/master/src/index.js#L83

It'd also be really nice to index the last used IDP by app URL. That way you can pick up the last used user session for the current app.

dan-f commented 7 years ago

I think we're saying the same thing

dmitrizagidulin commented 7 years ago

Ok, cool!