From 61d372b2882fc4e6dc95ee01dd3fee3cd1b810a0 Mon Sep 17 00:00:00 2001 From: Stavros Date: Sat, 22 Aug 2026 21:17:08 +0300 Subject: [PATCH] fix: don't use domain validator in acl matching logic --- internal/controller/oauth_controller.go | 4 +- internal/controller/proxy_controller_test.go | 1 + internal/service/access_controls_service.go | 41 +++++++++++++++---- .../service/access_controls_service_test.go | 33 +++++++-------- 4 files changed, 54 insertions(+), 25 deletions(-) diff --git a/internal/controller/oauth_controller.go b/internal/controller/oauth_controller.go index fd6c2658..f48c3fde 100644 --- a/internal/controller/oauth_controller.go +++ b/internal/controller/oauth_controller.go @@ -294,7 +294,9 @@ func (controller *OAuthController) getCookieDomain() string { func (controller *OAuthController) isRedirectSafe(redirectURI string) bool { v := validators.NewDomainValidator(validators.DomainValidatorOptions{ - WithPort: true, + WithPort: true, + WithScheme: true, + AllowedSchemes: []string{"https", "http"}, }) _, err := v.SafeHostname(controller.runtime.AppURL) diff --git a/internal/controller/proxy_controller_test.go b/internal/controller/proxy_controller_test.go index c7dee667..eff30935 100644 --- a/internal/controller/proxy_controller_test.go +++ b/internal/controller/proxy_controller_test.go @@ -978,6 +978,7 @@ func TestProxyController(t *testing.T) { NewProxyController(ProxyControllerInput{ Log: log, RuntimeConfig: &runtime, + Config: &cfg, RouterGroup: group, ACLsService: aclsService, AuthService: authService, diff --git a/internal/service/access_controls_service.go b/internal/service/access_controls_service.go index f8816a1f..6e9a3bb8 100644 --- a/internal/service/access_controls_service.go +++ b/internal/service/access_controls_service.go @@ -2,11 +2,13 @@ package service import ( "errors" + "fmt" + "net" "strings" + "unicode" "github.com/tinyauthapp/tinyauth/internal/model" "github.com/tinyauthapp/tinyauth/internal/utils/logger" - "github.com/tinyauthapp/tinyauth/pkg/validators" "go.uber.org/dig" ) @@ -37,8 +39,29 @@ func NewAccessControlsService(i AccessControlServiceInput) *AccessControlsServic } } +func (service *AccessControlsService) ensureAscii(str string) bool { + for i := 0; i < len(str); i++ { + if str[i] > unicode.MaxASCII { + return false + } + } + return true +} + +func (service *AccessControlsService) normalizeDomain(domain string) string { + if host, _, err := net.SplitHostPort(domain); err == nil { + domain = host + } + domain = strings.TrimRight(domain, ".") + return strings.ToLower(domain) +} + func (service *AccessControlsService) getACLs(domain string, lookup func(locator func(name string, app *model.App) bool) error) (*model.App, error) { - v := validators.NewDomainValidator(validators.DomainValidatorOptions{}) + if !service.ensureAscii(domain) { + return nil, errors.New("domain contains non-ascii characters") + } + + normalizedDomain := service.normalizeDomain(domain) var domainMatch *model.App var nameMatch *model.App @@ -46,16 +69,18 @@ func (service *AccessControlsService) getACLs(domain string, lookup func(locator locatorFunc := func(name string, app *model.App) bool { if app.Config.Domain != "" { - err := v.Validate(app.Config.Domain, domain) - if err == nil { + if !service.ensureAscii(app.Config.Domain) { + service.log.App.Warn().Str("name", name).Str("domain", app.Config.Domain).Msg("Domain contains non-ascii characters, skipping") + return false + } + if normalizedDomain == service.normalizeDomain(app.Config.Domain) { service.log.App.Debug().Str("name", name).Msg("Found matching container by domain") domainMatch = app return true - } else if !errors.Is(err, validators.ErrHostnameMismatch) { - service.log.App.Debug().Str("name", name).Err(err).Msg("Domain validation failed") } + return false } - if strings.HasPrefix(strings.ToLower(domain), strings.ToLower(name+".")) { + if strings.HasPrefix(normalizedDomain, strings.ToLower(name+".")) { service.log.App.Debug().Str("name", name).Msg("Found matching container by app name") nameMatch = app nameMatchedApps = append(nameMatchedApps, name) @@ -79,7 +104,7 @@ func (service *AccessControlsService) getACLs(domain string, lookup func(locator } if len(nameMatchedApps) > 1 { - service.log.App.Warn().Str("domain", domain).Strs("apps", nameMatchedApps).Msg("Multiple apps matched domain by name, app names must be unique, using last match") + return nil, fmt.Errorf("multiple apps matched domain by name, app names must be unique") } service.log.App.Debug().Str("domain", domain).Msg("Found matching app by app name") diff --git a/internal/service/access_controls_service_test.go b/internal/service/access_controls_service_test.go index 30415933..846d6a9f 100644 --- a/internal/service/access_controls_service_test.go +++ b/internal/service/access_controls_service_test.go @@ -4,6 +4,7 @@ import ( "errors" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tinyauthapp/tinyauth/internal/model" "github.com/tinyauthapp/tinyauth/internal/utils/logger" @@ -35,10 +36,11 @@ func TestAccessControlsService(t *testing.T) { log.Init() tests := []struct { - name string - domain string - acls map[string]model.App - want *model.App + name string + domain string + acls map[string]model.App + want *model.App + errorFunc func(t *testing.T, e error) }{ { name: "returns ACLs for domain", @@ -65,20 +67,11 @@ func TestAccessControlsService(t *testing.T) { want: &model.App{Config: model.AppConfig{Domain: "example.com"}}, }, { - name: "returns ACLs for non-ascii domain", + name: "returns error for non-ascii domain", domain: "bücher.example.com", - acls: map[string]model.App{ - "foo": {Config: model.AppConfig{Domain: "bücher.example.com"}}, + errorFunc: func(t *testing.T, e error) { + assert.ErrorContains(t, e, "domain contains non-ascii characters") }, - want: &model.App{Config: model.AppConfig{Domain: "bücher.example.com"}}, - }, - { - name: "returns ACLs for punycode domain and non-ascii config", - domain: "bücher.example.com", - acls: map[string]model.App{ - "foo": {Config: model.AppConfig{Domain: "xn--bcher-kva.example.com"}}, - }, - want: &model.App{Config: model.AppConfig{Domain: "xn--bcher-kva.example.com"}}, }, { name: "returns ACLs with case-insensitive matching", @@ -122,6 +115,10 @@ func TestAccessControlsService(t *testing.T) { LabelProvider: mock, }) app, err := acls.getACLs(test.domain, mock.Lookup) + if test.errorFunc != nil { + test.errorFunc(t, err) + return + } require.NoError(t, err) require.Equal(t, test.want, app) }) @@ -137,6 +134,10 @@ func TestAccessControlsService(t *testing.T) { }, }) app, err := acls.lookupStaticACLs(test.domain) + if test.errorFunc != nil { + test.errorFunc(t, err) + return + } require.NoError(t, err) require.Equal(t, test.want, app) })