diff --git a/internal/repository/integration_test.go b/internal/repository/integration_test.go deleted file mode 100644 index 73fa4a99..00000000 --- a/internal/repository/integration_test.go +++ /dev/null @@ -1,136 +0,0 @@ -package repository_test - -import ( - "context" - "database/sql" - "os" - "path/filepath" - "testing" - - "github.com/golang-migrate/migrate/v4" - pgxmigrate "github.com/golang-migrate/migrate/v4/database/pgx/v5" - "github.com/golang-migrate/migrate/v4/database/sqlite3" - "github.com/golang-migrate/migrate/v4/source/iofs" - _ "github.com/jackc/pgx/v5/stdlib" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - _ "modernc.org/sqlite" - - "github.com/tinyauthapp/tinyauth/internal/assets" - "github.com/tinyauthapp/tinyauth/internal/repository" - "github.com/tinyauthapp/tinyauth/internal/repository/postgres" - "github.com/tinyauthapp/tinyauth/internal/repository/sqlite" -) - -func setupSQLiteStore(t *testing.T) repository.Store { - t.Helper() - - db, err := sql.Open("sqlite", filepath.Join(t.TempDir(), "test.db")) - require.NoError(t, err) - - migrations, err := iofs.New(assets.Migrations, "migrations/sqlite") - require.NoError(t, err) - - target, err := sqlite3.WithInstance(db, &sqlite3.Config{}) - require.NoError(t, err) - - migrator, err := migrate.NewWithInstance("iofs", migrations, "sqlite3", target) - require.NoError(t, err) - - err = migrator.Up() - require.NoError(t, err) - - t.Cleanup(func() { db.Close() }) - - return sqlite.NewStore(sqlite.New(db)) -} - -func setupPostgresStore(t *testing.T) repository.Store { - t.Helper() - - url := os.Getenv("INTEGRATION_POSTGRES_URL") - if url == "" { - t.Skip("INTEGRATION_POSTGRES_URL not set, skipping postgres integration test") - } - - db, err := sql.Open("pgx", url) - require.NoError(t, err) - - migrations, err := iofs.New(assets.Migrations, "migrations/postgres") - require.NoError(t, err) - - target, err := pgxmigrate.WithInstance(db, &pgxmigrate.Config{}) - require.NoError(t, err) - - migrator, err := migrate.NewWithInstance("iofs", migrations, "pgx", target) - require.NoError(t, err) - - err = migrator.Up() - require.NoError(t, err) - - t.Cleanup(func() { db.Close() }) - - return postgres.NewStore(postgres.New(db)) -} - -func TestConsentIntegration(t *testing.T) { - t.Run("sqlite", func(t *testing.T) { - runConsentScenarios(t, setupSQLiteStore(t)) - }) - t.Run("postgres", func(t *testing.T) { - runConsentScenarios(t, setupPostgresStore(t)) - }) -} - -func runConsentScenarios(t *testing.T, store repository.Store) { - t.Helper() - - ctx := context.Background() - - // User consents to client A with openid profile, then client B with openid email - _, err := store.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{ - Username: "alice", ClientID: "client-a", Scope: "openid profile", CreatedAt: 100, - }) - require.NoError(t, err) - _, err = store.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{ - Username: "alice", ClientID: "client-b", Scope: "openid email", CreatedAt: 101, - }) - require.NoError(t, err) - - consents, err := store.ListOIDCConsents(ctx) - require.NoError(t, err) - assert.Len(t, consents, 2) - - gotA, err := store.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{Username: "alice", ClientID: "client-a"}) - require.NoError(t, err) - assert.Equal(t, "openid profile", gotA.Scope) - - gotB, err := store.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{Username: "alice", ClientID: "client-b"}) - require.NoError(t, err) - assert.Equal(t, "openid email", gotB.Scope) - - // Same user+client upsert keeps exactly one row - _, err = store.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{ - Username: "alice", ClientID: "client-a", Scope: "openid profile email", CreatedAt: 102, - }) - require.NoError(t, err) - - consents, err = store.ListOIDCConsents(ctx) - require.NoError(t, err) - assert.Len(t, consents, 2) - - gotA, err = store.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{Username: "alice", ClientID: "client-a"}) - require.NoError(t, err) - assert.Equal(t, "openid profile email", gotA.Scope) - - // Removing a client deletes its consent rows - require.NoError(t, store.DeleteOIDCConsentByClientID(ctx, "client-a")) - - consents, err = store.ListOIDCConsents(ctx) - require.NoError(t, err) - assert.Len(t, consents, 1) - assert.Equal(t, "client-b", consents[0].ClientID) - - _, err = store.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{Username: "alice", ClientID: "client-a"}) - assert.ErrorIs(t, err, repository.ErrNotFound) -} diff --git a/internal/service/oidc_consent_test.go b/internal/service/oidc_consent_test.go deleted file mode 100644 index 96e844fc..00000000 --- a/internal/service/oidc_consent_test.go +++ /dev/null @@ -1,180 +0,0 @@ -package service - -import ( - "context" - "strings" - "testing" - - "github.com/steveiliop56/ding" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/tinyauthapp/tinyauth/internal/model" - "github.com/tinyauthapp/tinyauth/internal/repository" - "github.com/tinyauthapp/tinyauth/internal/repository/memory" - "github.com/tinyauthapp/tinyauth/internal/test" - "github.com/tinyauthapp/tinyauth/internal/utils/logger" -) - -func newTestOIDCService(t *testing.T, cfg *model.Config, runtime *model.RuntimeConfig, store repository.Store) *OIDCService { - t.Helper() - - log := logger.NewLogger().WithTestConfig() - log.Init() - - ctx := context.Background() - dg := ding.New(ctx) - - svc, err := NewOIDCService(OIDCServiceInput{ - Log: log, - Config: cfg, - Runtime: runtime, - Queries: store, - Ding: dg, - }) - require.NoError(t, err) - require.NotNil(t, svc) - - return svc -} - -func TestOIDCConsentPerClient(t *testing.T) { - cfg, runtime := test.CreateTestConfigs(t) - - store := memory.New() - svc := newTestOIDCService(t, &cfg, &runtime, store) - - // Consent for client A - _, err := svc.UpsertOIDCConsent(context.Background(), "testuser", "openid profile", "some-client-id") - require.NoError(t, err) - - // Consent for client B - _, err = svc.UpsertOIDCConsent(context.Background(), "testuser", "openid email", "other-client-id") - require.NoError(t, err) - - // Two separate rows, one per client - consents, err := store.ListOIDCConsents(context.Background()) - require.NoError(t, err) - assert.Len(t, consents, 2) - - // Each lookup returns the consent for that specific client - gotA, err := svc.GetOIDCConsent(context.Background(), "testuser", "some-client-id") - require.NoError(t, err) - require.NotNil(t, gotA) - assert.Equal(t, "openid profile", gotA.Scope) - - gotB, err := svc.GetOIDCConsent(context.Background(), "testuser", "other-client-id") - require.NoError(t, err) - require.NotNil(t, gotB) - assert.Equal(t, "openid email", gotB.Scope) -} - -func TestOIDCConsentUnionKeepsOneRow(t *testing.T) { - cfg, runtime := test.CreateTestConfigs(t) - - store := memory.New() - svc := newTestOIDCService(t, &cfg, &runtime, store) - - _, err := svc.UpsertOIDCConsent(context.Background(), "testuser", "openid profile", "some-client-id") - require.NoError(t, err) - - // Second authorize with an additional scope - _, err = svc.UpsertOIDCConsent(context.Background(), "testuser", "openid email", "some-client-id") - require.NoError(t, err) - - // Still exactly one row - consents, err := store.ListOIDCConsents(context.Background()) - require.NoError(t, err) - assert.Len(t, consents, 1) - - // Stored scope is the union of both requests - got, err := svc.GetOIDCConsent(context.Background(), "testuser", "some-client-id") - require.NoError(t, err) - require.NotNil(t, got) - assert.Equal(t, []string{"email", "openid", "profile"}, splitScopes(got.Scope)) -} - -func TestOIDCConsentSubsetDoesNotShrink(t *testing.T) { - cfg, runtime := test.CreateTestConfigs(t) - - store := memory.New() - svc := newTestOIDCService(t, &cfg, &runtime, store) - - _, err := svc.UpsertOIDCConsent(context.Background(), "testuser", "openid profile email", "some-client-id") - require.NoError(t, err) - - // Authorize with a subset of already-granted scopes - _, err = svc.UpsertOIDCConsent(context.Background(), "testuser", "openid profile", "some-client-id") - require.NoError(t, err) - - // Stored scope is unchanged - got, err := svc.GetOIDCConsent(context.Background(), "testuser", "some-client-id") - require.NoError(t, err) - require.NotNil(t, got) - assert.Equal(t, []string{"email", "openid", "profile"}, splitScopes(got.Scope)) -} - -func TestOIDCConsentNotFound(t *testing.T) { - cfg, runtime := test.CreateTestConfigs(t) - - store := memory.New() - svc := newTestOIDCService(t, &cfg, &runtime, store) - - got, err := svc.GetOIDCConsent(context.Background(), "testuser", "some-client-id") - require.NoError(t, err) - assert.Nil(t, got) -} - -func TestOIDCConsentReconcileRemovesDeletedClients(t *testing.T) { - cfg, runtime := test.CreateTestConfigs(t) - - cfg.OIDC.Clients["client-a"] = model.OIDCClientConfig{ - ClientID: "client-a", - ClientSecret: "secret-a", - TrustedRedirectURIs: []string{"https://a.example.com/callback"}, - } - cfg.OIDC.Clients["client-b"] = model.OIDCClientConfig{ - ClientID: "client-b", - ClientSecret: "secret-b", - TrustedRedirectURIs: []string{"https://b.example.com/callback"}, - } - - // First startup: both clients configured - store := memory.New() - svc := newTestOIDCService(t, &cfg, &runtime, store) - - _, err := svc.UpsertOIDCConsent(context.Background(), "testuser", "openid profile", "client-a") - require.NoError(t, err) - _, err = svc.UpsertOIDCConsent(context.Background(), "testuser", "openid email", "client-b") - require.NoError(t, err) - // Stale consent for a client that was never configured - _, err = svc.UpsertOIDCConsent(context.Background(), "testuser", "openid", "stale-client") - require.NoError(t, err) - - // Second startup: client-b removed from configuration - delete(cfg.OIDC.Clients, "client-b") - - svc2 := newTestOIDCService(t, &cfg, &runtime, store) - - // Consents for the removed and unknown clients are gone - _, err = store.GetOIDCConsentByUsernameAndClientID(context.Background(), repository.GetOIDCConsentByUsernameAndClientIDParams{ - Username: "testuser", ClientID: "client-a", - }) - require.NoError(t, err) - - _, err = store.GetOIDCConsentByUsernameAndClientID(context.Background(), repository.GetOIDCConsentByUsernameAndClientIDParams{ - Username: "testuser", ClientID: "client-b", - }) - assert.ErrorIs(t, err, repository.ErrNotFound) - - _, err = store.GetOIDCConsentByUsernameAndClientID(context.Background(), repository.GetOIDCConsentByUsernameAndClientIDParams{ - Username: "testuser", ClientID: "stale-client", - }) - assert.ErrorIs(t, err, repository.ErrNotFound) - - require.NotNil(t, svc2) -} - -func splitScopes(scope string) []string { - return strings.Fields(scope) -} diff --git a/internal/service/oidc_service.go b/internal/service/oidc_service.go index c37bf771..59d4343d 100644 --- a/internal/service/oidc_service.go +++ b/internal/service/oidc_service.go @@ -16,6 +16,7 @@ import ( "os" "path/filepath" "strings" + "sync" "time" "slices" @@ -163,6 +164,10 @@ type OIDCService struct { usedCode *cache.CacheStore[UsedCodeEntry] authorize *cache.CacheStore[AuthorizeRequest] } + + mus struct { + consent *sync.RWMutex + } } type OIDCServiceInput struct { @@ -977,6 +982,9 @@ func (service *OIDCService) GetPrompt(prompt string) []OIDCPrompt { } func (service *OIDCService) GetOIDCConsent(ctx context.Context, username, clientId string) (*repository.OidcConsent, error) { + service.mus.consent.RLock() + defer service.mus.consent.RUnlock() + entry, err := service.queries.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{ Username: username, ClientID: clientId, @@ -993,6 +1001,9 @@ func (service *OIDCService) GetOIDCConsent(ctx context.Context, username, client } func (service *OIDCService) UpsertOIDCConsent(ctx context.Context, username, scope, clientId string) (repository.OidcConsent, error) { + service.mus.consent.Lock() + defer service.mus.consent.Unlock() + existing, err := service.GetOIDCConsent(ctx, username, clientId) if err != nil {