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: ability to define allowed email address/domain in upstream config #247

Merged
merged 2 commits into from
Sep 6, 2019

Conversation

Jusshersmith
Copy link
Contributor

@Jusshersmith Jusshersmith commented Aug 23, 2019

Problem

Allowed email addresses and domains can only be specified for SSO as a whole, setting the same restriction on all upstreams. It cannot be set on individual upstreams.

Solution

Allow these variables to be defined in two separate places:

  • in the environment (in the standard 'options' config), acting as a default setting for all upstreams
  • within individual upstream configs, overriding the 'default' setting

We still expect one of these to be set and will error if neither are.

Notes

Includes small bug fix to both the email address validator and email domain validator packages to avoid mutating the passed in slice, instead opting to create and append to a new one.

@Jusshersmith Jusshersmith force-pushed the jusshersmith-allowed-email-domain branch from 8d51b60 to ec433e9 Compare September 4, 2019 14:02
@codecov
Copy link

codecov bot commented Sep 4, 2019

Codecov Report

Merging #247 into master will increase coverage by 0.08%.
The diff coverage is 88.23%.

@@            Coverage Diff             @@
##           master     #247      +/-   ##
==========================================
+ Coverage   62.28%   62.37%   +0.08%     
==========================================
  Files          50       50              
  Lines        4078     4093      +15     
==========================================
+ Hits         2540     2553      +13     
- Misses       1350     1355       +5     
+ Partials      188      185       -3
Impacted Files Coverage Δ
internal/proxy/proxy.go 21.95% <0%> (-2.44%) ⬇️
internal/pkg/options/email_address_validator.go 100% <100%> (ø) ⬆️
internal/pkg/options/email_domain_validator.go 100% <100%> (ø) ⬆️
internal/proxy/options.go 84.67% <100%> (+1.07%) ⬆️
internal/proxy/proxy_config.go 78.49% <100%> (+0.23%) ⬆️
internal/proxy/oauthproxy.go 50.48% <0%> (-0.25%) ⬇️

@Jusshersmith Jusshersmith force-pushed the jusshersmith-allowed-email-domain branch 2 times, most recently from e712880 to c55aa73 Compare September 4, 2019 14:23
@Jusshersmith Jusshersmith marked this pull request as ready for review September 4, 2019 14:25
jphines
jphines previously approved these changes Sep 4, 2019
jphines
jphines previously approved these changes Sep 4, 2019
@Jusshersmith Jusshersmith force-pushed the jusshersmith-allowed-email-domain branch 2 times, most recently from 7d93f21 to c8872cd Compare September 4, 2019 21:34
@Jusshersmith Jusshersmith force-pushed the jusshersmith-allowed-email-domain branch from c8872cd to c1af8e0 Compare September 4, 2019 21:38
@Jusshersmith Jusshersmith force-pushed the jusshersmith-allowed-email-domain branch from c1af8e0 to 4a1ae62 Compare September 4, 2019 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants