buzzfeed / sso

sso, aka S.S.Octopus, aka octoboi, is a single sign-on solution for securing internal services
MIT License
3.09k stars 186 forks source link

Relax the use of Email Domains/Addresses to enforce authentication #236

Open jphines opened 5 years ago

jphines commented 5 years ago

Problem

We currently mandate the use of a global email domain to authenticate both on the proxy side as well as the authenticator side. This is limiting for organizations and upstreams that have more diverse requirements that require a more flexible authenticator model.

We originally implemented this requirement as a safety precaution when SSO was less mature. It no longer provides the same assurances now that group usage is more robust and SSO itself has matured.

Solution

We propose to move adjust this configuration in two ways:

  1. Move the configuration in the proxy to the upstream configuration block.
  2. Remove the configuration and mechanism on the authenticator side. Instead, we will rely on the identity providers to provide this authentication mechanism.

Notes

The way email domains, addresses, and groups interact with one another is becoming increasing confusing. We should think about ways to help simplify the model and make it more intuitive.

codecov[bot] commented 5 years ago

Codecov Report

Merging #236 into master will decrease coverage by 0.13%. The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master     #236      +/-   ##
==========================================
- Coverage   62.25%   62.11%   -0.14%     
==========================================
  Files          50       50              
  Lines        4069     4036      -33     
==========================================
- Hits         2533     2507      -26     
- Misses       1349     1350       +1     
+ Partials      187      179       -8
Impacted Files Coverage Δ
internal/auth/options.go 77.5% <ø> (-1.08%) :arrow_down:
internal/auth/authenticator.go 86.04% <ø> (-0.35%) :arrow_down:
internal/auth/mux.go 78% <ø> (+3%) :arrow_up:
internal/proxy/options.go 83.33% <ø> (-0.27%) :arrow_down:
internal/auth/configuration.go 49.71% <ø> (-0.02%) :arrow_down:
internal/proxy/proxy.go 21.95% <0%> (-2.44%) :arrow_down:
internal/proxy/proxy_config.go 78.37% <100%> (+0.23%) :arrow_up:
internal/proxy/oauthproxy.go 50.73% <0%> (-0.25%) :arrow_down:
Jusshersmith commented 5 years ago

This is largely just thinking out loud at the moment (I have no strong feelings), but is it worth allowing these configs to have a default set for them as well for where upstream specific settings are not required, but the restriction is still wanted. (I suppose here: https://github.com/buzzfeed/sso/blob/55bf4486549af125ba616c42f6b2a3749962b66d/internal/proxy/options.go#L172).

I'm not entirely sure if that would be useful to many, or if it's just further solidifying something that isn't all that useful anymore. (especially thinking of your comments in the 'Notes' section about simplifying this stuff)