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 c87ace154..e7b0dcf99 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, @@ -199,7 +198,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)) + }) +}