Skip to content

Add support for oauth2 device flow#186

Open
doanac wants to merge 4 commits into
mainfrom
oauth2-flow
Open

Add support for oauth2 device flow#186
doanac wants to merge 4 commits into
mainfrom
oauth2-flow

Conversation

@doanac

@doanac doanac commented Jun 25, 2026

Copy link
Copy Markdown
Member

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

@doanac doanac requested a review from vkhoroz June 25, 2026 14:10
Comment thread server/ui/web/templates/device_auth_confirm.html Dismissed
@doanac doanac force-pushed the oauth2-flow branch 2 times, most recently from 77350a5 to 7bf7608 Compare June 25, 2026 16:16
@doanac

doanac commented Jun 25, 2026

Copy link
Copy Markdown
Member Author

just force pushed several rounds of Claude code reviews for the branch.

Comment thread server/ui/api/handlers_test.go Outdated
return u.h.fs.Audit.ReadEvents(u.id)
}

func (u User) ApproveAuthorization(deviceCode string, tokenDescription string, scopes Scopes) error {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread auth/csrf.go
}

if c.Path() == "/oauth2/device/code" || c.Path() == "/oauth2/device/token" {
return next(c)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the problem with using CSRF for the Oauth2?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is device/api-client side.

Comment thread auth/provider_local.go Outdated
Andy Doan and others added 2 commits July 8, 2026 14:33
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
doanac added 2 commits July 8, 2026 15:24
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants