diff --git a/Dockerfile b/Dockerfile index 811a4837..a735bf84 100644 --- a/Dockerfile +++ b/Dockerfile @@ -39,6 +39,7 @@ RUN go mod download COPY ./cmd ./cmd COPY ./internal ./internal +COPY ./pkg ./pkg COPY --from=frontend-builder /frontend/dist ./internal/assets/dist RUN CGO_ENABLED=0 go build -tags "${BUILD_TAGS}" -ldflags "${LDFLAGS} \ diff --git a/Dockerfile.dev b/Dockerfile.dev index 1ea1c5f0..d4a9c386 100644 --- a/Dockerfile.dev +++ b/Dockerfile.dev @@ -12,6 +12,7 @@ RUN go install github.com/go-delve/delve/cmd/dlv@v1.26.3 COPY ./cmd ./cmd COPY ./internal ./internal +COPY ./pkg ./pkg COPY ./air.toml ./ EXPOSE 3000 diff --git a/Dockerfile.distroless b/Dockerfile.distroless index fb8b4962..91f22b32 100644 --- a/Dockerfile.distroless +++ b/Dockerfile.distroless @@ -39,6 +39,7 @@ RUN go mod download COPY ./cmd ./cmd/ COPY ./internal ./internal +COPY ./pkg ./pkg COPY --from=frontend-builder /frontend/dist ./internal/assets/dist RUN CGO_ENABLED=0 go build -tags "${BUILD_TAGS}" -ldflags "${LDFLAGS} \ diff --git a/internal/controller/oauth_controller.go b/internal/controller/oauth_controller.go index 27fca206..ee0f92fa 100644 --- a/internal/controller/oauth_controller.go +++ b/internal/controller/oauth_controller.go @@ -1,9 +1,9 @@ package controller import ( + "errors" "fmt" "net/http" - "net/url" "strings" "time" @@ -12,6 +12,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 +312,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) - au, err := url.Parse(controller.runtime.AppURL) - - if err != nil { - controller.log.App.Error().Err(err).Msg("Failed to parse app URL") - return false - } - - if u.Scheme != au.Scheme { - controller.log.App.Warn().Msg("Redirect URI scheme does not match app URL scheme") - 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 - } - - if strings.EqualFold(u.Hostname(), au.Hostname()) { + if err == nil { return true } + controller.log.App.Debug().Err(err).Msg("Failed to validate redirect URI") + + if errors.Is(err, validators.ErrInvalidURL) || + errors.Is(err, validators.ErrSchemeMismatch) || + errors.Is(err, validators.ErrPortMismatch) { + return false + } + if !controller.config.Auth.SubdomainsEnabled { return false } - if strings.HasSuffix(strings.ToLower(u.Hostname()), "."+strings.ToLower(controller.runtime.CookieDomain)) { + v = validators.NewDomainValidator(validators.DomainValidatorOptions{}) + + hostname, err := v.SafeHostname(redirectURI) + + if err != nil { + controller.log.App.Error().Err(err).Msg("Failed to get safe hostname from redirect URI") + return false + } + + 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..bb76c2a8 100644 --- a/internal/service/access_controls_service.go +++ b/internal/service/access_controls_service.go @@ -1,10 +1,12 @@ package service import ( + "errors" "strings" "github.com/tinyauthapp/tinyauth/internal/model" "github.com/tinyauthapp/tinyauth/internal/utils/logger" + "github.com/tinyauthapp/tinyauth/pkg/validators" "go.uber.org/dig" ) @@ -38,13 +40,19 @@ func NewAccessControlsService(i AccessControlServiceInput) *AccessControlsServic func (service *AccessControlsService) lookupStaticACLs(domain string) *model.App { 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) + if err == nil { service.log.App.Debug().Str("name", app).Msg("Found matching container by domain") return &config } - if strings.SplitN(domain, ".", 2)[0] == app { + if !errors.Is(err, validators.ErrHostnameMismatch) { + service.log.App.Debug().Str("name", app).Err(err).Msg("Domain validation failed") + } + if strings.HasPrefix(strings.ToLower(domain), strings.ToLower(app+".")) { service.log.App.Debug().Str("name", app).Msg("Found matching container by app name") nameMatch = &config } diff --git a/pkg/validators/domain_validator.go b/pkg/validators/domain_validator.go new file mode 100644 index 00000000..adeb7a85 --- /dev/null +++ b/pkg/validators/domain_validator.go @@ -0,0 +1,179 @@ +// 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" +) + +var ( + ErrInvalidURL = fmt.Errorf("invalid url") + ErrSchemeMismatch = fmt.Errorf("scheme mismatch") + ErrPortMismatch = fmt.Errorf("port mismatch") + ErrHostnameMismatch = fmt.Errorf("hostname mismatch") +) + +// 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, ErrInvalidURL + } + + 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, 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) { + 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 ErrSchemeMismatch + } + } + + 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 { + return ErrPortMismatch + } + } + + 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 ErrHostnameMismatch + } + + 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..a3df8701 --- /dev/null +++ b/pkg/validators/domain_validator_test.go @@ -0,0 +1,288 @@ +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.ErrorIs(t, e, ErrInvalidURL) + }, + }, + { + 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.ErrorIs(t, e, ErrInvalidURL) + }, + }, + { + 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.ErrorIs(t, e, ErrSchemeMismatch) + }, + }, + { + 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: "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, "failed to get effective port for url") + }, + }, + { + 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, "failed to get effective port for url") + }, + }, + { + 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) + }, + }, + { + 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.ErrorIs(t, e, ErrHostnameMismatch) + }, + }, + } + + 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) + }) + } +}