Skip to content

Commit

Permalink
sso_proxy: do not always run group validation during authentication
Browse files Browse the repository at this point in the history
  • Loading branch information
Jusshersmith committed Nov 12, 2019
1 parent a29ba90 commit 950db76
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 13 deletions.
2 changes: 1 addition & 1 deletion internal/auth/providers/okta.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func (p *OktaProvider) oktaRequest(method, endpoint string, params url.Values, t
if resp.StatusCode != http.StatusOK {
p.StatsdClient.Incr("provider.error", tags, 1.0)
logger.WithHTTPStatus(resp.StatusCode).WithEndpoint(stripToken(endpoint)).WithResponseBody(
respBody).Info()
respBody).Error("non-200 response returned from Okta")
switch resp.StatusCode {
case 400:
var response struct {
Expand Down
10 changes: 5 additions & 5 deletions internal/pkg/options/email_domain_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

var (
_ Validator = &EmailDomainValidator{}
_ Validator = EmailDomainValidator{}

// These error message should be formatted in such a way that is appropriate
// for display to the end user.
Expand All @@ -28,7 +28,7 @@ type EmailDomainValidator struct {
// - if the originally passed in list of domains consists only of "*", then all emails
// are considered valid based on their domain.
// If valid, nil is returned in place of an error.
func NewEmailDomainValidator(allowedDomains []string) *EmailDomainValidator {
func NewEmailDomainValidator(allowedDomains []string) EmailDomainValidator {
emailDomains := make([]string, 0, len(allowedDomains))

for _, domain := range allowedDomains {
Expand All @@ -39,12 +39,12 @@ func NewEmailDomainValidator(allowedDomains []string) *EmailDomainValidator {
emailDomains = append(emailDomains, emailDomain)
}
}
return &EmailDomainValidator{
return EmailDomainValidator{
AllowedDomains: emailDomains,
}
}

func (v *EmailDomainValidator) Validate(session *sessions.SessionState) error {
func (v EmailDomainValidator) Validate(session *sessions.SessionState) error {
if session.Email == "" {
return ErrInvalidEmailAddress
}
Expand All @@ -64,7 +64,7 @@ func (v *EmailDomainValidator) Validate(session *sessions.SessionState) error {
return nil
}

func (v *EmailDomainValidator) validate(session *sessions.SessionState) error {
func (v EmailDomainValidator) validate(session *sessions.SessionState) error {
email := strings.ToLower(session.Email)
for _, domain := range v.AllowedDomains {
if strings.HasSuffix(email, domain) {
Expand Down
24 changes: 17 additions & 7 deletions internal/proxy/oauthproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -781,13 +781,23 @@ func (p *OAuthProxy) Authenticate(rw http.ResponseWriter, req *http.Request) (er
}
}

errors := options.RunValidators(p.Validators, session)
if len(errors) == len(p.Validators) {
tags = append(tags, "error:validation_failed")
p.StatsdClient.Incr("application_error", tags, 1.0)
logger.WithRemoteAddress(remoteAddr).WithUser(session.Email).Info(
fmt.Sprintf("permission denied: unauthorized: %q", errors))
return ErrUserNotAuthorized
// We revalidate group membership whenever the session is refreshed or revalidated
// just above in the call to ValidateSessionState and RefreshSession.
// To reduce strain on upstream identity providers we only revalidate email domains and
// addresses on each request here.
for _, v := range p.Validators {
_, EmailGroupValidator := v.(options.EmailGroupValidator)

if !EmailGroupValidator {
err := v.Validate(session)
if err != nil {
tags = append(tags, "error:validation_failed")
p.StatsdClient.Incr("application_error", tags, 1.0)
logger.WithRemoteAddress(remoteAddr).WithUser(session.Email).Info(
fmt.Sprintf("permission denied: unauthorized: %q", err))
return ErrUserNotAuthorized
}
}
}

logger.WithRemoteAddress(remoteAddr).WithUser(session.Email).Info(
Expand Down

0 comments on commit 950db76

Please sign in to comment.