From 10e58eadb6014af9e53bbc852e3fbffd86ce2902 Mon Sep 17 00:00:00 2001 From: Sergio Date: Tue, 10 Mar 2026 03:43:13 -0700 Subject: [PATCH 1/2] fix(oauth): allow absolute authorization consent URL --- internal/api/oauthserver/authorize.go | 7 +++- internal/api/oauthserver/authorize_test.go | 37 ++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/internal/api/oauthserver/authorize.go b/internal/api/oauthserver/authorize.go index 42e14e335b..56332553ea 100644 --- a/internal/api/oauthserver/authorize.go +++ b/internal/api/oauthserver/authorize.go @@ -622,8 +622,13 @@ func (s *Server) buildErrorRedirectURL(redirectURI, errorCode, errorDescription, return u.String() } -// buildAuthorizationURL safely joins a base URL with a path, handling slashes correctly +// buildAuthorizationURL safely joins a base URL with a path, handling slashes correctly. +// If pathToJoin is an absolute URL, it is returned as-is. func (s *Server) buildAuthorizationURL(baseURL, pathToJoin string) string { + if parsed, err := url.Parse(pathToJoin); err == nil && parsed.IsAbs() { + return pathToJoin + } + // Trim trailing slash from baseURL baseURL = strings.TrimRight(baseURL, "/") diff --git a/internal/api/oauthserver/authorize_test.go b/internal/api/oauthserver/authorize_test.go index 3cfa2aeaca..56a366395e 100644 --- a/internal/api/oauthserver/authorize_test.go +++ b/internal/api/oauthserver/authorize_test.go @@ -122,6 +122,43 @@ func TestValidateRequestOrigin(t *testing.T) { } } +func TestBuildAuthorizationURL(t *testing.T) { + server := &Server{} + + tests := []struct { + name string + baseURL string + pathToJoin string + expected string + }{ + { + name: "joins relative path with leading slash", + baseURL: "https://example.com", + pathToJoin: "/oauth/consent", + expected: "https://example.com/oauth/consent", + }, + { + name: "joins relative path without leading slash", + baseURL: "https://example.com/", + pathToJoin: "oauth/consent", + expected: "https://example.com/oauth/consent", + }, + { + name: "returns absolute path unchanged", + baseURL: "https://example.com", + pathToJoin: "https://app.example.com/custom-consent", + expected: "https://app.example.com/custom-consent", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := server.buildAuthorizationURL(tt.baseURL, tt.pathToJoin) + assert.Equal(t, tt.expected, actual) + }) + } +} + func TestValidateRequestOriginEdgeCases(t *testing.T) { globalConfig, err := conf.LoadGlobal(oauthServerTestConfig) require.NoError(t, err) From 26ac7c033fd11f4d6770e5d8afc3d6ade826025e Mon Sep 17 00:00:00 2001 From: Sergio Date: Tue, 10 Mar 2026 03:51:53 -0700 Subject: [PATCH 2/2] fix(oauth): preserve query/fragment when appending authorization_id --- internal/api/oauthserver/authorize.go | 17 ++++++++-- internal/api/oauthserver/authorize_test.go | 39 ++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/internal/api/oauthserver/authorize.go b/internal/api/oauthserver/authorize.go index 56332553ea..5f3b3c0dc0 100644 --- a/internal/api/oauthserver/authorize.go +++ b/internal/api/oauthserver/authorize.go @@ -167,7 +167,7 @@ func (s *Server) OAuthServerAuthorize(w http.ResponseWriter, r *http.Request) er } baseURL := s.buildAuthorizationURL(config.SiteURL, config.OAuthServer.AuthorizationPath) - redirectURL := fmt.Sprintf("%s?authorization_id=%s", baseURL, authorization.AuthorizationID) + redirectURL := s.buildAuthorizationRedirectURL(baseURL, authorization.AuthorizationID) http.Redirect(w, r, redirectURL, http.StatusFound) return nil @@ -626,7 +626,7 @@ func (s *Server) buildErrorRedirectURL(redirectURI, errorCode, errorDescription, // If pathToJoin is an absolute URL, it is returned as-is. func (s *Server) buildAuthorizationURL(baseURL, pathToJoin string) string { if parsed, err := url.Parse(pathToJoin); err == nil && parsed.IsAbs() { - return pathToJoin + return parsed.String() } // Trim trailing slash from baseURL @@ -639,3 +639,16 @@ func (s *Server) buildAuthorizationURL(baseURL, pathToJoin string) string { return baseURL + pathToJoin } + +// buildAuthorizationRedirectURL appends authorization_id while preserving existing query params/fragments. +func (s *Server) buildAuthorizationRedirectURL(baseURL, authorizationID string) string { + u, err := url.Parse(baseURL) + if err != nil { + return fmt.Sprintf("%s?authorization_id=%s", baseURL, authorizationID) + } + + q := u.Query() + q.Set("authorization_id", authorizationID) + u.RawQuery = q.Encode() + return u.String() +} diff --git a/internal/api/oauthserver/authorize_test.go b/internal/api/oauthserver/authorize_test.go index 56a366395e..6bba841dee 100644 --- a/internal/api/oauthserver/authorize_test.go +++ b/internal/api/oauthserver/authorize_test.go @@ -159,6 +159,45 @@ func TestBuildAuthorizationURL(t *testing.T) { } } + + +func TestBuildAuthorizationRedirectURL(t *testing.T) { + server := &Server{} + + tests := []struct { + name string + baseURL string + authorizationID string + expected string + }{ + { + name: "adds authorization_id to URL without query", + baseURL: "https://app.example.com/custom-consent", + authorizationID: "auth-123", + expected: "https://app.example.com/custom-consent?authorization_id=auth-123", + }, + { + name: "preserves existing query parameters", + baseURL: "https://app.example.com/custom-consent?foo=bar", + authorizationID: "auth-123", + expected: "https://app.example.com/custom-consent?authorization_id=auth-123&foo=bar", + }, + { + name: "preserves fragment", + baseURL: "https://app.example.com/custom-consent?foo=bar#frag", + authorizationID: "auth-123", + expected: "https://app.example.com/custom-consent?authorization_id=auth-123&foo=bar#frag", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := server.buildAuthorizationRedirectURL(tt.baseURL, tt.authorizationID) + assert.Equal(t, tt.expected, actual) + }) + } +} + func TestValidateRequestOriginEdgeCases(t *testing.T) { globalConfig, err := conf.LoadGlobal(oauthServerTestConfig) require.NoError(t, err)