From 25a41bf62bf0e1e78bbeb9e1297dec4c21aaa832 Mon Sep 17 00:00:00 2001 From: Stavros Date: Mon, 13 Jul 2026 22:01:49 +0300 Subject: [PATCH 1/7] fix: fix domain check in acl service --- internal/controller/oauth_controller.go | 5 +- internal/service/access_controls_service.go | 54 ++++++++++++++++++--- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/internal/controller/oauth_controller.go b/internal/controller/oauth_controller.go index 27fca2067..1cbc520bc 100644 --- a/internal/controller/oauth_controller.go +++ b/internal/controller/oauth_controller.go @@ -350,7 +350,10 @@ func (controller *OAuthController) isRedirectSafe(redirectURI string) bool { return false } - if strings.EqualFold(u.Hostname(), au.Hostname()) { + nu := strings.TrimSuffix(u.Hostname(), ".") + nau := strings.TrimSuffix(au.Hostname(), ".") + + if strings.EqualFold(nu, nau) { return true } diff --git a/internal/service/access_controls_service.go b/internal/service/access_controls_service.go index 3615cce18..07233faa9 100644 --- a/internal/service/access_controls_service.go +++ b/internal/service/access_controls_service.go @@ -1,6 +1,8 @@ package service import ( + "fmt" + "net/url" "strings" "github.com/tinyauthapp/tinyauth/internal/model" @@ -35,27 +37,35 @@ 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 // 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 { + match, err := service.checkDomain(domain, config.Config.Domain) + if err != nil { + return nil, err + } + if match { 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") @@ -70,3 +80,35 @@ func (service *AccessControlsService) GetAccessControls(domain string) (*model.A // no labels return nil, nil } + +func (service *AccessControlsService) checkDomain(check, target string) (bool, error) { + // Domains don't have a scheme, so we use a mock one + cu, err := url.Parse("tinyauth://" + check) + + if err != nil { + return false, fmt.Errorf("failed to parse check domain: %w", err) + } + + if cu.Host == "" { + return false, fmt.Errorf("check domain is empty") + } + + tu, err := url.Parse("tinyauth://" + target) + + if err != nil { + return false, fmt.Errorf("failed to parse target domain: %w", err) + } + + if tu.Host == "" { + return false, fmt.Errorf("target domain is empty") + } + + // non-dns check url + ndcu := strings.TrimSuffix(cu.Hostname(), ".") + + // non-dns target url + ndtu := strings.TrimSuffix(tu.Hostname(), ".") + + // Strip out the port + return strings.EqualFold(ndcu, ndtu), nil +} From 3008ffad341195ec43a133343e4d80e568a05274 Mon Sep 17 00:00:00 2001 From: Stavros Date: Tue, 14 Jul 2026 00:08:53 +0300 Subject: [PATCH 2/7] feat: add domain validator pkg --- pkg/validators/domain_validator.go | 159 ++++++++++++++ pkg/validators/domain_validator_test.go | 267 ++++++++++++++++++++++++ 2 files changed, 426 insertions(+) create mode 100644 pkg/validators/domain_validator.go create mode 100644 pkg/validators/domain_validator_test.go diff --git a/pkg/validators/domain_validator.go b/pkg/validators/domain_validator.go new file mode 100644 index 000000000..28b089856 --- /dev/null +++ b/pkg/validators/domain_validator.go @@ -0,0 +1,159 @@ +// 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. + 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 !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 000000000..908700525 --- /dev/null +++ b/pkg/validators/domain_validator_test.go @@ -0,0 +1,267 @@ +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", + }, + } + + 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) + }) + } +} From 0c1cc98fd314e65e30d50caf9e1a630be98456fc Mon Sep 17 00:00:00 2001 From: Stavros Date: Tue, 14 Jul 2026 00:37:22 +0300 Subject: [PATCH 3/7] feat: adopt domain validator in oauth controller and acls service --- internal/controller/oauth_controller.go | 55 ++++++++----------- internal/controller/oauth_controller_test.go | 2 +- internal/service/access_controls_service.go | 45 +++------------ .../service/access_controls_service_test.go | 3 +- pkg/validators/domain_validator.go | 3 +- 5 files changed, 35 insertions(+), 73 deletions(-) diff --git a/internal/controller/oauth_controller.go b/internal/controller/oauth_controller.go index 1cbc520bc..5ef010c97 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,57 +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, + }) - if err != nil { - controller.log.App.Error().Err(err).Msg("Failed to parse redirect URI") - return false - } + _, err := v.SafeHostname(controller.runtime.AppURL) - if u.Scheme == "" || u.Host == "" { - controller.log.App.Warn().Msg("Redirect URI has invalid scheme or host") + if err != nil { + controller.log.App.Error().Err(err).Msg("App URL is invalid, cannot validate redirect URI") return false } - au, err := url.Parse(controller.runtime.AppURL) + err = v.Validate(redirectURI, controller.runtime.AppURL) - if err != nil { - controller.log.App.Error().Err(err).Msg("Failed to parse app URL") - return false + if err == nil { + return true } - if u.Scheme != au.Scheme { - controller.log.App.Warn().Msg("Redirect URI scheme does not match app URL scheme") - return false - } + controller.log.App.Debug().Err(err).Msg("Failed to validate redirect URI") - getEffectivePort := func(u *url.URL) string { - if u.Port() != "" { - return u.Port() - } - if u.Scheme == "https" { - return "443" - } - return "80" + if strings.HasPrefix(err.Error(), "expected port") || + strings.HasPrefix(err.Error(), "expected scheme") || + err.Error() == "input url is invalid" { + return false } - if getEffectivePort(u) != getEffectivePort(au) { - controller.log.App.Warn().Msg("Redirect URI port does not match app URL port") + if !controller.config.Auth.SubdomainsEnabled { return false } - nu := strings.TrimSuffix(u.Hostname(), ".") - nau := strings.TrimSuffix(au.Hostname(), ".") + v = validators.NewDomainValidator(validators.DomainValidatorOptions{}) - if strings.EqualFold(nu, nau) { - 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 1e3b8aeca..557eb8586 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 07233faa9..6a04ec880 100644 --- a/internal/service/access_controls_service.go +++ b/internal/service/access_controls_service.go @@ -1,12 +1,11 @@ package service import ( - "fmt" - "net/url" "strings" "github.com/tinyauthapp/tinyauth/internal/model" "github.com/tinyauthapp/tinyauth/internal/utils/logger" + "github.com/tinyauthapp/tinyauth/pkg/validators" "go.uber.org/dig" ) @@ -40,13 +39,17 @@ func NewAccessControlsService(i AccessControlServiceInput) *AccessControlsServic 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 { - match, err := service.checkDomain(domain, config.Config.Domain) - if err != nil { + 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 match { + if err == nil { service.log.App.Debug().Str("name", app).Msg("Found matching container by domain") return &config, nil } @@ -80,35 +83,3 @@ func (service *AccessControlsService) GetAccessControls(domain string) (*model.A // no labels return nil, nil } - -func (service *AccessControlsService) checkDomain(check, target string) (bool, error) { - // Domains don't have a scheme, so we use a mock one - cu, err := url.Parse("tinyauth://" + check) - - if err != nil { - return false, fmt.Errorf("failed to parse check domain: %w", err) - } - - if cu.Host == "" { - return false, fmt.Errorf("check domain is empty") - } - - tu, err := url.Parse("tinyauth://" + target) - - if err != nil { - return false, fmt.Errorf("failed to parse target domain: %w", err) - } - - if tu.Host == "" { - return false, fmt.Errorf("target domain is empty") - } - - // non-dns check url - ndcu := strings.TrimSuffix(cu.Hostname(), ".") - - // non-dns target url - ndtu := strings.TrimSuffix(tu.Hostname(), ".") - - // Strip out the port - return strings.EqualFold(ndcu, ndtu), nil -} diff --git a/internal/service/access_controls_service_test.go b/internal/service/access_controls_service_test.go index f4f4d24c9..24855a8e2 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 index 28b089856..e9cac7836 100644 --- a/pkg/validators/domain_validator.go +++ b/pkg/validators/domain_validator.go @@ -23,6 +23,7 @@ type DomainValidatorOptions struct { // 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 } @@ -64,7 +65,7 @@ func (v *DomainValidator) getURL(i string) (*url.URL, error) { if u.Scheme == "tinyauth" { return nil, fmt.Errorf("input url is missing scheme") } - if !slices.Contains(v.opts.AllowedSchemes, u.Scheme) { + if len(v.opts.AllowedSchemes) > 0 && !slices.Contains(v.opts.AllowedSchemes, u.Scheme) { return nil, fmt.Errorf("scheme %s not allowed", u.Scheme) } } From 25b434a012dd8bd61f7eba9d73729b8eb95e59cc Mon Sep 17 00:00:00 2001 From: Stavros Date: Tue, 14 Jul 2026 00:42:41 +0300 Subject: [PATCH 4/7] tests: fix validator tests --- pkg/validators/domain_validator_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/validators/domain_validator_test.go b/pkg/validators/domain_validator_test.go index 908700525..339e02472 100644 --- a/pkg/validators/domain_validator_test.go +++ b/pkg/validators/domain_validator_test.go @@ -250,6 +250,9 @@ func TestDomainValidator_Validate(t *testing.T) { 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") + }, }, } From db1d4232544d3553a6c4c22f6225b9a145f4ce28 Mon Sep 17 00:00:00 2001 From: Stavros Date: Tue, 14 Jul 2026 13:11:12 +0300 Subject: [PATCH 5/7] fix: review comments --- internal/controller/oauth_controller.go | 7 ++-- internal/service/access_controls_service.go | 21 ++++------ .../service/access_controls_service_test.go | 3 +- pkg/validators/domain_validator.go | 39 ++++++++++++++----- pkg/validators/domain_validator_test.go | 32 +++++++++++---- 5 files changed, 67 insertions(+), 35 deletions(-) diff --git a/internal/controller/oauth_controller.go b/internal/controller/oauth_controller.go index 5ef010c97..ee0f92fa9 100644 --- a/internal/controller/oauth_controller.go +++ b/internal/controller/oauth_controller.go @@ -1,6 +1,7 @@ package controller import ( + "errors" "fmt" "net/http" "strings" @@ -331,9 +332,9 @@ func (controller *OAuthController) isRedirectSafe(redirectURI string) bool { controller.log.App.Debug().Err(err).Msg("Failed to validate redirect URI") - if strings.HasPrefix(err.Error(), "expected port") || - strings.HasPrefix(err.Error(), "expected scheme") || - err.Error() == "input url is invalid" { + if errors.Is(err, validators.ErrInvalidURL) || + errors.Is(err, validators.ErrSchemeMismatch) || + errors.Is(err, validators.ErrPortMismatch) { return false } diff --git a/internal/service/access_controls_service.go b/internal/service/access_controls_service.go index 6a04ec880..181d29c1f 100644 --- a/internal/service/access_controls_service.go +++ b/internal/service/access_controls_service.go @@ -1,6 +1,7 @@ package service import ( + "errors" "strings" "github.com/tinyauthapp/tinyauth/internal/model" @@ -36,7 +37,7 @@ func NewAccessControlsService(i AccessControlServiceInput) *AccessControlsServic } } -func (service *AccessControlsService) lookupStaticACLs(domain string) (*model.App, error) { +func (service *AccessControlsService) lookupStaticACLs(domain string) *model.App { var nameMatch *model.App v := validators.NewDomainValidator(validators.DomainValidatorOptions{}) @@ -44,14 +45,12 @@ func (service *AccessControlsService) lookupStaticACLs(domain string) (*model.Ap // First try to find a matching app by domain, then fallback to matching by app name (subdomain) for app, config := range service.config.Apps { 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, nil + return &config + } + if !errors.Is(err, validators.ErrHostnameMismatch) { + service.log.App.Debug().Str("name", app).Err(err).Msg("Domain validation failed") } if strings.HasPrefix(domain, app+".") { service.log.App.Debug().Str("name", app).Msg("Found matching container by app name") @@ -59,16 +58,12 @@ func (service *AccessControlsService) lookupStaticACLs(domain string) (*model.Ap } } - return nameMatch, nil + return nameMatch } func (service *AccessControlsService) GetAccessControls(domain string) (*model.App, error) { // First check in the static config - app, err := service.lookupStaticACLs(domain) - - if err != nil { - return nil, err - } + app := service.lookupStaticACLs(domain) 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 24855a8e2..f4f4d24c9 100644 --- a/internal/service/access_controls_service_test.go +++ b/internal/service/access_controls_service_test.go @@ -92,8 +92,7 @@ func TestLookupStaticACLs(t *testing.T) { Config: &model.Config{Apps: tt.apps}, LabelProvider: nil, }) - got, err := svc.lookupStaticACLs(tt.domain) - require.NoError(t, err) + got := svc.lookupStaticACLs(tt.domain) if tt.expectNil { assert.Nil(t, got) return diff --git a/pkg/validators/domain_validator.go b/pkg/validators/domain_validator.go index e9cac7836..adeb7a859 100644 --- a/pkg/validators/domain_validator.go +++ b/pkg/validators/domain_validator.go @@ -16,6 +16,13 @@ import ( "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. @@ -53,7 +60,7 @@ func (v *DomainValidator) getURL(i string) (*url.URL, error) { } if u.Host == "" { - return nil, fmt.Errorf("input url is invalid") + return nil, ErrInvalidURL } if v.opts.WithPort && !v.opts.WithScheme && u.Port() == "" { @@ -73,14 +80,18 @@ func (v *DomainValidator) getURL(i string) (*url.URL, error) { return u, nil } -func (v *DomainValidator) getEffectivePort(u *url.URL) string { +func (v *DomainValidator) getEffectivePort(u *url.URL) (string, bool) { if u.Port() != "" { - return u.Port() + return u.Port(), true } - if u.Scheme == "https" { - return "443" + switch u.Scheme { + case "http": + return "80", true + case "https": + return "443", true + default: + return "", false } - return "80" } func (v *DomainValidator) formatHostname(hostname string) (string, error) { @@ -117,13 +128,21 @@ func (v *DomainValidator) Validate(expected, actual string) error { if v.opts.WithScheme { if eu.Scheme != au.Scheme { - return fmt.Errorf("expected scheme %s, got %s", eu.Scheme, au.Scheme) + return ErrSchemeMismatch } } 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)) + 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 } } @@ -140,7 +159,7 @@ func (v *DomainValidator) Validate(expected, actual string) error { } if euf != auf { - return fmt.Errorf("expected hostname %s, got %s", euf, auf) + return ErrHostnameMismatch } return nil diff --git a/pkg/validators/domain_validator_test.go b/pkg/validators/domain_validator_test.go index 339e02472..a3df87013 100644 --- a/pkg/validators/domain_validator_test.go +++ b/pkg/validators/domain_validator_test.go @@ -20,7 +20,7 @@ func TestDomainValidator_SafeHostname(t *testing.T) { { description: "Empty url fails", errorFunc: func(t *testing.T, e error) { - assert.ErrorContains(t, e, "input url is invalid") + assert.ErrorIs(t, e, ErrInvalidURL) }, }, { @@ -40,7 +40,7 @@ func TestDomainValidator_SafeHostname(t *testing.T) { options: DomainValidatorOptions{WithScheme: true}, input: "example.com", errorFunc: func(t *testing.T, e error) { - assert.ErrorContains(t, e, "input url is invalid") + assert.ErrorIs(t, e, ErrInvalidURL) }, }, { @@ -135,7 +135,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 input url:") }, }, { @@ -143,7 +143,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 input url:") }, }, { @@ -152,7 +152,7 @@ func TestDomainValidator_Validate(t *testing.T) { expected: "https://example.com", actual: "http://example.com", errorFunc: func(t *testing.T, e error) { - assert.ErrorContains(t, e, "expected scheme https, got http") + assert.ErrorIs(t, e, ErrSchemeMismatch) }, }, { @@ -188,13 +188,31 @@ func TestDomainValidator_Validate(t *testing.T) { 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.ErrorContains(t, e, "expected port 8080, got 8081") + assert.ErrorIs(t, e, ErrPortMismatch) }, }, { @@ -251,7 +269,7 @@ func TestDomainValidator_Validate(t *testing.T) { expected: "example.com", actual: "foo.com", errorFunc: func(t *testing.T, e error) { - assert.ErrorContains(t, e, "expected hostname example.com, got foo.com") + assert.ErrorIs(t, e, ErrHostnameMismatch) }, }, } From 8f8130949d2da47764525cc352f93369bce2190e Mon Sep 17 00:00:00 2001 From: Stavros Date: Tue, 14 Jul 2026 13:30:10 +0300 Subject: [PATCH 6/7] fix: lowercase app name check --- internal/service/access_controls_service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/access_controls_service.go b/internal/service/access_controls_service.go index 181d29c1f..bb76c2a8e 100644 --- a/internal/service/access_controls_service.go +++ b/internal/service/access_controls_service.go @@ -52,7 +52,7 @@ func (service *AccessControlsService) lookupStaticACLs(domain string) *model.App if !errors.Is(err, validators.ErrHostnameMismatch) { service.log.App.Debug().Str("name", app).Err(err).Msg("Domain validation failed") } - if strings.HasPrefix(domain, app+".") { + 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 } From edb282966483ae308d2d368f1bfdfff14ef1e25c Mon Sep 17 00:00:00 2001 From: Stavros Date: Tue, 14 Jul 2026 14:05:51 +0300 Subject: [PATCH 7/7] fix: add pkg to dockerfiles --- Dockerfile | 1 + Dockerfile.dev | 1 + Dockerfile.distroless | 1 + 3 files changed, 3 insertions(+) diff --git a/Dockerfile b/Dockerfile index 811a48375..a735bf844 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 1ea1c5f08..d4a9c3861 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 fb8b4962b..91f22b32e 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} \