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

CHANGELOG.md

@@ -20,6 +20,7 @@
 
 ### Bugs
 
+* Fix `AuthWorkflow: device_code` never getting a refresh token, so sessions ended after 8 hours #1508
 * Remove `--no-config-check` and `--sts-refresh` from the documented `login` flags #1451
 * Fix documentation / examples related to using the ECS server with Docker Compose #1462
 * Document `ssh -o ExitOnForwardFailure=yes` to prevent another user on the remote host from

cmd/aws-sso/helpers_e2e_test.go

@@ -234,6 +234,7 @@ func preAuth(t *testing.T, setup *e2eSetup) {
 			storage.GrantTypeDeviceCode,
 			storage.GrantTypeRefreshToken,
 		},
+		Scopes: []string{storage.SSOAccountAccessScope},
 	})
 	require.NoError(t, err)
 
@@ -261,6 +262,7 @@ func preAuthExpired(t *testing.T, setup *e2eSetup, refreshToken string) {
 			storage.GrantTypeDeviceCode,
 			storage.GrantTypeRefreshToken,
 		},
+		Scopes: []string{storage.SSOAccountAccessScope},
 	})
 	require.NoError(t, err)
 

cmd/aws-sso/login_e2e_test.go

@@ -21,6 +21,7 @@ package main
  */
 
 import (
+	"context"
 	"testing"
 	"time"
 
@@ -149,6 +150,77 @@ func TestE2ELogin_Force(t *testing.T) {
 	assert.False(t, ctr.Expired())
 }
 
+// TestE2ELogin_ForceScopelessRegistration is the upgrade path: a device_code
+// user whose registration was made before we sent the sso:account:access scope
+// still has a valid token. --force must still log out the old session once, and
+// the re-authentication must replace the registration with one that has the scope.
+func TestE2ELogin_ForceScopelessRegistration(t *testing.T) {
+	setup := newE2ESetup(t)
+	preAuth(t, setup)
+
+	// overwrite the seeded registration with one made without the scope
+	require.NoError(t, setup.Store.SaveRegisterClientData(context.Background(), AwsSSO.StoreKey(), storage.RegisterClientData{
+		ClientId:              "old-client-id",
+		ClientSecret:          "old-client-secret",
+		ClientIdIssuedAt:      time.Now().Unix(),
+		ClientSecretExpiresAt: time.Now().Add(90 * 24 * time.Hour).Unix(),
+		GrantTypes: []storage.GrantType{
+			storage.GrantTypeDeviceCode,
+			storage.GrantTypeRefreshToken,
+		},
+	}))
+
+	setup.Server.SSO.QueueLogout()
+	setup.Server.SSOOIDC.QueueRegisterClient(awsmock.RegisterClientResponse{
+		ClientID:              "new-client-id",
+		ClientSecret:          "new-client-secret",
+		ClientIDIssuedAt:      time.Now().Unix(),
+		ClientSecretExpiresAt: time.Now().Add(90 * 24 * time.Hour).Unix(),
+	})
+	setup.Server.SSOOIDC.QueueDeviceAuth(awsmock.DeviceAuthResponse{
+		DeviceCode:              "device-code",
+		UserCode:                "CODE-1234",
+		VerificationURI:         "https://verify.example.com",
+		VerificationURIComplete: "https://verify.example.com?user_code=CODE-1234",
+		ExpiresIn:               600,
+		Interval:                0,
+	})
+	setup.Server.SSOOIDC.QueueCreateToken(awsmock.OIDCTokenResponse{
+		AccessToken:  "forced-access-token",
+		ExpiresIn:    28800,
+		RefreshToken: "forced-refresh-token",
+		TokenType:    "Bearer",
+	})
+	setup.Server.SSO.QueueListAccounts(awsmock.ListAccountsResponse{
+		AccountList: []awsmock.AccountInfo{
+			{AccountID: "123456789012", AccountName: "TestAccount", EmailAddress: "admin@example.com"},
+		},
+	})
+	setup.Server.SSO.QueueListAccountRoles(awsmock.ListAccountRolesResponse{
+		RoleList: []awsmock.RoleInfo{
+			{AccountID: "123456789012", RoleName: "ReadOnly"},
+		},
+	})
+
+	ctx := newRunContext(setup, AUTH_SKIP)
+	ctx.Cli.Login = LoginCmd{UrlAction: "print", Threads: 1, Force: true}
+
+	cmd := &LoginCmd{}
+	require.NoError(t, cmd.Run(ctx))
+
+	assert.Equal(t, 1, setup.Server.SSO.LogoutCalls())
+
+	var rcd storage.RegisterClientData
+	require.NoError(t, setup.Store.GetRegisterClientData(AwsSSO.StoreKey(), &rcd))
+	assert.Equal(t, "new-client-id", rcd.ClientId)
+	assert.True(t, rcd.SupportsScope(storage.SSOAccountAccessScope))
+
+	var ctr storage.CreateTokenResponse
+	require.NoError(t, setup.Store.GetCreateTokenResponse(AwsSSO.StoreKey(), &ctr))
+	assert.Equal(t, "forced-access-token", ctr.AccessToken)
+	assert.Equal(t, "forced-refresh-token", ctr.RefreshToken)
+}
+
 // TestE2ELogin_ForceLogoutFailureStillLogsIn verifies a failed Logout is not fatal.
 func TestE2ELogin_ForceLogoutFailureStillLogsIn(t *testing.T) {
 	setup := newE2ESetup(t)

internal/sso/auth/awssso_auth.go

@@ -38,7 +38,7 @@ const (
 	DEFAULT_AUTH_COLOR       = "blue"
 	DEFAULT_AUTH_ICON        = "fingerprint"
 	VERIFY_MSG               = "\n\tVerify this code in your browser: %s\n"
-	SSO_ACCOUNT_ACCESS_SCOPE = "sso:account:access"
+	SSO_ACCOUNT_ACCESS_SCOPE = storage.SSOAccountAccessScope
 	awsSSOClientName         = "aws-sso-cli"
 	awsSSOClientType         = "public"
 	// The default values for ODIC defined in:
@@ -176,10 +176,17 @@ func (as *AWSSSO) registerClient(ctx context.Context, force bool) error {
 	if !force {
 		log.Trace("Checking cache for RegisterClientData", "storeKey", as.StoreKey())
 		err := as.store.GetRegisterClientData(as.StoreKey(), &as.ClientData)
-		if err == nil && !as.ClientData.Expired() {
+		// Registrations made without the scope never get a refresh token, even
+		// though they list the refresh_token grant type, so replace them here.
+		// A still-valid token from such a registration keeps working until then.
+		if err == nil && !as.ClientData.Expired() && as.ClientData.SupportsScope(SSO_ACCOUNT_ACCESS_SCOPE) {
 			log.Debug("Using RegisterClient from secure store", "storeKey", as.StoreKey())
 			return nil
 		}
+		if err == nil && !as.ClientData.Expired() {
+			log.Debug("cached client registration lacks necessary scope; registering a new client",
+				"storeKey", as.StoreKey(), "missingScope", SSO_ACCOUNT_ACCESS_SCOPE)
+		}
 	}
 
 	log.Trace("Registering new client with AWS SSO", "ClientName", as.ClientName, "ClientType", as.ClientType)
@@ -188,9 +195,10 @@ func (as *AWSSSO) registerClient(ctx context.Context, force bool) error {
 		ClientType: as.ClientType,
 		GrantTypes: as.authGrantTypes(),
 		IssuerUrl:  as.StartUrl,
+		// Without this scope AWS does not issue refresh tokens, for either workflow.
+		Scopes: []string{SSO_ACCOUNT_ACCESS_SCOPE},
 	}
 	if as.getAuthWorkflow() == oidc.AuthWorkflowPKCE {
-		input.Scopes = []string{SSO_ACCOUNT_ACCESS_SCOPE}
 		input.RedirectUris = []string{as.pkceRedirectURIBase()}
 	}
 	resp, err := as.oidcClient.RegisterClient(ctx, input)
@@ -203,6 +211,7 @@ func (as *AWSSSO) registerClient(ctx context.Context, force bool) error {
 	// AWS does not echo back the grant types we registered with, so we record
 	// them ourselves so ValidAuthToken() can check for refresh_token support.
 	as.ClientData.GrantTypes = as.GrantTypes()
+	as.ClientData.Scopes = input.Scopes
 	log.Trace("SaveRegisterClientData start", "storeKey", as.StoreKey())
 	err = as.store.SaveRegisterClientData(ctx, as.StoreKey(), as.ClientData)
 	if err != nil {

internal/sso/auth/awssso_auth_test.go

@@ -457,6 +457,7 @@ func TestValidAuthTokenPKCE(t *testing.T) { // nolint:dupl
 		ClientId:              "test-client-id",
 		ClientSecret:          "test-client-secret",
 		ClientSecretExpiresAt: 99999999999,
+		Scopes:                []string{SSO_ACCOUNT_ACCESS_SCOPE},
 		GrantTypes:            []storage.GrantType{storage.GrantTypeAuthorizationCode, storage.GrantTypeRefreshToken},
 	}
 	err = jstore.SaveRegisterClientData(context.Background(), key, clientData)
@@ -484,6 +485,7 @@ func TestValidAuthTokenDeviceCode(t *testing.T) { // nolint:dupl
 		ClientId:              "test-client-id",
 		ClientSecret:          "test-client-secret",
 		ClientSecretExpiresAt: 99999999999,
+		Scopes:                []string{SSO_ACCOUNT_ACCESS_SCOPE},
 		GrantTypes:            []storage.GrantType{storage.GrantTypeDeviceCode, storage.GrantTypeRefreshToken},
 	}
 	err = jstore.SaveRegisterClientData(context.Background(), key, clientData)
@@ -499,6 +501,18 @@ func TestValidAuthTokenDeviceCode(t *testing.T) { // nolint:dupl
 	err = jstore.SaveRegisterClientData(context.Background(), key, clientData)
 	assert.NoError(t, err)
 	assert.False(t, as.ValidAuthToken(context.Background()))
+
+	// A registration made before we sent the scope never gets a refresh token,
+	// but its token still works. Keep both until the next re-authentication
+	// (registerClient replaces the registration then).
+	clientData.GrantTypes = []storage.GrantType{storage.GrantTypeDeviceCode, storage.GrantTypeRefreshToken}
+	clientData.Scopes = nil
+	err = jstore.SaveRegisterClientData(context.Background(), key, clientData)
+	assert.NoError(t, err)
+	assert.True(t, as.ValidAuthToken(context.Background()))
+	got := storage.RegisterClientData{}
+	assert.NoError(t, jstore.GetRegisterClientData(key, &got), "scope-less registration should be kept")
+	assert.Equal(t, "test-client-id", got.ClientId)
 }
 
 func TestAuthenticateFailure(t *testing.T) {
@@ -998,6 +1012,166 @@ func TestRegisterClientPKCE(t *testing.T) {
 	}
 }
 
+func TestRegisterClientDeviceCode(t *testing.T) {
+	tfile, err := os.CreateTemp("", "*storage.json")
+	assert.NoError(t, err)
+	defer os.Remove(tfile.Name())
+
+	jstore, err := storage.OpenJsonStore(context.Background(), tfile.Name())
+	assert.NoError(t, err)
+
+	mock := &mockOIDCClient{
+		registerClientResult: storage.RegisterClientData{ // #nosec G101
+			ClientId:              "dc-client-id",
+			ClientSecret:          "dc-client-secret",
+			ClientSecretExpiresAt: time.Now().Add(time.Hour).Unix(),
+		},
+	}
+
+	as := &AWSSSO{
+		key:        "test",
+		SsoRegion:  "us-west-2",
+		StartUrl:   "https://d-test.awsapps.com/start",
+		ClientName: awsSSOClientName,
+		ClientType: awsSSOClientType,
+		store:      jstore,
+		oidcClient: mock,
+		SSOConfig:  &ssoconfig.SSOConfig{AuthWorkflow: oidc.AuthWorkflowDeviceCode},
+	}
+
+	err = as.registerClient(context.Background(), true)
+	assert.NoError(t, err)
+
+	// Without this scope AWS does not issue a refresh token.
+	if assert.Len(t, mock.registerClientInputs, 1) {
+		in := mock.registerClientInputs[0]
+		assert.Equal(t, []string{SSO_ACCOUNT_ACCESS_SCOPE}, in.Scopes)
+		assert.Empty(t, in.RedirectUris)
+	}
+
+	// The scope is stored so ValidAuthToken() can tell old registrations apart.
+	stored := storage.RegisterClientData{}
+	assert.NoError(t, jstore.GetRegisterClientData(as.StoreKey(), &stored))
+	assert.True(t, stored.SupportsScope(SSO_ACCOUNT_ACCESS_SCOPE))
+}
+
+func TestRegisterClientCachedScope(t *testing.T) {
+	newAS := func(t *testing.T, workflow oidc.AuthWorkflow, cached storage.RegisterClientData) (*AWSSSO, *mockOIDCClient, storage.SecureStorage, string) {
+		t.Helper()
+		tfile, err := os.CreateTemp("", "*storage.json")
+		require.NoError(t, err)
+		t.Cleanup(func() { _ = os.Remove(tfile.Name()) }) // nolint:gosec
+		jstore, err := storage.OpenJsonStore(context.Background(), tfile.Name())
+		require.NoError(t, err)
+
+		mock := &mockOIDCClient{
+			registerClientResult: storage.RegisterClientData{ // #nosec G101
+				ClientId:              "new-client-id",
+				ClientSecret:          "new-client-secret",
+				ClientSecretExpiresAt: time.Now().Add(time.Hour).Unix(),
+			},
+		}
+		as := &AWSSSO{
+			key:        "test",
+			SsoRegion:  "us-west-2",
+			StartUrl:   "https://d-test.awsapps.com/start",
+			ClientName: awsSSOClientName,
+			ClientType: awsSSOClientType,
+			store:      jstore,
+			oidcClient: mock,
+			SSOConfig:  &ssoconfig.SSOConfig{AuthWorkflow: workflow},
+		}
+		require.NoError(t, jstore.SaveRegisterClientData(context.Background(), as.StoreKey(), cached))
+		return as, mock, jstore, tfile.Name()
+	}
+	cached := func(scopes []string) storage.RegisterClientData {
+		return storage.RegisterClientData{ // #nosec G101
+			ClientId:              "old-client-id",
+			ClientSecret:          "old-client-secret",
+			ClientSecretExpiresAt: time.Now().Add(90 * 24 * time.Hour).Unix(),
+			GrantTypes:            []storage.GrantType{storage.GrantTypeDeviceCode, storage.GrantTypeRefreshToken},
+			Scopes:                scopes,
+		}
+	}
+
+	t.Run("scope-less registration is replaced", func(t *testing.T) {
+		as, mock, jstore, _ := newAS(t, oidc.AuthWorkflowDeviceCode, cached(nil))
+		require.NoError(t, as.registerClient(context.Background(), false))
+
+		require.Len(t, mock.registerClientInputs, 1)
+		assert.Equal(t, []string{SSO_ACCOUNT_ACCESS_SCOPE}, mock.registerClientInputs[0].Scopes)
+		stored := storage.RegisterClientData{}
+		require.NoError(t, jstore.GetRegisterClientData(as.StoreKey(), &stored))
+		assert.Equal(t, "new-client-id", stored.ClientId)
+		assert.True(t, stored.SupportsScope(SSO_ACCOUNT_ACCESS_SCOPE))
+	})
+
+	t.Run("registration with the scope is reused", func(t *testing.T) {
+		as, mock, _, _ := newAS(t, oidc.AuthWorkflowDeviceCode, cached([]string{SSO_ACCOUNT_ACCESS_SCOPE}))
+		require.NoError(t, as.registerClient(context.Background(), false))
+
+		assert.Empty(t, mock.registerClientInputs)
+		assert.Equal(t, "old-client-id", as.ClientData.ClientId)
+	})
+
+	t.Run("scope-less PKCE registration reloaded from the store file is reused", func(t *testing.T) {
+		// Older versions registered PKCE clients with the scope but did not
+		// record it. The JSON has no "scopes", and UnmarshalJSON backfills it.
+		pkce := cached(nil)
+		pkce.GrantTypes = []storage.GrantType{storage.GrantTypeAuthorizationCode, storage.GrantTypeRefreshToken}
+		as, mock, _, fname := newAS(t, oidc.AuthWorkflowPKCE, pkce)
+
+		// reopen the store so the registration is read back from the file
+		jstore, err := storage.OpenJsonStore(context.Background(), fname)
+		require.NoError(t, err)
+		as.store = jstore
+
+		require.NoError(t, as.registerClient(context.Background(), false))
+		assert.Empty(t, mock.registerClientInputs)
+		assert.Equal(t, "old-client-id", as.ClientData.ClientId)
+	})
+}
+
+func TestReauthenticateReplacesScopelessRegistration(t *testing.T) {
+	// An upgraded device_code user: expired token with no refresh token, and a
+	// registration made without the scope. Re-authentication must register a
+	// new client, or the next token has no refresh token either.
+	as, _, jstore := authTokenSetup(t, oidc.AuthWorkflowDeviceCode)
+	as.key = "test"
+	key := as.StoreKey()
+	require.NoError(t, jstore.SaveRegisterClientData(context.Background(), key, storage.RegisterClientData{ // #nosec G101
+		ClientId:              "old-client-id",
+		ClientSecret:          "old-client-secret",
+		ClientSecretExpiresAt: time.Now().Add(90 * 24 * time.Hour).Unix(),
+		GrantTypes:            []storage.GrantType{storage.GrantTypeDeviceCode, storage.GrantTypeRefreshToken},
+	}))
+	require.NoError(t, jstore.SaveCreateTokenResponse(context.Background(), key, storage.CreateTokenResponse{
+		AccessToken: "expired",
+		ExpiresAt:   time.Now().Add(-time.Hour).Unix(),
+	}))
+	assert.False(t, as.ValidAuthToken(context.Background()))
+
+	mock := &mockOIDCClient{
+		registerClientResult: storage.RegisterClientData{ // #nosec G101
+			ClientId:              "new-client-id",
+			ClientSecret:          "new-client-secret",
+			ClientSecretExpiresAt: time.Now().Add(time.Hour).Unix(),
+		},
+		// StartDeviceAuthorization succeeds with no verification URL, so the flow
+		// stops in getDeviceAuthInfo, before the forced re-register on error.
+	}
+	as.oidcClient = mock
+	as.ClientName = awsSSOClientName
+	as.ClientType = awsSSOClientType
+
+	assert.Error(t, as.reauthenticate(context.Background()))
+	require.Len(t, mock.registerClientInputs, 1)
+	assert.Equal(t, []string{SSO_ACCOUNT_ACCESS_SCOPE}, mock.registerClientInputs[0].Scopes)
+	stored := storage.RegisterClientData{}
+	require.NoError(t, jstore.GetRegisterClientData(key, &stored))
+	assert.Equal(t, "new-client-id", stored.ClientId)
+}
+
 func TestReauthenticatePKCE(t *testing.T) {
 	t.Run("success", func(t *testing.T) {
 		tfile, err := os.CreateTemp("", "*storage.json")
@@ -1272,6 +1446,7 @@ func TestValidAuthTokenRefresh(t *testing.T) {
 			ClientId:              "cid",
 			ClientSecret:          "csecret",
 			ClientSecretExpiresAt: time.Now().Add(time.Hour).Unix(),
+			Scopes:                []string{SSO_ACCOUNT_ACCESS_SCOPE},
 			GrantTypes:            []storage.GrantType{storage.GrantTypeAuthorizationCode, storage.GrantTypeRefreshToken},
 		}
 		err = jstore.SaveRegisterClientData(context.Background(), as.StoreKey(), clientData)
@@ -1319,6 +1494,7 @@ func TestValidAuthTokenRefresh(t *testing.T) {
 			ClientId:              "cid",
 			ClientSecret:          "csecret",
 			ClientSecretExpiresAt: time.Now().Add(time.Hour).Unix(),
+			Scopes:                []string{SSO_ACCOUNT_ACCESS_SCOPE},
 			GrantTypes:            []storage.GrantType{storage.GrantTypeAuthorizationCode, storage.GrantTypeRefreshToken},
 		}
 		err = jstore.SaveRegisterClientData(context.Background(), as.StoreKey(), clientData)
@@ -1357,6 +1533,7 @@ func TestValidAuthTokenRefresh(t *testing.T) {
 			ClientId:              "cid",
 			ClientSecret:          "csecret",
 			ClientSecretExpiresAt: time.Now().Add(time.Hour).Unix(),
+			Scopes:                []string{SSO_ACCOUNT_ACCESS_SCOPE},
 			GrantTypes:            []storage.GrantType{storage.GrantTypeAuthorizationCode, storage.GrantTypeRefreshToken},
 		}
 		err = jstore.SaveRegisterClientData(context.Background(), as.StoreKey(), clientData)