Why do we have a separate ISessionInfo object?
When reading the docs, I saw a weird
session.info.isLoggedIn
What's the rationale for having this ghost object, instead of:
-
session.isLoggedIn? -
session.webId? -
session.sessionId?
The notion of info objects is not something I see in interfaces; I think it may point to a design flaw.
I also want to point out that, even if there is some need for such an info object (read-only session, maybe?), that one does not exclude the other. We could perfectly have session.isLoggedIn and other.isLoggedIn. The session.info.isLoggedIn just reads like a flaw.
This object was initially introduced to match the return type of handleincomingRedirect, which returns not a Session, but a SessionInfo. However, I don't see any reason why the Session itself should not implement the ISessionInfo interface, and expose the isLoggedin, webId and sessionId properties instead of exposing an intermediary object that does.
Thanks, @NSeydoux!
handleincomingRedirect, which returns not aSession, but aSessionInfo
Can we iterate a bit more on this? What exactly does handleincomingRedirect do and why does it return this specific data structure?
In any case, SessionInfo (at least the Info bit of it) seems to be a misnomer.
And there is also SessionInfoManager; should probably be SessionManager or so.
I don't see any reason why the
Sessionitself should not implement theISessionInfointerface
Super, tracking in https://github.com/inrupt/solid-client-authn-js/issues/590
handleincomingRedirect is the function that completes the login: after the user is redirected from the OIDC provider, this reads the authorization code from the query params, retrieves the code verifier from storage, and makes the back channel request to the token endopoint to exchange the code for the ID, access and refresh tokens. Then, handleIncomingRedirect builds the authenticated fetch function holding the tokens in its closure, and binds this fetch to the Session. Therefore, only when handleIncomingRedirect returns is the login process complete. And for convenience, session.handleIncomingRedirect(someUrl) returns a SessionInfo object.
For a non-breaking change, what could be done is to make it so that handleIncomingRedirect returns the session it is bound to, which would implement the ISessionInfo interface. This way, libraries that use the return value would have no change to make, but the info could be deprecated easily.
For a non-breaking change, what could be done is to make it so that
handleIncomingRedirectreturns the session it is bound to, which would implement theISessionInfointerface. This way, libraries that use the return value would have no change to make, but theinfocould be deprecated easily.
Exactly. I think this is what we intended all along in the initial design; there was no ISessionInfo object.
Updated the steps in https://github.com/inrupt/solid-client-authn-js/issues/590 to reflect this.