ramosbugs / openidconnect-rs

OpenID Connect Library for Rust
MIT License
372 stars 98 forks source link

More usage applicable documentation #137

Open sidju opened 8 months ago

sidju commented 8 months ago

The current documentation showcases how to communicate with the Authentication Provider and verify the responses, but it doesn't mention a lot of how to build an authentication flow on it.

I'm very uncertain about these answers (as the best I can find is implications that something similar is done by a different OIDC implementation and no real examples), but I'll at least suggest the questions I've had while trying to implement an authentication with this library:

That is what I've figure this far, from a lot of research. It would be nice if this information was available on the types it relates to or in the examples. Just some quick hints in the style of:

If you can confirm this is something you want I can make a PR. I've clearly already written half of what I wanted to add.

ramosbugs commented 8 months ago

Hey @sidju,

Thanks for this feedback. I've had to spend so much time buried in the specs to write this crate (and oauth2) that it's difficult to look at the docs from a fresh end user's perspective.

Many of the questions you raised (e.g., "Are the claims encrypted, how tamper-proof are they?") are about the OIDC protocol in general, and not specific to this crate's interfaces. Answers to those questions already exist in the specs and various implementation-agnostic resources around the web. I don't think the documentation for every library in every programming language should attempt to fully explain the protocol, just as HTTP client libraries don't attempt to explain HTTP itself to their users.

Given that this protocol is for security-critical functionality, I would expect users to learn enough about the protocol to answer basic questions like "is my use case a confidential client that can store a client_secret?" and "which auth flow should I use?" before approaching this library to implement their desired auth flow. This crate attempts to express as many of the protocol's security requirements within Rust's type system as possible, but it doesn't abstract away the inherent security considerations that users still need to be aware of when implementing authentication in their applications. Consequently, I think the goal of this crate's documentation should be to help users go from an understanding of their desired auth flow to a secure implementation as easily as possible, not to serve as the only resource users need to consult when building auth.

This doesn't mean I think the current docs can't be improved significantly. They could, for example, explicitly list the decisions (e.g., choice of auth flow) users need to make before trying to use the library, along with links to good resources containing the information they'll need to make those decisions.

The sorts of hints you listed would also be a nice addition as a reminder to users what certain types are used for. We should be careful not to be overly prescriptive though, since, for example, the CSRF token and nonce could also be stored in a cookie depending on the type of application the user is working on. This is another good opportunity for links to external resources that discuss the various storage options and their tradeoffs.

sidju commented 8 months ago

Thank you for the quick response.

That is a very reasonable limitation of scope and expectation on the users.

I'll see if I can make a PR with some additional comments in the examples (mainly which type comes out and why, as that is a bit difficult to parse with all the type arguments the Client takes), maybe some getting started information in the crate documentation, and some extra documentation on the types. Mainly how you are expected to get an instance of a type and which type is the typical implementer of a trait should be documented, as there are so many types implementing so many traits that it is easy to get lost looking for a method in a trait that is implemented on a type.

ramosbugs commented 8 months ago

That sounds great, thanks!