Skip to content

Fix device_code login getting no refresh token (sessions end after 8 hours) - #1513

Merged
synfinatic merged 5 commits into
synfinatic:mainfrom
RyoKusnadi:fix/device-code-refresh-token-scope
Nov 6, 2026
Merged

synfinatic merged 5 commits into
synfinatic:mainfrom
RyoKusnadi:fix/device-code-refresh-token-scope

Conversation

@RyoKusnadi

@RyoKusnadi RyoKusnadi commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Problem

With AuthWorkflow: device_code (also the default over SSH/WSL), aws-sso login stores a token with no refresh token and expiresIn: 28800. The Identity Center session duration is ignored and you must log in again every 8 hours. PKCE logins do not have this problem.

How it happens: a user sets AuthWorkflow: device_code (or logs in over SSH), runs aws-sso login, and the stored token has an empty RefreshToken. Path: reauthenticate() -> registerClient() in internal/sso/auth/awssso_auth.go, which registered the OIDC client with no scopes for device_code.

Cause

registerClient() only set Scopes: ["sso:account:access"] for PKCE. AWS only issues refresh tokens to clients registered with that scope, even if refresh_token is in the grant types (the AWS CLI sets it too, via sso_registration_scopes). And registerClient(force=false) reused any cached, unexpired registration, so an old device_code registration would keep getting tokens with no refresh token.

Fix

  • registerClient() sends the scope for both workflows.
  • RegisterClientData records Scopes (like it already does for GrantTypes). registerClient(force=false) only reuses a cached registration that has the scope; otherwise it registers a new client. This is the one place registrations get reused, so Authenticate and the Unauthorized retry path are covered too.
  • ValidAuthToken() does not look at the scope. A still-valid token from an old registration keeps working, and login --force still logs out that session. The registration is replaced at the next real re-authentication.
  • Old stored PKCE registrations have no scopes field but were always registered with the scope, so UnmarshalJSON fills it in. They are not replaced.
  • SSO_ACCOUNT_ACCESS_SCOPE is now defined as storage.SSOAccountAccessScope, so there is one source.

Scope

StartPKCEAuthCodeFlow already requests the scope; no other call site registers clients. Not covered: whether AWS then returns a refresh token for a given Identity Center instance (needs a live account).

Testing

  • TestRegisterClientCachedScope: a cached scope-less registration is replaced (with the scope stored), one with the scope is reused, and a scope-less PKCE registration reloaded from the store file is reused.
  • TestReauthenticateReplacesScopelessRegistration: expired token with no refresh token plus a scope-less registration; re-authentication registers a new client with the scope.
  • TestValidAuthTokenDeviceCode: a valid token with a scope-less registration returns true and the registration is kept.
  • TestE2ELogin_ForceScopelessRegistration: upgraded user with a valid token runs login --force; one logout, new registration with the scope, token has a refresh token. Against the earlier version of this PR (check in ValidAuthToken) it fails with expected: 1, actual: 0 logouts.
  • TestRegisterClientDeviceCode and TestRegisterClientDataScopes from the first version.

Removing the scope check from registerClient makes the first two fail. preAuth / preAuthExpired seed the scope, like a new registration would.

Ran: go vet, gofmt -l, go test ./... and go test -tags e2etests ./cmd/aws-sso/ (with HOME pointed at a temp dir that has .config/aws-sso). All pass. Not run: a live AWS login (the tests use mocks only).
Environment: main @ 05d203b + this branch @ 74da2b0, go1.27.1, darwin/arm64

Risk

device_code users who upgrade keep their current session. Their next re-authentication registers a new client, after which tokens come with a refresh token. To roll back, revert this PR; stored data with a scopes field is still readable.

Fixes #1508

…t a refresh token

AWS only issues refresh tokens to clients registered with the
sso:account:access scope, but registerClient() sent it for PKCE only.
Send it for both workflows, record it in RegisterClientData, and replace
cached registrations that lack it.

Signed-off-by: Ryo Kusnadi <ryokusnadi@gmail.com>
@RyoKusnadi

Copy link
Copy Markdown
Contributor Author

@synfinatic friendly ping on this one when you have a moment. Happy to change anything.

Comment thread CHANGELOG.md Resolved
Comment thread internal/sso/auth/awssso_auth.go Resolved
…king token from an old registration is kept

Signed-off-by: Ryo Kusnadi <ryokusnadi@gmail.com>
@codecov

codecov Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.64%. Comparing base (05d203b) to head (ac0b823).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1513      +/-   ##
==========================================
+ Coverage   86.56%   86.64%   +0.08%     
==========================================
  Files          78       78              
  Lines        5200     5208       +8     
==========================================
+ Hits         4501     4512      +11     
+ Misses        472      470       -2     
+ Partials      227      226       -1     
Flag Coverage Δ
unittests 86.64% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
internal/sso/auth/awssso_auth.go 90.12% <100.00%> (+1.32%) ⬆️
internal/storage/storage.go 95.24% <100.00%> (+0.24%) ⬆️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05d203b...ac0b823. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@synfinatic

Copy link
Copy Markdown
Owner

@RyoKusnadi you really should be running make test before you push changes and not rely on my CI to find these issues.

Signed-off-by: Ryo Kusnadi <ryokusnadi@gmail.com>

@synfinatic synfinatic left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

other than the broken CI issues and the changelog still not conforming to the existing pattern, LGTM.

Comment thread CHANGELOG.md Resolved
Signed-off-by: Ryo Kusnadi <ryokusnadi@gmail.com>
@RyoKusnadi

Copy link
Copy Markdown
Contributor Author

Fair point, sorry about that. I ran go vet, the unit tests and the e2e tests but not golangci-lint, which is what caught both (an unused assignment and the gosec temp-file one). Fixed in 74da2b0, and the CHANGELOG entry is one line now (ac0b823).

@synfinatic

Copy link
Copy Markdown
Owner

Thanks for the contribution @RyoKusnadi. Typically I require all commits signed, but for first time contributors I do make an exception.

@synfinatic
synfinatic merged commit 2f92177 into synfinatic:main Nov 6, 2026
9 checks passed
@RyoKusnadi
RyoKusnadi deleted the fix/device-code-refresh-token-scope branch November 6, 2026 23:26
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.

device_code login gets no refresh token, so sessions end after 8 hours

2 participants