test(adminapi): add complete integration test suite for KMS and Public Keys by hmffa · Pull Request #53 · go-oidfed/lighthouse

hmffa · 2026-03-02T21:33:07Z

This PR introduces a complete, production-grade integration test suite for the api/adminapi/keys.go endpoints.

🛠️ Key Changes:

Test Setup: Added a temporary SQLite database and Fiber app setup to test the endpoints against a real database instance (DBPublicKeyStorage).
KMS Mocking: Created mockBasicKMS, mockFullKMS, and embedded structs to safely test all KMS interface interactions (including pending algorithm changes and rotation configs).

✅ Endpoints Tested (13/13):

Public Key Management (/entity-configuration/)

GET /keys
POST /keys (with Kid validation and missing body checks)
PUT /keys/:kid (Expiration updates)
POST /keys/:kid (Key rotation)
DELETE /keys/:kid (Hard delete and revocation)
GET /jwks (Ensuring expired keys are omitted)

KMS Management (/kms/)

The test suite also includes exhaustive "sad path" testing for malformed JSON and 400/404 states.

…/keys endpoint

…ys endpoint to update public key expiration

…otation test

…on before rotation

…ks endpoint

…e key rotation config

…ST /kms/rotate endpoints

…key rotation configuration

- Refactored repetitive setup code into reusable test helpers (`setupPublicKeyApp`, `doRequest`, etc.) using `t.Helper()`. - Organized tests into logical sub-tests using `t.Run` for better isolation and cleaner test output. - Added comprehensive "sad path" coverage including invalid JSON bodies, KID mismatches, and 404/400 error states. - Improved KMS mocking by using struct embedding to safely test pending algorithm states. - Extracted large RSA key payload strings into reusable constants to adhere to DRY principles and improve readability.

deepsource-io · 2026-03-02T21:34:20Z

DeepSource Code Review

We reviewed changes in 6a9b497...31f54c0 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade	Security Reliability Complexity Hygiene

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
Go		Mar 4, 2026 1:41p.m.	Review ↗

Copilot

Pull request overview

Adds an integration-style test suite for the Admin API key-management endpoints, exercising public key CRUD/rotation, JWKS publishing behavior, and KMS management/rotation endpoints against a real temporary SQLite-backed storage.

Changes:

Introduces a new api/adminapi/keys_test.go covering 13/13 key + KMS endpoints with happy-path and sad-path cases.
Adds test helpers for building a Fiber app, issuing requests, and seeding public keys.
Adds lightweight KMS mocks to drive KMS-related handlers (pending changes, rotation config, alg/rsa-len updates, rotate-all).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-02T21:38:24Z

api/adminapi/keys_test.go

+		registerKeys(app, km, store.KeyValue())
+
+		initialConfig := kms.KeyRotationConfig{Enabled: true}
+		_ = storage.SetKeyRotation(store.KeyValue(), initialConfig)


The error return from storage.SetKeyRotation is ignored. If persisting the rotation config fails, the test should fail immediately; otherwise later assertions can become misleading.

Suggested change

_ = storage.SetKeyRotation(store.KeyValue(), initialConfig)

if err := storage.SetKeyRotation(store.KeyValue(), initialConfig); err != nil {

t.Fatalf("failed to set key rotation config: %v", err)

}

Copilot · 2026-03-02T21:38:25Z

api/adminapi/keys_test.go

+		// Set initial config
+		_ = storage.SetKeyRotation(store.KeyValue(), kms.KeyRotationConfig{Enabled: false})
+


The error return from storage.SetKeyRotation is ignored when setting up the initial state. Please check this error so the test fails with the correct cause if KV persistence breaks.

Copilot · 2026-03-02T21:38:25Z

api/adminapi/keys_test.go

+
+		_ = storage.SetKeyRotation(store.KeyValue(), kms.KeyRotationConfig{Enabled: true})
+


The error return from storage.SetKeyRotation is ignored here. This setup step should be asserted so later expectations about the PATCH behavior are trustworthy.

Copilot · 2026-03-02T21:38:25Z

api/adminapi/keys_test.go

+	tempDir, err := os.MkdirTemp("", "lighthouse-test-*")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { os.RemoveAll(tempDir) })
+
+	store, err := storage.NewStorage(storage.Config{
+		Driver:  storage.DriverSQLite,
+		DataDir: tempDir,
+	})


newTestStorage creates a file-backed SQLite DB but never closes the underlying sql.DB connection; on some platforms (notably Windows) this can keep lighthouse.db locked and make the temp dir cleanup unreliable, and across many subtests it can also accumulate open file handles. Consider switching the SQLite DSN to an in-memory database for tests (e.g. a shared in-memory DSN) or exposing/using a storage Close method to close the connection in t.Cleanup before removing the temp directory.

