From c65a66c61db84e3446cb153606c13a4c404de91a Mon Sep 17 00:00:00 2001 From: Til Wegener <38760774+tilwegener@users.noreply.github.com> Date: Thu, 24 Sep 2026 21:24:08 +0000 Subject: [PATCH 1/5] feat(oidc): allow trusted clients to skip consent --- .env.example | 2 ++ internal/controller/oidc_controller.go | 8 +++++++ internal/controller/oidc_controller_test.go | 23 +++++++++++++++++++++ internal/model/config.go | 1 + internal/test/test.go | 7 +++++++ 5 files changed, 41 insertions(+) diff --git a/.env.example b/.env.example index bfdc357d..d843ae55 100644 --- a/.env.example +++ b/.env.example @@ -194,6 +194,8 @@ TINYAUTH_OIDC_CLIENTS_name_CLIENTSECRET= TINYAUTH_OIDC_CLIENTS_name_CLIENTSECRETFILE= # List of trusted redirect URIs. TINYAUTH_OIDC_CLIENTS_name_TRUSTEDREDIRECTURIS= +# Skip the consent screen for this trusted OIDC client. +TINYAUTH_OIDC_CLIENTS_name_TRUSTED=false # Client name in UI. TINYAUTH_OIDC_CLIENTS_name_NAME= diff --git a/internal/controller/oidc_controller.go b/internal/controller/oidc_controller.go index a4b2d76d..580b5874 100644 --- a/internal/controller/oidc_controller.go +++ b/internal/controller/oidc_controller.go @@ -327,6 +327,14 @@ func (controller *OIDCController) skipConsent(c *gin.Context) { return } + client, ok := controller.oidc.GetClient(authorizeReq.ClientID) + if ok && client.Trusted { + c.JSON(200, SkipConsentResponse{ + SkipConsent: true, + }) + return + } + consent, err := controller.oidc.GetOIDCConsent(c, userContext.GetUsername(), authorizeReq.ClientID) if err != nil || consent == nil { diff --git a/internal/controller/oidc_controller_test.go b/internal/controller/oidc_controller_test.go index e3da603b..f95113b0 100644 --- a/internal/controller/oidc_controller_test.go +++ b/internal/controller/oidc_controller_test.go @@ -351,6 +351,29 @@ func TestOIDCController(t *testing.T) { assert.False(t, res.SkipConsent) }, }, + { + description: "Skip consent returns true for a trusted client without prior consent", + middlewares: []gin.HandlerFunc{authedUser}, + run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { + require.NoError(t, store.DeleteOIDCConsentByClientID(ctx, "trusted-client-id")) + + ticket := oidcService.CreateAuthorizeRequestTicket(service.AuthorizeRequest{ + Scope: "openid profile", + ResponseType: "code", + ClientID: "trusted-client-id", + RedirectURI: "https://trusted.example.com/callback", + }) + + req := httptest.NewRequest("GET", "/api/oidc/skip-consent?oidc_ticket="+url.QueryEscape(ticket), nil) + router.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusOK, recorder.Code) + + var res SkipConsentResponse + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &res)) + assert.True(t, res.SkipConsent) + }, + }, { description: "Skip consent returns false when a new scope is requested", middlewares: []gin.HandlerFunc{authedUser}, diff --git a/internal/model/config.go b/internal/model/config.go index 24ba6302..b86cffa3 100644 --- a/internal/model/config.go +++ b/internal/model/config.go @@ -282,6 +282,7 @@ type OIDCClientConfig struct { ClientSecret string `description:"OIDC client secret." yaml:"clientSecret,omitempty"` ClientSecretFile string `description:"Path to the file containing the OIDC client secret." yaml:"clientSecretFile,omitempty"` TrustedRedirectURIs []string `description:"List of trusted redirect URIs." yaml:"trustedRedirectUris,omitempty"` + Trusted bool `description:"Skip the consent screen for this trusted OIDC client." yaml:"trusted,omitempty"` Name string `description:"Client name in UI." yaml:"name,omitempty"` } diff --git a/internal/test/test.go b/internal/test/test.go index 45daf2a1..6ff41a88 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -32,6 +32,13 @@ func CreateTestConfigs(t *testing.T) (model.Config, model.RuntimeConfig) { TrustedRedirectURIs: []string{"https://test.example.com/callback"}, Name: "Test Client", }, + "trusted-test": { + ClientID: "trusted-client-id", + ClientSecret: "trusted-client-secret", + TrustedRedirectURIs: []string{"https://trusted.example.com/callback"}, + Trusted: true, + Name: "Trusted Test Client", + }, }, PrivateKeyPath: filepath.Join(tempDir, "key.pem"), PublicKeyPath: filepath.Join(tempDir, "key.pub"), From e67210a2d1b2007a2831716f7f622cbc20a61200 Mon Sep 17 00:00:00 2001 From: Til Wegener <38760774+tilwegener@users.noreply.github.com> Date: Fri, 25 Sep 2026 08:33:47 +0000 Subject: [PATCH 2/5] fix(oidc): safely complete trusted authorization --- frontend/src/pages/authorize-page.tsx | 5 +++ internal/controller/oidc_controller.go | 49 ++++++++++++++------- internal/controller/oidc_controller_test.go | 32 ++++++++++++++ 3 files changed, 71 insertions(+), 15 deletions(-) diff --git a/frontend/src/pages/authorize-page.tsx b/frontend/src/pages/authorize-page.tsx index 261a7da3..9649b740 100644 --- a/frontend/src/pages/authorize-page.tsx +++ b/frontend/src/pages/authorize-page.tsx @@ -37,6 +37,7 @@ type Scope = { const skipConsentResponseSchema = z.object({ skipConsent: z.boolean(), + redirectUri: z.string().url().optional(), }) const scopeMapIconProps = { @@ -138,6 +139,10 @@ export const AuthorizePage = () => { const parsed = skipConsentResponseSchema.safeParse(await res.json()); if (!active || !parsed.success || !parsed.data.skipConsent) return; setAutoAuthorize(true); + if (parsed.data.redirectUri) { + window.location.replace(parsed.data.redirectUri); + return; + } authorizeMutate(); } catch { // Fall back to manual consent on any failure (including abort). diff --git a/internal/controller/oidc_controller.go b/internal/controller/oidc_controller.go index 580b5874..1ec3d657 100644 --- a/internal/controller/oidc_controller.go +++ b/internal/controller/oidc_controller.go @@ -74,7 +74,8 @@ type SkipConsentRequest struct { } type SkipConsentResponse struct { - SkipConsent bool `json:"skipConsent"` + SkipConsent bool `json:"skipConsent"` + RedirectURI string `json:"redirectUri,omitempty"` } type AuthorizeScreenParams struct { @@ -318,9 +319,10 @@ func (controller *OIDCController) skipConsent(c *gin.Context) { return } - controller.log.App.Debug().Str("client", authorizeReq.ClientID).Str("user", userContext.GetUsername()).Msg("User consented to OIDC") + controller.log.App.Debug().Str("client", authorizeReq.ClientID).Str("user", userContext.GetUsername()).Msg("Checking OIDC consent") - if authorizeReq.Prompt == service.OIDCPromptLogin.String() { + prompts := controller.oidc.GetPrompt(authorizeReq.Prompt) + if slices.Contains(prompts, service.OIDCPromptLogin) { c.JSON(200, SkipConsentResponse{ SkipConsent: false, }) @@ -329,8 +331,14 @@ func (controller *OIDCController) skipConsent(c *gin.Context) { client, ok := controller.oidc.GetClient(authorizeReq.ClientID) if ok && client.Trusted { + redirectURI, completed := controller.completeAuthorization(c, req.OIDCTicket, authorizeReq, userContext, false) + if !completed { + return + } + c.JSON(200, SkipConsentResponse{ SkipConsent: true, + RedirectURI: redirectURI, }) return } @@ -419,8 +427,20 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) { return } + redirectURI, completed := controller.completeAuthorization(c, req.Ticket, authorizeReq, userContext, true) + if !completed { + return + } + + c.JSON(200, gin.H{ + "status": 200, + "redirect_uri": redirectURI, + }) +} + +func (controller *OIDCController) completeAuthorization(c *gin.Context, ticket string, authorizeReq *service.AuthorizeRequest, userContext *model.UserContext, persistConsent bool) (string, bool) { // We no longer need the ticket - controller.oidc.DeleteAuthorizeRequestTicket(req.Ticket) + controller.oidc.DeleteAuthorizeRequestTicket(ticket) // Get the client client, ok := controller.oidc.GetClient(authorizeReq.ClientID) @@ -432,14 +452,14 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) { reasonPublic: "The client is not configured", json: true, }) - return + return "", false } // Create the sub to find and delete old sessions sub := controller.oidc.CreateSub(*userContext, authorizeReq.ClientID) // Before storing the code, delete old session - err = controller.oidc.DeleteOldSession(c, sub) + err := controller.oidc.DeleteOldSession(c, sub) if err != nil { controller.authorizeError(c, authorizeErrorParams{ err: err, @@ -450,7 +470,7 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) { state: authorizeReq.State, json: true, }) - return + return "", false } // Create the authorization code @@ -465,12 +485,14 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) { reasonPublic: "Failed to parse redirect URI", json: true, }) - return + return "", false } - // 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") + if persistConsent { + // Only store consent when the user explicitly approved the request. + 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() @@ -483,10 +505,7 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) { cu.RawQuery = q.Encode() - c.JSON(200, gin.H{ - "status": 200, - "redirect_uri": cu.String(), - }) + return cu.String(), true } func (controller *OIDCController) Token(c *gin.Context) { diff --git a/internal/controller/oidc_controller_test.go b/internal/controller/oidc_controller_test.go index f95113b0..0ec31112 100644 --- a/internal/controller/oidc_controller_test.go +++ b/internal/controller/oidc_controller_test.go @@ -372,6 +372,38 @@ func TestOIDCController(t *testing.T) { var res SkipConsentResponse require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &res)) assert.True(t, res.SkipConsent) + assert.Contains(t, res.RedirectURI, "https://trusted.example.com/callback?code=") + + _, err := store.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{ + Username: "testuser", + ClientID: "trusted-client-id", + }) + assert.ErrorIs(t, err, repository.ErrNotFound) + _, ok := oidcService.GetAuthorizeRequestByTicket(ticket) + assert.False(t, ok) + }, + }, + { + description: "Skip consent returns false for a trusted client when prompt includes login", + middlewares: []gin.HandlerFunc{authedUser}, + run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { + ticket := oidcService.CreateAuthorizeRequestTicket(service.AuthorizeRequest{ + Scope: "openid profile", + ResponseType: "code", + ClientID: "trusted-client-id", + RedirectURI: "https://trusted.example.com/callback", + Prompt: "login consent", + }) + + req := httptest.NewRequest("GET", "/api/oidc/skip-consent?oidc_ticket="+url.QueryEscape(ticket), nil) + router.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusOK, recorder.Code) + + var res SkipConsentResponse + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &res)) + assert.False(t, res.SkipConsent) + assert.Empty(t, res.RedirectURI) }, }, { From e9b0abf6fbc7e434357c6d327bc68994a2535cd3 Mon Sep 17 00:00:00 2001 From: Til Wegener <38760774+tilwegener@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:34:21 +0000 Subject: [PATCH 3/5] fix(oidc): make trusted authorization retryable --- internal/controller/oidc_controller.go | 77 ++++++++++++--------- internal/controller/oidc_controller_test.go | 13 ++++ internal/service/oidc_service.go | 31 ++++++++- 3 files changed, 86 insertions(+), 35 deletions(-) diff --git a/internal/controller/oidc_controller.go b/internal/controller/oidc_controller.go index 1ec3d657..7370e136 100644 --- a/internal/controller/oidc_controller.go +++ b/internal/controller/oidc_controller.go @@ -1,6 +1,7 @@ package controller import ( + "context" "crypto/sha256" "crypto/subtle" "encoding/json" @@ -33,6 +34,8 @@ type authorizeErrorParams struct { json bool } +var errOIDCClientNotFound = errors.New("oidc client not found") + type OIDCController struct { log *logger.Logger oidc *service.OIDCService @@ -313,6 +316,14 @@ func (controller *OIDCController) skipConsent(c *gin.Context) { authorizeReq, ok := controller.oidc.GetAuthorizeRequestByTicket(req.OIDCTicket) if !ok { + if redirectURI, completed := controller.oidc.GetCompletedAuthorizeRequest(req.OIDCTicket, userContext.GetUsername()); completed { + c.JSON(200, SkipConsentResponse{ + SkipConsent: true, + RedirectURI: redirectURI, + }) + return + } + c.JSON(200, SkipConsentResponse{ SkipConsent: false, }) @@ -331,10 +342,12 @@ func (controller *OIDCController) skipConsent(c *gin.Context) { client, ok := controller.oidc.GetClient(authorizeReq.ClientID) if ok && client.Trusted { - redirectURI, completed := controller.completeAuthorization(c, req.OIDCTicket, authorizeReq, userContext, false) - if !completed { + redirectURI, err := controller.completeAuthorization(c.Request.Context(), req.OIDCTicket, authorizeReq, userContext, false) + if err != nil { + controller.writeCompleteAuthorizationError(c, authorizeReq, err) return } + controller.oidc.StoreCompletedAuthorizeRequest(req.OIDCTicket, userContext.GetUsername(), redirectURI) c.JSON(200, SkipConsentResponse{ SkipConsent: true, @@ -427,8 +440,9 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) { return } - redirectURI, completed := controller.completeAuthorization(c, req.Ticket, authorizeReq, userContext, true) - if !completed { + redirectURI, err := controller.completeAuthorization(c.Request.Context(), req.Ticket, authorizeReq, userContext, true) + if err != nil { + controller.writeCompleteAuthorizationError(c, authorizeReq, err) return } @@ -438,7 +452,7 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) { }) } -func (controller *OIDCController) completeAuthorization(c *gin.Context, ticket string, authorizeReq *service.AuthorizeRequest, userContext *model.UserContext, persistConsent bool) (string, bool) { +func (controller *OIDCController) completeAuthorization(ctx context.Context, ticket string, authorizeReq *service.AuthorizeRequest, userContext *model.UserContext, persistConsent bool) (string, error) { // We no longer need the ticket controller.oidc.DeleteAuthorizeRequestTicket(ticket) @@ -446,31 +460,16 @@ func (controller *OIDCController) completeAuthorization(c *gin.Context, ticket s client, ok := controller.oidc.GetClient(authorizeReq.ClientID) if !ok { - controller.authorizeError(c, authorizeErrorParams{ - err: errors.New("client not found"), - reason: "Client not found", - reasonPublic: "The client is not configured", - json: true, - }) - return "", false + return "", errOIDCClientNotFound } // Create the sub to find and delete old sessions sub := controller.oidc.CreateSub(*userContext, authorizeReq.ClientID) // Before storing the code, delete old session - err := controller.oidc.DeleteOldSession(c, sub) + err := controller.oidc.DeleteOldSession(ctx, sub) if err != nil { - controller.authorizeError(c, authorizeErrorParams{ - err: err, - reason: "Failed to delete old sessions", - reasonPublic: "Failed to delete old sessions", - callback: authorizeReq.RedirectURI, - callbackError: "server_error", - state: authorizeReq.State, - json: true, - }) - return "", false + return "", fmt.Errorf("failed to delete old sessions: %w", err) } // Create the authorization code @@ -479,18 +478,12 @@ func (controller *OIDCController) completeAuthorization(c *gin.Context, ticket s cu, err := url.Parse(authorizeReq.RedirectURI) if err != nil { - controller.authorizeError(c, authorizeErrorParams{ - err: err, - reason: "Failed to parse redirect URI", - reasonPublic: "Failed to parse redirect URI", - json: true, - }) - return "", false + return "", fmt.Errorf("failed to parse redirect URI: %w", err) } if persistConsent { // Only store consent when the user explicitly approved the request. - if _, err := controller.oidc.UpsertOIDCConsent(c, userContext.GetUsername(), authorizeReq.Scope, client.ClientID); err != nil { + if _, err := controller.oidc.UpsertOIDCConsent(ctx, userContext.GetUsername(), authorizeReq.Scope, client.ClientID); err != nil { controller.log.App.Warn().Err(err).Msg("Failed to store OIDC consent") } } @@ -505,7 +498,27 @@ func (controller *OIDCController) completeAuthorization(c *gin.Context, ticket s cu.RawQuery = q.Encode() - return cu.String(), true + return cu.String(), nil +} + +func (controller *OIDCController) writeCompleteAuthorizationError(c *gin.Context, authorizeReq *service.AuthorizeRequest, err error) { + params := authorizeErrorParams{ + err: err, + reason: "Failed to complete authorization", + reasonPublic: "Failed to complete authorization", + json: true, + } + + if errors.Is(err, errOIDCClientNotFound) { + params.reason = "Client not found" + params.reasonPublic = "The client is not configured" + } else { + params.callback = authorizeReq.RedirectURI + params.callbackError = "server_error" + params.state = authorizeReq.State + } + + controller.authorizeError(c, params) } func (controller *OIDCController) Token(c *gin.Context) { diff --git a/internal/controller/oidc_controller_test.go b/internal/controller/oidc_controller_test.go index 0ec31112..90ddc8b4 100644 --- a/internal/controller/oidc_controller_test.go +++ b/internal/controller/oidc_controller_test.go @@ -381,6 +381,19 @@ func TestOIDCController(t *testing.T) { assert.ErrorIs(t, err, repository.ErrNotFound) _, ok := oidcService.GetAuthorizeRequestByTicket(ticket) assert.False(t, ok) + _, ok = oidcService.GetCompletedAuthorizeRequest(ticket, "otheruser") + assert.False(t, ok) + + retryRecorder := httptest.NewRecorder() + retryReq := httptest.NewRequest("GET", "/api/oidc/skip-consent?oidc_ticket="+url.QueryEscape(ticket), nil) + router.ServeHTTP(retryRecorder, retryReq) + + assert.Equal(t, http.StatusOK, retryRecorder.Code) + + var retryRes SkipConsentResponse + require.NoError(t, json.Unmarshal(retryRecorder.Body.Bytes(), &retryRes)) + assert.True(t, retryRes.SkipConsent) + assert.Equal(t, res.RedirectURI, retryRes.RedirectURI) }, }, { diff --git a/internal/service/oidc_service.go b/internal/service/oidc_service.go index c5f8ecd3..7b1eb797 100644 --- a/internal/service/oidc_service.go +++ b/internal/service/oidc_service.go @@ -160,6 +160,11 @@ type UsedCodeEntry struct { Sub string } +type CompletedAuthorizeEntry struct { + Username string + RedirectURI string +} + type OIDCService struct { log *logger.Logger config *model.Config @@ -172,9 +177,10 @@ type OIDCService struct { issuer string caches struct { - code *cache.CacheStore[AuthorizeCodeEntry] - usedCode *cache.CacheStore[UsedCodeEntry] - authorize *cache.CacheStore[AuthorizeRequest] + code *cache.CacheStore[AuthorizeCodeEntry] + usedCode *cache.CacheStore[UsedCodeEntry] + authorize *cache.CacheStore[AuthorizeRequest] + completedAuthorize *cache.CacheStore[CompletedAuthorizeEntry] } mus struct { @@ -365,10 +371,12 @@ func NewOIDCService(i OIDCServiceInput) (*OIDCService, error) { codeCache := cache.NewCacheStore[AuthorizeCodeEntry](256) usedCode := cache.NewCacheStore[UsedCodeEntry](256) authorize := cache.NewCacheStore[AuthorizeRequest](256) + completedAuthorize := cache.NewCacheStore[CompletedAuthorizeEntry](256) service.caches.code = codeCache service.caches.usedCode = usedCode service.caches.authorize = authorize + service.caches.completedAuthorize = completedAuthorize // Start cache cleanup routine i.Ding.Go(func(ctx context.Context) { @@ -381,6 +389,7 @@ func NewOIDCService(i OIDCServiceInput) (*OIDCService, error) { service.caches.code.Sweep() service.caches.usedCode.Sweep() service.caches.authorize.Sweep() + service.caches.completedAuthorize.Sweep() case <-ctx.Done(): return } @@ -942,6 +951,22 @@ func (service *OIDCService) DeleteAuthorizeRequestTicket(ticket string) { service.caches.authorize.Delete(ticket) } +func (service *OIDCService) StoreCompletedAuthorizeRequest(ticket, username, redirectURI string) { + service.caches.completedAuthorize.Set(ticket, CompletedAuthorizeEntry{ + Username: username, + RedirectURI: redirectURI, + }, time.Minute) +} + +func (service *OIDCService) GetCompletedAuthorizeRequest(ticket, username string) (string, bool) { + entry, ok := service.caches.completedAuthorize.Get(ticket) + if !ok || entry.Username != username { + return "", false + } + + return entry.RedirectURI, true +} + // DecodeAuthorizeJWT TODO: support signed request objects in the future func (service *OIDCService) DecodeAuthorizeJWT(tokenString string) (*AuthorizeRequest, error) { var claims jwt.MapClaims From 7cf04a0fd05945979af623a6a67732f56308a3c5 Mon Sep 17 00:00:00 2001 From: Til Wegener <38760774+tilwegener@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:57:37 +0000 Subject: [PATCH 4/5] fix(oidc): enforce trusted authorization constraints --- internal/controller/oidc_controller.go | 48 ++++++++++++++--- internal/controller/oidc_controller_test.go | 59 +++++++++++++++++++++ internal/service/oidc_service.go | 18 +++++++ 3 files changed, 119 insertions(+), 6 deletions(-) diff --git a/internal/controller/oidc_controller.go b/internal/controller/oidc_controller.go index 7370e136..d72bf14a 100644 --- a/internal/controller/oidc_controller.go +++ b/internal/controller/oidc_controller.go @@ -339,6 +339,45 @@ func (controller *OIDCController) skipConsent(c *gin.Context) { }) return } + if authorizeReq.MaxAge != "" { + maxAge, err := strconv.Atoi(authorizeReq.MaxAge) + if err != nil || time.Unix(userContext.AuthTime, 0).Add(time.Duration(maxAge)*time.Second).Before(time.Now()) { + c.JSON(200, SkipConsentResponse{ + SkipConsent: false, + }) + return + } + } + + client, ok := controller.oidc.GetClient(authorizeReq.ClientID) + if ok && client.Trusted { + authorizeReq, claimed := controller.oidc.ClaimAuthorizeRequestTicket(req.OIDCTicket) + if !claimed { + if redirectURI, completed := controller.oidc.GetCompletedAuthorizeRequest(req.OIDCTicket, userContext.GetUsername()); completed { + c.JSON(200, SkipConsentResponse{ + SkipConsent: true, + RedirectURI: redirectURI, + }) + return + } + + c.JSON(200, SkipConsentResponse{SkipConsent: false}) + return + } + + redirectURI, err := controller.completeAuthorization(c.Request.Context(), authorizeReq, userContext, false) + if err != nil { + controller.writeCompleteAuthorizationError(c, authorizeReq, err) + return + } + controller.oidc.StoreCompletedAuthorizeRequest(req.OIDCTicket, userContext.GetUsername(), redirectURI) + + c.JSON(200, SkipConsentResponse{ + SkipConsent: true, + RedirectURI: redirectURI, + }) + return + } client, ok := controller.oidc.GetClient(authorizeReq.ClientID) if ok && client.Trusted { @@ -428,7 +467,7 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) { return } - authorizeReq, ok := controller.oidc.GetAuthorizeRequestByTicket(req.Ticket) + authorizeReq, ok := controller.oidc.ClaimAuthorizeRequestTicket(req.Ticket) if !ok { controller.authorizeError(c, authorizeErrorParams{ @@ -440,7 +479,7 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) { return } - redirectURI, err := controller.completeAuthorization(c.Request.Context(), req.Ticket, authorizeReq, userContext, true) + redirectURI, err := controller.completeAuthorization(c.Request.Context(), authorizeReq, userContext, true) if err != nil { controller.writeCompleteAuthorizationError(c, authorizeReq, err) return @@ -452,10 +491,7 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) { }) } -func (controller *OIDCController) completeAuthorization(ctx context.Context, ticket string, authorizeReq *service.AuthorizeRequest, userContext *model.UserContext, persistConsent bool) (string, error) { - // We no longer need the ticket - controller.oidc.DeleteAuthorizeRequestTicket(ticket) - +func (controller *OIDCController) completeAuthorization(ctx context.Context, authorizeReq *service.AuthorizeRequest, userContext *model.UserContext, persistConsent bool) (string, error) { // Get the client client, ok := controller.oidc.GetClient(authorizeReq.ClientID) diff --git a/internal/controller/oidc_controller_test.go b/internal/controller/oidc_controller_test.go index 90ddc8b4..c77ff9f0 100644 --- a/internal/controller/oidc_controller_test.go +++ b/internal/controller/oidc_controller_test.go @@ -7,6 +7,8 @@ import ( "net/http/httptest" "net/url" "strings" + "sync" + "sync/atomic" "testing" "time" @@ -419,6 +421,63 @@ func TestOIDCController(t *testing.T) { assert.Empty(t, res.RedirectURI) }, }, + { + description: "Skip consent returns false for a trusted client when max age is exceeded", + middlewares: []gin.HandlerFunc{ + func(c *gin.Context) { + c.Set("context", &model.UserContext{ + Authenticated: true, + AuthTime: time.Now().Add(-time.Hour).Unix(), + Provider: model.ProviderLocal, + Local: &model.LocalContext{ + BaseContext: model.BaseContext{Username: "testuser"}, + }, + }) + }, + }, + run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { + ticket := oidcService.CreateAuthorizeRequestTicket(service.AuthorizeRequest{ + Scope: "openid profile", + ResponseType: "code", + ClientID: "trusted-client-id", + RedirectURI: "https://trusted.example.com/callback", + MaxAge: "60", + }) + + req := httptest.NewRequest("GET", "/api/oidc/skip-consent?oidc_ticket="+url.QueryEscape(ticket), nil) + router.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusOK, recorder.Code) + + var res SkipConsentResponse + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &res)) + assert.False(t, res.SkipConsent) + assert.Empty(t, res.RedirectURI) + _, ok := oidcService.GetAuthorizeRequestByTicket(ticket) + assert.True(t, ok) + }, + }, + { + description: "Authorize request ticket can only be claimed once", + run: func(t *testing.T, _ *gin.Engine, _ *httptest.ResponseRecorder) { + ticket := oidcService.CreateAuthorizeRequestTicket(service.AuthorizeRequest{ClientID: "trusted-client-id"}) + var claims atomic.Int32 + var wg sync.WaitGroup + + for range 16 { + wg.Add(1) + go func() { + defer wg.Done() + if _, ok := oidcService.ClaimAuthorizeRequestTicket(ticket); ok { + claims.Add(1) + } + }() + } + + wg.Wait() + assert.Equal(t, int32(1), claims.Load()) + }, + }, { description: "Skip consent returns false when a new scope is requested", middlewares: []gin.HandlerFunc{authedUser}, diff --git a/internal/service/oidc_service.go b/internal/service/oidc_service.go index 7b1eb797..dd9822c8 100644 --- a/internal/service/oidc_service.go +++ b/internal/service/oidc_service.go @@ -947,6 +947,24 @@ func (service *OIDCService) GetAuthorizeRequestByTicket(ticket string) (*Authori return &entry, true } +func (service *OIDCService) ClaimAuthorizeRequestTicket(ticket string) (*AuthorizeRequest, bool) { + var entry AuthorizeRequest + claimed := false + + service.caches.authorize.WithLock(func(actions cache.CacheStoreActions[AuthorizeRequest]) { + entry, claimed = actions.Get(ticket) + if claimed { + actions.Delete(ticket) + } + }) + + if !claimed { + return nil, false + } + + return &entry, true +} + func (service *OIDCService) DeleteAuthorizeRequestTicket(ticket string) { service.caches.authorize.Delete(ticket) } From 0f795906ad20f8b850d4dc17358649f5f7a8cd28 Mon Sep 17 00:00:00 2001 From: Til Wegener <38760774+tilwegener@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:14:30 +0000 Subject: [PATCH 5/5] fix(oidc): remove old trusted-client --- internal/controller/oidc_controller.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/internal/controller/oidc_controller.go b/internal/controller/oidc_controller.go index d72bf14a..f5489ba2 100644 --- a/internal/controller/oidc_controller.go +++ b/internal/controller/oidc_controller.go @@ -379,22 +379,6 @@ func (controller *OIDCController) skipConsent(c *gin.Context) { return } - client, ok := controller.oidc.GetClient(authorizeReq.ClientID) - if ok && client.Trusted { - redirectURI, err := controller.completeAuthorization(c.Request.Context(), req.OIDCTicket, authorizeReq, userContext, false) - if err != nil { - controller.writeCompleteAuthorizationError(c, authorizeReq, err) - return - } - controller.oidc.StoreCompletedAuthorizeRequest(req.OIDCTicket, userContext.GetUsername(), redirectURI) - - c.JSON(200, SkipConsentResponse{ - SkipConsent: true, - RedirectURI: redirectURI, - }) - return - } - consent, err := controller.oidc.GetOIDCConsent(c, userContext.GetUsername(), authorizeReq.ClientID) if err != nil || consent == nil {