Implement an async version of client credentials flow #159
Conversation
| @@ -121,7 +121,7 @@ pub enum ResponseStatus { | |||
| /// not derive this until this has shown unlikely but strongly requested. Please open an issue if you | |||
| /// think the pros or cons should be evaluated differently. | |||
| #[derive(Debug)] | |||
| enum InnerTemplate<'a> { | |||
| pub enum InnerTemplate<'a> { | |||
There was a problem hiding this comment.
Can you add and use constructors to Template instead.
| pub fn new(error: AccessTokenError) -> Self { | ||
| Self { error } | ||
| } |
There was a problem hiding this comment.
Being critical towards making too many internals public applies here, too, the constructor should only be needed by the response template internally. There's already constructors applicable to new_ok and new_unauthorized for instance.
| pub fn new(token: IssuedToken, scope: String) -> BearerToken { | ||
| Self(token, scope) | ||
| } |
There was a problem hiding this comment.
That should definitely not be the public signature, consider making the second parameter &Scope. But it's slightly controversial either way. This type consciously does not have a public constructor to nudge you towards properly using the flow and not issuing a badly protected token (e.g. to unauthenticated parties). I'll need to consider this, maybe you have arguments pro/con?
How complicated is it to duplicate? Seriously consider this alternative as well.
|
@HeroicKatora Thanks for the feedback. I will update the PR as requested. |
This is an attempt to port client credentials flow to the async version. I noticed that it seemed to be the only implemented flow that was missing an async version. I didn't know if this was a deliberate omission or not, but after some investigation I realized that the async work was done much earlier. So, I decided to try my hand at porting it over. The work itself doesn't contain anything new. It's a straight up port of the non-synchronous code to an async version. Please let me know if I missed anything. Also, I would appreciate any comments on the minor changes in oxide-auth that are needed to get the async version to compile (like changing the visibility of a couple of functions and ::new() constructor in a couple of places).
I license past and future contributions under the dual MIT/Apache-2.0 license, allowing licensees to chose either at their option.