Closed Jusshersmith closed 4 years ago
Merging #253 into master will increase coverage by
0.1%
. The diff coverage is62.04%
.
@@ Coverage Diff @@
## master #253 +/- ##
=========================================
+ Coverage 62.04% 62.14% +0.1%
=========================================
Files 50 53 +3
Lines 4105 4169 +64
=========================================
+ Hits 2547 2591 +44
- Misses 1370 1391 +21
+ Partials 188 187 -1
Impacted Files | Coverage Δ | |
---|---|---|
internal/pkg/options/email_group_validator.go | 0% <0%> (ø) |
|
internal/pkg/options/mock_validator.go | 0% <0%> (ø) |
|
internal/proxy/proxy.go | 20.45% <0%> (-1.5%) |
:arrow_down: |
internal/pkg/options/validators.go | 0% <0%> (ø) |
|
internal/pkg/options/email_address_validator.go | 100% <100%> (ø) |
:arrow_up: |
internal/auth/options.go | 78.57% <100%> (ø) |
:arrow_up: |
internal/auth/authenticator.go | 86.18% <100%> (+0.37%) |
:arrow_up: |
internal/pkg/options/email_domain_validator.go | 100% <100%> (ø) |
:arrow_up: |
internal/proxy/oauthproxy.go | 54.1% <55.55%> (+3.98%) |
:arrow_up: |
internal/auth/mux.go | 75% <75%> (ø) |
:arrow_up: |
... and 4 more |
Some additional test files are also needed, i.e: internal/pkg/options/email_group_validator_test.go
Problem
The connection between
AllowedEmailDomains
,AllowedEmailAddresses
, andAllowedGroups
is unclear. If all specified, they don't work together well which causes awkward workarounds to be put into place.Solution
Introduce a more structured 'Validator' type within SSO, allowing us to create better and clearer dynamics between these settings (and any other validators that may be added in the future).
There are currently three separate validators:
SSO will now allow the request through providing at least one of these validators pass.
Notes
I would suggest looking at the following files together:
internal/pkg/options/validators.go
internal/pkg/options/email_address_validator.go
internal/pkg/options/email_domain_validator.go
internal/pkg/options/email_group_validator.go
internal/proxy/proxy.go
internal/proxy/oauthproxy.go
internal/auth/authenticator.go
internal/auth/mux.go
internal/auth/options.go
internal/pkg/options/mock_validator.go
internal/auth/authenticator_test.go
internal/proxy/oauthproxy_test.go
internal/pkg/options/email_address_validator_test.go
internal/pkg/options/email_domain_validator_test.go
This also changes the use of the
*
wildcard withinAllowedEmailDomains
andAllowedEmailAddresses
. The wildcard will be ignored if other domains/email addresses are also passed with it, choosing to follow the most restrictive path.On second thoughts..I think it may have been clearer to recreate the validator files from scratch!