From d864e2414558319f9e1ee878b117a5578388f9eb Mon Sep 17 00:00:00 2001 From: Christoph Haas Date: Tue, 10 Jun 2025 17:51:45 +0200 Subject: [PATCH] improve logging of OAuth login issues, decrease auth-code exchange timeout (#451) (cherry picked from commit e3b65ca3370cdad96c414047a0a297fe41fe0119) --- .../app/api/core/assets/doc/v0_swagger.json | 46 +++++++++++++++++++ .../app/api/core/assets/doc/v0_swagger.yaml | 36 ++++++++------- .../v0/handlers/endpoint_authentication.go | 15 ++++-- 3 files changed, 77 insertions(+), 20 deletions(-) diff --git a/internal/app/api/core/assets/doc/v0_swagger.json b/internal/app/api/core/assets/doc/v0_swagger.json index 3ca06a1..f780025 100644 --- a/internal/app/api/core/assets/doc/v0_swagger.json +++ b/internal/app/api/core/assets/doc/v0_swagger.json @@ -57,6 +57,52 @@ } } }, + "/auth/login/{provider}/callback": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "Authentication" + ], + "summary": "Handle the OAuth callback.", + "operationId": "auth_handleOauthCallbackGet", + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/model.LoginProviderInfo" + } + } + } + } + } + }, + "/auth/login/{provider}/init": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "Authentication" + ], + "summary": "Initiate the OAuth login flow.", + "operationId": "auth_handleOauthInitiateGet", + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/model.LoginProviderInfo" + } + } + } + } + } + }, "/auth/logout": { "post": { "produces": [ diff --git a/internal/app/api/core/assets/doc/v0_swagger.yaml b/internal/app/api/core/assets/doc/v0_swagger.yaml index ab42b60..285fc8d 100644 --- a/internal/app/api/core/assets/doc/v0_swagger.yaml +++ b/internal/app/api/core/assets/doc/v0_swagger.yaml @@ -383,6 +383,8 @@ definitions: type: boolean MailLinkOnly: type: boolean + MinPasswordLength: + type: integer PersistentConfigSupported: type: boolean SelfProvisioning: @@ -456,7 +458,22 @@ paths: summary: Get all available audit entries. Ordered by timestamp. tags: - Audit - /auth/{provider}/callback: + /auth/login: + post: + operationId: auth_handleLoginPost + produces: + - application/json + responses: + "200": + description: OK + schema: + items: + $ref: '#/definitions/model.LoginProviderInfo' + type: array + summary: Get all available external login providers. + tags: + - Authentication + /auth/login/{provider}/callback: get: operationId: auth_handleOauthCallbackGet produces: @@ -471,7 +488,7 @@ paths: summary: Handle the OAuth callback. tags: - Authentication - /auth/{provider}/init: + /auth/login/{provider}/init: get: operationId: auth_handleOauthInitiateGet produces: @@ -486,21 +503,6 @@ paths: summary: Initiate the OAuth login flow. tags: - Authentication - /auth/login: - post: - operationId: auth_handleLoginPost - produces: - - application/json - responses: - "200": - description: OK - schema: - items: - $ref: '#/definitions/model.LoginProviderInfo' - type: array - summary: Get all available external login providers. - tags: - - Authentication /auth/logout: post: operationId: auth_handleLogoutPost diff --git a/internal/app/api/v0/handlers/endpoint_authentication.go b/internal/app/api/v0/handlers/endpoint_authentication.go index 56a889b..8d9d2cd 100644 --- a/internal/app/api/v0/handlers/endpoint_authentication.go +++ b/internal/app/api/v0/handlers/endpoint_authentication.go @@ -2,6 +2,7 @@ package handlers import ( "context" + "log/slog" "net/http" "net/url" "strconv" @@ -132,7 +133,7 @@ func (e AuthEndpoint) handleSessionInfoGet() http.HandlerFunc { // @Summary Initiate the OAuth login flow. // @Produce json // @Success 200 {object} []model.LoginProviderInfo -// @Router /auth/{provider}/init [get] +// @Router /auth/login/{provider}/init [get] func (e AuthEndpoint) handleOauthInitiateGet() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { currentSession := e.session.GetData(r.Context()) @@ -177,6 +178,8 @@ func (e AuthEndpoint) handleOauthInitiateGet() http.HandlerFunc { authCodeUrl, state, nonce, err := e.authService.OauthLoginStep1(context.Background(), provider) if err != nil { + slog.Debug("failed to create oauth auth code URL", + "provider", provider, "error", err) if autoRedirect && e.isValidReturnUrl(returnTo) { redirectToReturn() } else { @@ -211,7 +214,7 @@ func (e AuthEndpoint) handleOauthInitiateGet() http.HandlerFunc { // @Summary Handle the OAuth callback. // @Produce json // @Success 200 {object} []model.LoginProviderInfo -// @Router /auth/{provider}/callback [get] +// @Router /auth/login/{provider}/callback [get] func (e AuthEndpoint) handleOauthCallbackGet() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { currentSession := e.session.GetData(r.Context()) @@ -249,6 +252,8 @@ func (e AuthEndpoint) handleOauthCallbackGet() http.HandlerFunc { oauthState := request.Query(r, "state") if provider != currentSession.OauthProvider { + slog.Debug("invalid oauth provider in callback", + "expected", currentSession.OauthProvider, "got", provider, "state", oauthState) if returnUrl != nil && e.isValidReturnUrl(returnUrl.String()) { redirectToReturn() } else { @@ -258,6 +263,8 @@ func (e AuthEndpoint) handleOauthCallbackGet() http.HandlerFunc { return } if oauthState != currentSession.OauthState { + slog.Debug("invalid oauth state in callback", + "expected", currentSession.OauthState, "got", oauthState, "provider", provider) if returnUrl != nil && e.isValidReturnUrl(returnUrl.String()) { redirectToReturn() } else { @@ -267,11 +274,13 @@ func (e AuthEndpoint) handleOauthCallbackGet() http.HandlerFunc { return } - loginCtx, cancel := context.WithTimeout(context.Background(), 1000*time.Second) + loginCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) // avoid long waits user, err := e.authService.OauthLoginStep2(loginCtx, provider, currentSession.OauthNonce, oauthCode) cancel() if err != nil { + slog.Debug("failed to process oauth code", + "provider", provider, "state", oauthState, "error", err) if returnUrl != nil && e.isValidReturnUrl(returnUrl.String()) { redirectToReturn() } else {