fix: enforce API key lifetime and expiry constraints

Add checks to require positive API key lifetime and non-retro expiry.
Backfill bad rows before constraint to crash fast on regressions.
Update test API keys to assert sensible CreatedAt timestamps.

Author: ThomasK33

coderd/apikey_test.go

@@ -16,7 +16,6 @@ import (
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/database/dbgen"
 	"github.com/coder/coder/v2/coderd/database/dbtestutil"
-	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/coder/v2/coderd/httpapi"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/testutil"
@@ -342,7 +341,7 @@ func TestSessionExpiry(t *testing.T) {
 	err = db.UpdateAPIKeyByID(ctx, database.UpdateAPIKeyByIDParams{
 		ID:        apiKey.ID,
 		LastUsed:  apiKey.LastUsed,
-		ExpiresAt: dbtime.Now().Add(-time.Hour),
+		ExpiresAt: apiKey.CreatedAt,
 		IPAddress: apiKey.IPAddress,
 	})
 	require.NoError(t, err)

coderd/database/check_constraint.go

@@ -660,7 +660,7 @@ func TestExpireOldAPIKeys(t *testing.T) {
 			TemplateID: tpl.ID,
 		})
 		createAPIKey = func(userID uuid.UUID, name string) database.APIKey {
-			k, _ := dbgen.APIKey(t, db, database.APIKey{UserID: userID, TokenName: name, ExpiresAt: now.Add(time.Hour)}, func(iap *database.InsertAPIKeyParams) {
+			k, _ := dbgen.APIKey(t, db, database.APIKey{UserID: userID, TokenName: name, CreatedAt: now, ExpiresAt: now.Add(time.Hour)}, func(iap *database.InsertAPIKeyParams) {
 				iap.TokenName = name
 			})
 			return k

coderd/database/dbpurge/dbpurge_test.go

@@ -0,0 +1,6 @@
+-- Drop all CHECK constraints added in the up migration
+ALTER TABLE api_keys
+DROP CONSTRAINT api_keys_lifetime_seconds_positive;
+
+ALTER TABLE api_keys
+DROP CONSTRAINT api_keys_expires_at_after_created;

coderd/database/dump.sql

@@ -0,0 +1,24 @@
+-- Defensively update any API keys with zero or negative lifetime_seconds to default 86400 (24 hours)
+-- This aligns with application logic that converts 0 to 86400
+UPDATE api_keys
+SET lifetime_seconds = 86400
+WHERE lifetime_seconds <= 0;
+
+-- Add CHECK constraint to ensure lifetime_seconds is positive
+-- This enforces positive lifetimes at database level
+ALTER TABLE api_keys
+ADD CONSTRAINT api_keys_lifetime_seconds_positive
+CHECK (lifetime_seconds > 0);
+
+-- Defensivey update any API keys expires_at with its created_at value
+-- This ensures all existing keys have a non-null expires_at before
+-- adding the constraint.
+UPDATE api_keys
+SET expires_at = created_at
+WHERE expires_at < created_at;
+
+-- Add CHECK constraint to ensure expires_at is at or after created_at
+-- This prevents illogical data where a key expires before it's created
+ALTER TABLE api_keys
+ADD CONSTRAINT api_keys_expires_at_after_created
+CHECK (expires_at >= created_at);

coderd/database/migrations/000389_api_key_created_at_constraint.down.sql

@@ -247,6 +247,7 @@ func TestAPIKey(t *testing.T) {
 			user     = dbgen.User(t, db, database.User{})
 			_, token = dbgen.APIKey(t, db, database.APIKey{
 				UserID:    user.ID,
+				CreatedAt: dbtime.Now().Add(-2 * time.Hour),
 				ExpiresAt: time.Now().Add(time.Hour * -1),
 			})
 
@@ -515,6 +516,7 @@ func TestAPIKey(t *testing.T) {
 			sentAPIKey, token = dbgen.APIKey(t, db, database.APIKey{
 				UserID:    user.ID,
 				LastUsed:  dbtime.Now().AddDate(0, 0, -1),
+				CreatedAt: dbtime.Now().AddDate(0, 0, -2),
 				ExpiresAt: dbtime.Now().AddDate(0, 0, -1),
 				LoginType: database.LoginTypeOIDC,
 			})
@@ -565,6 +567,7 @@ func TestAPIKey(t *testing.T) {
 			sentAPIKey, token = dbgen.APIKey(t, db, database.APIKey{
 				UserID:    user.ID,
 				LastUsed:  dbtime.Now().AddDate(0, 0, -1),
+				CreatedAt: dbtime.Now().AddDate(0, 0, -2),
 				ExpiresAt: dbtime.Now().AddDate(0, 0, -1),
 				LoginType: database.LoginTypeOIDC,
 			})

coderd/database/migrations/000389_api_key_created_at_constraint.up.sql

@@ -374,6 +374,7 @@ func TestOAuth2WWWAuthenticateCompliance(t *testing.T) {
 		// Create an expired API key
 		_, expiredToken := dbgen.APIKey(t, db, database.APIKey{
 			UserID:    user.ID,
+			CreatedAt: dbtime.Now().Add(-2 * time.Hour),
 			ExpiresAt: dbtime.Now().Add(-time.Hour), // Expired 1 hour ago
 		})
 

coderd/httpmw/apikey_test.go

@@ -32,6 +32,7 @@ func TestRFC6750BearerTokenAuthentication(t *testing.T) {
 	// Create an OAuth2 provider app token (which should work with bearer token authentication)
 	key, token := dbgen.APIKey(t, db, database.APIKey{
 		UserID:    user.ID,
+		CreatedAt: dbtime.Now(),
 		ExpiresAt: dbtime.Now().Add(testutil.WaitLong),
 	})
 
@@ -115,6 +116,7 @@ func TestRFC6750BearerTokenAuthentication(t *testing.T) {
 		// Create an expired token
 		_, expiredToken := dbgen.APIKey(t, db, database.APIKey{
 			UserID:    user.ID,
+			CreatedAt: dbtime.Now().Add(-2 * testutil.WaitShort),
 			ExpiresAt: dbtime.Now().Add(-testutil.WaitShort), // Expired
 		})
 

coderd/httpmw/rfc6750_extended_test.go

@@ -134,7 +134,7 @@ func TestHeartbeat(t *testing.T) {
 		heartbeatInterval: testutil.IntervalFast,
 	})
 
-	for i := 0; i < numBeats; i++ {
+	for range numBeats {
 		testutil.TryReceive(ctx, t, heartbeatChan)
 	}
 	// goleak.VerifyTestMain ensures that the heartbeat goroutine does not leak
@@ -4069,6 +4069,8 @@ func TestServer_ExpirePrebuildsSessionToken(t *testing.T) {
 		existingKey, _ = dbgen.APIKey(t, db, database.APIKey{
 			UserID:    database.PrebuildsSystemUserID,
 			TokenName: provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, workspace.ID),
+			CreatedAt: dbtime.Now(),
+			ExpiresAt: dbtime.Now().Add(time.Hour),
 		})
 	)
 
@@ -4112,7 +4114,7 @@ func setup(t *testing.T, ignoreLogErrors bool, ov *overrides) (proto.DRPCProvisi
 	defOrg, err := db.GetDefaultOrganization(context.Background())
 	require.NoError(t, err, "default org not found")
 
-	deploymentValues := &codersdk.DeploymentValues{}
+	deploymentValues := coderdtest.DeploymentValues(t)
 	var externalAuthConfigs []*externalauth.Config
 	tss := testTemplateScheduleStore()
 	uqhss := testUserQuietHoursScheduleStore()
@@ -4280,7 +4282,7 @@ func (s *fakeStream) Send(j *proto.AcquiredJob) error {
 func (s *fakeStream) Recv() (*proto.CancelAcquire, error) {
 	s.c.L.Lock()
 	defer s.c.L.Unlock()
-	for !(s.canceled || s.closed) {
+	for !s.canceled && !s.closed {
 		s.c.Wait()
 	}
 	if s.canceled {
@@ -4323,7 +4325,7 @@ func (s *fakeStream) Close() error {
 func (s *fakeStream) waitForJob() (*proto.AcquiredJob, error) {
 	s.c.L.Lock()
 	defer s.c.L.Unlock()
-	for !(s.sendCalled || s.closed) {
+	for !s.sendCalled && !s.closed {
 		s.c.Wait()
 	}
 	if s.sendCalled {