Copilot · 2026-03-02T21:38:25Z

api/adminapi/keys_test.go

+		_ = km.APIManagedPKs.Load()
+		_ = km.KMSManagedPKs.Load()


Errors from Load() are being ignored here. If table creation/migration fails, the test should fail immediately with a clear message rather than proceeding and failing later in less obvious ways.

Suggested change

_ = km.APIManagedPKs.Load()

_ = km.KMSManagedPKs.Load()

if err := km.APIManagedPKs.Load(); err != nil {

t.Fatalf("failed to load API-managed public keys: %v", err)

}

if err := km.KMSManagedPKs.Load(); err != nil {

t.Fatalf("failed to load KMS-managed public keys: %v", err)

}

Copilot · 2026-03-02T21:38:26Z

api/adminapi/keys_test.go

+		_ = km.APIManagedPKs.Load()
+		_ = km.KMSManagedPKs.Load()


Errors from Load() are ignored here. Please assert these calls succeed (e.g., if err := ...; err != nil { t.Fatalf(...) }) so the test fails with the real root cause.

Copilot · 2026-03-02T21:38:26Z

api/adminapi/keys_test.go

+		_ = km.APIManagedPKs.Load()
+		_ = km.KMSManagedPKs.Load()


Errors from Load() are ignored here. These should be checked so the test doesn't silently proceed with an uninitialized storage and fail later with less actionable errors.

…age setup

…esAt checks

…eceiver

…sistency

hmffa added 12 commits February 25, 2026 17:04

test: add unit tests for KMS functionality with mock implementations

0ed1daf

test: add integration test for POST /entity-configuration/keys endpoint

e578d05

test: add integration test for GET /entity-configuration/keys endpoint

651fe19

test(adminapi): add integration test for DELETE /entity-configuration…

ff2b0f2

…/keys endpoint

test(adminapi): add integration test for PUT /entity-configuration/ke…

4100741

…ys endpoint to update public key expiration

test(adminapi): enhance tests for public key operations and add key r…

94e5979

…otation test

test(adminapi): enhance TestRotatePublicKey to verify old key inserti…

e7a2102

…on before rotation

test(adminapi): add integration test for GET /entity-configuration/jw…

7230cf6

…ks endpoint

test(adminapi): add integration test for PATCH /kms/rotation to updat…

bf5ae0a

…e key rotation config

test(adminapi): add integration tests for PUT /kms/rsa-key-len and PO…

f5b8086

…ST /kms/rotate endpoints

test(adminapi): add integration test for GET /kms/rotation to verify …

7320047

…key rotation configuration

Copilot AI review requested due to automatic review settings March 2, 2026 21:33

Copilot started reviewing on behalf of hmffa March 2, 2026 21:33 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

hmffa added 5 commits March 2, 2026 23:00

test(adminapi): Fixed linting warnings

e64945b

test(adminapi): improve error handling in key rotation tests and stor…

ee2b09a

…age setup

test(adminapi): enhance error handling in storage setup and fix Expir…

d99b532

…esAt checks

test(adminapi): update mock KMS methods to use blank identifier for r…

1c4d650

…eceiver

test(adminapi): update HTTP request bodies to use http.NoBody for con…

31f54c0

…sistency

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(adminapi): add complete integration test suite for KMS and Public Keys#53

test(adminapi): add complete integration test suite for KMS and Public Keys#53
hmffa wants to merge 17 commits intogo-oidfed:lighthouse-20from
hmffa:tests/admin-api-keys

hmffa commented Mar 2, 2026 •

edited

Loading

Uh oh!

deepsource-io bot commented Mar 2, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-		_ = storage.SetKeyRotation(store.KeyValue(), initialConfig)
+		if err := storage.SetKeyRotation(store.KeyValue(), initialConfig); err != nil {
+			t.Fatalf("failed to set key rotation config: %v", err)
+		}

		// Set initial config
		_ = storage.SetKeyRotation(store.KeyValue(), kms.KeyRotationConfig{Enabled: false})

-		_ = km.APIManagedPKs.Load()
-		_ = km.KMSManagedPKs.Load()
+		if err := km.APIManagedPKs.Load(); err != nil {
+			t.Fatalf("failed to load API-managed public keys: %v", err)
+		}
+		if err := km.KMSManagedPKs.Load(); err != nil {
+			t.Fatalf("failed to load KMS-managed public keys: %v", err)
+		}

Conversation

hmffa commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛠️ Key Changes:

✅ Endpoints Tested (13/13):

Uh oh!

deepsource-io bot commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DeepSource Code Review

PR Report Card

Code Review Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hmffa commented Mar 2, 2026 •

edited

Loading

deepsource-io bot commented Mar 2, 2026 •

edited

Loading