Skip to content

Commit 61d372b

Browse files
committed
fix: don't use domain validator in acl matching logic
1 parent 4dc12c6 commit 61d372b

4 files changed

Lines changed: 54 additions & 25 deletions

File tree

‎internal/controller/oauth_controller.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,9 @@ func (controller *OAuthController) getCookieDomain() string {
294294

295295
func (controller *OAuthController) isRedirectSafe(redirectURI string) bool {
296296
v := validators.NewDomainValidator(validators.DomainValidatorOptions{
297-
WithPort: true,
297+
WithPort: true,
298+
WithScheme: true,
299+
AllowedSchemes: []string{"https", "http"},
298300
})
299301

300302
_, err := v.SafeHostname(controller.runtime.AppURL)

‎internal/controller/proxy_controller_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -978,6 +978,7 @@ func TestProxyController(t *testing.T) {
978978
NewProxyController(ProxyControllerInput{
979979
Log: log,
980980
RuntimeConfig: &runtime,
981+
Config: &cfg,
981982
RouterGroup: group,
982983
ACLsService: aclsService,
983984
AuthService: authService,

‎internal/service/access_controls_service.go‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ package service
22

33
import (
44
"errors"
5+
"fmt"
6+
"net"
57
"strings"
8+
"unicode"
69

710
"github.com/tinyauthapp/tinyauth/internal/model"
811
"github.com/tinyauthapp/tinyauth/internal/utils/logger"
9-
"github.com/tinyauthapp/tinyauth/pkg/validators"
1012
"go.uber.org/dig"
1113
)
1214

@@ -37,25 +39,48 @@ func NewAccessControlsService(i AccessControlServiceInput) *AccessControlsServic
3739
}
3840
}
3941

42+
func (service *AccessControlsService) ensureAscii(str string) bool {
43+
for i := 0; i < len(str); i++ {
44+
if str[i] > unicode.MaxASCII {
45+
return false
46+
}
47+
}
48+
return true
49+
}
50+
51+
func (service *AccessControlsService) normalizeDomain(domain string) string {
52+
if host, _, err := net.SplitHostPort(domain); err == nil {
53+
domain = host
54+
}
55+
domain = strings.TrimRight(domain, ".")
56+
return strings.ToLower(domain)
57+
}
58+
4059
func (service *AccessControlsService) getACLs(domain string, lookup func(locator func(name string, app *model.App) bool) error) (*model.App, error) {
41-
v := validators.NewDomainValidator(validators.DomainValidatorOptions{})
60+
if !service.ensureAscii(domain) {
61+
return nil, errors.New("domain contains non-ascii characters")
62+
}
63+
64+
normalizedDomain := service.normalizeDomain(domain)
4265

4366
var domainMatch *model.App
4467
var nameMatch *model.App
4568
var nameMatchedApps []string
4669

4770
locatorFunc := func(name string, app *model.App) bool {
4871
if app.Config.Domain != "" {
49-
err := v.Validate(app.Config.Domain, domain)
50-
if err == nil {
72+
if !service.ensureAscii(app.Config.Domain) {
73+
service.log.App.Warn().Str("name", name).Str("domain", app.Config.Domain).Msg("Domain contains non-ascii characters, skipping")
74+
return false
75+
}
76+
if normalizedDomain == service.normalizeDomain(app.Config.Domain) {
5177
service.log.App.Debug().Str("name", name).Msg("Found matching container by domain")
5278
domainMatch = app
5379
return true
54-
} else if !errors.Is(err, validators.ErrHostnameMismatch) {
55-
service.log.App.Debug().Str("name", name).Err(err).Msg("Domain validation failed")
5680
}
81+
return false
5782
}
58-
if strings.HasPrefix(strings.ToLower(domain), strings.ToLower(name+".")) {
83+
if strings.HasPrefix(normalizedDomain, strings.ToLower(name+".")) {
5984
service.log.App.Debug().Str("name", name).Msg("Found matching container by app name")
6085
nameMatch = app
6186
nameMatchedApps = append(nameMatchedApps, name)
@@ -79,7 +104,7 @@ func (service *AccessControlsService) getACLs(domain string, lookup func(locator
79104
}
80105

81106
if len(nameMatchedApps) > 1 {
82-
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")
107+
return nil, fmt.Errorf("multiple apps matched domain by name, app names must be unique")
83108
}
84109

85110
service.log.App.Debug().Str("domain", domain).Msg("Found matching app by app name")

‎internal/service/access_controls_service_test.go‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55
"testing"
66

7+
"github.com/stretchr/testify/assert"
78
"github.com/stretchr/testify/require"
89
"github.com/tinyauthapp/tinyauth/internal/model"
910
"github.com/tinyauthapp/tinyauth/internal/utils/logger"
@@ -35,10 +36,11 @@ func TestAccessControlsService(t *testing.T) {
3536
log.Init()
3637

3738
tests := []struct {
38-
name string
39-
domain string
40-
acls map[string]model.App
41-
want *model.App
39+
name string
40+
domain string
41+
acls map[string]model.App
42+
want *model.App
43+
errorFunc func(t *testing.T, e error)
4244
}{
4345
{
4446
name: "returns ACLs for domain",
@@ -65,20 +67,11 @@ func TestAccessControlsService(t *testing.T) {
6567
want: &model.App{Config: model.AppConfig{Domain: "example.com"}},
6668
},
6769
{
68-
name: "returns ACLs for non-ascii domain",
70+
name: "returns error for non-ascii domain",
6971
domain: "bücher.example.com",
70-
acls: map[string]model.App{
71-
"foo": {Config: model.AppConfig{Domain: "bücher.example.com"}},
72-
},
73-
want: &model.App{Config: model.AppConfig{Domain: "bücher.example.com"}},
74-
},
75-
{
76-
name: "returns ACLs for punycode domain and non-ascii config",
77-
domain: "bücher.example.com",
78-
acls: map[string]model.App{
79-
"foo": {Config: model.AppConfig{Domain: "xn--bcher-kva.example.com"}},
72+
errorFunc: func(t *testing.T, e error) {
73+
assert.ErrorContains(t, e, "domain contains non-ascii characters")
8074
},
81-
want: &model.App{Config: model.AppConfig{Domain: "xn--bcher-kva.example.com"}},
8275
},
8376
{
8477
name: "returns ACLs with case-insensitive matching",
@@ -122,6 +115,10 @@ func TestAccessControlsService(t *testing.T) {
122115
LabelProvider: mock,
123116
})
124117
app, err := acls.getACLs(test.domain, mock.Lookup)
118+
if test.errorFunc != nil {
119+
test.errorFunc(t, err)
120+
return
121+
}
125122
require.NoError(t, err)
126123
require.Equal(t, test.want, app)
127124
})
@@ -137,6 +134,10 @@ func TestAccessControlsService(t *testing.T) {
137134
},
138135
})
139136
app, err := acls.lookupStaticACLs(test.domain)
137+
if test.errorFunc != nil {
138+
test.errorFunc(t, err)
139+
return
140+
}
140141
require.NoError(t, err)
141142
require.Equal(t, test.want, app)
142143
})

0 commit comments

Comments
 (0)