mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
83fd4cf5c2
Go's html/template has a built-in security filter (urlFilter) that only allows http, https, and mailto URL schemes. Any other scheme gets replaced with #ZgotmplZ. The OAuth2 app's callback URL uses custom URI scheme which the filter considers unsafe. For example the Coder JetBrains plugin exposes a callback URI with the scheme jetbrains:// - which was effectively changed by the template engine into #ZgotmplZ. Of course this is not an actual callback. When users clicked the cancel button nothing happened. The fix was simple - we now wrap the apps registered callback URI into htmltemplate.URL. Usually this needs some validation otherwise the linter will complain about it. The callback URI used by the Cancel logic is actually validated by our backend when the client app programmatically registered via the dynamic OAuth2 registration endpoints, so we refactored the validation around that code and re-used some of it in the Cancel handling to make sure we don't allow URIs like `javascript` and `data`, even though in theory these URIs were already validated. In addition, while testing this PR with https://github.com/coder/coder-jetbrains-toolbox/pull/209 I discovered that we are also not compliant with https://www.rfc-editor.org/rfc/rfc6749#section-4.1.2.1 which requires the server to attach the local state if it was provided by the client in the original request. Also it is optional but generally a good practice to include `error_description` in the error responses. In fact we follow this pattern for the other types of error responses. So this is not a one off. - resolves #20323 <img width="1485" height="771" alt="Cancel_page_with_invalid_uri" src="https://github.com/user-attachments/assets/5539d234-9ce3-4dda-b421-d023fc9aa99e" /> <img width="486" height="746" alt="Coder Toolbox handling the Cancel button" src="https://github.com/user-attachments/assets/acab71a6-d29c-4fa9-80ba-3c0095bbdc8f" /> <!-- If you have used AI to produce some or all of this PR, please ensure you have read our [AI Contribution guidelines](https://coder.com/docs/about/contributing/AI_CONTRIBUTING) before submitting. -->
317 lines
9.5 KiB
Go
317 lines
9.5 KiB
Go
package codersdk
|
|
|
|
import (
|
|
"net/url"
|
|
"slices"
|
|
"strings"
|
|
|
|
"golang.org/x/xerrors"
|
|
)
|
|
|
|
// RFC 7591 validation functions for Dynamic Client Registration
|
|
|
|
func (req *OAuth2ClientRegistrationRequest) Validate() error {
|
|
// Validate redirect URIs - required for authorization code flow
|
|
if len(req.RedirectURIs) == 0 {
|
|
return xerrors.New("redirect_uris is required for authorization code flow")
|
|
}
|
|
|
|
if err := validateRedirectURIs(req.RedirectURIs, req.TokenEndpointAuthMethod); err != nil {
|
|
return xerrors.Errorf("invalid redirect_uris: %w", err)
|
|
}
|
|
|
|
// Validate grant types if specified
|
|
if len(req.GrantTypes) > 0 {
|
|
if err := validateGrantTypes(req.GrantTypes); err != nil {
|
|
return xerrors.Errorf("invalid grant_types: %w", err)
|
|
}
|
|
}
|
|
|
|
// Validate response types if specified
|
|
if len(req.ResponseTypes) > 0 {
|
|
if err := validateResponseTypes(req.ResponseTypes); err != nil {
|
|
return xerrors.Errorf("invalid response_types: %w", err)
|
|
}
|
|
}
|
|
|
|
// Validate token endpoint auth method if specified
|
|
if req.TokenEndpointAuthMethod != "" {
|
|
if err := validateTokenEndpointAuthMethod(req.TokenEndpointAuthMethod); err != nil {
|
|
return xerrors.Errorf("invalid token_endpoint_auth_method: %w", err)
|
|
}
|
|
}
|
|
|
|
// Validate URI fields
|
|
if req.ClientURI != "" {
|
|
if err := validateURIField(req.ClientURI, "client_uri"); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
|
|
if req.LogoURI != "" {
|
|
if err := validateURIField(req.LogoURI, "logo_uri"); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
|
|
if req.TOSURI != "" {
|
|
if err := validateURIField(req.TOSURI, "tos_uri"); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
|
|
if req.PolicyURI != "" {
|
|
if err := validateURIField(req.PolicyURI, "policy_uri"); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
|
|
if req.JWKSURI != "" {
|
|
if err := validateURIField(req.JWKSURI, "jwks_uri"); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
// ValidateRedirectURIScheme reports whether the callback URL's scheme is
|
|
// safe to use as a redirect target. It returns an error when the scheme
|
|
// is empty, an unsupported URN, or one of the schemes that are dangerous
|
|
// in browser/HTML contexts (javascript, data, file, ftp).
|
|
//
|
|
// Legitimate custom schemes for native apps (e.g. vscode://, jetbrains://)
|
|
// are allowed.
|
|
// ValidateRedirectURIScheme reports whether the callback URL's scheme is
|
|
// safe to use as a redirect target. It returns an error when the scheme
|
|
// is empty, an unsupported URN, or one of the schemes that are dangerous
|
|
// in browser/HTML contexts (javascript, data, file, ftp).
|
|
//
|
|
// Legitimate custom schemes for native apps (e.g. vscode://, jetbrains://)
|
|
// are allowed.
|
|
func ValidateRedirectURIScheme(u *url.URL) error {
|
|
return validateScheme(u)
|
|
}
|
|
|
|
func validateScheme(u *url.URL) error {
|
|
if u.Scheme == "" {
|
|
return xerrors.New("redirect URI must have a scheme")
|
|
}
|
|
|
|
// Handle special URNs (RFC 6749 section 3.1.2.1).
|
|
if u.Scheme == "urn" {
|
|
if u.String() == "urn:ietf:wg:oauth:2.0:oob" {
|
|
return nil
|
|
}
|
|
return xerrors.New("redirect URI uses unsupported URN scheme")
|
|
}
|
|
|
|
// Block dangerous schemes for security (not allowed by RFCs
|
|
// for OAuth2).
|
|
dangerousSchemes := []string{"javascript", "data", "file", "ftp"}
|
|
for _, dangerous := range dangerousSchemes {
|
|
if strings.EqualFold(u.Scheme, dangerous) {
|
|
return xerrors.Errorf("redirect URI uses dangerous scheme %s which is not allowed", dangerous)
|
|
}
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
// validateRedirectURIs validates redirect URIs according to RFC 7591, 8252
|
|
func validateRedirectURIs(uris []string, tokenEndpointAuthMethod OAuth2TokenEndpointAuthMethod) error {
|
|
if len(uris) == 0 {
|
|
return xerrors.New("at least one redirect URI is required")
|
|
}
|
|
|
|
for i, uriStr := range uris {
|
|
if uriStr == "" {
|
|
return xerrors.Errorf("redirect URI at index %d cannot be empty", i)
|
|
}
|
|
|
|
uri, err := url.Parse(uriStr)
|
|
if err != nil {
|
|
return xerrors.Errorf("redirect URI at index %d is not a valid URL: %w", i, err)
|
|
}
|
|
|
|
if err := validateScheme(uri); err != nil {
|
|
return xerrors.Errorf("redirect URI at index %d: %w", i, err)
|
|
}
|
|
|
|
// The urn:ietf:wg:oauth:2.0:oob scheme passed validation
|
|
// above but needs no further checks.
|
|
if uri.Scheme == "urn" {
|
|
continue
|
|
}
|
|
|
|
// Determine if this is a public client based on token endpoint auth method
|
|
isPublicClient := tokenEndpointAuthMethod == OAuth2TokenEndpointAuthMethodNone
|
|
|
|
// Handle different validation for public vs confidential clients
|
|
if uri.Scheme == "http" || uri.Scheme == "https" {
|
|
// HTTP/HTTPS validation (RFC 8252 section 7.3)
|
|
if uri.Scheme == "http" {
|
|
if isPublicClient {
|
|
// For public clients, only allow loopback (RFC 8252)
|
|
if !isLoopbackAddress(uri.Hostname()) {
|
|
return xerrors.Errorf("redirect URI at index %d: public clients may only use http with loopback addresses (127.0.0.1, ::1, localhost)", i)
|
|
}
|
|
} else {
|
|
// For confidential clients, allow localhost for development
|
|
if !isLocalhost(uri.Hostname()) {
|
|
return xerrors.Errorf("redirect URI at index %d must use https scheme for non-localhost URLs", i)
|
|
}
|
|
}
|
|
}
|
|
} else {
|
|
// Custom scheme validation for public clients (RFC 8252 section 7.1)
|
|
if isPublicClient {
|
|
// For public clients, custom schemes should follow RFC 8252 recommendations
|
|
// Should be reverse domain notation based on domain under their control
|
|
if !isValidCustomScheme(uri.Scheme) {
|
|
return xerrors.Errorf("redirect URI at index %d: custom scheme %s should use reverse domain notation (e.g. com.example.app)", i, uri.Scheme)
|
|
}
|
|
}
|
|
// For confidential clients, custom schemes are less common but allowed
|
|
}
|
|
|
|
// Prevent URI fragments (RFC 6749 section 3.1.2)
|
|
if uri.Fragment != "" || strings.Contains(uriStr, "#") {
|
|
return xerrors.Errorf("redirect URI at index %d must not contain a fragment component", i)
|
|
}
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
// validateGrantTypes validates OAuth2 grant types
|
|
func validateGrantTypes(grantTypes []OAuth2ProviderGrantType) error {
|
|
for _, grant := range grantTypes {
|
|
if !isSupportedGrantType(grant) {
|
|
return xerrors.Errorf("unsupported grant type: %s", grant)
|
|
}
|
|
}
|
|
|
|
// Ensure authorization_code is present if redirect_uris are specified
|
|
hasAuthCode := slices.Contains(grantTypes, OAuth2ProviderGrantTypeAuthorizationCode)
|
|
if !hasAuthCode {
|
|
return xerrors.New("authorization_code grant type is required when redirect_uris are specified")
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
func isSupportedGrantType(grant OAuth2ProviderGrantType) bool {
|
|
switch grant {
|
|
case OAuth2ProviderGrantTypeAuthorizationCode, OAuth2ProviderGrantTypeRefreshToken:
|
|
return true
|
|
}
|
|
return false
|
|
}
|
|
|
|
// validateResponseTypes validates OAuth2 response types
|
|
func validateResponseTypes(responseTypes []OAuth2ProviderResponseType) error {
|
|
for _, responseType := range responseTypes {
|
|
if !isSupportedResponseType(responseType) {
|
|
return xerrors.Errorf("unsupported response type: %s", responseType)
|
|
}
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
func isSupportedResponseType(responseType OAuth2ProviderResponseType) bool {
|
|
return responseType == OAuth2ProviderResponseTypeCode
|
|
}
|
|
|
|
// validateTokenEndpointAuthMethod validates token endpoint authentication method
|
|
func validateTokenEndpointAuthMethod(method OAuth2TokenEndpointAuthMethod) error {
|
|
if !method.Valid() {
|
|
return xerrors.Errorf("unsupported token endpoint auth method: %s", method)
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
// ValidatePKCECodeChallengeMethod validates PKCE code_challenge_method parameter.
|
|
// Per OAuth 2.1, only S256 is supported; plain is rejected for security reasons.
|
|
func ValidatePKCECodeChallengeMethod(method string) error {
|
|
if method == "" {
|
|
return nil // Optional, defaults to S256 if code_challenge is provided
|
|
}
|
|
|
|
m := OAuth2PKCECodeChallengeMethod(method)
|
|
|
|
if m == OAuth2PKCECodeChallengeMethodPlain {
|
|
return xerrors.New("code_challenge_method 'plain' is not supported; use 'S256'")
|
|
}
|
|
|
|
if m != OAuth2PKCECodeChallengeMethodS256 {
|
|
return xerrors.Errorf("unsupported code_challenge_method: %s", method)
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
// validateURIField validates a URI field
|
|
func validateURIField(uriStr, fieldName string) error {
|
|
if uriStr == "" {
|
|
return nil // Empty URIs are allowed for optional fields
|
|
}
|
|
|
|
uri, err := url.Parse(uriStr)
|
|
if err != nil {
|
|
return xerrors.Errorf("invalid %s: %w", fieldName, err)
|
|
}
|
|
|
|
// Require absolute URLs with scheme
|
|
if !uri.IsAbs() {
|
|
return xerrors.Errorf("%s must be an absolute URL", fieldName)
|
|
}
|
|
|
|
// Only allow http/https schemes
|
|
if uri.Scheme != "http" && uri.Scheme != "https" {
|
|
return xerrors.Errorf("%s must use http or https scheme", fieldName)
|
|
}
|
|
|
|
// For production, prefer HTTPS
|
|
// Note: we allow HTTP for localhost but prefer HTTPS for production
|
|
// This could be made configurable in the future
|
|
|
|
return nil
|
|
}
|
|
|
|
// isLocalhost checks if hostname is localhost (allows broader development usage)
|
|
func isLocalhost(hostname string) bool {
|
|
return hostname == "localhost" ||
|
|
hostname == "127.0.0.1" ||
|
|
hostname == "::1" ||
|
|
strings.HasSuffix(hostname, ".localhost")
|
|
}
|
|
|
|
// isLoopbackAddress checks if hostname is a strict loopback address (RFC 8252)
|
|
func isLoopbackAddress(hostname string) bool {
|
|
return hostname == "localhost" ||
|
|
hostname == "127.0.0.1" ||
|
|
hostname == "::1"
|
|
}
|
|
|
|
// isValidCustomScheme validates custom schemes for public clients (RFC 8252)
|
|
func isValidCustomScheme(scheme string) bool {
|
|
// For security 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 {
|
|
if strings.EqualFold(scheme, wellKnown) {
|
|
return false
|
|
}
|
|
}
|
|
|
|
return true
|
|
}
|