Explicitly declare which values can be trusted
- Vorherrschende Sprache
- Rust
- Sterne
- 783
- Forks
- 102
- PR-Merge-Kennzahlen
- Keine gemergten PRs in 30 T.
Beschreibung
# Project Improvement
Hey! I've been going through implementing my own (async) implementation with this, and I've been collecting thoughts that I think are actionable to better the crate.
Notably, I'm very thankful for the primitives: once you understand what most pieces do it makes it a lot easier to implement things correctly. I've certainly
However, there's a bunch of things that I felt I was lost on, and had to read source code (and play around with what was being provided) to various traits:
By far the biggest concern is the documentation on the individual traits, specifically on `Registrar`, `Authorizer`, and `Issuer`. The documentation on all these traits currently do not inform a trait implementation whether or not an input can be trusted or not. For example:
- [ ] In `Registrar::negotiate`, is the `Option` raw user input? Can we directly trust `BoundClient` to have a server validated redirect uri, or must we validate it before generating a `PreGrant`?
- [ ] In `Issuer::issue`, can we trust the `Grant` provided to us? Where is the `Grant.until` value being populated from?
- [ ] In `Issuer::refresh`, can we trust the `Grant` provided to us?
This probably expands to all traits, but I think these are the most important ones when it comes to getting up and running in a secure manner.
There's also a couple clarifying questions I can't seem to figure out either that I think documentation could be extremely helpful on:
- [ ] In `Issuer::recover_refresh` for an Issuer that supports refresh tokens, are we to return a `Grant` with an `until` value of the refresh token expiration or the session token expiration?
- [ ] In the Issuer's recover flows, do we need to populate all fields of the Grant? As far as I can tell I was able to populate dummy values in the `redirect_uri` and `extensions` fields without any issues, though I suspect that's only because I currently am only doing the authorization grant flow.
- [ ] As far as I can tell, a "solicitor" isn't part of the OAuth spec. You can figure it out as the "render-the-oauth-grant/deny-page", but it took a while for me to digest everything.
Finally, I think it would be really, really helpful embedding some of the expectations of the Flow structs. It took me quite a bit of time to figure out that you need to use `HTTP Basic` authorization to exchange an authorization code into bearer tokens. Suggestions on how long those bearer tokens should last would also be appreciated
## Other context
I'm very appreciative of the work done already. It's definitely helped clear things up with OAuth on the server implementation side.
# Tracking pull request
- [ ] A pull request does not yet exist
Beitragsleitfaden
Bewertung
Dieses Issue wurde noch nicht bewertet.