panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

Missing docs for session storage without passport #145

Closed atuttle closed 5 years ago

atuttle commented 5 years ago

The word session only appears in the readme 3 times: once under the heading "Processing callback", presumably referring to some data we should store before redirecting out to the issuer, and twice under the heading "Getting RP-Initiated Logout url", which is irrelevant to initial client setup (as far as I can tell). That first reference has thrown me for a bit of a loop... where is this data coming from?

I am attempting to put together a client that doesn't use passport, using express and express-session. What do I need to save in the session? At what point in the process? And where do I get it from?

All in all I'm very optimistic about this library, though. It seems to otherwise be really well documented. Nice job!

panva commented 5 years ago

That first reference has thrown me for a bit of a loop...

Why? What's not clear about it? You need to persist some data before starting the oauth redirect ritual to be able detect abuse and misuse when your callback gets triggered. This is data that goes to a session.

What do I need to save in the session?

state, nonce and other parameters you might be passing to the callback's third argument

At what point in the process?

before redirecting the user to authorization

And where do I get it from?

state and nonce are random values preferably cryptographically bound to something else in your session

doesn't use passport, using express and express-session

You can use whatever you want, the client does not dictate any frameworks to you, the fact that it bundles a passport strategy is just to cater to the masses who already use passport.

panva commented 5 years ago

I'm open to suggestions about the documentation, if you have a proposal that would've helped you before opening the issue please reach out either here or open a PR.

atuttle commented 5 years ago

Why? What's not clear about it?

I actually asked questions that I thought would answer this. You haven't given any example of what sorts of things one might use for state and response_type. You say here in the ticket that I need to save:

state, nonce and other parameters you might be passing to the callback's third argument

And I can see in the example that I previously referenced ("Processing callback") that you've indicated that { state, response_type } might be reasonable arguments, but there is no documentation on that third argument of authorizationCallback at all. What is expected in that argument? How is it used? In fact the only other place in the readme where authorizationCallback gets mentioned is under the heading "TokenSet" and that example (1) doesn't include a third argument at all, and (2) doesn't document the first two arguments either.

I can gather from context clues that the first argument should be the callback url that was presented to the issuer (presumably because it will be included in the response for validation purposes), and the second argument should be (guesstimating here) an object of key/value pairs representing query string arguments from the callback request. Did I get those correct?

I'm still left wondering what the point of the 3rd argument is, and what has to or can be optionally included in it.

It seems that a working knowledge of OIDC might be required before I can fully understand how to use this library, which I think is unfortunate. It would be nice if the docs didn't assume much/any pre-existing knowledge. This issue was an attempt at pointing out some of those shortcomings.

I am familiar with what a nonce is, but I guess what was more at question was, does this OIDC library provide these values (state, nonce, response_type, anything else) as return values from previous calls, that I will need to save, or do I make them all up myself? And for the state and the response type, it would be helpful if some example were given as to their intended uses. As answered so far here in the ticket, I could just pick a random letter from the alphabet and use that for each, and as long as they match in the callback, that would work... But if there's some reason to use anything else, could you please explain it? And consider adding it to the documentation?

Lastly, I'm uncertain what you meant by "cryptographically bound." Do you mean encrypted using a key in the session? Encrypted with the same key as the session? Something entirely different?

Apologies if these are introductory questions, but none of this information is readily apparent to someone without prior knowledge of OIDC internals who is just trying to get a working client on its feet.

panva commented 5 years ago

Let’s see if i can address some of it in the coming days. But so far, yes, it requires prior knowledge of oidc and i’m ok with it. There already other libraries using mine as a dependency that take care of e.g. just exposing express middlewares and take care of generating nonces, states, etc.

Ultimately you’d following the provider’s documentation and things would become apparent to you. Your provider’s integration docs are your first touchpoint with the protocol and they ought to make recommendations for you.

I could indeed hold the developer’s hands every step of the way here but i intended to provide a level of abstraction so that sdks and abstractions can be built on top. I had an example use folder in this repo before but people took it for standard implementation when it wasnt. I also don’t feel like copying the RFCs and BCPs here.