From 58c8cb8958692b02f8937d3fde1c03085f91ec8f Mon Sep 17 00:00:00 2001 From: Marcus Pasell <3690498+rickyrombo@users.noreply.github.com> Date: Sat, 14 Mar 2026 17:11:19 -0700 Subject: [PATCH 1/3] Move redirect URI validation to backend for non-registered case --- api/v1_oauth.go | 30 +++++++++++++- api/v1_oauth_test.go | 95 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 110 insertions(+), 15 deletions(-) diff --git a/api/v1_oauth.go b/api/v1_oauth.go index dbb956b6..adf1fcd4 100644 --- a/api/v1_oauth.go +++ b/api/v1_oauth.go @@ -7,6 +7,8 @@ import ( "encoding/base64" "encoding/json" "fmt" + "net" + "net/url" "strings" "time" @@ -155,9 +157,12 @@ func (app *ApiServer) v1OAuthAuthorize(c *fiber.Ctx) error { // 3. Validate redirect_uri // Fetch all registered redirect URIs for this client. - // If none are registered, any redirect_uri is allowed (including postmessage). // If any are registered, the submitted redirect_uri must exactly match one of them. - // postmessage is not special-cased — it must be explicitly registered to be used. + // If none are registered (legacy apps), apply format-based validation as a fallback: + // - scheme must be http or https (or postmessage) + // - no credentials or fragment + // - no path traversal sequences + // - no non-loopback IP addresses { rows, err := app.pool.Query(c.Context(), ` SELECT redirect_uri FROM oauth_redirect_uris @@ -187,6 +192,27 @@ func (app *ApiServer) v1OAuthAuthorize(c *fiber.Ctx) error { if !allowed { return oauthError(c, fiber.StatusBadRequest, "invalid_request", "redirect_uri not registered") } + } else { + // No registered URIs: apply legacy format validation. + // postmessage is always allowed for backwards compatibility. + if strings.ToLower(body.RedirectURI) != "postmessage" { + parsed, err := url.Parse(body.RedirectURI) + if err != nil || parsed.Host == "" { + return oauthError(c, fiber.StatusBadRequest, "invalid_request", "redirect_uri is not a valid URL") + } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return oauthError(c, fiber.StatusBadRequest, "invalid_request", "redirect_uri scheme must be http or https") + } + if parsed.Fragment != "" || parsed.User != nil { + return oauthError(c, fiber.StatusBadRequest, "invalid_request", "redirect_uri must not contain credentials or a fragment") + } + if strings.Contains(parsed.Path, "/..") || strings.Contains(parsed.Path, "\\..") || strings.Contains(parsed.Path, "../") { + return oauthError(c, fiber.StatusBadRequest, "invalid_request", "redirect_uri contains a path traversal sequence") + } + if ip := net.ParseIP(parsed.Hostname()); ip != nil && !ip.IsLoopback() { + return oauthError(c, fiber.StatusBadRequest, "invalid_request", "redirect_uri must not use a bare IP address") + } + } } } diff --git a/api/v1_oauth_test.go b/api/v1_oauth_test.go index fa7862ca..022ec96a 100644 --- a/api/v1_oauth_test.go +++ b/api/v1_oauth_test.go @@ -434,40 +434,109 @@ func TestOAuthAuthorize_UnregisteredRedirectURI(t *testing.T) { assert.Contains(t, gjson.GetBytes(body, "error_description").String(), "redirect_uri") } -func TestOAuthAuthorize_NoRegisteredRedirectURIs(t *testing.T) { - app := emptyTestApp(t) - seedOAuthTestData(t, app) - - // Register a new developer app with no redirect URIs. - // Policy: if no URIs are registered, any redirect_uri is accepted. - clientIDNoURIs := "0xccdd000000000000000000000000000000000003" +// seedAppWithNoRedirectURIs registers a developer app with no redirect URIs and returns its client_id. +func seedAppWithNoRedirectURIs(t *testing.T, app *ApiServer) string { + t.Helper() + clientID := "0xccdd000000000000000000000000000000000003" database.SeedTable(app.pool.Replicas[0], "developer_apps", []map[string]any{ { - "address": clientIDNoURIs, + "address": clientID, "user_id": 100, "name": "App Without Redirect URIs", "is_delete": false, }, }) + return clientID +} +func authorizeWithNoURIsApp(t *testing.T, app *ApiServer, redirectURI string) (int, []byte) { + t.Helper() + clientID := seedAppWithNoRedirectURIs(t, app) token := makeOAuthJWT(t, 100, oauthTestPrivKeyHex) h := sha256.Sum256([]byte("verifier")) codeChallenge := base64.RawURLEncoding.EncodeToString(h[:]) - - status, body := oauthPostJSON(t, app, "/v1/oauth/authorize", map[string]string{ + return oauthPostJSON(t, app, "/v1/oauth/authorize", map[string]string{ "token": token, - "client_id": clientIDNoURIs, - "redirect_uri": "https://any-uri.example.com/callback", + "client_id": clientID, + "redirect_uri": redirectURI, "code_challenge": codeChallenge, "code_challenge_method": "S256", "scope": "read", }) +} +func TestOAuthAuthorize_NoRegisteredRedirectURIs_ValidHTTPS(t *testing.T) { + app := emptyTestApp(t) + seedOAuthTestData(t, app) + status, body := authorizeWithNoURIsApp(t, app, "https://any-uri.example.com/callback") + assert.Equal(t, 200, status) + assert.NotEmpty(t, gjson.GetBytes(body, "code").String()) +} + +func TestOAuthAuthorize_NoRegisteredRedirectURIs_ValidHTTP(t *testing.T) { + app := emptyTestApp(t) + seedOAuthTestData(t, app) + status, body := authorizeWithNoURIsApp(t, app, "http://localhost:3000/callback") + assert.Equal(t, 200, status) + assert.NotEmpty(t, gjson.GetBytes(body, "code").String()) +} + +func TestOAuthAuthorize_NoRegisteredRedirectURIs_PostMessage(t *testing.T) { + app := emptyTestApp(t) + seedOAuthTestData(t, app) + status, body := authorizeWithNoURIsApp(t, app, "postMessage") + assert.Equal(t, 200, status) + assert.NotEmpty(t, gjson.GetBytes(body, "code").String()) +} + +func TestOAuthAuthorize_NoRegisteredRedirectURIs_LoopbackIP(t *testing.T) { + app := emptyTestApp(t) + seedOAuthTestData(t, app) + status, body := authorizeWithNoURIsApp(t, app, "http://127.0.0.1:8080/callback") assert.Equal(t, 200, status) - assert.True(t, gjson.GetBytes(body, "code").Exists()) assert.NotEmpty(t, gjson.GetBytes(body, "code").String()) } +func TestOAuthAuthorize_NoRegisteredRedirectURIs_NonHTTPScheme(t *testing.T) { + app := emptyTestApp(t) + seedOAuthTestData(t, app) + status, body := authorizeWithNoURIsApp(t, app, "myapp://oauth/callback") + assert.Equal(t, 400, status) + jsonAssert(t, body, map[string]any{"error": "invalid_request"}) +} + +func TestOAuthAuthorize_NoRegisteredRedirectURIs_BareIP(t *testing.T) { + app := emptyTestApp(t) + seedOAuthTestData(t, app) + status, body := authorizeWithNoURIsApp(t, app, "https://192.168.1.1/callback") + assert.Equal(t, 400, status) + jsonAssert(t, body, map[string]any{"error": "invalid_request"}) +} + +func TestOAuthAuthorize_NoRegisteredRedirectURIs_Credentials(t *testing.T) { + app := emptyTestApp(t) + seedOAuthTestData(t, app) + status, body := authorizeWithNoURIsApp(t, app, "https://user:pass@evil.com/callback") + assert.Equal(t, 400, status) + jsonAssert(t, body, map[string]any{"error": "invalid_request"}) +} + +func TestOAuthAuthorize_NoRegisteredRedirectURIs_Fragment(t *testing.T) { + app := emptyTestApp(t) + seedOAuthTestData(t, app) + status, body := authorizeWithNoURIsApp(t, app, "https://example.com/callback#fragment") + assert.Equal(t, 400, status) + jsonAssert(t, body, map[string]any{"error": "invalid_request"}) +} + +func TestOAuthAuthorize_NoRegisteredRedirectURIs_PathTraversal(t *testing.T) { + app := emptyTestApp(t) + seedOAuthTestData(t, app) + status, body := authorizeWithNoURIsApp(t, app, "https://example.com/foo/../../../callback") + assert.Equal(t, 400, status) + jsonAssert(t, body, map[string]any{"error": "invalid_request"}) +} + // --- /oauth/token (authorization_code grant) --- func TestOAuthTokenAuthorizationCode(t *testing.T) { From 2d00d9d65db5bc76365c1c730fff72ace4eeb8c2 Mon Sep 17 00:00:00 2001 From: Marcus Pasell <3690498+rickyrombo@users.noreply.github.com> Date: Sat, 14 Mar 2026 17:21:36 -0700 Subject: [PATCH 2/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- api/v1_oauth.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/api/v1_oauth.go b/api/v1_oauth.go index adf1fcd4..fec70d61 100644 --- a/api/v1_oauth.go +++ b/api/v1_oauth.go @@ -206,8 +206,11 @@ func (app *ApiServer) v1OAuthAuthorize(c *fiber.Ctx) error { if parsed.Fragment != "" || parsed.User != nil { return oauthError(c, fiber.StatusBadRequest, "invalid_request", "redirect_uri must not contain credentials or a fragment") } - if strings.Contains(parsed.Path, "/..") || strings.Contains(parsed.Path, "\\..") || strings.Contains(parsed.Path, "../") { - return oauthError(c, fiber.StatusBadRequest, "invalid_request", "redirect_uri contains a path traversal sequence") + normalizedPath := strings.ReplaceAll(parsed.Path, "\\", "/") + for _, segment := range strings.Split(normalizedPath, "/") { + if segment == ".." { + return oauthError(c, fiber.StatusBadRequest, "invalid_request", "redirect_uri contains a path traversal sequence") + } } if ip := net.ParseIP(parsed.Hostname()); ip != nil && !ip.IsLoopback() { return oauthError(c, fiber.StatusBadRequest, "invalid_request", "redirect_uri must not use a bare IP address") From c71fdfe6a60858c28f47ae33708389b204efdb84 Mon Sep 17 00:00:00 2001 From: Marcus Pasell <3690498+rickyrombo@users.noreply.github.com> Date: Sat, 14 Mar 2026 17:21:41 -0700 Subject: [PATCH 3/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- api/v1_oauth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1_oauth.go b/api/v1_oauth.go index fec70d61..bafdbe58 100644 --- a/api/v1_oauth.go +++ b/api/v1_oauth.go @@ -213,7 +213,7 @@ func (app *ApiServer) v1OAuthAuthorize(c *fiber.Ctx) error { } } if ip := net.ParseIP(parsed.Hostname()); ip != nil && !ip.IsLoopback() { - return oauthError(c, fiber.StatusBadRequest, "invalid_request", "redirect_uri must not use a bare IP address") + return oauthError(c, fiber.StatusBadRequest, "invalid_request", "redirect_uri must not use a non-loopback IP address") } } }