From 787d7d89077887eb2fa21f8dfab4adf8f3bc4546 Mon Sep 17 00:00:00 2001 From: Stavros Date: Mon, 25 May 2026 18:37:30 +0300 Subject: [PATCH 1/2] fix: use policy engine in email whitelist check --- internal/bootstrap/service_bootstrap.go | 4 ++- internal/controller/proxy_controller_test.go | 3 +- internal/controller/user_controller_test.go | 5 ++- .../middleware/context_middleware_test.go | 5 ++- internal/model/runtime.go | 8 +++++ internal/service/auth_service.go | 33 +++++++++++++------ internal/service/policy_engine.go | 25 +++++++------- internal/service/policy_engine_test.go | 13 ++++---- internal/utils/security_utils.go | 7 +++- 9 files changed, 68 insertions(+), 35 deletions(-) diff --git a/internal/bootstrap/service_bootstrap.go b/internal/bootstrap/service_bootstrap.go index 7474ec27..a7bc48f9 100644 --- a/internal/bootstrap/service_bootstrap.go +++ b/internal/bootstrap/service_bootstrap.go @@ -39,10 +39,12 @@ func (app *BootstrapApp) setupServices() error { return fmt.Errorf("failed to initialize policy engine: %w", err) } + app.runtime.Policy = app.services.policyEngine.Policy() + oauthBrokerService := service.NewOAuthBrokerService(app.log, app.runtime.OAuthProviders, app.ctx) app.services.oauthBrokerService = oauthBrokerService - authService := service.NewAuthService(app.log, app.config, app.runtime, app.ctx, &app.wg, app.services.ldapService, app.queries, app.services.oauthBrokerService, app.services.tailscaleService) + authService := service.NewAuthService(app.log, app.config, app.runtime, app.ctx, &app.wg, app.services.ldapService, app.queries, app.services.oauthBrokerService, app.services.tailscaleService, app.services.policyEngine) app.services.authService = authService oidcService, err := service.NewOIDCService(app.log, app.config, app.runtime, app.queries, app.ctx, &app.wg) diff --git a/internal/controller/proxy_controller_test.go b/internal/controller/proxy_controller_test.go index ef004f8f..cc2226a1 100644 --- a/internal/controller/proxy_controller_test.go +++ b/internal/controller/proxy_controller_test.go @@ -357,7 +357,6 @@ func TestProxyController(t *testing.T) { ctx := context.TODO() broker := service.NewOAuthBrokerService(log, map[string]model.OAuthServiceConfig{}, ctx) - authService := service.NewAuthService(log, cfg, runtime, ctx, wg, nil, store, broker, nil) aclsService := service.NewAccessControlsService(log, cfg, nil) policyEngine, err := service.NewPolicyEngine(cfg, log) @@ -383,6 +382,8 @@ func TestProxyController(t *testing.T) { Log: log, }) + authService := service.NewAuthService(log, cfg, runtime, ctx, wg, nil, store, broker, nil, policyEngine) + for _, test := range tests { t.Run(test.description, func(t *testing.T) { router := gin.Default() diff --git a/internal/controller/user_controller_test.go b/internal/controller/user_controller_test.go index 39b343c0..7527d752 100644 --- a/internal/controller/user_controller_test.go +++ b/internal/controller/user_controller_test.go @@ -414,8 +414,11 @@ func TestUserController(t *testing.T) { ctx := context.TODO() wg := &sync.WaitGroup{} + policyEngine, err := service.NewPolicyEngine(cfg, log) + require.NoError(t, err) + broker := service.NewOAuthBrokerService(log, map[string]model.OAuthServiceConfig{}, ctx) - authService := service.NewAuthService(log, cfg, runtime, ctx, wg, nil, store, broker, nil) + authService := service.NewAuthService(log, cfg, runtime, ctx, wg, nil, store, broker, nil, policyEngine) beforeEach := func() { // Clear failed login attempts before each test diff --git a/internal/middleware/context_middleware_test.go b/internal/middleware/context_middleware_test.go index b672684f..b31231fa 100644 --- a/internal/middleware/context_middleware_test.go +++ b/internal/middleware/context_middleware_test.go @@ -254,8 +254,11 @@ func TestContextMiddleware(t *testing.T) { store := memory.New() + policyEngine, err := service.NewPolicyEngine(cfg, log) + require.NoError(t, err) + broker := service.NewOAuthBrokerService(log, map[string]model.OAuthServiceConfig{}, ctx) - authService := service.NewAuthService(log, cfg, runtime, ctx, wg, nil, store, broker, nil) + authService := service.NewAuthService(log, cfg, runtime, ctx, wg, nil, store, broker, nil, policyEngine) contextMiddleware := middleware.NewContextMiddleware(log, runtime, authService, broker, nil) diff --git a/internal/model/runtime.go b/internal/model/runtime.go index 9df20b85..6e4047fd 100644 --- a/internal/model/runtime.go +++ b/internal/model/runtime.go @@ -14,6 +14,7 @@ type RuntimeConfig struct { ConfiguredProviders []Provider OIDCClients []OIDCClientConfig TrustedDomains []string + Policy Policy } type Provider struct { @@ -21,3 +22,10 @@ type Provider struct { ID string `json:"id"` OAuth bool `json:"oauth"` } + +type Policy string + +const ( + PolicyAllow Policy = "allow" + PolicyDeny Policy = "deny" +) diff --git a/internal/service/auth_service.go b/internal/service/auth_service.go index 76fdafbd..2cb647b3 100644 --- a/internal/service/auth_service.go +++ b/internal/service/auth_service.go @@ -75,10 +75,11 @@ type AuthService struct { runtime model.RuntimeConfig context context.Context - ldap *LdapService - queries repository.Store - oauthBroker *OAuthBrokerService - tailscale *TailscaleService + ldap *LdapService + queries repository.Store + oauthBroker *OAuthBrokerService + tailscale *TailscaleService + policyEngine *PolicyEngine loginAttempts map[string]*LoginAttempt ldapGroupsCache map[string]*LdapGroupsCache @@ -101,6 +102,7 @@ func NewAuthService( queries repository.Store, oauthBroker *OAuthBrokerService, tailscale *TailscaleService, + policy *PolicyEngine, ) *AuthService { service := &AuthService{ log: log, @@ -114,6 +116,7 @@ func NewAuthService( queries: queries, oauthBroker: oauthBroker, tailscale: tailscale, + policyEngine: policy, } wg.Go(service.CleanupOAuthSessionsRoutine) @@ -285,13 +288,23 @@ func (auth *AuthService) RecordLoginAttempt(identifier string, success bool) { } } +// We could also directly access the policyEngine.effectToAccess but +// I believe it's better to use the exported functions instead func (auth *AuthService) IsEmailWhitelisted(email string) bool { - match, err := utils.CheckFilter(strings.Join(auth.runtime.OAuthWhitelist, ","), email) - if err != nil { - auth.log.App.Warn().Err(err).Str("email", email).Msg("Invalid email filter pattern") - return false - } - return match + return auth.policyEngine.EvaluateFunc(func() Effect { + match, err := utils.CheckFilter(strings.Join(auth.runtime.OAuthWhitelist, ","), email) + if err != nil { + if err == utils.ErrFilterEmpty { + return EffectAbstain + } + auth.log.App.Error().Err(err).Str("email", email).Msg("Failed to evaluate email whitelist filter, defaulting to deny") + return EffectDeny + } + if match { + return EffectAllow + } + return EffectDeny + }) } func (auth *AuthService) CreateSession(ctx context.Context, data repository.Session) (*http.Cookie, error) { diff --git a/internal/service/policy_engine.go b/internal/service/policy_engine.go index 4250d8a0..159cc630 100644 --- a/internal/service/policy_engine.go +++ b/internal/service/policy_engine.go @@ -8,13 +8,6 @@ import ( "github.com/tinyauthapp/tinyauth/internal/utils/logger" ) -type Policy string - -const ( - PolicyAllow Policy = "allow" - PolicyDeny Policy = "deny" -) - type Effect int const ( @@ -37,7 +30,7 @@ type ACLContext struct { type PolicyEngine struct { log *logger.Logger rules map[RuleName]Rule - policy Policy + policy model.Policy } func NewPolicyEngine(config model.Config, log *logger.Logger) (*PolicyEngine, error) { @@ -47,12 +40,12 @@ func NewPolicyEngine(config model.Config, log *logger.Logger) (*PolicyEngine, er } switch config.Auth.ACLs.Policy { - case string(PolicyAllow): + case string(model.PolicyAllow): log.App.Debug().Msg("Using 'allow' ACL policy: access to apps will be allowed by default unless explicitly blocked") - engine.policy = PolicyAllow - case string(PolicyDeny): + engine.policy = model.PolicyAllow + case string(model.PolicyDeny): log.App.Debug().Msg("Using 'deny' ACL policy: access to apps will be blocked by default unless explicitly allowed") - engine.policy = PolicyDeny + engine.policy = model.PolicyDeny default: return nil, fmt.Errorf("invalid acl policy: %s", config.Auth.ACLs.Policy) } @@ -84,7 +77,7 @@ func (engine *PolicyEngine) effectToAccess(effect Effect) bool { return false default: // If the effect is abstain, we fall back to the default policy - return engine.policy == PolicyAllow + return engine.policy == model.PolicyAllow } } @@ -101,10 +94,14 @@ func (engine *PolicyEngine) Evaluate(name RuleName, ctx *ACLContext) bool { return access } -func (engine *PolicyEngine) Policy() Policy { +func (engine *PolicyEngine) Policy() model.Policy { return engine.policy } func (engine *PolicyEngine) Rules() map[RuleName]Rule { return engine.rules } + +func (engine *PolicyEngine) EvaluateFunc(f func() Effect) bool { + return engine.effectToAccess(f()) +} diff --git a/internal/service/policy_engine_test.go b/internal/service/policy_engine_test.go index d1ef4796..3142c0e3 100644 --- a/internal/service/policy_engine_test.go +++ b/internal/service/policy_engine_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/tinyauthapp/tinyauth/internal/model" "github.com/tinyauthapp/tinyauth/internal/service" "github.com/tinyauthapp/tinyauth/internal/test" "github.com/tinyauthapp/tinyauth/internal/utils/logger" @@ -37,16 +38,16 @@ func TestPolicyEngine(t *testing.T) { assert.Error(t, err) // Engine should initialize with 'allow' policy - cfg.Auth.ACLs.Policy = string(service.PolicyAllow) + cfg.Auth.ACLs.Policy = string(model.PolicyAllow) engine, err := service.NewPolicyEngine(cfg, log) assert.NoError(t, err) - assert.Equal(t, service.PolicyAllow, engine.Policy()) + assert.Equal(t, model.PolicyAllow, engine.Policy()) // Engine should initialize with 'deny' policy - cfg.Auth.ACLs.Policy = string(service.PolicyDeny) + cfg.Auth.ACLs.Policy = string(model.PolicyDeny) engine, err = service.NewPolicyEngine(cfg, log) assert.NoError(t, err) - assert.Equal(t, service.PolicyDeny, engine.Policy()) + assert.Equal(t, model.PolicyDeny, engine.Policy()) // Engine should allow adding rules engine, err = service.NewPolicyEngine(cfg, log) @@ -56,7 +57,7 @@ func TestPolicyEngine(t *testing.T) { assert.True(t, ok) // Begin allow policy tests - cfg.Auth.ACLs.Policy = string(service.PolicyAllow) + cfg.Auth.ACLs.Policy = string(model.PolicyAllow) engine, err = service.NewPolicyEngine(cfg, log) assert.NoError(t, err) engine.RegisterRule("test-rule", testRule) @@ -74,7 +75,7 @@ func TestPolicyEngine(t *testing.T) { assert.Equal(t, true, engine.Evaluate("test-rule", ctx)) // Begin deny policy tests - cfg.Auth.ACLs.Policy = string(service.PolicyDeny) + cfg.Auth.ACLs.Policy = string(model.PolicyDeny) engine, err = service.NewPolicyEngine(cfg, log) assert.NoError(t, err) engine.RegisterRule("test-rule", testRule) diff --git a/internal/utils/security_utils.go b/internal/utils/security_utils.go index 8e8dd23b..71b59d41 100644 --- a/internal/utils/security_utils.go +++ b/internal/utils/security_utils.go @@ -3,6 +3,7 @@ package utils import ( "crypto/rand" "encoding/base64" + "errors" "fmt" "net" "regexp" @@ -11,6 +12,10 @@ import ( "github.com/google/uuid" ) +var ( + ErrFilterEmpty = errors.New("filter is empty") +) + func GetSecret(conf string, file string) string { if conf == "" && file == "" { return "" @@ -78,7 +83,7 @@ func CheckIPFilter(filter string, ip string) (bool, error) { func CheckFilter(filter string, input string) (bool, error) { if len(strings.TrimSpace(filter)) == 0 { - return false, fmt.Errorf("filter is empty") + return false, ErrFilterEmpty } if strings.HasPrefix(filter, "/") && strings.HasSuffix(filter, "/") { From a40c08f363bde5f1c4b82a63704b245d0ed79bc5 Mon Sep 17 00:00:00 2001 From: Stavros Date: Mon, 25 May 2026 18:41:11 +0300 Subject: [PATCH 2/2] chore: no need to store policy in runtime config --- internal/bootstrap/service_bootstrap.go | 2 -- internal/model/runtime.go | 8 -------- internal/service/policy_engine.go | 21 ++++++++++++++------- internal/service/policy_engine_test.go | 13 ++++++------- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/internal/bootstrap/service_bootstrap.go b/internal/bootstrap/service_bootstrap.go index a7bc48f9..151cdeb8 100644 --- a/internal/bootstrap/service_bootstrap.go +++ b/internal/bootstrap/service_bootstrap.go @@ -39,8 +39,6 @@ func (app *BootstrapApp) setupServices() error { return fmt.Errorf("failed to initialize policy engine: %w", err) } - app.runtime.Policy = app.services.policyEngine.Policy() - oauthBrokerService := service.NewOAuthBrokerService(app.log, app.runtime.OAuthProviders, app.ctx) app.services.oauthBrokerService = oauthBrokerService diff --git a/internal/model/runtime.go b/internal/model/runtime.go index 6e4047fd..9df20b85 100644 --- a/internal/model/runtime.go +++ b/internal/model/runtime.go @@ -14,7 +14,6 @@ type RuntimeConfig struct { ConfiguredProviders []Provider OIDCClients []OIDCClientConfig TrustedDomains []string - Policy Policy } type Provider struct { @@ -22,10 +21,3 @@ type Provider struct { ID string `json:"id"` OAuth bool `json:"oauth"` } - -type Policy string - -const ( - PolicyAllow Policy = "allow" - PolicyDeny Policy = "deny" -) diff --git a/internal/service/policy_engine.go b/internal/service/policy_engine.go index 159cc630..7f301da6 100644 --- a/internal/service/policy_engine.go +++ b/internal/service/policy_engine.go @@ -8,6 +8,13 @@ import ( "github.com/tinyauthapp/tinyauth/internal/utils/logger" ) +type Policy string + +const ( + PolicyAllow Policy = "allow" + PolicyDeny Policy = "deny" +) + type Effect int const ( @@ -30,7 +37,7 @@ type ACLContext struct { type PolicyEngine struct { log *logger.Logger rules map[RuleName]Rule - policy model.Policy + policy Policy } func NewPolicyEngine(config model.Config, log *logger.Logger) (*PolicyEngine, error) { @@ -40,12 +47,12 @@ func NewPolicyEngine(config model.Config, log *logger.Logger) (*PolicyEngine, er } switch config.Auth.ACLs.Policy { - case string(model.PolicyAllow): + case string(PolicyAllow): log.App.Debug().Msg("Using 'allow' ACL policy: access to apps will be allowed by default unless explicitly blocked") - engine.policy = model.PolicyAllow - case string(model.PolicyDeny): + engine.policy = PolicyAllow + case string(PolicyDeny): log.App.Debug().Msg("Using 'deny' ACL policy: access to apps will be blocked by default unless explicitly allowed") - engine.policy = model.PolicyDeny + engine.policy = PolicyDeny default: return nil, fmt.Errorf("invalid acl policy: %s", config.Auth.ACLs.Policy) } @@ -77,7 +84,7 @@ func (engine *PolicyEngine) effectToAccess(effect Effect) bool { return false default: // If the effect is abstain, we fall back to the default policy - return engine.policy == model.PolicyAllow + return engine.policy == PolicyAllow } } @@ -94,7 +101,7 @@ func (engine *PolicyEngine) Evaluate(name RuleName, ctx *ACLContext) bool { return access } -func (engine *PolicyEngine) Policy() model.Policy { +func (engine *PolicyEngine) Policy() Policy { return engine.policy } diff --git a/internal/service/policy_engine_test.go b/internal/service/policy_engine_test.go index 3142c0e3..d1ef4796 100644 --- a/internal/service/policy_engine_test.go +++ b/internal/service/policy_engine_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/tinyauthapp/tinyauth/internal/model" "github.com/tinyauthapp/tinyauth/internal/service" "github.com/tinyauthapp/tinyauth/internal/test" "github.com/tinyauthapp/tinyauth/internal/utils/logger" @@ -38,16 +37,16 @@ func TestPolicyEngine(t *testing.T) { assert.Error(t, err) // Engine should initialize with 'allow' policy - cfg.Auth.ACLs.Policy = string(model.PolicyAllow) + cfg.Auth.ACLs.Policy = string(service.PolicyAllow) engine, err := service.NewPolicyEngine(cfg, log) assert.NoError(t, err) - assert.Equal(t, model.PolicyAllow, engine.Policy()) + assert.Equal(t, service.PolicyAllow, engine.Policy()) // Engine should initialize with 'deny' policy - cfg.Auth.ACLs.Policy = string(model.PolicyDeny) + cfg.Auth.ACLs.Policy = string(service.PolicyDeny) engine, err = service.NewPolicyEngine(cfg, log) assert.NoError(t, err) - assert.Equal(t, model.PolicyDeny, engine.Policy()) + assert.Equal(t, service.PolicyDeny, engine.Policy()) // Engine should allow adding rules engine, err = service.NewPolicyEngine(cfg, log) @@ -57,7 +56,7 @@ func TestPolicyEngine(t *testing.T) { assert.True(t, ok) // Begin allow policy tests - cfg.Auth.ACLs.Policy = string(model.PolicyAllow) + cfg.Auth.ACLs.Policy = string(service.PolicyAllow) engine, err = service.NewPolicyEngine(cfg, log) assert.NoError(t, err) engine.RegisterRule("test-rule", testRule) @@ -75,7 +74,7 @@ func TestPolicyEngine(t *testing.T) { assert.Equal(t, true, engine.Evaluate("test-rule", ctx)) // Begin deny policy tests - cfg.Auth.ACLs.Policy = string(model.PolicyDeny) + cfg.Auth.ACLs.Policy = string(service.PolicyDeny) engine, err = service.NewPolicyEngine(cfg, log) assert.NoError(t, err) engine.RegisterRule("test-rule", testRule)