Skip to content

Commit 0b47133

Browse files
ThomasK33ThomasK33
committed
fix(oauth2): allow custom URI schemes without reverse domain notation for native apps
Change-Id: I4000cd39caa994efe0b76c4984e968f2963063ca Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent 13de8e2 commit 0b47133

File tree

3 files changed

+71
-8
lines changed

3 files changed

+71
-8
lines changed

‎coderd/httpapi/httpapi.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"flag"
1010
"fmt"
1111
"net/http"
12+
"net/url"
1213
"reflect"
1314
"strings"
1415
"time"
@@ -26,6 +27,61 @@ import (
2627

2728
var Validate *validator.Validate
2829

30+
// isValidOAuth2RedirectURI validates OAuth2 redirect URIs according to RFC 6749.
31+
// It requires a proper scheme and host, rejecting malformed URIs that would be
32+
// problematic for OAuth2 flows.
33+
func isValidOAuth2RedirectURI(uri string) bool {
34+
if uri == "" {
35+
return false
36+
}
37+
38+
parsed, err := url.Parse(uri)
39+
if err != nil {
40+
return false
41+
}
42+
43+
// Must have a scheme
44+
if parsed.Scheme == "" {
45+
return false
46+
}
47+
48+
// Reject patterns that look like "host:port" without proper scheme
49+
// These get parsed as scheme="host" and path="port" which is ambiguous
50+
if parsed.Host == "" && parsed.Path != "" && !strings.HasPrefix(uri, parsed.Scheme+"://") {
51+
// Check if this looks like a host:port pattern (contains digits after colon)
52+
if strings.Contains(parsed.Path, ":") {
53+
return false
54+
}
55+
// Also reject if the "scheme" part looks like a hostname
56+
if strings.Contains(parsed.Scheme, ".") || parsed.Scheme == "localhost" {
57+
return false
58+
}
59+
}
60+
61+
// For standard schemes (http/https), host is required
62+
if parsed.Scheme == "http" || parsed.Scheme == "https" {
63+
if parsed.Host == "" {
64+
return false
65+
}
66+
}
67+
68+
// Reject scheme-only URIs like "http://"
69+
if parsed.Host == "" && parsed.Path == "" {
70+
return false
71+
}
72+
73+
// For custom schemes, we allow no host (like "myapp://callback")
74+
// But if there's a host, it should be valid
75+
if parsed.Host != "" {
76+
// Basic host validation - should not be empty after parsing
77+
if strings.TrimSpace(parsed.Host) == "" {
78+
return false
79+
}
80+
}
81+
82+
return true
83+
}
84+
2985
// This init is used to create a validator and register validation-specific
3086
// functionality for the HTTP API.
3187
//
@@ -113,6 +169,19 @@ func init() {
113169
if err != nil {
114170
panic(err)
115171
}
172+
173+
oauth2RedirectURIValidator := func(fl validator.FieldLevel) bool {
174+
f := fl.Field().Interface()
175+
str, ok := f.(string)
176+
if !ok {
177+
return false
178+
}
179+
return isValidOAuth2RedirectURI(str)
180+
}
181+
err = Validate.RegisterValidation("oauth2_redirect_uri", oauth2RedirectURIValidator)
182+
if err != nil {
183+
panic(err)
184+
}
116185
}
117186

118187
// Is404Error returns true if the given error should return a 404 status code.

‎codersdk/oauth2.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func (c *Client) OAuth2ProviderApp(ctx context.Context, id uuid.UUID) (OAuth2Pro
9393

9494
type PostOAuth2ProviderAppRequest struct {
9595
Name string `json:"name" validate:"required,oauth2_app_display_name"`
96-
RedirectURIs []string `json:"redirect_uris" validate:"dive,http_url"`
96+
RedirectURIs []string `json:"redirect_uris" validate:"dive,oauth2_redirect_uri"`
9797
Icon string `json:"icon" validate:"omitempty"`
9898
GrantTypes []OAuth2ProviderGrantType `json:"grant_types,omitempty" validate:"dive,oneof=authorization_code refresh_token client_credentials urn:ietf:params:oauth:grant-type:device_code"`
9999
}
@@ -150,7 +150,7 @@ func (c *Client) PostOAuth2ProviderApp(ctx context.Context, app PostOAuth2Provid
150150

151151
type PutOAuth2ProviderAppRequest struct {
152152
Name string `json:"name" validate:"required,oauth2_app_display_name"`
153-
RedirectURIs []string `json:"redirect_uris" validate:"dive,http_url"`
153+
RedirectURIs []string `json:"redirect_uris" validate:"dive,oauth2_redirect_uri"`
154154
Icon string `json:"icon" validate:"omitempty"`
155155
GrantTypes []OAuth2ProviderGrantType `json:"grant_types,omitempty" validate:"dive,oneof=authorization_code refresh_token client_credentials urn:ietf:params:oauth:grant-type:device_code"`
156156
}

‎codersdk/oauth2_validation.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -257,12 +257,6 @@ func isLoopbackAddress(hostname string) bool {
257257

258258
// isValidCustomScheme validates custom schemes for public clients (RFC 8252)
259259
func isValidCustomScheme(scheme string) bool {
260-
// For security and RFC compliance, require reverse domain notation
261-
// Should contain at least one period and not be a well-known scheme
262-
if !strings.Contains(scheme, ".") {
263-
return false
264-
}
265-
266260
// Block schemes that look like well-known protocols
267261
wellKnownSchemes := []string{"http", "https", "ftp", "mailto", "tel", "sms"}
268262
for _, wellKnown := range wellKnownSchemes {

0 commit comments

Comments
 (0)