197g / 197g/oxide-auth

Explicitly declare which values can be trusted

Aberta Para iniciantes
#184 0 comentários 1 reação 0 responsáveis Ver no GitHub
improvement
Linguagem predominante
Rust
Estrelas
783
Forks
102
Métricas de merge de PRs
Nenhum PR com merge em 30d

Descrição

# 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

Guia de contribuição

Abrir o guia de contribuição

Direção de pesquisa

A issue aponta para a falta de documentação sobre suposições de confiança para traits como Registrar, Authorizer e Issuer na crate oxide-auth. Comece lendo o código-fonte desses traits no arquivo lib.rs ou nos arquivos de módulo relevantes da crate. Examine implementações ou exemplos existentes para entender o comportamento esperado. Em seguida, atualize a documentação dos traits para esclarecer quais entradas são consideradas confiáveis e quais precisam de validação, referenciando a especificação OAuth2 quando aplicável.

Escrita pelo modelo de indexação a partir do texto da issue.

Avaliação

Domínio
documentation
Tipo de issue
Documentação
Dificuldade
2/5
Tempo estimado
1-3 horas
Status de atividade
Estagnada
Clareza
Claramente especificada
Facilidade para iniciantes
65/100

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.