diff --git a/internal/assets/migrations/postgres/000003_oidc_consent.down.sql b/internal/assets/migrations/postgres/000003_oidc_consent.down.sql new file mode 100644 index 00000000..f3d144d8 --- /dev/null +++ b/internal/assets/migrations/postgres/000003_oidc_consent.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS "oidc_consents"; \ No newline at end of file diff --git a/internal/assets/migrations/postgres/000003_oidc_consent.up.sql b/internal/assets/migrations/postgres/000003_oidc_consent.up.sql new file mode 100644 index 00000000..a6ff3b65 --- /dev/null +++ b/internal/assets/migrations/postgres/000003_oidc_consent.up.sql @@ -0,0 +1,7 @@ +CREATE TABLE IF NOT EXISTS "oidc_consents" ( + "username" TEXT NOT NULL, + "client_id" TEXT NOT NULL, + "scope" TEXT NOT NULL, + "created_at" BIGINT NOT NULL, + PRIMARY KEY ("username", "client_id") +); diff --git a/internal/assets/migrations/sqlite/000011_oidc_consent.down.sql b/internal/assets/migrations/sqlite/000011_oidc_consent.down.sql new file mode 100644 index 00000000..f3d144d8 --- /dev/null +++ b/internal/assets/migrations/sqlite/000011_oidc_consent.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS "oidc_consents"; \ No newline at end of file diff --git a/internal/assets/migrations/sqlite/000011_oidc_consent.up.sql b/internal/assets/migrations/sqlite/000011_oidc_consent.up.sql new file mode 100644 index 00000000..6d67a89c --- /dev/null +++ b/internal/assets/migrations/sqlite/000011_oidc_consent.up.sql @@ -0,0 +1,7 @@ +CREATE TABLE IF NOT EXISTS "oidc_consents" ( + "username" TEXT NOT NULL, + "client_id" TEXT NOT NULL, + "scope" TEXT NOT NULL, + "created_at" INTEGER NOT NULL, + PRIMARY KEY ("username", "client_id") +); diff --git a/internal/bootstrap/app_bootstrap.go b/internal/bootstrap/app_bootstrap.go index 7c8e1ea0..6fc95408 100644 --- a/internal/bootstrap/app_bootstrap.go +++ b/internal/bootstrap/app_bootstrap.go @@ -179,7 +179,6 @@ func (app *BootstrapApp) Setup() error { cookieId := strings.Split(app.runtime.UUID, "-")[0] // first 8 characters of the uuid should be good enough app.runtime.SessionCookieName = fmt.Sprintf("%s-%s", model.SessionCookieName, cookieId) - app.runtime.ScopeCookieName = fmt.Sprintf("%s-%s", model.OIDCScopeCookieName, cookieId) app.runtime.OAuthSessionCookieName = fmt.Sprintf("%s-%s", model.OAuthSessionCookieName, cookieId) // database diff --git a/internal/controller/oidc_controller.go b/internal/controller/oidc_controller.go index 3826aeef..9064135f 100644 --- a/internal/controller/oidc_controller.go +++ b/internal/controller/oidc_controller.go @@ -35,7 +35,6 @@ type OIDCController struct { log *logger.Logger oidc *service.OIDCService runtime *model.RuntimeConfig - config *model.Config } type AuthorizeCallback struct { @@ -89,7 +88,6 @@ type OIDCControllerInput struct { Log *logger.Logger OIDCService *service.OIDCService - Config *model.Config RuntimeConfig *model.RuntimeConfig RouterGroup *gin.RouterGroup `name:"apiRouterGroup"` MainRouter *gin.RouterGroup `name:"mainRouterGroup"` @@ -100,7 +98,6 @@ func NewOIDCController(i OIDCControllerInput) *OIDCController { log: i.Log, oidc: i.OIDCService, runtime: i.RuntimeConfig, - config: i.Config, } i.MainRouter.POST("/authorize", controller.authorize) @@ -245,40 +242,15 @@ func (controller *OIDCController) authorize(c *gin.Context) { } } - checkSkipAuthorize := func() { - cookieId := client.ClientID[8:] - cookieName := fmt.Sprintf("%s-%s", controller.runtime.ScopeCookieName, cookieId) - scopeCookie, err := c.Cookie(cookieName) + if userContext != nil && userContext.Authenticated && values.OIDCPrompt != service.OIDCPromptLogin { + consent, err := controller.oidc.GetOIDCConsent(c, userContext.GetUsername(), req.ClientID) - if err != nil || userContext == nil || !userContext.Authenticated { - return - } - - kv, err := controller.oidc.DecryptSecureValue(client.ClientSecret, scopeCookie) if err != nil { - controller.log.App.Warn().Err(err).Msg("Failed to decrypt scope cookie") - return - } - - scope, ok := kv["scope"] - if !ok { - controller.log.App.Warn().Str("cookieName", cookieName).Msg("Failed to get scopes from scope cookie") - return - } - - username, ok := kv["username"] - if !ok { - controller.log.App.Warn().Str("cookieName", cookieName).Msg("Failed to get username from scope cookie") - return - } - - if username == userContext.GetUsername() && - values.OIDCPrompt != service.OIDCPromptLogin && - scope == req.Scope { + controller.log.App.Warn().Err(err).Msg("Failed to get OIDC consent") + } else if consent != nil && scopesGranted(consent.Scope, req.Scope) { values.OIDCPrompt = service.OIDCPromptNone } } - checkSkipAuthorize() queries, err := query.Values(values) @@ -407,28 +379,9 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) { return } - // Set a cookie for the consent screen (approved scopes) - cookieId := client.ClientID[8:] - cookieName := fmt.Sprintf("%s-%s", controller.runtime.ScopeCookieName, cookieId) - - secureSignedValue, err := controller.oidc.CreateSecureValue(client.ClientSecret, map[string]string{ - "username": userContext.GetUsername(), - "scope": authorizeReq.Scope, - }) - - if err == nil { - cookie := &http.Cookie{ - Name: cookieName, - Value: secureSignedValue, - Path: "/", - Secure: controller.config.Auth.SecureCookie, - HttpOnly: true, - SameSite: http.SameSiteLaxMode, - } - - http.SetCookie(c.Writer, cookie) - } else { - controller.log.App.Warn().Err(err).Msg("Failed to create scope cookie") + // Store the consent granted by the user for this client + if _, err := controller.oidc.UpsertOIDCConsent(c, userContext.GetUsername(), authorizeReq.Scope, client.ClientID); err != nil { + controller.log.App.Warn().Err(err).Msg("Failed to store OIDC consent") } q := cu.Query() @@ -831,3 +784,20 @@ func (controller *OIDCController) resolveNormalParams(c *gin.Context) (*service. return &req, nil } + +// scopesGranted reports whether every scope in requested is present in the +// space-separated granted scope string. +func scopesGranted(granted, requested string) bool { + grantedScopes := strings.Split(granted, " ") + + for _, scope := range strings.Split(requested, " ") { + if scope == "" { + continue + } + if !slices.Contains(grantedScopes, scope) { + return false + } + } + + return true +} diff --git a/internal/controller/oidc_controller_test.go b/internal/controller/oidc_controller_test.go index b22ddc54..d73ef34f 100644 --- a/internal/controller/oidc_controller_test.go +++ b/internal/controller/oidc_controller_test.go @@ -170,6 +170,102 @@ func TestOIDCController(t *testing.T) { assert.Contains(t, location, "oidc_name="+url.QueryEscape("Test Client")) }, }, + { + description: "Authorize skips the consent screen when all requested scopes were already granted", + middlewares: []gin.HandlerFunc{authedUser}, + run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { + _, err := store.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{ + Username: "testuser", ClientID: "some-client-id", + Scope: "openid profile", CreatedAt: time.Now().Unix(), + }) + require.NoError(t, err) + + q := url.Values{} + q.Set("scope", "openid profile") + q.Set("response_type", "code") + q.Set("client_id", "some-client-id") + q.Set("redirect_uri", "https://test.example.com/callback") + + req := httptest.NewRequest("GET", "/authorize?"+q.Encode(), nil) + router.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusFound, recorder.Code) + location := recorder.Header().Get("Location") + assert.True(t, strings.HasPrefix(location, oidcService.GetIssuer()+"/oidc/authorize?")) + assert.Contains(t, location, "oidc_prompt=none") + }, + }, + { + description: "Authorize shows the consent screen when a new scope is requested", + middlewares: []gin.HandlerFunc{authedUser}, + run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { + _, err := store.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{ + Username: "testuser", ClientID: "some-client-id", + Scope: "openid", CreatedAt: time.Now().Unix(), + }) + require.NoError(t, err) + + q := url.Values{} + q.Set("scope", "openid profile") + q.Set("response_type", "code") + q.Set("client_id", "some-client-id") + q.Set("redirect_uri", "https://test.example.com/callback") + + req := httptest.NewRequest("GET", "/authorize?"+q.Encode(), nil) + router.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusFound, recorder.Code) + location := recorder.Header().Get("Location") + assert.True(t, strings.HasPrefix(location, oidcService.GetIssuer()+"/oidc/authorize?")) + assert.NotContains(t, location, "oidc_prompt=none") + }, + }, + { + description: "Authorize skips the consent screen for a subset of already granted scopes", + middlewares: []gin.HandlerFunc{authedUser}, + run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { + _, err := store.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{ + Username: "testuser", ClientID: "some-client-id", + Scope: "openid profile email", CreatedAt: time.Now().Unix(), + }) + require.NoError(t, err) + + q := url.Values{} + q.Set("scope", "openid profile") + q.Set("response_type", "code") + q.Set("client_id", "some-client-id") + q.Set("redirect_uri", "https://test.example.com/callback") + + req := httptest.NewRequest("GET", "/authorize?"+q.Encode(), nil) + router.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusFound, recorder.Code) + location := recorder.Header().Get("Location") + assert.True(t, strings.HasPrefix(location, oidcService.GetIssuer()+"/oidc/authorize?")) + assert.Contains(t, location, "oidc_prompt=none") + }, + }, + { + description: "Authorize shows the consent screen when no consent was granted yet", + middlewares: []gin.HandlerFunc{authedUser}, + run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { + require.NoError(t, store.DeleteOIDCConsentByClientID(ctx, "some-client-id")) + + q := url.Values{} + q.Set("scope", "openid profile") + q.Set("response_type", "code") + q.Set("client_id", "some-client-id") + q.Set("redirect_uri", "https://test.example.com/callback") + + req := httptest.NewRequest("GET", "/authorize?"+q.Encode(), nil) + router.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusFound, recorder.Code) + location := recorder.Header().Get("Location") + assert.True(t, strings.HasPrefix(location, oidcService.GetIssuer()+"/oidc/authorize?")) + assert.NotContains(t, location, "oidc_prompt=none") + }, + }, { description: "Authorize redirects to error screen when the request object is invalid", run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { diff --git a/internal/model/constants.go b/internal/model/constants.go index f4ebc346..7a743d98 100644 --- a/internal/model/constants.go +++ b/internal/model/constants.go @@ -21,8 +21,5 @@ var ReservedProviderNames = []string{"local", "ldap", "tailscale"} const SessionCookieName = "tinyauth-session" const OAuthSessionCookieName = "tinyauth-oauth" -const OIDCScopeCookieName = "tinyauth-scope" const GracefulShutdownTimeout = 5 // seconds - -const HKDFSalt = "tinyauth-hkdf-salt-v1" diff --git a/internal/model/runtime.go b/internal/model/runtime.go index 0722e8e1..66811952 100644 --- a/internal/model/runtime.go +++ b/internal/model/runtime.go @@ -5,7 +5,6 @@ type RuntimeConfig struct { UUID string CookieDomain string SessionCookieName string - ScopeCookieName string OAuthSessionCookieName string LocalUsers []LocalUser OAuthProviders map[string]OAuthServiceConfig diff --git a/internal/repository/integration_test.go b/internal/repository/integration_test.go new file mode 100644 index 00000000..73fa4a99 --- /dev/null +++ b/internal/repository/integration_test.go @@ -0,0 +1,136 @@ +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/repository/memory/memory_test.go b/internal/repository/memory/memory_test.go index 558ed234..c2ae4ca8 100644 --- a/internal/repository/memory/memory_test.go +++ b/internal/repository/memory/memory_test.go @@ -277,6 +277,80 @@ func TestMemoryStore(t *testing.T) { assert.NoError(t, err) }, }, + { + description: "Upsert creates a consent for each user+client pair", + run: func(t *testing.T, s repository.Store) { + _, err := s.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{ + Username: "alice", ClientID: "client-a", Scope: "openid profile", CreatedAt: 1, + }) + require.NoError(t, err) + _, err = s.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{ + Username: "alice", ClientID: "client-b", Scope: "openid email", CreatedAt: 2, + }) + require.NoError(t, err) + + consents, err := s.ListOIDCConsents(ctx) + require.NoError(t, err) + assert.Len(t, consents, 2) + + gotA, err := s.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{Username: "alice", ClientID: "client-a"}) + require.NoError(t, err) + assert.Equal(t, "openid profile", gotA.Scope) + + gotB, err := s.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{Username: "alice", ClientID: "client-b"}) + require.NoError(t, err) + assert.Equal(t, "openid email", gotB.Scope) + }, + }, + { + description: "Upsert overwrites the same consent row", + run: func(t *testing.T, s repository.Store) { + _, err := s.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{ + Username: "alice", ClientID: "client-a", Scope: "openid", CreatedAt: 1, + }) + require.NoError(t, err) + + _, err = s.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{ + Username: "alice", ClientID: "client-a", Scope: "openid email", CreatedAt: 2, + }) + require.NoError(t, err) + + consents, err := s.ListOIDCConsents(ctx) + require.NoError(t, err) + assert.Len(t, consents, 1) + + got, err := s.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{Username: "alice", ClientID: "client-a"}) + require.NoError(t, err) + assert.Equal(t, "openid email", got.Scope) + }, + }, + { + description: "Get consent by username and client not found", + run: func(t *testing.T, s repository.Store) { + _, err := s.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{Username: "alice", ClientID: "client-a"}) + assert.ErrorIs(t, err, repository.ErrNotFound) + }, + }, + { + description: "Delete consent by client id", + run: func(t *testing.T, s repository.Store) { + _, err := s.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{ + Username: "alice", ClientID: "client-a", Scope: "openid", CreatedAt: 1, + }) + require.NoError(t, err) + _, err = s.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{ + Username: "alice", ClientID: "client-b", Scope: "openid", CreatedAt: 2, + }) + require.NoError(t, err) + + require.NoError(t, s.DeleteOIDCConsentByClientID(ctx, "client-a")) + + consents, err := s.ListOIDCConsents(ctx) + require.NoError(t, err) + assert.Len(t, consents, 1) + assert.Equal(t, "client-b", consents[0].ClientID) + }, + }, } for _, test := range tests { diff --git a/internal/repository/memory/oidc_queries.go b/internal/repository/memory/oidc_queries.go index 1ee81c8b..e06a4346 100644 --- a/internal/repository/memory/oidc_queries.go +++ b/internal/repository/memory/oidc_queries.go @@ -94,3 +94,46 @@ func (s *Store) DeleteExpiredOIDCSessions(_ context.Context, arg repository.Dele } return nil } + +func consentKey(username, clientID string) string { + return username + "\x00" + clientID +} + +func (s *Store) UpsertOIDCConsent(_ context.Context, arg repository.UpsertOIDCConsentParams) (repository.OidcConsent, error) { + s.mu.Lock() + defer s.mu.Unlock() + oc := repository.OidcConsent(arg) + s.oidcConsents[consentKey(arg.Username, arg.ClientID)] = oc + return oc, nil +} + +func (s *Store) GetOIDCConsentByUsernameAndClientID(_ context.Context, arg repository.GetOIDCConsentByUsernameAndClientIDParams) (repository.OidcConsent, error) { + s.mu.RLock() + defer s.mu.RUnlock() + oc, ok := s.oidcConsents[consentKey(arg.Username, arg.ClientID)] + if !ok { + return repository.OidcConsent{}, repository.ErrNotFound + } + return oc, nil +} + +func (s *Store) DeleteOIDCConsentByClientID(_ context.Context, clientID string) error { + s.mu.Lock() + defer s.mu.Unlock() + for key, oc := range s.oidcConsents { + if oc.ClientID == clientID { + delete(s.oidcConsents, key) + } + } + return nil +} + +func (s *Store) ListOIDCConsents(_ context.Context) ([]repository.OidcConsent, error) { + s.mu.RLock() + defer s.mu.RUnlock() + out := make([]repository.OidcConsent, 0, len(s.oidcConsents)) + for _, oc := range s.oidcConsents { + out = append(out, oc) + } + return out, nil +} diff --git a/internal/repository/memory/store.go b/internal/repository/memory/store.go index 684ddeb3..5aa4750b 100644 --- a/internal/repository/memory/store.go +++ b/internal/repository/memory/store.go @@ -12,6 +12,7 @@ type Store struct { mu sync.RWMutex sessions map[string]repository.Session oidcSessions map[string]repository.OidcSession + oidcConsents map[string]repository.OidcConsent } // New returns a new empty in-memory Store. @@ -19,5 +20,6 @@ func New() repository.Store { return &Store{ sessions: make(map[string]repository.Session), oidcSessions: make(map[string]repository.OidcSession), + oidcConsents: make(map[string]repository.OidcConsent), } } diff --git a/internal/repository/models.go b/internal/repository/models.go index 39538a00..9e356680 100644 --- a/internal/repository/models.go +++ b/internal/repository/models.go @@ -84,3 +84,22 @@ type DeleteExpiredOIDCSessionsParams struct { TokenExpiresAt int64 RefreshTokenExpiresAt int64 } + +type OidcConsent struct { + Username string + ClientID string + Scope string + CreatedAt int64 +} + +type UpsertOIDCConsentParams struct { + Username string + ClientID string + Scope string + CreatedAt int64 +} + +type GetOIDCConsentByUsernameAndClientIDParams struct { + Username string + ClientID string +} diff --git a/internal/repository/postgres/models.go b/internal/repository/postgres/models.go index f957e1fd..ccf7ce62 100644 --- a/internal/repository/postgres/models.go +++ b/internal/repository/postgres/models.go @@ -4,6 +4,13 @@ package postgres +type OidcConsent struct { + Username string + ClientID string + Scope string + CreatedAt int64 +} + type OidcSession struct { Sub string AccessTokenHash string diff --git a/internal/repository/postgres/oidc_queries.sql.go b/internal/repository/postgres/oidc_queries.sql.go index b5b9789c..a2bb9631 100644 --- a/internal/repository/postgres/oidc_queries.sql.go +++ b/internal/repository/postgres/oidc_queries.sql.go @@ -80,6 +80,16 @@ func (q *Queries) DeleteExpiredOIDCSessions(ctx context.Context, arg DeleteExpir return err } +const deleteOIDCConsentByClientID = `-- name: DeleteOIDCConsentByClientID :exec +DELETE FROM "oidc_consents" +WHERE "client_id" = $1 +` + +func (q *Queries) DeleteOIDCConsentByClientID(ctx context.Context, clientID string) error { + _, err := q.db.ExecContext(ctx, deleteOIDCConsentByClientID, clientID) + return err +} + const deleteOIDCSessionBySub = `-- name: DeleteOIDCSessionBySub :exec DELETE FROM "oidc_sessions" WHERE "sub" = $1 @@ -90,6 +100,28 @@ func (q *Queries) DeleteOIDCSessionBySub(ctx context.Context, sub string) error return err } +const getOIDCConsentByUsernameAndClientID = `-- name: GetOIDCConsentByUsernameAndClientID :one +SELECT username, client_id, scope, created_at FROM "oidc_consents" +WHERE "username" = $1 AND "client_id" = $2 +` + +type GetOIDCConsentByUsernameAndClientIDParams struct { + Username string + ClientID string +} + +func (q *Queries) GetOIDCConsentByUsernameAndClientID(ctx context.Context, arg GetOIDCConsentByUsernameAndClientIDParams) (OidcConsent, error) { + row := q.db.QueryRowContext(ctx, getOIDCConsentByUsernameAndClientID, arg.Username, arg.ClientID) + var i OidcConsent + err := row.Scan( + &i.Username, + &i.ClientID, + &i.Scope, + &i.CreatedAt, + ) + return i, err +} + const getOIDCSessionByAccessTokenHash = `-- name: GetOIDCSessionByAccessTokenHash :one SELECT sub, access_token_hash, refresh_token_hash, scope, client_id, token_expires_at, refresh_token_expires_at, nonce, userinfo_json FROM "oidc_sessions" WHERE "access_token_hash" = $1 @@ -156,6 +188,38 @@ func (q *Queries) GetOIDCSessionBySub(ctx context.Context, sub string) (OidcSess return i, err } +const listOIDCConsents = `-- name: ListOIDCConsents :many +SELECT username, client_id, scope, created_at FROM "oidc_consents" +` + +func (q *Queries) ListOIDCConsents(ctx context.Context) ([]OidcConsent, error) { + rows, err := q.db.QueryContext(ctx, listOIDCConsents) + if err != nil { + return nil, err + } + defer rows.Close() + var items []OidcConsent + for rows.Next() { + var i OidcConsent + if err := rows.Scan( + &i.Username, + &i.ClientID, + &i.Scope, + &i.CreatedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const updateOIDCSession = `-- name: UpdateOIDCSession :one UPDATE "oidc_sessions" SET "access_token_hash" = $1, @@ -208,3 +272,43 @@ func (q *Queries) UpdateOIDCSession(ctx context.Context, arg UpdateOIDCSessionPa ) return i, err } + +const upsertOIDCConsent = `-- name: UpsertOIDCConsent :one +INSERT INTO "oidc_consents" ( + "username", + "client_id", + "scope", + "created_at" +) VALUES ( + $1, $2, $3, $4 +) +ON CONFLICT ("username", "client_id") +DO UPDATE SET + "scope" = excluded.scope, + "created_at" = excluded.created_at +RETURNING username, client_id, scope, created_at +` + +type UpsertOIDCConsentParams struct { + Username string + ClientID string + Scope string + CreatedAt int64 +} + +func (q *Queries) UpsertOIDCConsent(ctx context.Context, arg UpsertOIDCConsentParams) (OidcConsent, error) { + row := q.db.QueryRowContext(ctx, upsertOIDCConsent, + arg.Username, + arg.ClientID, + arg.Scope, + arg.CreatedAt, + ) + var i OidcConsent + err := row.Scan( + &i.Username, + &i.ClientID, + &i.Scope, + &i.CreatedAt, + ) + return i, err +} diff --git a/internal/repository/postgres/store.go b/internal/repository/postgres/store.go index b3e79c80..1857016a 100644 --- a/internal/repository/postgres/store.go +++ b/internal/repository/postgres/store.go @@ -56,6 +56,10 @@ func (s *Store) DeleteExpiredSessions(ctx context.Context, expiry int64) error { return mapErr(s.q.DeleteExpiredSessions(ctx, expiry)) } +func (s *Store) DeleteOIDCConsentByClientID(ctx context.Context, clientID string) error { + return mapErr(s.q.DeleteOIDCConsentByClientID(ctx, clientID)) +} + func (s *Store) DeleteOIDCSessionBySub(ctx context.Context, sub string) error { return mapErr(s.q.DeleteOIDCSessionBySub(ctx, sub)) } @@ -64,6 +68,14 @@ func (s *Store) DeleteSession(ctx context.Context, uuid string) error { return mapErr(s.q.DeleteSession(ctx, uuid)) } +func (s *Store) GetOIDCConsentByUsernameAndClientID(ctx context.Context, arg repository.GetOIDCConsentByUsernameAndClientIDParams) (repository.OidcConsent, error) { + r, err := s.q.GetOIDCConsentByUsernameAndClientID(ctx, GetOIDCConsentByUsernameAndClientIDParams(arg)) + if err != nil { + return repository.OidcConsent{}, mapErr(err) + } + return repository.OidcConsent(r), nil +} + func (s *Store) GetOIDCSessionByAccessTokenHash(ctx context.Context, accessTokenHash string) (repository.OidcSession, error) { r, err := s.q.GetOIDCSessionByAccessTokenHash(ctx, accessTokenHash) if err != nil { @@ -96,6 +108,18 @@ func (s *Store) GetSession(ctx context.Context, uuid string) (repository.Session return repository.Session(r), nil } +func (s *Store) ListOIDCConsents(ctx context.Context) ([]repository.OidcConsent, error) { + rows, err := s.q.ListOIDCConsents(ctx) + if err != nil { + return nil, mapErr(err) + } + out := make([]repository.OidcConsent, len(rows)) + for i, row := range rows { + out[i] = repository.OidcConsent(row) + } + return out, nil +} + func (s *Store) UpdateOIDCSession(ctx context.Context, arg repository.UpdateOIDCSessionParams) (repository.OidcSession, error) { r, err := s.q.UpdateOIDCSession(ctx, UpdateOIDCSessionParams(arg)) if err != nil { @@ -111,3 +135,11 @@ func (s *Store) UpdateSession(ctx context.Context, arg repository.UpdateSessionP } return repository.Session(r), nil } + +func (s *Store) UpsertOIDCConsent(ctx context.Context, arg repository.UpsertOIDCConsentParams) (repository.OidcConsent, error) { + r, err := s.q.UpsertOIDCConsent(ctx, UpsertOIDCConsentParams(arg)) + if err != nil { + return repository.OidcConsent{}, mapErr(err) + } + return repository.OidcConsent(r), nil +} diff --git a/internal/repository/sqlite/models.go b/internal/repository/sqlite/models.go index 2ced8a2b..f30ae672 100644 --- a/internal/repository/sqlite/models.go +++ b/internal/repository/sqlite/models.go @@ -4,6 +4,13 @@ package sqlite +type OidcConsent struct { + Username string + ClientID string + Scope string + CreatedAt int64 +} + type OidcSession struct { Sub string AccessTokenHash string diff --git a/internal/repository/sqlite/oidc_queries.sql.go b/internal/repository/sqlite/oidc_queries.sql.go index a5aa08a8..e574197a 100644 --- a/internal/repository/sqlite/oidc_queries.sql.go +++ b/internal/repository/sqlite/oidc_queries.sql.go @@ -80,6 +80,16 @@ func (q *Queries) DeleteExpiredOIDCSessions(ctx context.Context, arg DeleteExpir return err } +const deleteOIDCConsentByClientID = `-- name: DeleteOIDCConsentByClientID :exec +DELETE FROM "oidc_consents" +WHERE "client_id" = ? +` + +func (q *Queries) DeleteOIDCConsentByClientID(ctx context.Context, clientID string) error { + _, err := q.db.ExecContext(ctx, deleteOIDCConsentByClientID, clientID) + return err +} + const deleteOIDCSessionBySub = `-- name: DeleteOIDCSessionBySub :exec DELETE FROM "oidc_sessions" WHERE "sub" = ? @@ -90,6 +100,28 @@ func (q *Queries) DeleteOIDCSessionBySub(ctx context.Context, sub string) error return err } +const getOIDCConsentByUsernameAndClientID = `-- name: GetOIDCConsentByUsernameAndClientID :one +SELECT username, client_id, scope, created_at FROM "oidc_consents" +WHERE "username" = ? AND "client_id" = ? +` + +type GetOIDCConsentByUsernameAndClientIDParams struct { + Username string + ClientID string +} + +func (q *Queries) GetOIDCConsentByUsernameAndClientID(ctx context.Context, arg GetOIDCConsentByUsernameAndClientIDParams) (OidcConsent, error) { + row := q.db.QueryRowContext(ctx, getOIDCConsentByUsernameAndClientID, arg.Username, arg.ClientID) + var i OidcConsent + err := row.Scan( + &i.Username, + &i.ClientID, + &i.Scope, + &i.CreatedAt, + ) + return i, err +} + const getOIDCSessionByAccessTokenHash = `-- name: GetOIDCSessionByAccessTokenHash :one SELECT sub, access_token_hash, refresh_token_hash, scope, client_id, token_expires_at, refresh_token_expires_at, nonce, userinfo_json FROM "oidc_sessions" WHERE "access_token_hash" = ? @@ -156,6 +188,38 @@ func (q *Queries) GetOIDCSessionBySub(ctx context.Context, sub string) (OidcSess return i, err } +const listOIDCConsents = `-- name: ListOIDCConsents :many +SELECT username, client_id, scope, created_at FROM "oidc_consents" +` + +func (q *Queries) ListOIDCConsents(ctx context.Context) ([]OidcConsent, error) { + rows, err := q.db.QueryContext(ctx, listOIDCConsents) + if err != nil { + return nil, err + } + defer rows.Close() + var items []OidcConsent + for rows.Next() { + var i OidcConsent + if err := rows.Scan( + &i.Username, + &i.ClientID, + &i.Scope, + &i.CreatedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const updateOIDCSession = `-- name: UpdateOIDCSession :one UPDATE "oidc_sessions" SET "access_token_hash" = ?, @@ -208,3 +272,43 @@ func (q *Queries) UpdateOIDCSession(ctx context.Context, arg UpdateOIDCSessionPa ) return i, err } + +const upsertOIDCConsent = `-- name: UpsertOIDCConsent :one +INSERT INTO "oidc_consents" ( + "username", + "client_id", + "scope", + "created_at" +) VALUES ( + ?, ?, ?, ? +) +ON CONFLICT ("username", "client_id") +DO UPDATE SET + "scope" = excluded.scope, + "created_at" = excluded.created_at +RETURNING username, client_id, scope, created_at +` + +type UpsertOIDCConsentParams struct { + Username string + ClientID string + Scope string + CreatedAt int64 +} + +func (q *Queries) UpsertOIDCConsent(ctx context.Context, arg UpsertOIDCConsentParams) (OidcConsent, error) { + row := q.db.QueryRowContext(ctx, upsertOIDCConsent, + arg.Username, + arg.ClientID, + arg.Scope, + arg.CreatedAt, + ) + var i OidcConsent + err := row.Scan( + &i.Username, + &i.ClientID, + &i.Scope, + &i.CreatedAt, + ) + return i, err +} diff --git a/internal/repository/sqlite/store.go b/internal/repository/sqlite/store.go index a567c871..3c0d8a78 100644 --- a/internal/repository/sqlite/store.go +++ b/internal/repository/sqlite/store.go @@ -56,6 +56,10 @@ func (s *Store) DeleteExpiredSessions(ctx context.Context, expiry int64) error { return mapErr(s.q.DeleteExpiredSessions(ctx, expiry)) } +func (s *Store) DeleteOIDCConsentByClientID(ctx context.Context, clientID string) error { + return mapErr(s.q.DeleteOIDCConsentByClientID(ctx, clientID)) +} + func (s *Store) DeleteOIDCSessionBySub(ctx context.Context, sub string) error { return mapErr(s.q.DeleteOIDCSessionBySub(ctx, sub)) } @@ -64,6 +68,14 @@ func (s *Store) DeleteSession(ctx context.Context, uuid string) error { return mapErr(s.q.DeleteSession(ctx, uuid)) } +func (s *Store) GetOIDCConsentByUsernameAndClientID(ctx context.Context, arg repository.GetOIDCConsentByUsernameAndClientIDParams) (repository.OidcConsent, error) { + r, err := s.q.GetOIDCConsentByUsernameAndClientID(ctx, GetOIDCConsentByUsernameAndClientIDParams(arg)) + if err != nil { + return repository.OidcConsent{}, mapErr(err) + } + return repository.OidcConsent(r), nil +} + func (s *Store) GetOIDCSessionByAccessTokenHash(ctx context.Context, accessTokenHash string) (repository.OidcSession, error) { r, err := s.q.GetOIDCSessionByAccessTokenHash(ctx, accessTokenHash) if err != nil { @@ -96,6 +108,18 @@ func (s *Store) GetSession(ctx context.Context, uuid string) (repository.Session return repository.Session(r), nil } +func (s *Store) ListOIDCConsents(ctx context.Context) ([]repository.OidcConsent, error) { + rows, err := s.q.ListOIDCConsents(ctx) + if err != nil { + return nil, mapErr(err) + } + out := make([]repository.OidcConsent, len(rows)) + for i, row := range rows { + out[i] = repository.OidcConsent(row) + } + return out, nil +} + func (s *Store) UpdateOIDCSession(ctx context.Context, arg repository.UpdateOIDCSessionParams) (repository.OidcSession, error) { r, err := s.q.UpdateOIDCSession(ctx, UpdateOIDCSessionParams(arg)) if err != nil { @@ -111,3 +135,11 @@ func (s *Store) UpdateSession(ctx context.Context, arg repository.UpdateSessionP } return repository.Session(r), nil } + +func (s *Store) UpsertOIDCConsent(ctx context.Context, arg repository.UpsertOIDCConsentParams) (repository.OidcConsent, error) { + r, err := s.q.UpsertOIDCConsent(ctx, UpsertOIDCConsentParams(arg)) + if err != nil { + return repository.OidcConsent{}, mapErr(err) + } + return repository.OidcConsent(r), nil +} diff --git a/internal/repository/store.go b/internal/repository/store.go index abd70bd3..4291cd46 100644 --- a/internal/repository/store.go +++ b/internal/repository/store.go @@ -27,4 +27,10 @@ type Store interface { GetOIDCSessionByRefreshTokenHash(ctx context.Context, refreshTokenHash string) (OidcSession, error) GetOIDCSessionBySub(ctx context.Context, sub string) (OidcSession, error) UpdateOIDCSession(ctx context.Context, arg UpdateOIDCSessionParams) (OidcSession, error) + + // OIDC Consents + UpsertOIDCConsent(ctx context.Context, arg UpsertOIDCConsentParams) (OidcConsent, error) + GetOIDCConsentByUsernameAndClientID(ctx context.Context, arg GetOIDCConsentByUsernameAndClientIDParams) (OidcConsent, error) + DeleteOIDCConsentByClientID(ctx context.Context, clientID string) error + ListOIDCConsents(ctx context.Context) ([]OidcConsent, error) } diff --git a/internal/service/oidc_consent_test.go b/internal/service/oidc_consent_test.go new file mode 100644 index 00000000..96e844fc --- /dev/null +++ b/internal/service/oidc_consent_test.go @@ -0,0 +1,180 @@ +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 418ac574..c37bf771 100644 --- a/internal/service/oidc_service.go +++ b/internal/service/oidc_service.go @@ -3,8 +3,6 @@ package service import ( "context" "crypto" - "crypto/aes" - "crypto/cipher" "crypto/rand" "crypto/rsa" "crypto/sha256" @@ -14,7 +12,6 @@ import ( "encoding/pem" "errors" "fmt" - "io" "net/url" "os" "path/filepath" @@ -32,7 +29,6 @@ import ( "github.com/tinyauthapp/tinyauth/internal/utils/logger" "github.com/tinyauthapp/tinyauth/pkg/cache" "go.uber.org/dig" - "golang.org/x/crypto/hkdf" ) var ( @@ -340,6 +336,11 @@ func NewOIDCService(i OIDCServiceInput) (*OIDCService, error) { issuer: issuer, } + // Remove consents for clients that are no longer configured + if err := service.reconcileOIDCConsents(context.Background()); err != nil { + i.Log.App.Warn().Err(err).Msg("Failed to reconcile OIDC consents") + } + // Start cleanup routine i.Ding.Go(service.cleanupRoutine, ding.RingMinor) @@ -975,82 +976,104 @@ func (service *OIDCService) GetPrompt(prompt string) []OIDCPrompt { return parsedPromps } -func (service *OIDCService) deriveKey(key string, info string) ([]byte, error) { - kdf := hkdf.New(sha256.New, []byte(key), []byte(model.HKDFSalt), []byte(info)) - derived := make([]byte, 32) - if _, err := io.ReadFull(kdf, derived); err != nil { - return nil, fmt.Errorf("failed to derive key: %w", err) +func (service *OIDCService) GetOIDCConsent(ctx context.Context, username, clientId string) (*repository.OidcConsent, error) { + entry, err := service.queries.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{ + Username: username, + ClientID: clientId, + }) + + if err != nil { + if errors.Is(err, repository.ErrNotFound) { + return nil, nil + } + return nil, fmt.Errorf("failed to get oidc consent: %w", err) } - return derived, nil + + return &entry, nil } -func (service *OIDCService) CreateSecureValue(key string, kv map[string]string) (string, error) { - aesKey, err := service.deriveKey(key, "oidc-scope-v1") +func (service *OIDCService) UpsertOIDCConsent(ctx context.Context, username, scope, clientId string) (repository.OidcConsent, error) { + existing, err := service.GetOIDCConsent(ctx, username, clientId) + if err != nil { - return "", err + return repository.OidcConsent{}, err } - c, err := aes.NewCipher(aesKey) + merged := scope + + if existing != nil { + merged = mergeScopes(existing.Scope, scope) + } + + entry := repository.UpsertOIDCConsentParams{ + Username: username, + Scope: merged, + ClientID: clientId, + CreatedAt: time.Now().Unix(), + } + + consent, err := service.queries.UpsertOIDCConsent(ctx, entry) + if err != nil { - return "", fmt.Errorf("failed to create aes cipher: %w", err) + service.log.App.Error().Err(err).Msg("Failed to upsert OIDC consent") + return repository.OidcConsent{}, err } - gcm, err := cipher.NewGCM(c) - if err != nil { - return "", fmt.Errorf("failed to create gcm: %w", err) - } - - nonce := make([]byte, gcm.NonceSize()) - if _, err := io.ReadFull(rand.Reader, nonce); err != nil { - return "", fmt.Errorf("failed to generate nonce: %w", err) - } - - plain, err := json.Marshal(kv) - if err != nil { - return "", fmt.Errorf("failed to marshal data: %w", err) - } - - sealed := gcm.Seal(nonce, nonce, plain, nil) - return base64.RawURLEncoding.EncodeToString(sealed), nil + return consent, nil } -func (service *OIDCService) DecryptSecureValue(key string, encrypted string) (map[string]string, error) { - data, err := base64.RawURLEncoding.DecodeString(encrypted) +func (service *OIDCService) reconcileOIDCConsents(ctx context.Context) error { + consents, err := service.queries.ListOIDCConsents(ctx) + if err != nil { - return nil, fmt.Errorf("failed to decode value: %w", err) + return fmt.Errorf("failed to list oidc consents: %w", err) } - aesKey, err := service.deriveKey(key, "oidc-scope-v1") - if err != nil { - return nil, err + cleaned := make(map[string]struct{}) + + for _, consent := range consents { + if _, ok := cleaned[consent.ClientID]; ok { + continue + } + + cleaned[consent.ClientID] = struct{}{} + + if _, ok := service.clients[consent.ClientID]; ok { + continue + } + + service.log.App.Info().Str("clientId", consent.ClientID).Msg("Removed OIDC client no longer in configuration, deleting its consents") + + if err := service.queries.DeleteOIDCConsentByClientID(ctx, consent.ClientID); err != nil { + service.log.App.Warn().Err(err).Str("clientId", consent.ClientID).Msg("Failed to delete OIDC consents for removed client") + } } - c, err := aes.NewCipher(aesKey) - if err != nil { - return nil, fmt.Errorf("failed to create aes cipher: %w", err) - } + return nil +} + +func mergeScopes(existing, requested string) string { + set := make(map[string]struct{}) - gcm, err := cipher.NewGCM(c) - if err != nil { - return nil, fmt.Errorf("failed to create gcm: %w", err) + for _, scope := range strings.Split(existing, " ") { + if scope != "" { + set[scope] = struct{}{} + } } - nonceSize := gcm.NonceSize() - if len(data) < nonceSize { - return nil, fmt.Errorf("ciphertext too short") + for _, scope := range strings.Split(requested, " ") { + if scope != "" { + set[scope] = struct{}{} + } } - nonce, ciphertext := data[:nonceSize], data[nonceSize:] + scopes := make([]string, 0, len(set)) - plain, err := gcm.Open(nil, nonce, ciphertext, nil) - if err != nil { - return nil, fmt.Errorf("failed to decrypt secure value: %w", err) + for scope := range set { + scopes = append(scopes, scope) } - kv := make(map[string]string) // make(), not var — nil map writes panic - if err := json.Unmarshal(plain, &kv); err != nil { - return nil, fmt.Errorf("failed to parse secure value: %w", err) - } + slices.Sort(scopes) - return kv, nil + return strings.Join(scopes, " ") } diff --git a/sql/postgres/oidc_queries.sql b/sql/postgres/oidc_queries.sql index 3cd5ff99..12d2cf50 100644 --- a/sql/postgres/oidc_queries.sql +++ b/sql/postgres/oidc_queries.sql @@ -46,3 +46,29 @@ UPDATE "oidc_sessions" SET "userinfo_json" = $8 WHERE "sub" = $9 RETURNING *; + +-- name: UpsertOIDCConsent :one +INSERT INTO "oidc_consents" ( + "username", + "client_id", + "scope", + "created_at" +) VALUES ( + $1, $2, $3, $4 +) +ON CONFLICT ("username", "client_id") +DO UPDATE SET + "scope" = excluded.scope, + "created_at" = excluded.created_at +RETURNING *; + +-- name: GetOIDCConsentByUsernameAndClientID :one +SELECT * FROM "oidc_consents" +WHERE "username" = $1 AND "client_id" = $2; + +-- name: DeleteOIDCConsentByClientID :exec +DELETE FROM "oidc_consents" +WHERE "client_id" = $1; + +-- name: ListOIDCConsents :many +SELECT * FROM "oidc_consents"; diff --git a/sql/postgres/oidc_schemas.sql b/sql/postgres/oidc_schemas.sql index 2376c1d4..dad3d332 100644 --- a/sql/postgres/oidc_schemas.sql +++ b/sql/postgres/oidc_schemas.sql @@ -9,3 +9,12 @@ CREATE TABLE IF NOT EXISTS "oidc_sessions" ( "nonce" TEXT NOT NULL DEFAULT '', "userinfo_json" TEXT NOT NULL ); + +CREATE TABLE IF NOT EXISTS "oidc_consents" ( + "username" TEXT NOT NULL, + "client_id" TEXT NOT NULL, + "scope" TEXT NOT NULL, + "created_at" BIGINT NOT NULL, + PRIMARY KEY ("username", "client_id") +); + diff --git a/sql/sqlite/oidc_queries.sql b/sql/sqlite/oidc_queries.sql index 49b33cff..ee4528ea 100644 --- a/sql/sqlite/oidc_queries.sql +++ b/sql/sqlite/oidc_queries.sql @@ -46,3 +46,29 @@ UPDATE "oidc_sessions" SET "userinfo_json" = ? WHERE "sub" = ? RETURNING *; + +-- name: UpsertOIDCConsent :one +INSERT INTO "oidc_consents" ( + "username", + "client_id", + "scope", + "created_at" +) VALUES ( + ?, ?, ?, ? +) +ON CONFLICT ("username", "client_id") +DO UPDATE SET + "scope" = excluded.scope, + "created_at" = excluded.created_at +RETURNING *; + +-- name: GetOIDCConsentByUsernameAndClientID :one +SELECT * FROM "oidc_consents" +WHERE "username" = ? AND "client_id" = ?; + +-- name: DeleteOIDCConsentByClientID :exec +DELETE FROM "oidc_consents" +WHERE "client_id" = ?; + +-- name: ListOIDCConsents :many +SELECT * FROM "oidc_consents"; diff --git a/sql/sqlite/oidc_schemas.sql b/sql/sqlite/oidc_schemas.sql index 5a851033..aac3aeac 100644 --- a/sql/sqlite/oidc_schemas.sql +++ b/sql/sqlite/oidc_schemas.sql @@ -9,3 +9,11 @@ CREATE TABLE IF NOT EXISTS "oidc_sessions" ( "nonce" TEXT NOT NULL DEFAULT "", "userinfo_json" TEXT NOT NULL ); + +CREATE TABLE IF NOT EXISTS "oidc_consents" ( + "username" TEXT NOT NULL, + "client_id" TEXT NOT NULL, + "scope" TEXT NOT NULL, + "created_at" INTEGER NOT NULL, + PRIMARY KEY ("username", "client_id") +);