diff --git a/frontend/src/lib/hooks/redirect-uri.ts b/frontend/src/lib/hooks/redirect-uri.ts index f7048ffe..286773be 100644 --- a/frontend/src/lib/hooks/redirect-uri.ts +++ b/frontend/src/lib/hooks/redirect-uri.ts @@ -75,19 +75,6 @@ export const useRedirectUri = ( }; }; -// ported from internal/controller/oauth_controller.go -const getEffectivePort = (url: URL): string => { - if (url.port) { - return url.port; - } - - if (url.protocol == "https:") { - return "443"; - } - - return "80"; -}; - // https://www.geeksforgeeks.org/javascript/how-to-check-if-a-string-is-a-valid-ip-address-format-in-javascript const isIP = (str: string): boolean => { const ipv4 = @@ -114,7 +101,7 @@ export const isTrustedDomain = ( return false; } - if (getEffectivePort(url) != getEffectivePort(appUrl)) { + if (url.port != appUrl.port) { return false; } diff --git a/internal/controller/oauth_controller.go b/internal/controller/oauth_controller.go index 00eae837..66756a82 100644 --- a/internal/controller/oauth_controller.go +++ b/internal/controller/oauth_controller.go @@ -331,8 +331,7 @@ func (controller *OAuthController) isRedirectSafe(redirectURI string) bool { controller.log.App.Debug().Err(err).Msg("Failed to validate redirect URI") - if errors.Is(err, validators.ErrInvalidURL) || - errors.Is(err, validators.ErrPortMismatch) { + if !errors.Is(err, validators.ErrHostnameMismatch) { return false } diff --git a/internal/controller/oauth_controller_test.go b/internal/controller/oauth_controller_test.go index 80939ab2..72929d42 100644 --- a/internal/controller/oauth_controller_test.go +++ b/internal/controller/oauth_controller_test.go @@ -81,14 +81,6 @@ func TestOAuthController_isRedirectSafe(t *testing.T) { redirectURI: "https://sub.example.com", expected: false, }, - { - description: "Different scheme returns false", - appURL: "https://tinyauth.example.com", - cookieDomain: "example.com", - subdomainsEnabled: true, - redirectURI: "http://tinyauth.example.com", - expected: false, - }, { description: "Different port returns false", appURL: "https://tinyauth.example.com", diff --git a/internal/service/access_controls_service.go b/internal/service/access_controls_service.go index d29f2a97..c6ffff7d 100644 --- a/internal/service/access_controls_service.go +++ b/internal/service/access_controls_service.go @@ -50,9 +50,7 @@ func (service *AccessControlsService) lookupStaticACLs(domain string) *model.App service.log.App.Debug().Str("name", app).Msg("Found matching container by domain") return &config } - if !errors.Is(err, validators.ErrHostnameMismatch) && - !errors.Is(err, validators.ErrPortMismatch) && - !errors.Is(err, validators.ErrSchemeMismatch) { + if !errors.Is(err, validators.ErrHostnameMismatch) { service.log.App.Debug().Str("name", app).Err(err).Msg("Domain validation failed") } } diff --git a/pkg/README.md b/pkg/README.md new file mode 100644 index 00000000..abf6a416 --- /dev/null +++ b/pkg/README.md @@ -0,0 +1,10 @@ +# Public packages + +This directory contains packages that can be used by +other projects. + +While we try to maintain a consistent API, no promises +can be made for non-breaking changes throughout updates +as we constantly need to make changes to comply with the +needs of Tinyauth. We advise pinning the version of the +package you wish to use. \ No newline at end of file diff --git a/pkg/validators/domain_validator.go b/pkg/validators/domain_validator.go index c16966f4..d41d82b3 100644 --- a/pkg/validators/domain_validator.go +++ b/pkg/validators/domain_validator.go @@ -10,14 +10,13 @@ import ( "fmt" "net" "net/url" - "slices" "strings" "golang.org/x/net/idna" ) +// Errors var ( - ErrInvalidURL = fmt.Errorf("invalid url") ErrSchemeMismatch = fmt.Errorf("scheme mismatch") ErrPortMismatch = fmt.Errorf("port mismatch") ErrHostnameMismatch = fmt.Errorf("hostname mismatch") @@ -29,8 +28,7 @@ type DomainValidatorOptions struct { WithScheme bool // Ensure domains have the same port. WithPort bool - // Specify a list of allowed schemes IF WithScheme is set to true. - // Leave empty to allow any scheme. + // Specify a list of allowed schemes if WithScheme is set to true. AllowedSchemes []string } @@ -48,53 +46,74 @@ func NewDomainValidator(opts DomainValidatorOptions) *DomainValidator { } } +func (v *DomainValidator) checkScheme(rawURL string) error { + if !v.opts.WithScheme { + return nil + } + + if len(v.opts.AllowedSchemes) == 0 { + return fmt.Errorf("allowed schemes must be specified") + } + + for _, scheme := range v.opts.AllowedSchemes { + if strings.HasPrefix(strings.ToLower(rawURL), strings.ToLower(scheme)+"://") { + return nil + } + } + + return fmt.Errorf("invalid scheme") + +} + func (v *DomainValidator) getURL(i string) (*url.URL, error) { - u, err := url.Parse(i) - - if !v.opts.WithScheme && (err != nil || u.Host == "") { - u, err = url.Parse("tinyauth://" + i) - } - - if err != nil { - return nil, fmt.Errorf("failed to parse input url: %w", err) - } - - if u.Host == "" { - return nil, ErrInvalidURL - } - - if v.opts.WithPort && u.Port() == "" && (u.Scheme != "http" && u.Scheme != "https") { - return nil, fmt.Errorf("port validation is enabled but port is missing in input url and schemes are not enabled") + if i == "" { + return nil, fmt.Errorf("url cannot be empty") } if v.opts.WithScheme { - // Empty scheme means that we parsed the url with the tinyauth:// placeholder - if u.Scheme == "tinyauth" { - return nil, fmt.Errorf("input url is missing scheme") + err := v.checkScheme(i) + + if err != nil { + return nil, fmt.Errorf("invalid scheme: %w", err) } - if len(v.opts.AllowedSchemes) > 0 && !slices.Contains(v.opts.AllowedSchemes, u.Scheme) { - return nil, fmt.Errorf("scheme %s not allowed", u.Scheme) + + u, err := url.Parse(i) + + if err != nil { + return nil, fmt.Errorf("failed to parse input url: %w", err) } + + if u.Host == "" || u.Scheme == "" { + return nil, fmt.Errorf("missing host or scheme in url: %s", i) + } + + return u, nil + } + + rawURL := i + + if !strings.Contains(i, "://") { + // From godoc: [scheme:][//[userinfo@]host][/]path[?query][#fragment] + // So, we can omit the colon and tell the Go URL lib that we want + // to parse the URL without the scheme. If we don't do this, + // the URL lib will parse our entire domain as the path. + rawURL = "//" + i + } + + u, err := url.Parse(rawURL) + + if err != nil { + return nil, fmt.Errorf("failed to parse host: %w", err) + } + + if u.Host == "" { + return nil, fmt.Errorf("missing host in url: %s", i) } return u, nil } -func (v *DomainValidator) getEffectivePort(u *url.URL) (string, bool) { - if u.Port() != "" { - return u.Port(), true - } - switch u.Scheme { - case "http": - return "80", true - case "https": - return "443", true - default: - return "", false - } -} - -func (v *DomainValidator) formatHostname(hostname string) (string, error) { +func (v *DomainValidator) getHostname(hostname string) (string, error) { hostname = strings.ToLower(hostname) hostname = strings.TrimSuffix(hostname, ".") if net.ParseIP(hostname) != nil { @@ -133,26 +152,18 @@ func (v *DomainValidator) Validate(expected, actual string) error { } if v.opts.WithPort { - eup, ok := v.getEffectivePort(eu) - if !ok { - return fmt.Errorf("failed to get effective port for url: %s", eu.String()) - } - aup, ok := v.getEffectivePort(au) - if !ok { - return fmt.Errorf("failed to get effective port for url: %s", au.String()) - } - if eup != aup { + if eu.Port() != au.Port() { return ErrPortMismatch } } - euf, err := v.formatHostname(eu.Hostname()) + euf, err := v.getHostname(eu.Hostname()) if err != nil { return err } - auf, err := v.formatHostname(au.Hostname()) + auf, err := v.getHostname(au.Hostname()) if err != nil { return err @@ -165,7 +176,7 @@ func (v *DomainValidator) Validate(expected, actual string) error { return nil } -// SafeHostname uses the internal validation for domains that Validator uses +// SafeHostname uses the internal validation for domains that the validator uses // to parse a hostname. It ensures the input URL is a valid URL, that a host // is present and that the hostname is lowercased and without a trailing dot. func (v *DomainValidator) SafeHostname(input string) (string, error) { @@ -175,5 +186,5 @@ func (v *DomainValidator) SafeHostname(input string) (string, error) { return "", err } - return v.formatHostname(u.Hostname()) + return v.getHostname(u.Hostname()) } diff --git a/pkg/validators/domain_validator_test.go b/pkg/validators/domain_validator_test.go index 9789b139..aa7587f1 100644 --- a/pkg/validators/domain_validator_test.go +++ b/pkg/validators/domain_validator_test.go @@ -20,16 +20,36 @@ func TestDomainValidator_SafeHostname(t *testing.T) { { description: "Empty url fails", errorFunc: func(t *testing.T, e error) { - assert.ErrorIs(t, e, ErrInvalidURL) + assert.ErrorContains(t, e, "url cannot be empty") + }, + }, + { + description: "URL without host fails", + input: "/foo", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "missing host in url") }, }, { description: "Invalid url fails", input: "foo:foo", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "failed to parse host") + }, + }, + { + description: "With scheme and invalid url should fail", + options: DomainValidatorOptions{WithScheme: true, AllowedSchemes: []string{"https"}}, + input: "https://foo:foo", errorFunc: func(t *testing.T, e error) { assert.ErrorContains(t, e, "failed to parse input url") }, }, + { + description: "Scheme disabled with scheme should pass", + input: "https://example.com", + expected: "example.com", + }, { description: "Domain without scheme should parse if scheme is disabled", input: "example.com", @@ -40,7 +60,7 @@ func TestDomainValidator_SafeHostname(t *testing.T) { options: DomainValidatorOptions{WithScheme: true}, input: "example.com", errorFunc: func(t *testing.T, e error) { - assert.ErrorIs(t, e, ErrInvalidURL) + assert.ErrorContains(t, e, "invalid scheme") }, }, { @@ -48,7 +68,7 @@ func TestDomainValidator_SafeHostname(t *testing.T) { options: DomainValidatorOptions{WithScheme: true, AllowedSchemes: []string{"https"}}, input: "foo://example.com", errorFunc: func(t *testing.T, e error) { - assert.ErrorContains(t, e, "foo not allowed") + assert.ErrorContains(t, e, "invalid scheme") }, }, { @@ -94,32 +114,9 @@ func TestDomainValidator_SafeHostname(t *testing.T) { }, }, { - // Placeholder should not be used by users and is reserved for the validator. - // Using it is like not using any scheme for the validator, and thus it will fail - // with schemes enabled. - description: "Placeholder scheme supplied directly should fail", - options: DomainValidatorOptions{WithScheme: true, AllowedSchemes: []string{"https"}}, - input: "tinyauth://example.com", - errorFunc: func(t *testing.T, e error) { - assert.ErrorContains(t, e, "input url is missing scheme") - }, - }, - { - description: "With port enabled but without scheme and url with https should work", + description: "With port enabled without any port should work", options: DomainValidatorOptions{WithPort: true}, - input: "https://example.com", - expected: "example.com", - }, - { - description: "With port enabled but without scheme and url with http should work", - options: DomainValidatorOptions{WithPort: true}, - input: "http://example.com", - expected: "example.com", - }, - { - description: "With port enabled but without scheme and url with port should work", - options: DomainValidatorOptions{WithPort: true}, - input: "example.com:8080", + input: "example.com", expected: "example.com", }, } @@ -153,7 +150,7 @@ func TestDomainValidator_Validate(t *testing.T) { expected: "foo:foo", actual: "bar.com", errorFunc: func(t *testing.T, e error) { - assert.ErrorContains(t, e, "failed to parse input url:") + assert.ErrorContains(t, e, "failed to parse host:") }, }, { @@ -161,7 +158,7 @@ func TestDomainValidator_Validate(t *testing.T) { expected: "example.com", actual: "foo:foo", errorFunc: func(t *testing.T, e error) { - assert.ErrorContains(t, e, "failed to parse input url:") + assert.ErrorContains(t, e, "failed to parse host:") }, }, { @@ -180,70 +177,22 @@ func TestDomainValidator_Validate(t *testing.T) { actual: "https://example.com", }, { - description: "Port validation without ports and schemes disabled should fail", + description: "Port validation with ports enabled and empty ports should work", options: DomainValidatorOptions{WithPort: true}, expected: "example.com", actual: "example.com", - errorFunc: func(t *testing.T, e error) { - assert.ErrorContains(t, e, "port validation is enabled but port is missing in input url and schemes are not enabled") - }, }, { - description: "Port validation with no port and http should pass", - options: DomainValidatorOptions{WithPort: true, WithScheme: true, AllowedSchemes: []string{"http"}}, - expected: "http://example.com", - actual: "http://example.com", - }, - { - description: "Port validation with no port and https should pass", - options: DomainValidatorOptions{WithPort: true, WithScheme: true, AllowedSchemes: []string{"https"}}, - expected: "https://example.com", - actual: "https://example.com", - }, - { - description: "Port validation with port and no scheme should pass with same port", + description: "Port validation should pass with same port", options: DomainValidatorOptions{WithPort: true}, expected: "example.com:8080", actual: "example.com:8080", }, { - description: "Domains with unknown scheme and port enabled but no port should fail", - options: DomainValidatorOptions{WithPort: true, WithScheme: true}, - expected: "ssh://example.com:22", - actual: "ssh://example.com", - errorFunc: func(t *testing.T, e error) { - assert.ErrorContains(t, e, "port validation is enabled but port is missing in input url and schemes are not enabled") - }, - }, - { - description: "Domains with unknown scheme and port enabled but no port should fail, reverse", - options: DomainValidatorOptions{WithPort: true, WithScheme: true}, - expected: "ssh://example.com", - actual: "ssh://example.com:22", - errorFunc: func(t *testing.T, e error) { - assert.ErrorContains(t, e, "port validation is enabled but port is missing in input url and schemes are not enabled") - }, - }, - { - description: "Port enabled but no scheme and https scheme should pass", + description: "Port enabled with scheme and matching port should pass", options: DomainValidatorOptions{WithPort: true}, - expected: "https://example.com", - actual: "https://example.com", - }, - { - description: "Port enabled but no scheme and http scheme should pass", - options: DomainValidatorOptions{WithPort: true}, - expected: "http://example.com", - actual: "http://example.com", - }, - { - description: "Port validation with port and no scheme should fail with different port", - options: DomainValidatorOptions{WithPort: true}, - expected: "example.com:8080", - actual: "example.com:8081", - errorFunc: func(t *testing.T, e error) { - assert.ErrorIs(t, e, ErrPortMismatch) - }, + expected: "https://example.com:443", + actual: "https://example.com:443", }, { description: "Failure to format expected domain should fail", @@ -274,10 +223,13 @@ func TestDomainValidator_Validate(t *testing.T) { actual: "https://example.com", }, { - description: "Valid domains with matching ports should pass", + description: "Valid domains with non matching ports should fail", options: DomainValidatorOptions{WithPort: true}, expected: "example.com:8080", - actual: "example.com:8080", + actual: "example.com:8085", + errorFunc: func(t *testing.T, e error) { + assert.ErrorIs(t, e, ErrPortMismatch) + }, }, { description: "Valid domains without ports or schemes should pass",