197g / 197g/oxide-auth

Explicitly declare which values can be trusted

未關閉 適合新手
#184 0 則留言 1 個 reaction 已指派 0 人 在 GitHub 檢視
improvement
主要語言
Rust
星號
783
分支
102
PR 合併指標
30 天內沒有已合併 PR

描述

# 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

貢獻指南

開啟貢獻指南

研究方向

該 issue 指出 oxide-auth crate 中缺少關於 Registrar、Authorizer 和 Issuer 等 trait 的信任假設的文件。首先閱讀 crate 的 lib.rs 或相關模組檔案中這些 trait 的原始碼。查看現有實作或範例以理解預期行為。然後,更新 trait 文件以釐清哪些輸入被視為可信、哪些需要驗證,並在適用時參考 OAuth2 規範。

由索引模型根據 Issue 內容生成。

評估

領域
documentation
Issue 類型
文件
難度
2/5
預估耗時
1-3 小時
活躍度
停滯
描述清晰度
描述清楚
新手友好度
65/100

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。