Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sso_*: allow group validator to be used standalone #264

Merged
merged 1 commit into from
Oct 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions internal/pkg/options/email_group_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions internal/proxy/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/proxy/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
3 changes: 0 additions & 3 deletions internal/proxy/providers/sso.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions internal/proxy/providers/sso_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -311,15 +311,15 @@ 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",
},
ProviderResponse: http.StatusOK,
Groups: []string{},
ProxyGroupIds: []string{},
ExpectedValid: true,
ExpectedValid: false,
},
{
Name: "invalid when response is is not 200",
Expand Down
4 changes: 3 additions & 1 deletion internal/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down