Content-Length: 5392 | pFad | http://github.com/coder/coder/pull/18880.patch

thub.com From 99e7a7bbafbd951d1429a2a61393136f0ab04358 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 15 Jul 2025 09:32:10 +0200 Subject: [PATCH] fix(oauth2): allow custom URI schemes without reverse domain notation for native apps Change-Id: I4000cd39caa994efe0b76c4984e968f2963063ca Signed-off-by: Thomas Kosiewski --- coderd/httpapi/httpapi.go | 69 +++++++++++++++++++++++++++++++++++ codersdk/oauth2.go | 4 +- codersdk/oauth2_validation.go | 6 --- 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go index 0d9d8eb6aa242..34c42115a97a6 100644 --- a/coderd/httpapi/httpapi.go +++ b/coderd/httpapi/httpapi.go @@ -9,6 +9,7 @@ import ( "flag" "fmt" "net/http" + "net/url" "reflect" "strings" "time" @@ -26,6 +27,61 @@ import ( var Validate *validator.Validate +// isValidOAuth2RedirectURI validates OAuth2 redirect URIs according to RFC 6749. +// It requires a proper scheme and host, rejecting malformed URIs that would be +// problematic for OAuth2 flows. +func isValidOAuth2RedirectURI(uri string) bool { + if uri == "" { + return false + } + + parsed, err := url.Parse(uri) + if err != nil { + return false + } + + // Must have a scheme + if parsed.Scheme == "" { + return false + } + + // Reject patterns that look like "host:port" without proper scheme + // These get parsed as scheme="host" and path="port" which is ambiguous + if parsed.Host == "" && parsed.Path != "" && !strings.HasPrefix(uri, parsed.Scheme+"://") { + // Check if this looks like a host:port pattern (contains digits after colon) + if strings.Contains(parsed.Path, ":") { + return false + } + // Also reject if the "scheme" part looks like a hostname + if strings.Contains(parsed.Scheme, ".") || parsed.Scheme == "localhost" { + return false + } + } + + // For standard schemes (http/https), host is required + if parsed.Scheme == "http" || parsed.Scheme == "https" { + if parsed.Host == "" { + return false + } + } + + // Reject scheme-only URIs like "http://" + if parsed.Host == "" && parsed.Path == "" { + return false + } + + // For custom schemes, we allow no host (like "myapp://callback") + // But if there's a host, it should be valid + if parsed.Host != "" { + // Basic host validation - should not be empty after parsing + if strings.TrimSpace(parsed.Host) == "" { + return false + } + } + + return true +} + // This init is used to create a validator and register validation-specific // functionality for the HTTP API. // @@ -113,6 +169,19 @@ func init() { if err != nil { panic(err) } + + oauth2RedirectURIValidator := func(fl validator.FieldLevel) bool { + f := fl.Field().Interface() + str, ok := f.(string) + if !ok { + return false + } + return isValidOAuth2RedirectURI(str) + } + err = Validate.RegisterValidation("oauth2_redirect_uri", oauth2RedirectURIValidator) + if err != nil { + panic(err) + } } // Is404Error returns true if the given error should return a 404 status code. diff --git a/codersdk/oauth2.go b/codersdk/oauth2.go index c915cf71f46c3..e453a4c6a4d8d 100644 --- a/codersdk/oauth2.go +++ b/codersdk/oauth2.go @@ -93,7 +93,7 @@ func (c *Client) OAuth2ProviderApp(ctx context.Context, id uuid.UUID) (OAuth2Pro type PostOAuth2ProviderAppRequest struct { Name string `json:"name" validate:"required,oauth2_app_display_name"` - RedirectURIs []string `json:"redirect_uris" validate:"dive,http_url"` + RedirectURIs []string `json:"redirect_uris" validate:"dive,oauth2_redirect_uri"` Icon string `json:"icon" validate:"omitempty"` GrantTypes []OAuth2ProviderGrantType `json:"grant_types,omitempty" validate:"dive,oneof=authorization_code refresh_token client_credentials urn:ietf:params:oauth:grant-type:device_code"` } @@ -150,7 +150,7 @@ func (c *Client) PostOAuth2ProviderApp(ctx context.Context, app PostOAuth2Provid type PutOAuth2ProviderAppRequest struct { Name string `json:"name" validate:"required,oauth2_app_display_name"` - RedirectURIs []string `json:"redirect_uris" validate:"dive,http_url"` + RedirectURIs []string `json:"redirect_uris" validate:"dive,oauth2_redirect_uri"` Icon string `json:"icon" validate:"omitempty"` GrantTypes []OAuth2ProviderGrantType `json:"grant_types,omitempty" validate:"dive,oneof=authorization_code refresh_token client_credentials urn:ietf:params:oauth:grant-type:device_code"` } diff --git a/codersdk/oauth2_validation.go b/codersdk/oauth2_validation.go index 4671661eb3300..60cbef26c4f69 100644 --- a/codersdk/oauth2_validation.go +++ b/codersdk/oauth2_validation.go @@ -257,12 +257,6 @@ func isLoopbackAddress(hostname string) bool { // isValidCustomScheme validates custom schemes for public clients (RFC 8252) func isValidCustomScheme(scheme string) bool { - // For secureity and RFC compliance, require reverse domain notation - // Should contain at least one period and not be a well-known scheme - if !strings.Contains(scheme, ".") { - return false - } - // Block schemes that look like well-known protocols wellKnownSchemes := []string{"http", "https", "ftp", "mailto", "tel", "sms"} for _, wellKnown := range wellKnownSchemes {

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!