Inconsistent validation logic between init and refresh

[griffinmyers]

Describe the bug

This proxy supports a variety of mechanisms to authorize access to a host:

https://github.com/buzzfeed/sso/blob/290f27ea84bcf1c83a06a5908ee8afd1d15cf7ab/internal/proxy/proxy.go#L48-L59

When responding to the Oauth callback (that is, when a user first authenticates and initiates a session), the proxy requires any of its validators be satisfied before proceeding:

https://github.com/buzzfeed/sso/blob/290f27ea84bcf1c83a06a5908ee8afd1d15cf7ab/internal/proxy/oauthproxy.go#L481-L482

However, when the proxy is refreshing a session sometime later, it requires that a user satisfy "groups" validation, exclusively:

https://github.com/buzzfeed/sso/blob/290f27ea84bcf1c83a06a5908ee8afd1d15cf7ab/internal/proxy/oauthproxy.go#L660-L663

https://github.com/buzzfeed/sso/blob/290f27ea84bcf1c83a06a5908ee8afd1d15cf7ab/internal/proxy/providers/sso.go#L274-L289

https://github.com/buzzfeed/sso/blob/290f27ea84bcf1c83a06a5908ee8afd1d15cf7ab/internal/proxy/providers/sso.go#L188-L201

I might be reading things wrong, but I have produced this with a live instance, and it seems like strange behavior to me.

The consequence of this is a user who satisfies email domain validation but fails group validation will be able to authenticate and briefly see the service, until the refresh period expires (or validation period, for that matter), where they'll be logged out.

To Reproduce

upstream_configs.yml

Create a group named nobody@corp.com with 0 people (or as close to it as the provider allows). Register a backend that uses this group in the allowed_groups field:

- service: my-service
  default:
    from: my-service.int.corp.com
    to: http://my-service.corp-internal.com
    options:
      allowed_groups:
        # an empty google group
        - nobody@corp.com

run the service, ensuring that DEFAULT_ALLOWED_EMAIL_DOMAINS=corp.com. Then, as a user not in nobody@corp.com, attempt to access my-service.int.corp.com. You will succeed, but be logged out as soon as the session refreshes.

Expected behavior

The user should not be logged out when the session is refreshed.

[Jusshersmith]

Hey @griffinmyers,

Thanks for sending in this issue, and with the level of detail you've included.

Just wanted to acknowledge it -- I'm going to try to find some time (realistically) next week to look into this.

[Jusshersmith]

Just to check in here -- I'm still working through this. Finding time has been difficult, but I'm getting there!