From a5fbaed70240fb8d2743baf34afb9e27092cf772 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 27 Oct 2026 16:04:44 +0000 Subject: [PATCH] api/v2: list token-made auth keys to a token without all=true Keys an OAuth token makes are marked so. No listing holds an invalid key. --- hscontrol/api/v2/keys.go | 17 ++++- hscontrol/apiv2_compat_test.go | 14 +--- hscontrol/apiv2_keys_test.go | 4 +- hscontrol/apiv2_oauth_test.go | 67 ++++++++++++++++++- hscontrol/db/db_test.go | 47 +++++++++++++ hscontrol/db/preauth_keys.go | 26 +++++++ hscontrol/db/preauth_keys_test.go | 25 +++++++ hscontrol/servertest/lifecycle_compat_test.go | 9 +-- hscontrol/state/state.go | 5 ++ 9 files changed, 189 insertions(+), 25 deletions(-) diff --git a/hscontrol/api/v2/keys.go b/hscontrol/api/v2/keys.go index 9f4c67243..cf5fe953f 100644 --- a/hscontrol/api/v2/keys.go +++ b/hscontrol/api/v2/keys.go @@ -104,7 +104,7 @@ type ( listKeysInput struct { Tailnet string `path:"tailnet"` - All bool `doc:"Accepted for compatibility; Headscale returns all keys." query:"all"` + All bool `doc:"Without it, a token sees only the auth keys tokens made." query:"all"` } keyByIDInput struct { @@ -199,7 +199,15 @@ func registerKeys(api huma.API, b Backend) { return nil, huma.Error500InternalServerError("listing auth keys", err) } + now := time.Now() + + // As Tailscale lists them: never an invalid key, and to a token + // without all=true only the keys tokens made. for i := range keys { + if keys[i].ValidAt(now) != nil || isOAuth && !in.All && !keys[i].CreatedByToken { + continue + } + out.Body.Keys = append(out.Body.Keys, keyFromStored(&keys[i])) } } @@ -412,7 +420,12 @@ func createAuthKey(ctx context.Context, b Backend, body CreateKeyRequest) (*keyO expiration := time.Now().Add(expiryDuration(body.ExpirySeconds)) - pak, err := b.State.CreatePreAuthKey( + createKey := b.State.CreatePreAuthKey + if isOAuth { + createKey = b.State.CreateTokenPreAuthKey + } + + pak, err := createKey( userID, create.Reusable, create.Ephemeral, diff --git a/hscontrol/apiv2_compat_test.go b/hscontrol/apiv2_compat_test.go index 689889b42..c21c64e20 100644 --- a/hscontrol/apiv2_compat_test.go +++ b/hscontrol/apiv2_compat_test.go @@ -87,16 +87,7 @@ var apiv2CompatSkipReasons = map[string]string{ "oauth/T5/2/tags": retagRemovesUnownedTag, "oauth/T5/2/owner": retagRemovesUnownedTag, - "oauth/D0/1/listed": keyListWithoutAll, - "oauth/B11/0/listed": keyListWithoutAll, - "oauth/B11/2/listed": keyListWithoutAll, - "oauth/D0/2/listed": keyListInvalidKeys, - - "key-list/L-untagged/1/listed": keyListWithoutAll, - "key-list/L-prod/1/listed": keyListWithoutAll, - "key-list/L-prod-server/1/listed": keyListWithoutAll, - "key-list/L-make-prod/0/listed": keyListWithoutAll, - "key-list/L-make-prod-server/0/listed": keyListWithoutAll, + "oauth/D0/1/listed": emptyKeyListNull, "oauth/D7/2/status": deletedClientGone, "oauth/D7/2/outcome": deletedClientGone, @@ -197,8 +188,7 @@ const ( mintTagsNotEchoed = "TOKEN_TAGS_NOT_ECHOED: SaaS's token answer names the token's tags; headscale's has no tags" unknownKeyType = "UNKNOWN_KEY_TYPE: SaaS refuses an unknown keyType with 400 before it checks scopes; headscale checks the scope first and answers 403" tagRefusalStatus = "TAG_REFUSAL_STATUS: SaaS refuses a tag outside the token's grant with 400 requested tags are invalid or not permitted; headscale with 403" - keyListWithoutAll = "KEY_LIST_WITHOUT_ALL: without all=true SaaS lists an OAuth token only the auth keys OAuth tokens made, whatever its tags and theirs; headscale lists every key" - keyListInvalidKeys = "KEY_LIST_INVALID_KEYS: SaaS leaves spent and revoked auth keys out of a listing; headscale lists them" + emptyKeyListNull = "EMPTY_KEY_LIST_NULL: SaaS answers a listing that holds no key with keys null; headscale with []" deletedClientGone = "DELETED_CLIENT_GONE: SaaS keeps a deleted OAuth client readable as invalid and revoked; headscale removes it" deletedClientCredentialText = "DELETED_CLIENT_CREDENTIAL_TEXT: SaaS refuses a deleted client's secret and tokens with API token invalid; headscale says invalid client credentials and unauthorized" missingScopeText = "MISSING_SCOPE_TEXT: SaaS refuses a delete outside the token's scopes with calling actor does not have enough permissions; headscale names the missing scope" diff --git a/hscontrol/apiv2_keys_test.go b/hscontrol/apiv2_keys_test.go index 2c46741e9..0aefccd0f 100644 --- a/hscontrol/apiv2_keys_test.go +++ b/hscontrol/apiv2_keys_test.go @@ -286,11 +286,11 @@ func TestAPIv2Key_Delete(t *testing.T) { require.Equal(t, http.StatusOK, api.Delete("/api/v2/tailnet/-/keys/"+created.ID).Code) // DELETE soft-revokes (Tailscale-faithful): the key stays retrievable with - // invalid set and a revoked timestamp, and is still listed. + // invalid set and a revoked timestamp, but listings leave it out. got := getKey(t, api, created.ID) assert.True(t, got.Invalid, "revoked key reports invalid") assert.NotNil(t, got.Revoked, "revoked timestamp is populated") - assert.True(t, containsKeyID(listKeys(t, api), created.ID), "revoked key still listed") + assert.False(t, containsKeyID(listKeys(t, api), created.ID), "revoked key not listed") // Server-side: row kept, revoked stamped. pak := srvKey(t, app, created.ID) diff --git a/hscontrol/apiv2_oauth_test.go b/hscontrol/apiv2_oauth_test.go index 08150ed09..7df1a1d9d 100644 --- a/hscontrol/apiv2_oauth_test.go +++ b/hscontrol/apiv2_oauth_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "slices" "strconv" "strings" "testing" @@ -338,6 +339,69 @@ func TestAPIv2OAuth_TaggedKeyCreator(t *testing.T) { assert.False(t, nv.AuthKey().User().Valid()) } +// TestAPIv2OAuth_KeyListing: a listing never holds an invalid key; without +// all=true a token sees only the auth keys tokens made, whatever their tags. +func TestAPIv2OAuth_KeyListing(t *testing.T) { + app, baseURL, admin := newOAuthTestServer(t) + + adminKey := createAdminAuthKey(t, baseURL, admin) + + revoked := createAdminAuthKey(t, baseURL, admin) + status, _ := apiReq(t, http.MethodDelete, baseURL+"/api/v2/tailnet/-/keys/"+revoked, admin, nil) + require.Equal(t, http.StatusOK, status) + + maker := mintScopedToken(t, baseURL, admin, scope.AuthKeys) + require.Equal(t, http.StatusOK, createTaggedKey(t, baseURL, maker, []string{"tag:ci"})) + + keys, err := app.state.ListPreAuthKeys() + require.NoError(t, err) + + var tokenKey string + + for _, k := range keys { + if k.CreatedByToken { + tokenKey = k.StringID() + } + } + + require.NotEmpty(t, tokenKey, "the token's key is marked") + + _, readerSecret := createClient(t, baseURL, admin, []string{"auth_keys:read"}, nil) + reader := tokenFor(t, baseURL, readerSecret) + + listed := func(bearer, query string) []string { + t.Helper() + + status, body := apiReq(t, http.MethodGet, baseURL+"/api/v2/tailnet/-/keys"+query, bearer, nil) + require.Equalf(t, http.StatusOK, status, "body: %s", body) + + var out struct{ Keys []apiv2.Key } + require.NoError(t, json.Unmarshal(body, &out)) + + ids := make([]string, 0, len(out.Keys)) + for _, k := range out.Keys { + if k.KeyType == "auth" { + ids = append(ids, k.ID) + } + } + + slices.Sort(ids) + + return ids + } + + every := []string{adminKey, tokenKey} + slices.Sort(every) + + for _, bearer := range []string{reader, maker} { + assert.Equal(t, []string{tokenKey}, listed(bearer, "")) + assert.Equal(t, every, listed(bearer, "?all=true")) + } + + assert.Equal(t, every, listed(admin, "")) + assert.Equal(t, every, listed(admin, "?all=true")) +} + // TestAPIv2OAuth_UnknownKeyTypeRefused: a keyType other than auth or client // is refused, not taken as an auth key. func TestAPIv2OAuth_UnknownKeyTypeRefused(t *testing.T) { @@ -676,8 +740,9 @@ func TestAPIv2OAuth_KeysMultiplexIsolation(t *testing.T) { clientID, _ := createClient(t, baseURL, admin, []string{"oauth_keys"}, []string{"tag:ci"}) require.Equal(t, http.StatusOK, createTaggedKey(t, baseURL, admin, []string{"tag:ci"})) + // all=true, so a token sees the admin's auth key too. listKinds := func(bearer string) map[string]int { - st, body := apiReq(t, http.MethodGet, keysURL, bearer, nil) + st, body := apiReq(t, http.MethodGet, keysURL+"?all=true", bearer, nil) require.Equalf(t, http.StatusOK, st, "%s", body) var out struct { diff --git a/hscontrol/db/db_test.go b/hscontrol/db/db_test.go index fb1acfca5..dbb7636b8 100644 --- a/hscontrol/db/db_test.go +++ b/hscontrol/db/db_test.go @@ -705,6 +705,53 @@ func TestSQLiteMigrationToCredentialsIsAtomic(t *testing.T) { assert.True(t, pak.Reusable) } +// TestSQLiteMigrationAddsCreatedByToken upgrades a database whose credentials +// table predates the created_by_token column: existing keys read as not made +// by a token, and the schema matches schema.sql again. +func TestSQLiteMigrationAddsCreatedByToken(t *testing.T) { + dbPath := t.TempDir() + "/headscale_test.db" + cfg := &types.Config{ + Database: types.DatabaseConfig{ + Type: "sqlite3", + Sqlite: types.SqliteConfig{Path: dbPath}, + }, + Policy: types.PolicyConfig{Mode: types.PolicyModeDB}, + } + + hsdb, err := NewHeadscaleDatabase(cfg) + require.NoError(t, err) + + pak, err := hsdb.CreatePreAuthKey(nil, true, false, nil, []string{"tag:ci"}) + require.NoError(t, err) + require.NoError(t, hsdb.Close()) + + raw, err := sql.Open("sqlite", dbPath) + require.NoError(t, err) + + defer raw.Close() + + for _, stmt := range []string{ + `ALTER TABLE credentials DROP COLUMN created_by_token`, + `DELETE FROM migrations WHERE id = '202610221200-credential-created-by-token'`, + } { + _, err = raw.ExecContext(context.Background(), stmt) + require.NoError(t, err, stmt) + } + + hsdb, err = NewHeadscaleDatabase(cfg) + require.NoError(t, err) + + got, err := hsdb.GetPreAuthKeyByID(pak.ID) + require.NoError(t, err) + assert.False(t, got.CreatedByToken) + + made, err := hsdb.CreateTokenPreAuthKey(nil, true, false, nil, []string{"tag:ci"}) + require.NoError(t, err) + got, err = hsdb.GetPreAuthKeyByID(made.ID) + require.NoError(t, err) + assert.True(t, got.CreatedByToken) +} + // TestSQLiteRejectsPre029Database ensures a real pre-0.29 database is refused // rather than silently skipping the migrations that were removed. func TestSQLiteRejectsPre029Database(t *testing.T) { diff --git a/hscontrol/db/preauth_keys.go b/hscontrol/db/preauth_keys.go index 77d71129e..37647bb4c 100644 --- a/hscontrol/db/preauth_keys.go +++ b/hscontrol/db/preauth_keys.go @@ -57,6 +57,32 @@ func (hsdb *HSDatabase) CreatePreAuthKey( }) } +// CreateTokenPreAuthKey is [HSDatabase.CreatePreAuthKey] for a key an OAuth +// access token makes, marked as such in the same transaction. +func (hsdb *HSDatabase) CreateTokenPreAuthKey( + uid *types.UserID, + reusable bool, + ephemeral bool, + expiration *time.Time, + aclTags []string, +) (*types.PreAuthKeyNew, error) { + return Write(hsdb.DB, func(tx *gorm.DB) (*types.PreAuthKeyNew, error) { + pak, err := CreatePreAuthKey(tx, uid, reusable, ephemeral, expiration, aclTags) + if err != nil { + return nil, err + } + + err = tx.Model(&types.Credential{}). + Where("id = ?", pak.ID). + Update("created_by_token", true).Error + if err != nil { + return nil, fmt.Errorf("marking key as made by a token: %w", err) + } + + return pak, nil + }) +} + const authKeyPrefix = "hskey-auth-" // CreatePreAuthKey creates a new [types.PreAuthKey] in a user, and returns it. diff --git a/hscontrol/db/preauth_keys_test.go b/hscontrol/db/preauth_keys_test.go index a60d8114e..dd89444ba 100644 --- a/hscontrol/db/preauth_keys_test.go +++ b/hscontrol/db/preauth_keys_test.go @@ -162,6 +162,31 @@ func TestDestroyPreAuthKeyKeepsReferencedEphemeralKey(t *testing.T) { require.ErrorIs(t, err, gorm.ErrRecordNotFound) } +// TestCreateTokenPreAuthKeyIsAtomic: a key a token makes is marked in the +// transaction that creates it, so a failed mark leaves no unmarked key. +func TestCreateTokenPreAuthKeyIsAtomic(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + + pak, err := db.CreateTokenPreAuthKey(nil, true, false, nil, []string{"tag:ci"}) + require.NoError(t, err) + + got, err := db.GetPreAuthKeyByID(pak.ID) + require.NoError(t, err) + assert.True(t, got.CreatedByToken) + + require.NoError(t, db.DB.Exec(`CREATE TRIGGER refuse_mark + BEFORE UPDATE OF created_by_token ON credentials + BEGIN SELECT RAISE(ABORT, 'refused'); END`).Error) + + _, err = db.CreateTokenPreAuthKey(nil, true, false, nil, []string{"tag:ci"}) + require.Error(t, err) + + keys, err := db.ListPreAuthKeys() + require.NoError(t, err) + assert.Len(t, keys, 1, "the failed create left a key behind") +} + func TestCannotDeleteAssignedPreAuthKey(t *testing.T) { db, err := newSQLiteTestDB() require.NoError(t, err) diff --git a/hscontrol/servertest/lifecycle_compat_test.go b/hscontrol/servertest/lifecycle_compat_test.go index 2ad439789..f4621a09b 100644 --- a/hscontrol/servertest/lifecycle_compat_test.go +++ b/hscontrol/servertest/lifecycle_compat_test.go @@ -35,14 +35,7 @@ import ( // /[/]/, with "collection" for // the step of what killed nodes leave behind. A listed check that matches // fails, so a fix must delete its entry. -var lifecycleSkipReasons = map[string]string{ - "ephemeral-lifecycle/E3-key-delete/2/keys": deletedKeyListed, - "ephemeral-lifecycle/E3-key-delete/3/keys": deletedKeyListed, -} - -const ( - deletedKeyListed = "DELETED_KEY_LISTED: headscale lists a deleted auth key; SaaS leaves it out of both key lists, while a read by ID still answers" -) +var lifecycleSkipReasons = map[string]string{} // TestRouteApprovalCompat replays a user-owned router through admin // approvals and revokes, same-key and forced re-auth, a hostname change, diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 5d5f6bfda..4b20d4fa6 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -2051,6 +2051,11 @@ func (s *State) CreatePreAuthKey(userID *types.UserID, reusable bool, ephemeral return s.db.CreatePreAuthKey(userID, reusable, ephemeral, expiration, aclTags) } +// CreateTokenPreAuthKey creates a key an OAuth access token makes. +func (s *State) CreateTokenPreAuthKey(userID *types.UserID, reusable bool, ephemeral bool, expiration *time.Time, aclTags []string) (*types.PreAuthKeyNew, error) { + return s.db.CreateTokenPreAuthKey(userID, reusable, ephemeral, expiration, aclTags) +} + // Test helpers for the state layer // CreateUserForTest creates a test user. This is a convenience wrapper around the database layer.