Add support for oauth2 device flow#186
Conversation
77350a5 to
7bf7608
Compare
|
just force pushed several rounds of Claude code reviews for the branch. |
| return u.h.fs.Audit.ReadEvents(u.id) | ||
| } | ||
|
|
||
| func (u User) ApproveAuthorization(deviceCode string, tokenDescription string, scopes Scopes) error { |
There was a problem hiding this comment.
These two functions can be a part of oauth2.go too.
I have no preference, just pointing at the possibility.
| // @Produce json | ||
| // @Success 200 | ||
| // @Router /oauth2/device/code [post] | ||
| func (h oauth2Handlers) oauth2DeviceCode(c echo.Context) error { |
There was a problem hiding this comment.
Why can't we reuse the handlers class for this?
I suspect you're trying to start spinning off individual handlers classes for different groups.
But, that would better be done as an overhaul; or we risk getting into a mess.
There was a problem hiding this comment.
We need a slightly different middleware setup for this that:
- doesn't require authenticated clients
- does include some ratelimits (since it doesn't require authentication)
| // @Produce json | ||
| // @Success 200 | ||
| // @Router /oauth2/device/token [post] | ||
| func (h oauth2Handlers) oauth2DeviceToken(c echo.Context) error { |
There was a problem hiding this comment.
Why didn't some standard server library (like e.g. https://pkg.go.dev/github.com/go-oauth2/oauth2/v4) suit our needs with some minimal customization?
For the above example, it looks like all we needed was to implement the store interface from https://github.com/go-oauth2/oauth2/blob/master/store.go and some minimal handlers customization.
Just asking, as I don't feel expert enough to validate the full correctness of this Oauth2 implementation.
There was a problem hiding this comment.
That library doesn't implement the device-flow part of the spec that we need. Chatting with claude/copilot, they both said this library doesn't buy us much. Especially since we have token generation, validation, and scope logic already implemented.
| } | ||
|
|
||
| if c.Path() == "/oauth2/device/code" || c.Path() == "/oauth2/device/token" { | ||
| return next(c) |
There was a problem hiding this comment.
What's the problem with using CSRF for the Oauth2?
There was a problem hiding this comment.
This is device/api-client side.
There are some upcoming functions we need this for when doing oauth2 device flow Signed-off-by: Andy Doan <doanac@qti.qualcomm.com>
Signed-off-by: Andy Doan <doanac@qti.qualcomm.com> Assisted-by: GitHub Copilot:claude-4-sonnet
Signed-off-by: Andy Doan <doanac@qti.qualcomm.com> Assisted-by: GitHub Copilot:claude-4-sonnet
Signed-off-by: Andy Doan <doanac@qti.qualcomm.com> Assisted-by: GitHub Copilot:claude-4-sonnet
This set of changes adds support for oauth2 device flow to streamline the CLI login process. It also sets the basis for followup work to support lmp-device-register