diff --git a/internal/controller/oauth_controller.go b/internal/controller/oauth_controller.go index 1cbc520b..5ef010c9 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, + }) + + _, 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 - } - - nu := strings.TrimSuffix(u.Hostname(), ".") - nau := strings.TrimSuffix(au.Hostname(), ".") - - if strings.EqualFold(nu, nau) { + if err == nil { return true } + 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" { + 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 07233faa..6a04ec88 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 f4f4d24c..24855a8e 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 28b08985..e9cac783 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) } }