mirror of
https://github.com/tinyauthapp/tinyauth.git
synced 2026-08-30 01:53:31 +08:00
fix: don't use domain validator in acl matching logic
This commit is contained in:
@@ -294,7 +294,9 @@ func (controller *OAuthController) getCookieDomain() string {
|
|||||||
|
|
||||||
func (controller *OAuthController) isRedirectSafe(redirectURI string) bool {
|
func (controller *OAuthController) isRedirectSafe(redirectURI string) bool {
|
||||||
v := validators.NewDomainValidator(validators.DomainValidatorOptions{
|
v := validators.NewDomainValidator(validators.DomainValidatorOptions{
|
||||||
WithPort: true,
|
WithPort: true,
|
||||||
|
WithScheme: true,
|
||||||
|
AllowedSchemes: []string{"https", "http"},
|
||||||
})
|
})
|
||||||
|
|
||||||
_, err := v.SafeHostname(controller.runtime.AppURL)
|
_, err := v.SafeHostname(controller.runtime.AppURL)
|
||||||
|
|||||||
@@ -978,6 +978,7 @@ func TestProxyController(t *testing.T) {
|
|||||||
NewProxyController(ProxyControllerInput{
|
NewProxyController(ProxyControllerInput{
|
||||||
Log: log,
|
Log: log,
|
||||||
RuntimeConfig: &runtime,
|
RuntimeConfig: &runtime,
|
||||||
|
Config: &cfg,
|
||||||
RouterGroup: group,
|
RouterGroup: group,
|
||||||
ACLsService: aclsService,
|
ACLsService: aclsService,
|
||||||
AuthService: authService,
|
AuthService: authService,
|
||||||
|
|||||||
@@ -2,11 +2,13 @@ package service
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"errors"
|
"errors"
|
||||||
|
"fmt"
|
||||||
|
"net"
|
||||||
"strings"
|
"strings"
|
||||||
|
"unicode"
|
||||||
|
|
||||||
"github.com/tinyauthapp/tinyauth/internal/model"
|
"github.com/tinyauthapp/tinyauth/internal/model"
|
||||||
"github.com/tinyauthapp/tinyauth/internal/utils/logger"
|
"github.com/tinyauthapp/tinyauth/internal/utils/logger"
|
||||||
"github.com/tinyauthapp/tinyauth/pkg/validators"
|
|
||||||
"go.uber.org/dig"
|
"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) {
|
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 domainMatch *model.App
|
||||||
var nameMatch *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 {
|
locatorFunc := func(name string, app *model.App) bool {
|
||||||
if app.Config.Domain != "" {
|
if app.Config.Domain != "" {
|
||||||
err := v.Validate(app.Config.Domain, domain)
|
if !service.ensureAscii(app.Config.Domain) {
|
||||||
if err == nil {
|
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")
|
service.log.App.Debug().Str("name", name).Msg("Found matching container by domain")
|
||||||
domainMatch = app
|
domainMatch = app
|
||||||
return true
|
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")
|
service.log.App.Debug().Str("name", name).Msg("Found matching container by app name")
|
||||||
nameMatch = app
|
nameMatch = app
|
||||||
nameMatchedApps = append(nameMatchedApps, name)
|
nameMatchedApps = append(nameMatchedApps, name)
|
||||||
@@ -79,7 +104,7 @@ func (service *AccessControlsService) getACLs(domain string, lookup func(locator
|
|||||||
}
|
}
|
||||||
|
|
||||||
if len(nameMatchedApps) > 1 {
|
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")
|
service.log.App.Debug().Str("domain", domain).Msg("Found matching app by app name")
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
"github.com/tinyauthapp/tinyauth/internal/model"
|
"github.com/tinyauthapp/tinyauth/internal/model"
|
||||||
"github.com/tinyauthapp/tinyauth/internal/utils/logger"
|
"github.com/tinyauthapp/tinyauth/internal/utils/logger"
|
||||||
@@ -35,10 +36,11 @@ func TestAccessControlsService(t *testing.T) {
|
|||||||
log.Init()
|
log.Init()
|
||||||
|
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
domain string
|
domain string
|
||||||
acls map[string]model.App
|
acls map[string]model.App
|
||||||
want *model.App
|
want *model.App
|
||||||
|
errorFunc func(t *testing.T, e error)
|
||||||
}{
|
}{
|
||||||
{
|
{
|
||||||
name: "returns ACLs for domain",
|
name: "returns ACLs for domain",
|
||||||
@@ -65,20 +67,11 @@ func TestAccessControlsService(t *testing.T) {
|
|||||||
want: &model.App{Config: model.AppConfig{Domain: "example.com"}},
|
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",
|
domain: "bücher.example.com",
|
||||||
acls: map[string]model.App{
|
errorFunc: func(t *testing.T, e error) {
|
||||||
"foo": {Config: model.AppConfig{Domain: "bücher.example.com"}},
|
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",
|
name: "returns ACLs with case-insensitive matching",
|
||||||
@@ -122,6 +115,10 @@ func TestAccessControlsService(t *testing.T) {
|
|||||||
LabelProvider: mock,
|
LabelProvider: mock,
|
||||||
})
|
})
|
||||||
app, err := acls.getACLs(test.domain, mock.Lookup)
|
app, err := acls.getACLs(test.domain, mock.Lookup)
|
||||||
|
if test.errorFunc != nil {
|
||||||
|
test.errorFunc(t, err)
|
||||||
|
return
|
||||||
|
}
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
require.Equal(t, test.want, app)
|
require.Equal(t, test.want, app)
|
||||||
})
|
})
|
||||||
@@ -137,6 +134,10 @@ func TestAccessControlsService(t *testing.T) {
|
|||||||
},
|
},
|
||||||
})
|
})
|
||||||
app, err := acls.lookupStaticACLs(test.domain)
|
app, err := acls.lookupStaticACLs(test.domain)
|
||||||
|
if test.errorFunc != nil {
|
||||||
|
test.errorFunc(t, err)
|
||||||
|
return
|
||||||
|
}
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
require.Equal(t, test.want, app)
|
require.Equal(t, test.want, app)
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user