diff --git a/internal/controller/oauth_controller.go b/internal/controller/oauth_controller.go index 27fca206..5ef010c9 100644 --- a/internal/controller/oauth_controller.go +++ b/internal/controller/oauth_controller.go @@ -3,7 +3,6 @@ package controller import ( "fmt" "net/http" - "net/url" "strings" "time" @@ -12,6 +11,7 @@ import ( "github.com/tinyauthapp/tinyauth/internal/service" "github.com/tinyauthapp/tinyauth/internal/utils" "github.com/tinyauthapp/tinyauth/internal/utils/logger" + "github.com/tinyauthapp/tinyauth/pkg/validators" "go.uber.org/dig" "github.com/gin-gonic/gin" @@ -311,54 +311,46 @@ func (controller *OAuthController) getCookieDomain() string { } func (controller *OAuthController) isRedirectSafe(redirectURI string) bool { - u, err := url.Parse(redirectURI) + v := validators.NewDomainValidator(validators.DomainValidatorOptions{ + WithScheme: true, + WithPort: true, + }) + + _, err := v.SafeHostname(controller.runtime.AppURL) if err != nil { - controller.log.App.Error().Err(err).Msg("Failed to parse redirect URI") + controller.log.App.Error().Err(err).Msg("App URL is invalid, cannot validate redirect URI") return false } - if u.Scheme == "" || u.Host == "" { - controller.log.App.Warn().Msg("Redirect URI has invalid scheme or host") - return false + err = v.Validate(redirectURI, controller.runtime.AppURL) + + if err == nil { + return true } - au, err := url.Parse(controller.runtime.AppURL) + controller.log.App.Debug().Err(err).Msg("Failed to validate redirect URI") - if err != nil { - controller.log.App.Error().Err(err).Msg("Failed to parse app URL") + if strings.HasPrefix(err.Error(), "expected port") || + strings.HasPrefix(err.Error(), "expected scheme") || + err.Error() == "input url is invalid" { return false } - if u.Scheme != au.Scheme { - controller.log.App.Warn().Msg("Redirect URI scheme does not match app URL scheme") + if !controller.config.Auth.SubdomainsEnabled { return false } - getEffectivePort := func(u *url.URL) string { - if u.Port() != "" { - return u.Port() - } - if u.Scheme == "https" { - return "443" - } - return "80" - } - - if getEffectivePort(u) != getEffectivePort(au) { - controller.log.App.Warn().Msg("Redirect URI port does not match app URL port") - return false - } + v = validators.NewDomainValidator(validators.DomainValidatorOptions{}) - if strings.EqualFold(u.Hostname(), au.Hostname()) { - return true - } + hostname, err := v.SafeHostname(redirectURI) - if !controller.config.Auth.SubdomainsEnabled { + if err != nil { + controller.log.App.Error().Err(err).Msg("Failed to get safe hostname from redirect URI") return false } - if strings.HasSuffix(strings.ToLower(u.Hostname()), "."+strings.ToLower(controller.runtime.CookieDomain)) { + if strings.HasSuffix(hostname, "."+strings.ToLower(controller.runtime.CookieDomain)) { return true } diff --git a/internal/controller/oauth_controller_test.go b/internal/controller/oauth_controller_test.go index 1e3b8aec..557eb858 100644 --- a/internal/controller/oauth_controller_test.go +++ b/internal/controller/oauth_controller_test.go @@ -9,7 +9,7 @@ import ( "github.com/tinyauthapp/tinyauth/internal/utils/logger" ) -func TestOAuthControllerIsRedirectSafe(t *testing.T) { +func TestOAuthController_isRedirectSafe(t *testing.T) { log := logger.NewLogger().WithTestConfig() log.Init() diff --git a/internal/service/access_controls_service.go b/internal/service/access_controls_service.go index 3615cce1..6a04ec88 100644 --- a/internal/service/access_controls_service.go +++ b/internal/service/access_controls_service.go @@ -5,6 +5,7 @@ import ( "github.com/tinyauthapp/tinyauth/internal/model" "github.com/tinyauthapp/tinyauth/internal/utils/logger" + "github.com/tinyauthapp/tinyauth/pkg/validators" "go.uber.org/dig" ) @@ -35,27 +36,39 @@ func NewAccessControlsService(i AccessControlServiceInput) *AccessControlsServic } } -func (service *AccessControlsService) lookupStaticACLs(domain string) *model.App { +func (service *AccessControlsService) lookupStaticACLs(domain string) (*model.App, error) { var nameMatch *model.App + v := validators.NewDomainValidator(validators.DomainValidatorOptions{}) + // First try to find a matching app by domain, then fallback to matching by app name (subdomain) for app, config := range service.config.Apps { - if config.Config.Domain == domain { + err := v.Validate(config.Config.Domain, domain) + // Maybe not the best way to check if a match succeeded, but expected is only + // used on match errors and not parsing errors + if err != nil && !strings.HasPrefix(err.Error(), "expected") { + return nil, err + } + if err == nil { service.log.App.Debug().Str("name", app).Msg("Found matching container by domain") - return &config + return &config, nil } - if strings.SplitN(domain, ".", 2)[0] == app { + if strings.HasPrefix(domain, app+".") { service.log.App.Debug().Str("name", app).Msg("Found matching container by app name") nameMatch = &config } } - return nameMatch + return nameMatch, nil } func (service *AccessControlsService) GetAccessControls(domain string) (*model.App, error) { // First check in the static config - app := service.lookupStaticACLs(domain) + app, err := service.lookupStaticACLs(domain) + + if err != nil { + return nil, err + } if app != nil { service.log.App.Debug().Msg("Using static ACLs for app") diff --git a/internal/service/access_controls_service_test.go b/internal/service/access_controls_service_test.go index f4f4d24c..24855a8e 100644 --- a/internal/service/access_controls_service_test.go +++ b/internal/service/access_controls_service_test.go @@ -92,7 +92,8 @@ func TestLookupStaticACLs(t *testing.T) { Config: &model.Config{Apps: tt.apps}, LabelProvider: nil, }) - got := svc.lookupStaticACLs(tt.domain) + got, err := svc.lookupStaticACLs(tt.domain) + require.NoError(t, err) if tt.expectNil { assert.Nil(t, got) return diff --git a/pkg/validators/domain_validator.go b/pkg/validators/domain_validator.go new file mode 100644 index 00000000..e9cac783 --- /dev/null +++ b/pkg/validators/domain_validator.go @@ -0,0 +1,160 @@ +// Package validators provides validators for various types of data. +// +// Domain validator is a simple utility that ensures two domains are exact +// matches while ensuring that techniques used to bypass such checks do +// not impact the validation. + +package validators + +import ( + "fmt" + "net" + "net/url" + "slices" + "strings" + + "golang.org/x/net/idna" +) + +// DomainValidatorOptions is a set of options for DomainValidator. +type DomainValidatorOptions struct { + // Ensure domains have the same scheme. + 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. + AllowedSchemes []string +} + +// DomainValidator is a simple utility that ensures two domains are exact +// matches while ensuring that techniques used to bypass such checks do +// not impact the validation. +type DomainValidator struct { + opts DomainValidatorOptions +} + +// NewDomainValidator creates a new DomainValidator. +func NewDomainValidator(opts DomainValidatorOptions) *DomainValidator { + return &DomainValidator{ + opts: opts, + } +} + +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, fmt.Errorf("input url is invalid") + } + + if v.opts.WithPort && !v.opts.WithScheme && u.Port() == "" { + return nil, fmt.Errorf("port validation is enabled but port is missing in input url and schemes are not enabled") + } + + 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") + } + if len(v.opts.AllowedSchemes) > 0 && !slices.Contains(v.opts.AllowedSchemes, u.Scheme) { + return nil, fmt.Errorf("scheme %s not allowed", u.Scheme) + } + } + + return u, nil +} + +func (v *DomainValidator) getEffectivePort(u *url.URL) string { + if u.Port() != "" { + return u.Port() + } + if u.Scheme == "https" { + return "443" + } + return "80" +} + +func (v *DomainValidator) formatHostname(hostname string) (string, error) { + hostname = strings.ToLower(hostname) + hostname = strings.TrimSuffix(hostname, ".") + if net.ParseIP(hostname) != nil { + return "", fmt.Errorf("ip addresses are not supported") + } + hostname, err := idna.Lookup.ToASCII(hostname) + if err != nil { + return "", fmt.Errorf("failed to convert hostname to ascii: %w", err) + } + return hostname, nil +} + +// Validate ensures that two domains are exact matches with the +// options defined in the DomainValidatorOptions. It ensures that the +// inputs are proper URLs and contain a host. It lowercases the hostnames +// and removes the trailing dot. Finally, it checks that the hostnames are +// equal unless WithScheme or WithPort is set to true where it also +// validates the scheme and port respectively. +func (v *DomainValidator) Validate(expected, actual string) error { + eu, err := v.getURL(expected) + + if err != nil { + return err + } + + au, err := v.getURL(actual) + + if err != nil { + return err + } + + if v.opts.WithScheme { + if eu.Scheme != au.Scheme { + return fmt.Errorf("expected scheme %s, got %s", eu.Scheme, au.Scheme) + } + } + + if v.opts.WithPort { + if v.getEffectivePort(eu) != v.getEffectivePort(au) { + return fmt.Errorf("expected port %s, got %s", v.getEffectivePort(eu), v.getEffectivePort(au)) + } + } + + euf, err := v.formatHostname(eu.Hostname()) + + if err != nil { + return err + } + + auf, err := v.formatHostname(au.Hostname()) + + if err != nil { + return err + } + + if euf != auf { + return fmt.Errorf("expected hostname %s, got %s", euf, auf) + } + + return nil +} + +// SafeHostname uses the internal validation for domains that 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) { + u, err := v.getURL(input) + + if err != nil { + return "", err + } + + return v.formatHostname(u.Hostname()) +} diff --git a/pkg/validators/domain_validator_test.go b/pkg/validators/domain_validator_test.go new file mode 100644 index 00000000..339e0247 --- /dev/null +++ b/pkg/validators/domain_validator_test.go @@ -0,0 +1,270 @@ +package validators + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDomainValidator_SafeHostname(t *testing.T) { + type testCase struct { + description string + options DomainValidatorOptions + input string + expected string + errorFunc func(t *testing.T, e error) + } + + tests := []testCase{ + { + description: "Empty url fails", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "input url is invalid") + }, + }, + { + description: "Invalid url fails", + input: "foo:foo", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "failed to parse input url") + }, + }, + { + description: "Domain without scheme should parse if scheme is disabled", + input: "example.com", + expected: "example.com", + }, + { + description: "Domain without scheme should not parse if scheme is enabled", + options: DomainValidatorOptions{WithScheme: true}, + input: "example.com", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "input url is invalid") + }, + }, + { + description: "Domain with scheme and disallowed scheme should fail", + 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") + }, + }, + { + description: "Domain with scheme and allowed scheme should pass", + options: DomainValidatorOptions{WithScheme: true, AllowedSchemes: []string{"https"}}, + input: "https://example.com", + expected: "example.com", + }, + { + description: "Domain should get lowercased", + input: "EXAMPLE.COM", + expected: "example.com", + }, + { + description: "DNS dot should be removed", + input: "example.com.", + expected: "example.com", + }, + { + description: "IPv4 address should fail", + input: "127.0.0.1", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "ip addresses are not supported") + }, + }, + { + description: "IPv6 address should fail", + input: "[::1]", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "ip addresses are not supported") + }, + }, + { + description: "Domains with unicode characters should be allowed", + input: "bücher.example.com", + expected: "xn--bcher-kva.example.com", + }, + { + description: "Invalid IDNA domain should fail", + input: "ab--cd.example.com", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "invalid label") + }, + }, + { + // 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") + }, + }, + } + + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + v := NewDomainValidator(test.options) + res, err := v.SafeHostname(test.input) + if test.errorFunc != nil { + test.errorFunc(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, test.expected, res) + }) + } +} + +func TestDomainValidator_Validate(t *testing.T) { + type testCase struct { + description string + options DomainValidatorOptions + expected string + actual string + errorFunc func(t *testing.T, e error) + } + + tests := []testCase{ + { + description: "Invalid expected domain fails checks", + expected: "foo:foo", + actual: "bar.com", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "failed to parse input url") + }, + }, + { + description: "Invalid check domain fails checks", + expected: "example.com", + actual: "foo:foo", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "failed to parse input url") + }, + }, + { + description: "Valid domains with non-matching schemes should fail", + options: DomainValidatorOptions{WithScheme: true, AllowedSchemes: []string{"https", "http"}}, + expected: "https://example.com", + actual: "http://example.com", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "expected scheme https, got http") + }, + }, + { + description: "Valid domains with matching schemes should pass", + options: DomainValidatorOptions{WithScheme: true, AllowedSchemes: []string{"https", "http"}}, + expected: "https://example.com", + actual: "https://example.com", + }, + { + description: "Port validation without ports and schemes disabled should fail", + 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", + options: DomainValidatorOptions{WithPort: true}, + expected: "example.com:8080", + actual: "example.com:8080", + }, + { + 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.ErrorContains(t, e, "expected port 8080, got 8081") + }, + }, + { + description: "Failure to format expected domain should fail", + expected: "ab--cd.example.com", + actual: "example.com", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "idna: invalid label") + }, + }, + { + description: "Failure to format check domain should fail", + expected: "example.com", + actual: "ab--cd.example.com", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "idna: invalid label") + }, + }, + { + description: "Valid domains with matching schemes and ports should pass", + options: DomainValidatorOptions{WithScheme: true, AllowedSchemes: []string{"https", "http"}, WithPort: true}, + expected: "https://example.com:8080", + actual: "https://example.com:8080", + }, + { + description: "Valid domains with matching schemes should pass", + options: DomainValidatorOptions{WithScheme: true, AllowedSchemes: []string{"https", "http"}}, + expected: "https://example.com", + actual: "https://example.com", + }, + { + description: "Valid domains with matching ports should pass", + options: DomainValidatorOptions{WithPort: true}, + expected: "example.com:8080", + actual: "example.com:8080", + }, + { + description: "Valid domains without ports or schemes should pass", + actual: "example.com", + expected: "example.com", + }, + { + description: "Unicode valid domains should pass", + expected: "xn--bcher-kva.example.com", + actual: "bücher.example.com", + }, + { + description: "Unicode valid domains should pass (reverse)", + expected: "bücher.example.com", + actual: "xn--bcher-kva.example.com", + }, + { + description: "Non matching hostnames should fail", + expected: "example.com", + actual: "foo.com", + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "expected hostname example.com, got foo.com") + }, + }, + } + + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + v := NewDomainValidator(test.options) + err := v.Validate(test.expected, test.actual) + if test.errorFunc != nil { + test.errorFunc(t, err) + return + } + require.NoError(t, err) + }) + } +}