fix(oauth2): add proper redirect URI validation to prevent invalid URIs

blink-so[bot] · mattvollmer · blink-so[bot] · commit 78f85788c0a3 · 2025-07-17T15:54:57.000Z
The OAuth2 provider app validation was too permissive, allowing invalid
redirect URIs like 'localhost:3000', '/path/only', and 'http://' to pass
validation. This caused test failures in TestOAuth2ProviderAppValidation.

Changes:
- Updated PostOAuth2ProviderAppRequest.Validate() to call validateRedirectURIs
- Updated PutOAuth2ProviderAppRequest.Validate() to call validateRedirectURIs
- Added isHostnameScheme() function to detect hostname-like schemes
- Added validation to catch common patterns like 'localhost:3000' that are
  missing the http:// or https:// prefix

Fixes the failing test cases:
- URLNoHost: 'http://' now fails with scheme validation
- URLLocalhostNoScheme: 'localhost:3000' now fails with hostname detection
- URLPathOnly: '/bar/baz/qux' now fails with missing scheme validation

Co-authored-by: mattvollmer &lt;95866673+mattvollmer@users.noreply.github.com&gt;
diff --git a/codersdk/oauth2.go b/codersdk/oauth2.go
@@ -130,7 +130,20 @@ func validateOAuth2ProviderAppRequest(grantTypes []OAuth2ProviderGrantType, redi
 
 // Validate validates OAuth2 app creation request
 func (req *PostOAuth2ProviderAppRequest) Validate() error {
-	return validateOAuth2ProviderAppRequest(req.GrantTypes, req.RedirectURIs)
+	if err := validateOAuth2ProviderAppRequest(req.GrantTypes, req.RedirectURIs); err != nil {
+		return err
+	}
+
+	// Validate redirect URI format if any are provided
+	if len(req.RedirectURIs) > 0 {
+		// For OAuth2 provider apps, we assume confidential client by default
+		// (token endpoint auth method is not "none")
+		if err := validateRedirectURIs(req.RedirectURIs, "client_secret_basic"); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }
 
 // PostOAuth2ProviderApp adds an application that can authenticate using Coder
@@ -157,7 +170,20 @@ type PutOAuth2ProviderAppRequest struct {
 
 // Validate validates OAuth2 app update request
 func (req *PutOAuth2ProviderAppRequest) Validate() error {
-	return validateOAuth2ProviderAppRequest(req.GrantTypes, req.RedirectURIs)
+	if err := validateOAuth2ProviderAppRequest(req.GrantTypes, req.RedirectURIs); err != nil {
+		return err
+	}
+
+	// Validate redirect URI format if any are provided
+	if len(req.RedirectURIs) > 0 {
+		// For OAuth2 provider apps, we assume confidential client by default
+		// (token endpoint auth method is not "none")
+		if err := validateRedirectURIs(req.RedirectURIs, "client_secret_basic"); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }
 
 // PutOAuth2ProviderApp updates an application that can authenticate using Coder
diff --git a/codersdk/oauth2_validation.go b/codersdk/oauth2_validation.go
@@ -96,6 +96,11 @@ func validateRedirectURIs(uris []string, tokenEndpointAuthMethod string) error {
 			return xerrors.Errorf("redirect URI at index %d must have a scheme", i)
 		}
 
+		// Check for common hostname patterns used as schemes (likely missing http:// prefix)
+		if isHostnameScheme(uri.Scheme) {
+			return xerrors.Errorf("redirect URI at index %d: '%s' appears to be a hostname, not a valid scheme. Did you mean 'http://%s' or 'https://%s'?", i, uri.Scheme, uriStr, uriStr)
+		}
+
 		// Handle special URNs (RFC 6749 section 3.1.2.1)
 		if uri.Scheme == "urn" {
 			// Allow the out-of-band redirect URI for native apps
@@ -267,3 +272,26 @@ func isValidCustomScheme(scheme string) bool {
 
 	return true
 }
+
+// isHostnameScheme detects if a scheme looks like a hostname that's missing http:// prefix
+func isHostnameScheme(scheme string) bool {
+	// Common hostname patterns that shouldn't be used as schemes
+	hostnamePatterns := []string{
+		"localhost",
+		"127.0.0.1",
+		"::1",
+	}
+	
+	for _, pattern := range hostnamePatterns {
+		if strings.EqualFold(scheme, pattern) {
+			return true
+		}
+	}
+	
+	// Check if it looks like a domain name (contains dots)
+	if strings.Contains(scheme, ".") {
+		return true
+	}
+	
+	return false
+}
