Skip to content

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