diff --git a/internal/pkg/options/email_group_validator.go b/internal/pkg/options/email_group_validator.go index 36b1d988..14d1c6dd 100644 --- a/internal/pkg/options/email_group_validator.go +++ b/internal/pkg/options/email_group_validator.go @@ -22,8 +22,6 @@ type EmailGroupValidator struct { // NewEmailGroupValidator takes in a Provider object and a list of groups, and returns a Validator object. // The validator can be used to validate that the session.Email: -// - if an empty list is passed in in place of a list of groups, all session.Emails will be considered valid -// regardless of group membership with that particular Provider. // - according to the Provider that was passed in, is a member of one of the originally passed in groups. // If valid, nil is returned in place of an error. func NewEmailGroupValidator(provider providers.Provider, allowedGroups []string) EmailGroupValidator { @@ -34,10 +32,6 @@ func NewEmailGroupValidator(provider providers.Provider, allowedGroups []string) } func (v EmailGroupValidator) Validate(session *sessions.SessionState) error { - if len(v.AllowedGroups) == 0 { - return nil - } - err := v.validate(session) if err != nil { return err diff --git a/internal/proxy/options.go b/internal/proxy/options.go index 14a2a952..06da8533 100644 --- a/internal/proxy/options.go +++ b/internal/proxy/options.go @@ -199,13 +199,13 @@ func (o *Options) Validate() error { o.TCPWriteTimeout = uc.Timeout } - if len(uc.AllowedEmailDomains) == 0 && len(uc.AllowedEmailAddresses) == 0 { + if len(uc.AllowedEmailDomains) == 0 && len(uc.AllowedEmailAddresses) == 0 && len(uc.AllowedGroups) == 0 { invalidUpstreams = append(invalidUpstreams, uc.Service) } } if len(invalidUpstreams) != 0 { msgs = append(msgs, fmt.Sprintf( - "missing setting: DEFAULT_ALLOWED_EMAIL_DOMAINS or DEFAULT_ALLOWED_EMAIL_ADDRESSES in either environment or upstream config in the following upstreams: %v", + "missing setting: ALLOWED_EMAIL_DOMAINS, ALLOWED_EMAIL_ADDRESSES, ALLOWED_GROUPS default in environment or override in upstream config in the following upstreams: %v", invalidUpstreams)) } } diff --git a/internal/proxy/options_test.go b/internal/proxy/options_test.go index 246b1783..e2fdc2f1 100644 --- a/internal/proxy/options_test.go +++ b/internal/proxy/options_test.go @@ -59,7 +59,7 @@ func TestNewOptions(t *testing.T) { "missing setting: client-secret", "missing setting: statsd-host", "missing setting: statsd-port", - "missing setting: DEFAULT_ALLOWED_EMAIL_DOMAINS or DEFAULT_ALLOWED_EMAIL_ADDRESSES in either environment or upstream config in the following upstreams: [testService]", + "missing setting: ALLOWED_EMAIL_DOMAINS, ALLOWED_EMAIL_ADDRESSES, ALLOWED_GROUPS default in environment or override in upstream config in the following upstreams: [testService]", "Invalid value for COOKIE_SECRET; must decode to 32 or 64 bytes, but decoded to 0 bytes", }) testutil.Equal(t, expected, err.Error()) diff --git a/internal/proxy/providers/sso.go b/internal/proxy/providers/sso.go index 08a496a6..1aca089c 100644 --- a/internal/proxy/providers/sso.go +++ b/internal/proxy/providers/sso.go @@ -181,9 +181,6 @@ func (p *SSOProvider) ValidateGroup(email string, allowedGroups []string, access logger.WithUser(email).WithAllowedGroups(allowedGroups).Info("validating groups") inGroups := []string{} - if len(allowedGroups) == 0 { - return inGroups, true, nil - } userGroups, err := p.UserGroups(email, allowedGroups, accessToken) if err != nil { diff --git a/internal/proxy/providers/sso_test.go b/internal/proxy/providers/sso_test.go index 237cd540..1b9a2244 100644 --- a/internal/proxy/providers/sso_test.go +++ b/internal/proxy/providers/sso_test.go @@ -144,11 +144,11 @@ func TestSSOProviderGroups(t *testing.T) { ProfileStatus int }{ { - Name: "valid when no group id set", + Name: "invalid when no group id set", Email: "michael.bland@gsa.gov", Groups: []string{}, ProxyGroupIds: []string{}, - ExpectedValid: true, + ExpectedValid: false, ExpectedInGroups: []string{}, ExpectError: nil, }, @@ -311,7 +311,7 @@ func TestSSOProviderValidateSessionState(t *testing.T) { ExpectedValid bool }{ { - Name: "valid when no group id set", + Name: "invalid when no group id set", SessionState: &sessions.SessionState{ AccessToken: "abc", Email: "michael.bland@gsa.gov", @@ -319,7 +319,7 @@ func TestSSOProviderValidateSessionState(t *testing.T) { ProviderResponse: http.StatusOK, Groups: []string{}, ProxyGroupIds: []string{}, - ExpectedValid: true, + ExpectedValid: false, }, { Name: "invalid when response is is not 200", diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 7aa2ee49..5cc5c1c0 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -47,7 +47,9 @@ func New(opts *Options) (*SSOProxy, error) { validators = append(validators, options.NewEmailDomainValidator(upstreamConfig.AllowedEmailDomains)) } - validators = append(validators, options.NewEmailGroupValidator(provider, upstreamConfig.AllowedGroups)) + if len(upstreamConfig.AllowedGroups) != 0 { + validators = append(validators, options.NewEmailGroupValidator(provider, upstreamConfig.AllowedGroups)) + } optFuncs = append(optFuncs, SetProvider(provider),