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_proxy: reduce amount of group validations #267

Merged
merged 1 commit into from
Nov 15, 2019

Conversation

Jusshersmith
Copy link
Contributor

@Jusshersmith Jusshersmith commented Nov 12, 2019

Problem

With the validator abstraction work that was recently done we inadvertently started to run group validations for each authenticated request. See 'Notes' section for specific details.

This increased volume of requests increases the potential to cause extra strain on upstream providers

Solution

We don't need to validate the groups again here. This pull request brings us closer to previous functionality where we re-validate group membership after refreshing or validating the session, and re-validate email domains and addresses upon each request.

Notes

  • Now that the group membership check is an official 'validator' within sso-proxy it's ran each time we call RunValidators().

    The problematic call in question:

    errors := options.RunValidators(p.Validators, session)

    Previously, we were only checking email address/domains here, with the group check being ran just above that when refreshing or validating the session:

    ok, err := p.provider.RefreshSession(session, allowedGroups)
    &
    ok := p.provider.ValidateSessionState(session, allowedGroups)

  • An alternative solution to validator pkg: control when each validator is ran #266

  • Also included in this is a change to the group validator (it's no longer used as a pointer). Largely to bring it in line with the other validators.

@Jusshersmith Jusshersmith added the bug Something isn't working label Nov 13, 2019
@Jusshersmith Jusshersmith merged commit 99e69f8 into master Nov 15, 2019
@Jusshersmith Jusshersmith deleted the jusshersmith-group-validator-bug branch November 15, 2019 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants