From e2ddea6b4f4e123e135ac661fd0e769b1c18d271 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 17 Oct 2026 15:52:17 +0000 Subject: [PATCH] db: keep at most 1000 access tokens per OAuth client Minting needs only the client secret, whatever its scopes, and added one row per mint. Each mint now ends tokens past the newest 1000. --- docs/ref/api.md | 4 ++- hscontrol/api/v2/oauth.go | 4 +-- hscontrol/apiv2_compat_test.go | 2 -- hscontrol/apiv2_oauth_test.go | 5 ++++ hscontrol/db/oauth.go | 25 +++++++++++++++- hscontrol/db/oauth_test.go | 52 ++++++++++++++++++++++++++++++++++ 6 files changed, 86 insertions(+), 6 deletions(-) diff --git a/docs/ref/api.md b/docs/ref/api.md index f1cd4c44f..858aa1d1b 100644 --- a/docs/ref/api.md +++ b/docs/ref/api.md @@ -67,7 +67,9 @@ secret for an access token, mints a single-use tagged auth key and registers wit it. This works anywhere the client takes an auth key: `tailscale up`, the container image, `tsnet` and the [`tailscale/github-action`](https://github.com/tailscale/github-action). One -long-lived secret joins any number of nodes. +long-lived secret joins any number of nodes. A client keeps its newest 1000 +access tokens and minting more ends the oldest, so give a long-lived integration +its own client. Create an OAuth client with the `auth_keys` scope and the tags its nodes get. The secret is shown once: diff --git a/hscontrol/api/v2/oauth.go b/hscontrol/api/v2/oauth.go index 452fed482..fb6085a51 100644 --- a/hscontrol/api/v2/oauth.go +++ b/hscontrol/api/v2/oauth.go @@ -103,7 +103,7 @@ func oauthTokenHandler(b Backend) http.HandlerFunc { expiry := time.Now().Add(accessTokenTTL) - tokenStr, _, err := b.State.MintAccessToken(client.ClientID, scopes, tags, &expiry) + tokenStr, token, err := b.State.MintAccessToken(client.ClientID, scopes, tags, &expiry) if err != nil { writeOAuthError(w, http.StatusInternalServerError, "server_error", "could not mint access token") @@ -114,7 +114,7 @@ func oauthTokenHandler(b Backend) http.HandlerFunc { AccessToken: tokenStr, TokenType: "Bearer", ExpiresIn: int(accessTokenTTL.Seconds()), - Scope: strings.Join(scopes, " "), + Scope: strings.Join(token.Scopes, " "), }) } } diff --git a/hscontrol/apiv2_compat_test.go b/hscontrol/apiv2_compat_test.go index 7c25347a7..44803ab7f 100644 --- a/hscontrol/apiv2_compat_test.go +++ b/hscontrol/apiv2_compat_test.go @@ -54,7 +54,6 @@ var apiv2CompatSkipReasons = map[string]string{ "oauth/K4/0/status": missingScopeNotFound, "oauth/D1/0/message": missingScopeText, - "oauth/O1/0/scope": scopeEchoAsSent, "oauth/O3/0/status": mintScopeRefusalStatus, "oauth/O4/0/scope": unknownScopeGranted, @@ -211,7 +210,6 @@ func notCompared(prefixes []string, key string) bool { // Differences a planned change removes. const ( - scopeEchoAsSent = "SCOPE_ECHO_AS_SENT: SaaS echoes a token's scopes sorted and without repeats; headscale echoes the request as sent" retagRemovesUnownedTag = "RETAG_REMOVES_UNOWNED_TAG: SaaS refuses a token's retag of a device holding a tag outside the token's grant with 400 cannot remove current tags; headscale replaces the tags" ) diff --git a/hscontrol/apiv2_oauth_test.go b/hscontrol/apiv2_oauth_test.go index 39db71454..f3dbc9434 100644 --- a/hscontrol/apiv2_oauth_test.go +++ b/hscontrol/apiv2_oauth_test.go @@ -205,6 +205,11 @@ func TestAPIv2OAuth_TokenEndpoint(t *testing.T) { require.Equalf(t, http.StatusOK, statusBasic, "%v", mBasic) assert.NotEmpty(t, mBasic["access_token"]) + // The answer names the scopes stored on the token, each once. + statusDup, mDup := mintToken(t, baseURL, clientID, secret, strings.Repeat("auth_keys ", 2), false) + require.Equalf(t, http.StatusOK, statusDup, "%v", mDup) + assert.Equal(t, "auth_keys", mDup["scope"]) + // Bad credentials → RFC 6749 invalid_client. statusBad, mBad := mintToken(t, baseURL, clientID, "hskey-client-deadbeefdead-"+stringOf("0", 64), "", false) assert.Equal(t, http.StatusUnauthorized, statusBad) diff --git a/hscontrol/db/oauth.go b/hscontrol/db/oauth.go index 7e6e4d397..527760896 100644 --- a/hscontrol/db/oauth.go +++ b/hscontrol/db/oauth.go @@ -26,6 +26,11 @@ var ( ErrAccessTokenClientRevoked = errors.New("oauth access token issuing client revoked or deleted") ) +// maxAccessTokensPerClient bounds the access tokens kept for one OAuth client. +// Minting needs only the client secret, whatever its scopes, so each mint ends +// the client's tokens beyond the newest this many. +const maxAccessTokensPerClient = 1000 + // validateScopes rejects any value outside the vocabulary in [scope.Known]. An // unknown scope satisfies no requirement, so storing one would silently mint a // client with fewer permissions than asked for. Every offending scope is @@ -193,6 +198,12 @@ func (hsdb *HSDatabase) MintAccessToken( ) (string, *types.OAuthAccessToken, error) { tokenStr, prefix, hash := generateSecret(types.AccessTokenPrefix) + scopes = set.SetOf(scopes).Slice() + slices.Sort(scopes) + + tags = set.SetOf(tags).Slice() + slices.Sort(tags) + now := time.Now().UTC() cred := types.Credential{ Kind: types.CredentialOAuthToken, @@ -223,7 +234,19 @@ func (hsdb *HSDatabase) MintAccessToken( return ErrOAuthClientRevoked } - return tx.Save(&cred).Error + err = tx.Save(&cred).Error + if err != nil { + return err + } + + // Under the limit the subquery is NULL and nothing is deleted. + newestSurplus := tx.Model(&types.Credential{}).Select("id"). + Where("kind = ? AND client_id = ?", types.CredentialOAuthToken, clientID). + Order("id DESC").Limit(1).Offset(maxAccessTokensPerClient) + + return tx.Where("kind = ? AND client_id = ? AND id <= (?)", + types.CredentialOAuthToken, clientID, newestSurplus). + Delete(&types.Credential{}).Error }) if err != nil { return "", nil, fmt.Errorf("saving oauth access token: %w", err) diff --git a/hscontrol/db/oauth_test.go b/hscontrol/db/oauth_test.go index b070fc5bc..89062ae58 100644 --- a/hscontrol/db/oauth_test.go +++ b/hscontrol/db/oauth_test.go @@ -469,3 +469,55 @@ func TestOAuthClientCreateReportsEveryUnknownScope(t *testing.T) { assert.Contains(t, err.Error(), "devices:everything") assert.Contains(t, err.Error(), "EVIL:superuser") } + +// TestMintAccessTokenKeepsNewestPerClient: minting needs only the client +// secret, so the rows one client can hold must be bounded. +func TestMintAccessTokenKeepsNewestPerClient(t *testing.T) { + test := func(t *testing.T, db *HSDatabase) { + t.Helper() + + _, client, err := db.CreateOAuthClient([]string{"users:read"}, nil, "", nil) + require.NoError(t, err) + _, other, err := db.CreateOAuthClient([]string{"users:read"}, nil, "", nil) + require.NoError(t, err) + + future := time.Now().Add(time.Hour) + otherTok, _, err := db.MintAccessToken(other.ClientID, other.Scopes, nil, &future) + require.NoError(t, err) + + toks := make([]string, maxAccessTokensPerClient+2) + for i := range toks { + toks[i], _, err = db.MintAccessToken( + client.ClientID, []string{"users:read", "users:read"}, nil, &future, + ) + require.NoError(t, err) + } + + var rows int64 + require.NoError(t, db.DB.Model(&types.Credential{}). + Where("kind = ? AND client_id = ?", types.CredentialOAuthToken, client.ClientID). + Count(&rows).Error) + assert.EqualValues(t, maxAccessTokensPerClient, rows) + + for _, tok := range toks[:2] { + _, err = db.AuthenticateAccessToken(tok) + require.ErrorIs(t, err, ErrAccessTokenNotFound) + } + + for _, tok := range []string{toks[2], toks[len(toks)-1], otherTok} { + at, err := db.AuthenticateAccessToken(tok) + require.NoError(t, err) + assert.Equal(t, []string{"users:read"}, at.Scopes) + } + } + + t.Run("sqlite", func(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + + test(t, db) + }) + t.Run("postgres", func(t *testing.T) { + test(t, newPostgresTestDB(t)) + }) +}