Closed RubenVerborgh closed 2 years ago
Observations regarding AuthFetcher
:
void
, why not a Session
?fetch
is a function that can be passed around; this is good and crucial.AuthFetcher
and a Session
?
getSessions
which sessions exactly do we get back?getSession
do we mean getDefaultSession
?fetch
use?uniqueLogin
I find very confusing. Is this the function that gives the default session? From the documentation, it seems actually to do the opposite of what I'd expect.AuthFetcher
has many Session
s, is it addSession
or createSession
, conceptually?uniqueLogin
not be the function that creates a unique session, i.e., closes all others onLogin
.handleRedirect
seems out of place. Not sure if this should be exposed; it's internal mechanics, specific to OIDC.Observations regarding ISolidSession
:
localUserId
is weird; not sure it belongs here. What problem are we solving?
Map
on the level of AuthFetcher
loggedIn
so there are sessions where I'm not logged in? Or they expire? Because this conflicts with the (incorrect?) notion above that logged in creates a session and logging out destroys it.webId?
with a question mark is weirdwebId
being a string
as opposed to a URL
?state
? Do we really need to expose it? Is it OIDC-specific?neededAction
is really weird. We likely need to rethink this. What problem are we solving?logout
, so a session can "log out" itself? But so can AuthFetcher
. We need a clear single point of responsibility.fetch
idemThe last two points in particular are confusing; hope they will be answered with a point above regarding the relation between AuthFetcher
and Session
.
Observations regarding ILoginInputOptions
:
redirect
and why is it a string? Is it OIDC-specific? Should it be exposed?popUpRedirectPath
is oddly specific as wellstate
ah okay, so this is just passed on as-is… now I get state
better in Session
, but I think it is wrongly named. Also, I don't think it belongs on Session
; it should be something you need once, but not repeatedly.doNotAutoRedirect
is weirdly namedObservations regarding the public API:
AuthFetcher
. I understand the rationale behind that. However, I wonder whether we actually, as a default
object, want to export the globalAuthFetcher
. So we can just pass around that fetcher object. And maybe, for simplicity, we can also export its fetch
. But all more specific functions, shall we just keep them as methods? Then we can also keep the number of exports down.customAuthFetcher
. I think we should just export AuthFetcher
so people can new AuthFetcher()
.
AuthFetcher
and Session
.So summarizing, I'd suggest to export three things: default
being the default AuthFetcher
instance, fetch
being its fetch
method, and the AuthFetcher
constructor.
@jaxoncreed Just making sure that this one doesn’t get forgotten. Is there any newer version that I need to adjust the review to, or would you be able to discuss the remarks above here?
No newer version of the auth fetcher. My focus has been on updating NSS to support the new token type. But I'll ping you if there's a new interface change.
Okay, thanks. But given that there is no interface change yet then, let's discuss the comments above at some point before they happen (and definitely before release).
login returns a void, why not a Session? Good catch. I assume you got this from https://github.com/inrupt/solid-auth-fetcher/tree/91bc153b88968674f81d02838a7aada85732e229#authfetcher which is the one place in the documentation that wasn't properly updated. It does return a session.
fetch is a function that can be passed around; this is good and crucial.
Noice
What is the exact relationship between an AuthFetcher and a Session?
AuthFetcher contains all methods to do anything in the library. A session represents only one login instance. If you perform
fetch
with auth fetcher it will fetch for the "global" user. Butfetch
in a Session applies to whatever that user's session is.one to many?
Yep getSessions which sessions exactly do we get back? It will return all sessions that are currently logged in on this client. Both global and non-global. getSession do we mean getDefaultSession? Yeah. I've been calling it the "global" session, but "default" works too. The reason this isn't named "getGlobalSession" is because most users are going to be using "global." Non-global use cases will be out of the norm, so the naming for anything dealing with the non-global use case is distinct (ie
loginUnique
), but the naming for global things is not.Which session(s) will fetch use?
The global session
uniqueLogin I find very confusing. Is this the function that gives the default session? From the documentation, it seems actually to do the opposite of what I'd expect.
Pat doesn't like this either. It allows you to log in a new user without overwriting the global user.
If AuthFetcher has many Sessions, is it addSession or createSession, conceptually?
Yeah that could be a way of thinking about it
Should uniqueLogin not be the function that creates a unique session, i.e., closes all others
Not sure what's being asked here. In my mind "creating a unique" session would not close all others. It would simply create a new session.
What is a login compared to a session? Is a login the action that creates a session? Is logout the method that destroys a login?
Yep a login creates a new session. Logout destroys it.
Weird that there is no onLogin.
It's handled by the
onSession
method since login creates a session.handleRedirect seems out of place. Not sure if this should be exposed; it's internal mechanics, specific to OIDC.
It needs to be exposed for the server use case. It's automatically trigged in the web browser, but on the server the developer must manually link it to a route (see the server use case https://github.com/inrupt/solid-auth-fetcher#for-use-on-the-server)
localUserId is weird; not sure it belongs here. What problem are we solving?
There must be some kind of ID to identify the session. Because not all sessions are logged in, the webId cannot be used.
I think this possibly needs to be a Map on the level of AuthFetcher
What would be the key of that map? If the key is just the
localUserId
, then we could certainly have a map, but why not also put it in the session as well?loggedIn so there are sessions where I'm not logged in? Or they expire? Because this conflicts with the (incorrect?) notion above that logged in creates a session and logging out destroys it.
I can see the confusion in naming here. I think it comes from trying to keep the API similar to Auth Client. Login creates a session, but not necessarily a logged-in session. Sometimes you need to do a "needed action" to continue with the login.
webId? with a question mark is weird
You might not have a webId since not all sessions are logged in.
What are the pros/cons of webId being a string as opposed to a URL?
I've been passing around URLs internally but have always converted to strings before exposing it externally. I thought this was best practice as strings are more universal. People can convert them into URLs if they want to.
What exactly is state? Do we really need to expose it? Is it OIDC-specific?
It is OIDC specific, but helpful. Because your execution environment may be interrupted, you might want to pass some state along (It's an option in the login method). "state" in the session is where you can recover your lost state.
neededAction is really weird. We likely need to rethink this. What problem are we solving?
Sometimes when you log in (especially on the server), the library can't do an action for you (like a redirect), so you need to do it yourself.
logout, so a session can "log out" itself? But so can AuthFetcher. We need a clear single point of responsibility. fetch idem
logout
andfetch
on the AuthFetcher are simply shortcuts for the global user.
What is redirect and why is it a string? Is it OIDC-specific? Should it be exposed?
Redirect is the URL to which you should be redirected back to once you're done with the login process. It should definitely be exposed so developers can set custom routes. If we want to it could be made optional and by default made to be the current URL.
popUpRedirectPath is oddly specific as well
I implemented Auth Fetcher to not require the special PopUp server that Auth-Client required. However, because of that, you're going to want to implement it within your app. This is the place to put that route.
state ah okay, so this is just passed on as-is… now I get state better in Session, but I think it is wrongly named. Also, I don't think it belongs on Session; it should be something you need once, but not repeatedly.
It's called
state
in OIDC, so I think people will be used to it. It's in session because you get the session on the "onSession" call. Alternatively, It could be another parameter that's passed into the function.doNotAutoRedirect is weirdly named
How about
disableAutoRedirect
So it seems that we are re-exporting every member method of AuthFetcher. I understand the rationale behind that. However, I wonder whether we actually, as a default object, want to export the globalAuthFetcher. So we can just pass around that fetcher object. And maybe, for simplicity, we can also export its fetch. But all more specific functions, shall we just keep them as methods? Then we can also keep the number of exports down.
I don't think the goal should be to keep the number of exports down. But, I think there still is a problem with the current way things are exported. Using a global AuthFetcher object means that you can't tree shake anything since all files are eventually imported into AuthFetcher. This isn't good for a case where someone wants to use "fetch" but not "login"
Also I just see it as more elegant to be able to do import { login } from "solid-auth-fetcher"
rather than
import authFetcher from "solid-auth-fetcher"
authFetcher.login({})
I don't like customAuthFetcher. I think we should just export AuthFetcher so people can new AuthFetcher().
If we just export AuthFetcher, people will need to inject a loginHandler, redirectHandler, logoutHandler, sessionCreator, authenticatedFetcher, and environmentDetector. If you wanted to do this, you could just
import AuthFetcher from "solid-auth-fetcher/dist/AuthFetcher.js"
. But, most people don't care about those injections. They only want to set a custom storage system. So "customAuthFetcher" provides the convenience by injecting your custom storage system in the right place.The necessity of which also again depends on the relation between AuthFetcher and Session.
Okay
@RubenVerborgh @pmcb55 All in all good comments. I see where some confusion exists in naming (like how the login method doesn't necessarily log you in). I could take another pass at it next sprint!
Thanks for all the effort! Will study in detail, and split some points up in dedicated issues if necessary.
If you perform
fetch
with auth fetcher it will fetch for the "global" user.
We might actually want to reserve that method for some more intelligent binding. For instance: when fetching from a.com
, I am user A; b.com
, user B.
Non-global use cases will be out of the norm
I'd be careful there. I agree we need to hide multiple sessions from devs who don't need to see them, but they should not make assumptions about this. The notion of "global" (or "primary" etc.) I don't think is the right one.
I also observe there might be some inconsistencies regarding what is a session. For instance, got getSessions
I read that:
It will return all sessions that are currently logged in on this client.
So sessions that are not logged in are not returned? Are they even sessions then? Created an issue to zoom out on this: https://github.com/inrupt/solid-auth-fetcher/issues/58
I don't think the goal should be to keep the number of exports down.
Not in itself; but as part of good API design, we should think carefully about every member we export.
Using a global AuthFetcher object means that you can't tree shake anything since all files are eventually imported into AuthFetcher. This isn't good for a case where someone wants to use "fetch" but not "login"
Well there will be no fetch
without login
, so something will have to ship the login
to the browser anyway. I.e., a component that only uses fetch
will eventually be webpacked together with something that uses login
, or there is no reason to use solid-auth-fetch fetch
at all, and it can just be changed to window.fetch
.
So this might be premature optimization.
If we just export AuthFetcher, people will need to inject a loginHandler, redirectHandler, logoutHandler, sessionCreator, authenticatedFetcher, and environmentDetector.
Not really, just fill it with sensible defaults.
It's not because the internal AuthFetcher
accepts a lot of dependencies, that the external API cannot have those prefilled.
I also noticed OIDC-specific stuff leaking out, and I don't think that's a good thing.
handleRedirect seems out of place. Not sure if this should be exposed; it's internal mechanics, specific to OIDC.
It needs to be exposed for the server use case. It's automatically trigged in the web browser, but on the server the developer must manually link it to a route
But does it need to be exposed there?
I would expose it somewhere else. Like OidcServerAdapter
or so.
Which is a component that only the sever needs to care about.
Redirect is the URL to which you should be redirected back to once you're done with the login process. It should definitely be exposed so developers can set custom routes. If we want to it could be made optional and by default made to be the current URL.
Yes to default, but also to renaming it in a non-OIDC centric way. It's not a redirect from the point of the client, it's the afterLogin
or so.
It's called state in OIDC, so I think people will be used to it.
Our intended audience are not people who do OIDC. And we're using the word in a context where there's no explicit suggestion of OIDC.
popUpRedirectPath is oddly specific as well
I implemented Auth Fetcher to not require the special PopUp server that Auth-Client required.
A different name would help already.
Just to say that I've only just caught up now on all the comments from Ruben, and responses from Jackson. In summary all I'll say at this point is that I totally agree with all of Ruben's comments. I do think the API needs another full sweep to clear up it's structure and naming, and I think the README needs a good bit more prose to explain a lot of the subtleties (especially the 'global user' vs 'uniqueLogin', non-logged-in sessions, 'neededAction' (e.g. renamed to something like 'postLogin()' instead)). I know Jackson wanted to keep the README short and concise, which indeed would be ideal, but first I think we need a longer version that clarifies the overall operation, and the intent of each function, and then hopefully later (once the API is well structured and more intuitive), a shorter, more concise README will come out of that.
Actually, just pondering on the API structure question - would it make sense to split IAuthFetcher into an IAuthFetcher
and an IAuthFetcherMutliSession
or IAuthFetcherManager
. The Multi-Session case is an 'advanced' usage, so forcing it into a separate, explicit interface seems appropriate, rather than it's concepts polluting the common IAuthFetcher
.
Can onSession
be called onLoginStarted
, and then we can add an onLoginComplete
?
// Main interface object
interface IAuthFetcher {
login: (options: {}) => Promise<Session>
fetch: (url: string | URL, options: {}) => Promise<Response>
logout: () => Promise<void>
onLoginStarted: (callback: (session: Session) => any) => void
onLoginComplete: (callback: (session: Session) => any) => void
// I don't think we need an 'onLogoutStarted' and 'onLogoutCompleted'.
onLogout: (callback: (session: Session) => any) => void
// I'm still uncomfortable with this convenience method, as I feel
// it should be up to the developer to manage this 'global variable'
// themselves...
getDefaultSession: () => Promise<Session>
}
interface IAuthFetcherManager {
getSessions: () => Promise<Session[]>
// These functions operate on all sessions managed by this Manager.
onLoginStarted: (callback: (session: Session) => any) => void
onLoginComplete: (callback: (session: Session) => any) => void
onLogout: (callback: (session: Session) => any) => void
}
interface IOidcAdapter {
handleRedirect: (url: string | URL) => Promise<void>
}
^ I think the splitting is a good suggestion.
Reminder that this split is the external API, which might be different from how we do it internally (where such a split would likely not be a good idea).
@RubenVerborgh @pmcb55 Here's the new proposed docs. Tell me what you think:
The Solid Auth Fetcher Ecosystem is comprised of a pair of libraries designed to make authenticated fetch requests to Solid-compatible servers.
There are two auth fetcher libraries you can use: solid-auth-fetcher
and solid-auth-fetcher-sessions
.
solid-auth-fetcher
Solid-auth-fetcher is designed to provide a streamlined experience to implementing authentication in apps that are focused on one user. You'd use solid-auth-fetcher to
solid-auth-fetcher-sessions
Solid-auth-fetcher-sessions allows you to manage multiple sessions for multiple logged in users. You'd use solid-auth-fetcher-sessions to
git clone https://github.com/inrupt/solid-auth-fetcher.git
cd solid-auth-fetcher
npm i
npm run bootstrap-examples
# Run each example
npm run dev-bundle
npm run dev-script
npm run dev-server
npm run dev-ios
npm run dev-android
At this point, a test application will be running on port 3001
and a test solid server will be running on port 9001
Solid-auth-fetcher provides a streamlined experience for authenticating with and making fetch requests to Solid-compatible servers.
Solid-auth-fetcher works best in cases where one user is logged into an application at a time. If your use case requires managing multiple logged-in users at once (for example, a server-based app) see solid-auth-fetcher-sessions.
npm install @inrupt/solid-auth-fetcher
// TODO: complete tutorial once Ruben and Pat approve of the API
In the browser via the script
tag:
<script src="/path/to/solidAuthFetcher.bundle.js"></script>
</script>
solidAuthFetcher.getSession().then((session) => console.log(session))
</script>
Using import
import { getSession } from "@inrupt/solid-auth-fetcher"
getSession().then((session) => console.log(session))
Using require
const solidAuthFetcher = require("@inrupt/solid-auth-fetcher")
solidAuthFetcher.getSession().then((session) => console.log(session))
Kick off the login process for the user:
import { login } from '@inrupt/solid-auth-fetcher';
login({
issuer: 'https://identityprovider.com',
redirectUrl: 'https://mysite.com/redirect'
}).then((neededAction) => {})
Options: | Field Name | Required? | Type | Description | Default |
---|---|---|---|---|---|
issuer |
Yes, unless webId is provided |
String or URL | The user's issuer | undefined | |
returnToUrl |
Yes | String or URL | The URI within this application that the user should be redirected to after successful login. This can be either a web URL or a mobile URL scheme | undefined | |
clientId |
Only if you don't want to do dynamic registration | String or URL | The id of a statically registered application. | undefined | |
popUp |
No | Boolean | If true, the login process will initiate via a popup. This only works on web clients. | false | |
doNotAutoRedirect |
No | Boolean | If true, the browser will not auto redirect. Note that auto redirect only happens if Solid-Auth-Fetcher is running in the browser | false |
Send an HTTP request to a Solid Pod:
import { fetch } from '@inrupt/solid-auth-fetcher';
fetch('https://example.com/resource', {
method: 'POST',
headers: {
"Content-Type": "text/plain"
},
body: 'What a cool body!'
}).then((result) => {})
Fetch follows the WHATWG Fetch Standard.
Log the user out:
import { logout } from '@inrupt/solid-auth-fetcher';
logout().then(() => {})
Retrieve the user's session or provides null if the user is not logged in:
import { getSession } from '@inrupt/solid-auth-fetcher';
await getSession().then((session) => {})
Register a callback function to be called when a user completes login:
import { onLogin } from '@inrupt/solid-auth-fetcher'
onLogin((session) => {
console.log(session.webId)
})
Register a callback function to be called when a user logs out:
import { onLogout } from '@inrupt/solid-auth-fetcher'
onLogout(() => {})
A method for configuring a custom backend for solid-auth-fetcher. This will return an object containing all functions in the API (login
, fetch
, logout
, getSession
, onLogin
, onLogout
) configured with your custom input.
Options: | Field Name | Required? | Type | Description | Default |
---|---|---|---|---|---|
storage |
No | IStorage | A storage object to help you access your custom storage. | Local Storage |
If you'd like to configure custom storage, provide an object following this interface:
{
get: (key: string) => Promise<string | null>;
set: (key: string, value: string) => Promise<void>;
delete: (key: string) => Promise<void>;
}
The session object contains information about the logged in user.
{
webId: String // The user's WebID.
}
A needed action object describes an action you must implement. It is returned by the login function in the case that you are not operating in the web browser. If you are operating in the web browser, no additional action needs to be taken.
There are a few needed actions that can be returned:
If a NeededInaction
is returned by login
, no action is needed. A NeededInaction
looks like:
{
actionType: "inaction"
}
If a NeededRedirect
is returned by login
, you must redirect the user to the provided url. A NeededRedirect
looks like:
{
actionType: "redirect";
redirectUrl: "https://identityprovider.com/auth?client_id=coolApp";
}
The oidc adapter contains functions that should be called to handle edge cases that can't be handled automatically by solid-auth-fetcher
{
handleRedirect: (url: string | URL) => Promise<void>,
handlePopUpRedirect: (url: string | URL) => Promise<void>
}
Handles redirects as a part of the OIDC process. Servers using solid-auth-fetcher must manually call this method on redirect, but is done automatically on web and mobile.
import { oidcAdapter } from 'solid-auth-fetcher'
oidcAdapter.handleRedirect(window.location.href)
Handles the redirect of a popup window. This function should be called when a request to your redirectUrl
is made after a login
function has been called with popup=true
. (See example in the tutorial)
import { oidcAdapter } from 'solid-auth-fetcher'
oidcAdapter.handlePopUpRedirect(window.location.href)
Solid-auth-fetcher-sessions provides a flexible interface for authenticating with and making fetch requests to Solid-compatible servers.
Solid-auth-fetcher-sessions exposes a wide array of tools to adapt to any kind of environment. If your app is designed only to have a single user logged in at a time, consider using the streamlined interface in solid-auth-fetcher.
npm install @inrupt/solid-auth-fetcher-sessions
// TODO: complete tutorial once Ruben and Pat approve of the API
<script src="/path/to/solidAuthFetcherSessions.bundle.js"></script>
</script>
solidAuthFetcher.getSession().then((session) => console.log(session))
</script>
Using import
import { getSession } from "@inrupt/solid-auth-fetcher-sessions"
getSession().then((session) => console.log(session))
Using require
const solidAuthFetcher = require("@inrupt/solid-auth-fetcher-sessions")
solidAuthFetcher.getSession().then((session) => console.log(session))
A class that manages all sessions in your application.
Creates a SessionManager object.
import { SessionManager } from "@inrupt/solid-auth-fetcher-sessions";
import customStorage from "./myCustomStorage"
const sessionManager = new SessionManager({
storage: customStorage
})
Options: | Field Name | Required? | Type | Description | Default |
---|---|---|---|---|---|
storage |
No | IStorage | A storage object to help you access your custom storage. | In Memory Storage |
Returns all sessions currently in the session manager.
sessionManager.getSessions().then(sessions => {})
Creates a new session and adds it to the session manager. If a session id is not provided, a random UUID will be assigned as the session id. If the session of the provided id already exists, that session will be returned.
const session = sessionManager.createSession("mySessionid")
Returns a session of the given id. If the session does not exist in storage, null will be returned.
sessionManager.getSessionById("mySessionId").then(session => {})
Registers a callback to be called when a session is logged in.
sessionManager.onLogin((session) => {})
Registers a callback to be called when a session is logged out.
sessionManager.onLogout((session) => {})
Part of the OIDC flow is a redirect. If solid-auth-fetcher-sessions is deployed in the web-browser, this redirect is handled automatically, but on the server, it must be handled manually. Use the handleOidcRedirect
at the redirect route for your app.
app.get("/redirect", async (req, res) => {
await sessionManager.handleOidcRedirect(req.url)
res.redirect("/home");
})
Handles the redirect of a popup window. This function should be called when a request to your redirectUrl
is made after a login
function has been called with popup=true
. (See example in the tutorial).
This should only be used for in a web browser.
sessionManager.handleOidcPopUpRedirect(window.location.href)
Creates a Session object. If sessionId is not present, a random UUID will be generated.
Warning: you should either use sessionManager.createSession() or call session.init() directly after calling the constructor.
import { Session } from "@inrupt/solid-auth-fetcher-sessions"
const session = new Session(sessionManager, "mySessionId")
session.init().then(() => {})
Initializes the session by saving it to storage if a session with the given id does not already exist, or by syncing it with the session in storage if it already exists.
session.init().then(() => {})
Kick off the login process for this session:
session.login({
issuer: 'https://identityprovider.com',
redirectUrl: 'https://mysite.com/redirect'
}).then((neededAction) => {})
Options: | Field Name | Required? | Type | Description | Default |
---|---|---|---|---|---|
issuer |
Yes, unless webId is provided |
String or URL | The user's issuer | undefined | |
returnToUrl |
Yes | String or URL | The URI within this application that the user should be redirected to after successful login. This can be either a web URL or a mobile URL scheme | undefined | |
clientId |
Only if you don't want to do dynamic registration | String or URL | The id of a statically registered application. | undefined | |
doNotAutoRedirect |
No | Boolean | If true, the browser will not auto redirect. Note that auto redirect only happens if Solid-Auth-Fetcher is running in the browser | false |
Send an HTTP request to a Solid Pod:
session.fetch('https://example.com/resource', {
method: 'POST',
headers: {
"Content-Type": "text/plain"
},
body: 'What a cool body!'
}).then((result) => {})
Fetch follows the WHATWG Fetch Standard.
Log the user out:
session.logout().then(() => {})
If you'd like to configure custom storage, provide an object following this interface:
{
get: (key: string) => Promise<string | null>;
set: (key: string, value: string) => Promise<void>;
delete: (key: string) => Promise<void>;
}
A needed action object describes an action you must implement. It is returned by the login function in the case that you are not operating in the web browser. If you are operating in the web browser, no additional action needs to be taken.
There are a few needed actions that can be returned:
If a NeededInaction
is returned by login
, no action is needed. A NeededInaction
looks like:
{
actionType: "inaction"
}
If a NeededRedirect
is returned by login
, you must redirect the user to the provided url. A NeededRedirect
looks like:
{
actionType: "redirect";
redirectUrl: "https://identityprovider.com/auth?client_id=coolApp";
}
Preliminary remark (will do a detailed review):
I like the idea of separate interfaces; but maybe not the separate solid-auth-fetcher-sessions
. After all, I'd expect solid-auth-fetcher
to simply be a front for solid-auth-fetcher-sessions
, right? I.e., the front that just makes everything look like one session.
I'd want to make switching as easy as possible, so perhaps just different exports from the same lib or so.
Just a thought; much more coming up.
@RubenVerborgh I think if we're going to spit the interface it makes sense to split the library too. Mainly because in the single-instance use case, an instance of SessionManager is created automatically at import. If they were combined, that session manager would just be hanging around without a use if the developer didn't want to use the single-instance use case.
an instance of SessionManager is created automatically at import
Perhaps not the best idea though (regardless of other outcomes).
The reason why we want that is so that you can run import { fetch } from "@inrupt/solid-auth-fetcher"
and it will just work. No need to do additional setup in the file or pass around a variable within your code.
I think it still makes sense to make it two libraries beause they are going after two different kinds of set ups. (Also if not already understood, solid-auth-fetcher-sessions would be a dependency of solid-auth-fetcher)
Note that
The reason why we want that is so that you can run
import { fetch } from "@inrupt/solid-auth-fetcher"
and it will just work.
and
an instance of SessionManager is created automatically at import
are orthogonal 🙂
And also dangerous from an ES6 modules perspective, where imports should not perform work. Now we risk different behaviors before and after tree shaking.
I think it still makes sense to make it two libraries beause they are going after two different kinds of set ups
Will investigate in the context of my larger review.
In terms of tree shaking, this module is not tree shakable because it uses dependency injection. The dependency tree imports everything in the module.
In terms of tree shaking, this module is not tree shakable because it uses dependency injection. The dependency tree imports everything in the module.
There is no shaking inside of solid-auth-fetcher, okay.
However, assume a module A that imports B, which in turn imports fetch
from @inrupt/solid-auth-fetcher
.
Now if A only imports specific parts of B, and those do not involve fetch
, then fetch
will be "tree-shaken" off.
But what I'm hearing from the above, is that this will result in different behavior rather than an unnoticeable difference, which is not how modules should behave.
Below is my full API review in two parts.
In this first part, I start with the "Solid Auth Fetcher Sessions", given that this is the generic API.
Overall comments:
NeededAction
abstraction is probably better replaced by an event handlerSessionManager
might be missing a fetch
method,
where it decides in a (configurable) intelligent way what session to useUsing
import
import { getSession } from "@inrupt/solid-auth-fetcher-sessions" getSession().then((session) => console.log(session))
So I still object to splitting the generic interface from the specific case where there is one session.
I think everything should just be @inrupt/solid-auth-fetcher
.
The splitting creates the perception that they are different libraries, whereas they're really not; one is simply a front API for another.
I think it's unnecessarily complicating and confusing to split. It's our job to keep things simple.
A class that manages all sessions in your application.
The below is more a note about how the API is documented rather than the API itself; I will prefix such notes with [doc].
[doc] So we're going to need to start with the basics here; specifically what exactly a session is.
[doc] And let's start with a session and what you can do with it, before looking at a complex object managing sessions.
- storage
That's not exposed, so not a part of the API.
|
storage
| No | IStorage | A storage object to help you access your custom storage. | In Memory Storage |
[doc] It is the storage. Also, what is it storing? (session settings I presume)
Returns all sessions currently in the session manager.
[doc] But what does it mean for a session to be in the manager?
Creates a new session and adds it to the session manager. If a session id is not provided, a random UUID will be assigned as the session id. If the session of the provided id already exists, that session will be returned.
[doc] The ID mechanism will need to be explained first.
I wonder whether ID should be part of the session; I see that it is now.
A session has many more properties than the constructor allows here; how are those others initialized?
How can we know the session ID before we initialize it?
Registers a callback to be called when a session is logged in.
What is the difference between creating a session and logging in to one?
Why is onLogin
a method on the manager,
and not a onSession
when a session is created,
such that we can listen to individual sessions?
sessionManager.onLogin((session) => {})
I wonder if session ID should be in the callback.
OIDC-specific; does not belong here but rather on an OIDC-specific class.
Perhaps something like an OidcAdapter that is registered to the session manager. Or elsewhere.
OIDC-specific; does not belong here but rather on an OIDC-specific class.
Same remark here; are those internal variables? Or exposed properties>?
- sessionId
- loggedIn
What does it mean for a session to not be logged in? Isn't it the same as it not existing?
- webId
- neededAction
What is this, and does it really belong here?
- sessionManager
Can a session only belong to one manager? (Why) is it important to a) keep a link to that manager b) expose it?
Should sessions know about managers at all?
constructor(sessionManager, sessionId?)
Creates a Session object. If sessionId is not present, a random UUID will be generated.
With those constructor arguments, it seems that the coupling between the manager and session is too strong.
Initializes the session by saving it to storage if a session with the given id does not already exist, or by syncing it with the session in storage if it already exists.
This seems to be in the wrong place. Should a session know how to init itself? It rather seems to be the task of the object above it.
At the very least, storage should be a parameter passed to this method. Now, it seems that session knows how to look up storage in a manager; that's a Law of Demeter violation.
I wonder if the return value is needed here, or whether we should instead solve this with action-specific (event) handlers.
|
returnToUrl
| Yes | String or URL | The URI within this application that the user should be redirected to after successful login. This can be either a web URL or a mobile URL scheme | undefined |
Might need a better name. Reads like a boolean to me know.
|
clientId
| Only if you don't want to do dynamic registration | String or URL | The id of a statically registered application. | undefined |
[doc] Can we specify the positive case here?
What does passing a clientId
mean/do?
|
doNotAutoRedirect
| No | Boolean | If true, the browser will not auto redirect. Note that auto redirect only happens if Solid-Auth-Fetcher is running in the browser | false |
Rename to autoRedirect
and invert value.
Shall we make explicit that this method can also be used as an independent function?
So that I can detach fetch
from the object, and it still works?
If you'd like to configure custom storage, provide an object following this interface:
Name proposal: IAsyncKeyValueStore
(or IAsyncKeyValueStore
).
A needed action object describes an action you must implement.
As per my above comment, let's get rid of this concept.
Instead, let's add event handlers such as onRedirect
(possibly with default implementations).
In this second part, I discuss the simplified single-session API.
Overall comments:
getSession
and key functionality of that session being exposed as direct exports.let defaultSession;
export function getSession() {
if (typeof defaultSession === 'undefined')
defaultSession = createDefaultSession();
return defaultSession;
}
export function fetch(url, options) {
return getSession().fetch(url, options);
}
<script src="/path/to/solidAuthFetcher.bundle.js"></script> </script> solidAuthFetcher.getSession().then((session) => console.log(session)) </script>
Shall we put it in solid.fetcher
?
[doc] Add return type
[doc] Add return value and type
Register a callback function to be called when a user completes login:
Specifically, when the default user logs in?
Register a callback function to be called when a user logs out:
Idem?
A method for configuring a custom backend for solid-auth-fetcher.
It's not a backend, and I don't think this method belongs here.
Extra evidence to me that we want to keep this as one library, so devs can just use the manager for these things.
Perhaps .adapters.oidc
to keep it generic?
Yeah, I agree with Ruben on splitting the interface, but not the library. Also, I think there should always be a SessionManager (lazy initialized though), and when only a single user, that manager instance will simply manage a single 'default' session.
I also agree that a session should not know about it's manager (just a general design princple).
So I'd rewrite Ruben's example as:
let defaultSessionManager;
export function getSession() {
if (typeof defaultSessionManager === 'undefined')
defaultSessionManager = new SessionManager();
if (defaultSessionManager.hasDefaultSession())
return defaultSessionManager.defaultSession();
// Creates a new session and sets it as the 'defaultSession'
// inside the SessionManager if there isn't one already.
return defaultSessionManager.createSession();
}
export function fetch(url, options) {
return getSession().fetch(url, options);
}
// If the developer wants their own session manager instance
// to act as the default one (perhaps they want to use one with
// their own custom storage, but they still want use all the methods
// we expose here that don't explicitly dereference a
// SessionManager instance).
export function setDefaultSessionManager(manager: SessionManager) {
defaultSessionManager = manager;
}
@pmcb55 Agreed with this implementation for createDefaultSession
. Might want the explicit "default" out of session managers though; "default"
can just be a session ID.
Agree with Ruben that "withCustomBackend(options): { ...API }" does not seem to belong where it is (but I'm not sure!). It seems to be a SessionManager concept - i.e. for managing the storage of sessions, or is it for use by a session for storing session-specific state? Regardless, I'd just call it 'Storage' instead of 'Backend' too.
Okay here's the new one:
The Solid Auth Fetcher Ecosystem is comprised of a pair of libraries designed to make authenticated fetch requests to Solid-compatible servers.
Solid auth fetcher includes two APIs for building applications: a "Simple API" and a "Flexible API".
Solid-auth-fetcher's "Simple API" is designed to provide a streamlined experience to implementing authentication in apps that are focused on one user. You'd use solid-auth-fetcher to
Solid-auth-fetcher's "Flexible API" allows you to manage multiple sessions for multiple logged in users. You'd use solid-auth-fetcher to
git clone https://github.com/inrupt/solid-auth-fetcher.git
cd solid-auth-fetcher
npm i
npm run bootstrap-examples
# Run each example
npm run dev-bundle
npm run dev-script
npm run dev-server
At this point, a test application will be running on port 3001
and a test solid server will be running on port 9001
Be sure that you point
npm install @inrupt/solid-auth-fetcher
In the browser via the script
tag:
<script src="/path/to/solidAuthFetcher.bundle.js"></script>
</script>
solidAuthFetcher.getSessionInfo()
.then((sessionInfo) => console.log(sessionInfo))
</script>
Using import
import { getSession } from "@inrupt/solid-auth-fetcher"
getSessionInfo()
.then((sessionInfo) => console.log(sessionInfo))
Using require
const solidAuthFetcher = require("@inrupt/solid-auth-fetcher")
solidAuthFetcher.getSessionInfo()
.then((sessionInfo) => console.log(sessionInfo))
If solid-auth-fetcher is installed on an application that operates in the web browser, triggering login is a simple process:
import {
login,
getSessionInfo,
onLogin
} from 'solid-auth-fetcher'
getSessionInfo().then(async (sessionInfo) => {
// Check if the user is already logged in
if (!sessionInfo.loggedIn) {
await login({
// The URL of the user's OIDC issuer
oidcIssuer: 'https://identityProvider.com',
// The url the system should redirect to after login
returnToUrl: 'https://mysite.com/redirect'
});
}
});
onLogin((sessionInfo) => {
// Logs the user's webId
console.log(sessionInfo.webId);
});
You can use the fetch
function anywhere in your application. If you've already logged in, the fetch
function automatically fills in the user's credentials, if not, it will attempt to make a request without the user's credentials.
import { fetch } from '@inrupt/solid-auth-fetcher'
fetch('https://example.com/resource', {
method: 'post',
body: 'What a cool string!'
}).then(async (response) => {
console.log(await response.text());
})
By default, the user is redirected to the login page within the same window, but you might want to maintain the state of your application without it being interrupted by a redirect. To do so, you can use a popup.
index.html
<html>
<head>
<script src="/path/to/solidAuthFetcher.bundle.js"></script>
<script>
function login() {
solidAuthFetcher.login({
oidcIssuer: "https://identityProvider.com",
popUp: true,
redirectUrl: "https://mysite.com/popup.html"
})
}
</script>
<head>
<body>
<button onClick="login()">login</button>
</body>
</html>
If you want to use the simple API to log in anywhere other than a web-browser, you must call handleRedirect
at the redirect route.
import express from 'express'
import {
login,
adapters,
fetch
} from "@inrupt/solid-auth-fetcher"
const app = express()
app.get('/login', async (req, res) => {
// Begin login process and get the needed action.
// (Note the needed action can also be retieved by
// using the onNeededAction function)
const neededAction = await login({
oidcIssuer: 'https://identityProvider.com',
redirectUrl: 'https://mysite.com/redirect'
})
if (neededAction.type === 'redirect') {
res.redirect(neededAction.redirectUrl)
}
})
app.get('/redirect', async (req, res) => {
// Provide the adapter here
await adapters.oidc.handleRedirect(req.url)
})
app.get('/fetch', async (req, res) => {
const result = await fetch("https://example.com/resource")
res.send(await result.text())
})
By default, Solid-Auth-Fetcher redirects automatically upon login, and automatically handles a redirect back to the app when it is initialized. But, you may want to handle redirects manually. To do this you can use the doNotAutoRedirect
flags along with the handleRedirect method.
import {
login,
getSessionInfo,
onLogin
} from 'solid-auth-fetcher'
async function login() {
await onLogin((sessionInfo) => {
console.log(sessionInfo.webId);
});
// Handle the redirect in a custom way here
await onNeededRedirect((redirectUrl) => {
window.location.href = redirectUrl
})
const sessionInfo = await getSessionInfo()
if (!sessionInfo.loggedIn) {
await login({
oidcIssuer: 'https://identityProvider.com',
returnToUrl: 'https://mysite.com/redirect',
// Prevent auto redirecting by setting this flag
doNotAutoRedirect: true
});
}
}
login()
Kick off the login process for the user:
import { login } from '@inrupt/solid-auth-fetcher';
login({
issuer: 'https://identityprovider.com',
returnToUrl: 'https://mysite.com/redirect'
}).then((neededAction) => {})
Options: | Field Name | Required? | Type | Description | Default |
---|---|---|---|---|---|
issuer |
Yes, unless webId is provided |
String or URL | The user's issuer | undefined | |
returnToUrl |
Yes | String or URL | The URI within this application that the user should be redirected to after successful login. This can be either a web URL or a mobile URL scheme | undefined | |
clientId |
Only if you don't want to do dynamic registration | String or URL | The id of a statically registered application. | undefined | |
popUp |
No | Boolean | If true, the login process will initiate via a popup. This only works on web clients. | false | |
doNotAutoRedirect |
No | Boolean | If true, the browser will not auto redirect. Note that auto redirect only happens if Solid-Auth-Fetcher is running in the browser | false |
Send an HTTP request to a Solid Pod:
import { fetch } from '@inrupt/solid-auth-fetcher';
fetch('https://example.com/resource', {
method: 'POST',
headers: {
"Content-Type": "text/plain"
},
body: 'What a cool body!'
}).then((result) => {})
Fetch follows the WHATWG Fetch Standard.
Log the user out:
import { logout } from '@inrupt/solid-auth-fetcher';
logout().then(() => {})
Retrieve the user's session or provides null if the user is not logged in:
import { getSession } from '@inrupt/solid-auth-fetcher';
await getSession().then((sessionInfo) => {
console.log()
})
Register a callback function to be called when a user completes login:
import { onLogin } from '@inrupt/solid-auth-fetcher'
onLogin((sessionInfo) => {
console.log(session.webId)
})
Register a callback function to be called when a user logs out:
import { onLogout } from '@inrupt/solid-auth-fetcher'
onLogout(() => {})
Register a callback function to be called when a needed action must be taken.
import { onNeededAction } from '@inrupt/solid-auth-fetcher'
onNeededAction((neededAction) => {
if (neededAction.type === "redirect") {
window.location.href = neededAction.redirectUrl
}
})
Register a callback function to be called when a needed action of type "redirect" must be handled.
import { onNeededRedirect } from '@inrupt/solid-auth-fetcher'
onNeededAction((redirectUrl) => {
window.location.href = redirectUrl
})
The session object contains information about the logged in user.
{
loggedIn: boolean // True if the user is currently logged in
webId?: String // The user's WebID.
}
A needed action object describes an action you must implement. It is returned by the login function in the case that you are not operating in the web browser. If you are operating in the web browser, no additional action needs to be taken.
There are a few needed actions that can be returned:
If a NeededInaction
is returned by login
, no action is needed. A NeededInaction
looks like:
{
actionType: "inaction"
}
If a NeededRedirect
is returned by login
, you must redirect the user to the provided url. A NeededRedirect
looks like:
{
actionType: "redirect";
redirectUrl: "https://identityprovider.com/auth?client_id=coolApp";
}
A place for special functions that have to do with a specific login technique.
The oidc adapter contains functions that should be called to handle edge cases that can't be handled automatically by solid-auth-fetcher
{
handleRedirect: (url: string | URL) => Promise<void>
}
Handles redirects as a part of the OIDC process. Servers using solid-auth-fetcher must manually call this method on redirect, but is done automatically on web and mobile.
import { adapters } from '@inrupt/solid-auth-fetcher'
adapters.oidc.handleRedirect(window.location.href)
import { adapters } from '@inrupt/solid-auth-fetcher'
adapters.oidc.handlePopUpRedirect(window.location.href)
// Todo explain more about what's happening here
import { SessionManager } from "@inrupt/solid-auth-fetcher";
import express, { Request, Response } from "express";
import bodyParser from "body-parser";
import session from "express-session";
const PORT = 3001;
const BASE_URL = `http://localhost:${PORT}`;
const app = express();
const sessionManager = new SessionManager();
app.use(
session({
secret: "I let Kevin's son beat me in foosball",
cookie: { secure: false }
})
);
app.use(bodyParser.urlencoded({ extended: false }));
app.set("view engine", "ejs");
app.get("/", (req, res) => {
res.render("home");
});
app.post("/login", async (req: Request, res: Response) => {
if (req.session && req.session.localUserId) {
res.status(400).send("already logged in");
}
const issuer: string = req.body.issuer;
const session = await sessionManager.createSession();
const neededAction = await session.login({
issuer: issuer,
redirectUrl: `${BASE_URL}/redirect`
});
if (neededAction.type === "redirect") {
res.redirect(neededAction.redirectUrl);
} else {
res.redirect("/dashboard");
}
});
app.get("/redirect", async (req: Request, res: Response) => {
const session = await sessionManager.handleRedirect(req.url);
if (session.loggedIn) {
res.session.sessionId = session.sessionId;
res.redirect("/dashboard");
} else {
res.redirect("/login");
}
});
app.get("/dashboard", async (req: Request, res: Response) => {
const session = await sessionManager.getSessionById(req.session.sessionId);
if (session && session.loggedIn) {
res.render("dashboard", {
webId: session.webId,
fetchResult: ""
});
} else {
res.status(401).send("You are not logged in");
}
});
app.post("/fetch", async (req: Request, res: Response) => {
const session = await sessionManager.getSessionById(req.session.sessionId);
if (session && session.loggedIn) {
const result = await session.fetch("http://localhost:10100/");
res.render("dashboard", {
webId: session.webId,
fetchResult: JSON.stringify(await result.text(), null, 2)
});
} else {
res.status(401).send("You are not logged in");
}
});
app.post("/logout", async (req: Request, res: Response) => {
const session = await sessionManager.getSessionById(req.session.sessionId);
if (session && session.loggedIn) {
await session.logout();
delete req.session.sessionId;
res.redirect("/");
}
});
app.listen(PORT, () => console.log(`Listening on port ${PORT}`));
// TODO: describe what's going on
import React, { Component } from "react";
import { SessionManager } from "@inrupt/solid-auth-fetcher";
class Form extends Component {
constructor(props) {
super(props);
this.state = {
status: "loading",
loginIssuer: "https://localhost:8443",
fetchRoute: "https://jackson.localhost:8443/private",
fetchBody: "",
session: null,
sessionManager: new SessionManager()
};
if (window.location.pathname === "/popup") {
this.state.status = "popup";
}
this.sessionManager.onLogin(session => {
this.setState({ status: "dashboard", session });
});
this.sessionManager.onNeededRedirect(redirectUrl => {
window.location.href = redirectUrl;
});
this.handleLogin = this.handleLogin.bind(this);
this.handleLogout = this.handleLogout.bind(this);
this.handleFetch = this.handleFetch.bind(this);
}
async componentDidMount() {
const session = await sessionManager.getSessionById("default");
if (session && session.loggedIn) {
this.setState({ status: "dashboard", session });
} else {
this.setState({ status: "login" });
}
}
async handleLogin(e, isPopup = false) {
e.preventDefault();
this.setState({ status: "loading" });
await this.sessionManager.login({
redirect: isPopup
? "http://localhost:3001/popup"
: "http://localhost:3001/",
oidcIssuer: this.state.loginIssuer,
popUp: isPopup,
doNotAutoRedirect: true
});
}
async handleLogout(e) {
e.preventDefault();
this.setState({ status: "loading" });
await this.state.session.logout();
this.setState({
status: "login",
fetchBody: "",
session: null
});
}
async handleFetch(e) {
e.preventDefault();
this.setState({ status: "loading", fetchBody: "" });
const response = await (
await this.state.session.fetch(this.state.fetchRoute, {})
).text();
this.setState({ status: "dashboard", fetchBody: response });
}
render() {
switch (this.state.status) {
case "popup":
return <h1>Popup Redirected</h1>;
case "loading":
return <h1>Loading</h1>;
case "login":
return (
<form>
<h1>Solid Auth Fetcher Demo Login</h1>
<input
type="text"
value={this.state.loginIssuer}
onChange={e => this.setState({ loginIssuer: e.target.value })}
/>
<button onClick={this.handleLogin}>Log In</button>
<button onClick={e => this.handleLogin(e, true)}>
Log In with Popup
</button>
</form>
);
case "dashboard":
return (
<div>
<h1>Solid Auth Fetcher Demo Dashboad</h1>
<p>WebId: {this.state.session.webId}</p>
<form>
<input
type="text"
value={this.state.fetchRoute}
onChange={e => this.setState({ fetchRoute: e.target.value })}
/>
<button onClick={this.handleFetch}>Fetch</button>
<pre>{this.state.fetchBody}</pre>
</form>
<form>
<button onClick={this.handleLogout}>Log Out</button>
</form>
</div>
);
}
}
}
export default Form;
A class that manages all sessions in your application.
Creates a SessionManager object.
import { SessionManager } from "@inrupt/solid-auth-fetcher";
import customStorage from "./myCustomStorage"
const sessionManager = new SessionManager({
storage: customStorage
})
Options: | Field Name | Required? | Type | Description | Default |
---|---|---|---|---|---|
storage |
No | IStorage | A storage object to help you access your custom storage. | In Memory Storage |
Auto initializes your session. This should be called directly after the constructor.
import { SessionManager } from "@inrupt/solid-auth-fetcher";
import customStorage from "./myCustomStorage"
const sessionManager = new SessionManager()
sessionManager.init().then(() => {})
Kick off the login process for this session:
session.login({
issuer: 'https://identityprovider.com',
redirectUrl: 'https://mysite.com/redirect'
}).then((neededAction) => {})
Options: | Field Name | Required? | Type | Description | Default |
---|---|---|---|---|---|
issuer |
Yes, unless webId is provided |
String or URL | The user's issuer | undefined | |
returnToUrl |
Yes | String or URL | The URI within this application that the user should be redirected to after successful login. This can be either a web URL or a mobile URL scheme | undefined | |
clientId |
Only if you don't want to do dynamic registration | String or URL | The id of a statically registered application. | undefined | |
doNotAutoRedirect |
No | Boolean | If true, the browser will not auto redirect. Note that auto redirect only happens if Solid-Auth-Fetcher is running in the browser | false |
Returns all sessions currently in the session manager.
sessionManager.getSessions().then(sessions => {})
Creates a new session and adds it to the session manager. If a session id is not provided, a random UUID will be assigned as the session id. If the session of the provided id already exists, that session will be returned.
const session = sessionManager.createSession("mySessionid")
Returns a session of the given id. If the session does not exist in storage, null will be returned.
sessionManager.getSessionById("mySessionId").then(session => {})
Registers a callback to be called when a session is logged in.
sessionManager.onLogin((session) => {})
Registers a callback to be called when a session is logged out.
sessionManager.onLogout((session) => {})
Registers a callback to be called when an action is needed.
sessionManager.onNeededAction((neededAction) => {
if (neededAction.type === "redirect") {
window.location.href = neededAction.redirectUrl
}
})
sessionManager.onNeededAction((redirectUrl) => {
window.location.href = redirectUrl
})
Part of the OIDC flow is a redirect. If solid-auth-fetcher-sessions is deployed in the web-browser, this redirect is handled automatically, but on the server, it must be handled manually. Use the handleOidcRedirect
at the redirect route for your app.
app.get("/redirect", async (req, res) => {
await sessionManager.handleOidcRedirect(req.url)
res.redirect("/home");
})
Creates a Session object. If sessionId is not present, a random UUID will be generated.
Warning: you should either use sessionManager.createSession() or call session.init() directly after calling the constructor.
import { Session } from "@inrupt/solid-auth-fetcher-sessions"
const session = new Session(sessionManager, "mySessionId")
session.init().then(() => {})
Initializes the session by saving it to storage if a session with the given id does not already exist, or by syncing it with the session in storage if it already exists.
session.init().then(() => {})
Send an HTTP request to a Solid Pod:
session.fetch('https://example.com/resource', {
method: 'POST',
headers: {
"Content-Type": "text/plain"
},
body: 'What a cool body!'
}).then((result) => {})
Fetch follows the WHATWG Fetch Standard.
Log the user out:
session.logout().then(() => {})
If you'd like to configure custom storage, provide an object following this interface:
{
get: (key: string) => Promise<string | null>;
set: (key: string, value: string) => Promise<void>;
delete: (key: string) => Promise<void>;
}
A needed action object describes an action you must implement. It is returned by the login function in the case that you are not operating in the web browser. If you are operating in the web browser, no additional action needs to be taken.
There are a few needed actions that can be returned:
If a NeededInaction
is returned by login
, no action is needed. A NeededInaction
looks like:
{
actionType: "inaction"
}
If a NeededRedirect
is returned by login
, you must redirect the user to the provided url. A NeededRedirect
looks like:
{
actionType: "redirect";
redirectUrl: "https://identityprovider.com/auth?client_id=coolApp";
}
Hi @jaxoncreed,
Quick review comments below.
Naming suggestion: Flexible API
to become Sessions
API.
Phrasing suggestion: "one user" => "one identity" perhaps.
SessionInfo
just Session
getSessionInfo
should be getSession
.doNotAutoRedirect
=> autoRedirect
onRedirect
handler (see below)NeededAction
login
should not return NeededAction
login
return promise reject when logging in fails?onNeededAction
by specific actions such as onRedirect
(or onRequestRedirect
or something)handleOidcRedirect
does not belong on SessionManager
Session
should not know about SessionManager
. Potentially about Storage
.Session
know its sessionId
? I thought only SessionManager
did.Session#init
.
fetch
?Session
sNeededAction
section would need to go.
Current API
distilled from the
popup
branch at 91bc153b88968674f81d02838a7aada85732e229