From add45c360ceeb6b12702f4f95dc447df72653393 Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Sat, 26 Sep 2020 13:19:08 -0700 Subject: [PATCH 1/8] Split session enrichment from code redemption --- oauthproxy.go | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index e64ffe91f6..653701d97f 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -357,22 +357,24 @@ func (p *OAuthProxy) redeemCode(ctx context.Context, host, code string) (*sessio if err != nil { return nil, err } + return s, nil +} +func (p *OAuthProxy) enrichSession(ctx context.Context, s *sessionsapi.SessionState) error { + var err error if s.Email == "" { s.Email, err = p.provider.GetEmailAddress(ctx, s) if err != nil && err.Error() != "not implemented" { - return nil, err + return err } } - if s.User == "" { s.User, err = p.provider.GetUserName(ctx, s) if err != nil && err.Error() != "not implemented" { - return nil, err + return err } } - - return s, nil + return nil } // MakeCSRFCookie creates a cookie for CSRF @@ -829,14 +831,21 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { return } - s := strings.SplitN(req.Form.Get("state"), ":", 2) - if len(s) != 2 { + err = p.enrichSession(req.Context(), session) + if err != nil { + logger.Errorf("Error creating session during OAuth2 callback: %v", err) + p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", "Internal Error") + return + } + + state := strings.SplitN(req.Form.Get("state"), ":", 2) + if len(state) != 2 { logger.Error("Error while parsing OAuth2 state: invalid length") p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", "Invalid State") return } - nonce := s[0] - redirect := s[1] + nonce := state[0] + redirect := state[1] c, err := req.Cookie(p.CSRFCookieName) if err != nil { logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: unable to obtain CSRF cookie") From 0bd8eb31910bb09a765eadae84bb103b744bda77 Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Sat, 26 Sep 2020 14:06:52 -0700 Subject: [PATCH 2/8] Setup provider.ErrNotImplemented sentinel error --- oauthproxy.go | 4 ++-- providers/provider_default.go | 14 ++++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index 653701d97f..9f80f643ab 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -364,13 +364,13 @@ func (p *OAuthProxy) enrichSession(ctx context.Context, s *sessionsapi.SessionSt var err error if s.Email == "" { s.Email, err = p.provider.GetEmailAddress(ctx, s) - if err != nil && err.Error() != "not implemented" { + if err != nil && !errors.Is(err, providers.ErrNotImplemented) { return err } } if s.User == "" { s.User, err = p.provider.GetUserName(ctx, s) - if err != nil && err.Error() != "not implemented" { + if err != nil && !errors.Is(err, providers.ErrNotImplemented) { return err } } diff --git a/providers/provider_default.go b/providers/provider_default.go index 8a5a98acc8..4e96f0e014 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -14,7 +14,13 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" ) -var _ Provider = (*ProviderData)(nil) +var ( + // ErrNotImplemented is returned when a provider did not override a default + // implementation method that doesn't have sensible defaults + ErrNotImplemented = errors.New("not implemented") + + _ Provider = (*ProviderData)(nil) +) // Redeem provides a default implementation of the OAuth2 token redemption process func (p *ProviderData) Redeem(ctx context.Context, redirectURL, code string) (s *sessions.SessionState, err error) { @@ -82,12 +88,12 @@ func (p *ProviderData) GetLoginURL(redirectURI, state string) string { // GetEmailAddress returns the Account email address func (p *ProviderData) GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) { - return "", errors.New("not implemented") + return "", ErrNotImplemented } // GetUserName returns the Account username func (p *ProviderData) GetUserName(ctx context.Context, s *sessions.SessionState) (string, error) { - return "", errors.New("not implemented") + return "", ErrNotImplemented } // ValidateGroup validates that the provided email exists in the configured provider @@ -110,5 +116,5 @@ func (p *ProviderData) RefreshSessionIfNeeded(ctx context.Context, s *sessions.S // CreateSessionStateFromBearerToken should be implemented to allow providers // to convert ID tokens into sessions func (p *ProviderData) CreateSessionStateFromBearerToken(ctx context.Context, rawIDToken string, idToken *oidc.IDToken) (*sessions.SessionState, error) { - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } From b6061f08036fee813556ad4981a5e2ed0d03409d Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Sat, 26 Sep 2020 15:17:26 -0700 Subject: [PATCH 3/8] Adds tests for redeemCode and enrichSession --- oauthproxy_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index d0fd9481a7..9f60439bb8 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -404,6 +404,78 @@ func (tp *TestProvider) ValidateSessionState(ctx context.Context, session *sessi return tp.ValidToken } +func Test_redeemCode(t *testing.T) { + opts := baseTestOptions() + err := validation.Validate(opts) + assert.NoError(t, err) + + proxy, err := NewOAuthProxy(opts, func(string) bool { return true }) + if err != nil { + t.Fatal(err) + } + + _, err = proxy.redeemCode(context.Background(), "www.example.com", "") + assert.Error(t, err) +} + +func Test_enrichSession(t *testing.T) { + const ( + sessionUser = "Mr Session" + sessionEmail = "session@example.com" + providerEmail = "provider@example.com" + ) + + testCases := map[string]struct { + session *sessions.SessionState + expectedUser string + expectedEmail string + }{ + "Session already has enrichable fields": { + session: &sessions.SessionState{ + User: sessionUser, + Email: sessionEmail, + }, + expectedUser: sessionUser, + expectedEmail: sessionEmail, + }, + "Session is missing Email and GetEmailAddress is implemented": { + session: &sessions.SessionState{ + User: sessionUser, + }, + expectedUser: sessionUser, + expectedEmail: providerEmail, + }, + "Session is missing User and GetUserName is not implemented": { + session: &sessions.SessionState{ + Email: sessionEmail, + }, + expectedUser: "", + expectedEmail: sessionEmail, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + opts := baseTestOptions() + err := validation.Validate(opts) + assert.NoError(t, err) + + // intentionally set after validation.Validate(opts) since it will clobber + // our TestProvider and call `providers.New` defaulting to `providers.GoogleProvider` + opts.SetProvider(NewTestProvider(&url.URL{Host: "www.example.com"}, providerEmail)) + proxy, err := NewOAuthProxy(opts, func(string) bool { return true }) + if err != nil { + t.Fatal(err) + } + + err = proxy.enrichSession(context.Background(), tc.session) + assert.NoError(t, err) + assert.Equal(t, tc.expectedUser, tc.session.User) + assert.Equal(t, tc.expectedEmail, tc.session.Email) + }) + } +} + func TestBasicAuthPassword(t *testing.T) { providerServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { logger.Printf("%#v", r) From 2b9e1bbba0998095cea6cbb8dd4625d93fc3d6fc Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Sun, 27 Sep 2020 11:46:29 -0700 Subject: [PATCH 4/8] Add EnrichSessionState as main post-Redeem session updater --- CHANGELOG.md | 2 ++ oauthproxy.go | 7 ++++--- oauthproxy_test.go | 8 ++++---- providers/provider_default.go | 18 +++++++++++++----- providers/provider_default_test.go | 6 ++++++ providers/providers.go | 3 +++ 6 files changed, 32 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eaca5c8675..5dfc03e4ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,9 +26,11 @@ ## Changes since v6.1.1 - [#825](https://github.com/oauth2-proxy/oauth2-proxy/pull/825) Fix code coverage reporting on GitHub actions(@JoelSpeed) +- [#767](https://github.com/oauth2-proxy/oauth2-proxy/pull/796) Deprecate GetUserName & GetEmailAdress for EnrichSessionState (@NickMeves) - [#705](https://github.com/oauth2-proxy/oauth2-proxy/pull/705) Add generic Header injectors for upstream request and response headers (@JoelSpeed) - [#753](https://github.com/oauth2-proxy/oauth2-proxy/pull/753) Pass resource parameter in login url (@codablock) - [#789](https://github.com/oauth2-proxy/oauth2-proxy/pull/789) Add `--skip-auth-route` configuration option for `METHOD=pathRegex` based allowlists (@NickMeves) +- [#767](https://github.com/oauth2-proxy/oauth2-proxy/pull/796) Deprecate GetUserName & GetEmailAdress for EnrichSessionState (@NickMeves) - [#575](https://github.com/oauth2-proxy/oauth2-proxy/pull/575) Stop accepting legacy SHA1 signed cookies (@NickMeves) - [#722](https://github.com/oauth2-proxy/oauth2-proxy/pull/722) Validate Redis configuration options at startup (@NickMeves) - [#791](https://github.com/oauth2-proxy/oauth2-proxy/pull/791) Remove GetPreferredUsername method from provider interface (@NickMeves) diff --git a/oauthproxy.go b/oauthproxy.go index 9f80f643ab..db18e660a9 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -360,7 +360,7 @@ func (p *OAuthProxy) redeemCode(ctx context.Context, host, code string) (*sessio return s, nil } -func (p *OAuthProxy) enrichSession(ctx context.Context, s *sessionsapi.SessionState) error { +func (p *OAuthProxy) enrichSessionState(ctx context.Context, s *sessionsapi.SessionState) error { var err error if s.Email == "" { s.Email, err = p.provider.GetEmailAddress(ctx, s) @@ -374,7 +374,8 @@ func (p *OAuthProxy) enrichSession(ctx context.Context, s *sessionsapi.SessionSt return err } } - return nil + + return p.provider.EnrichSessionState(ctx, s) } // MakeCSRFCookie creates a cookie for CSRF @@ -831,7 +832,7 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { return } - err = p.enrichSession(req.Context(), session) + err = p.enrichSessionState(req.Context(), session) if err != nil { logger.Errorf("Error creating session during OAuth2 callback: %v", err) p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", "Internal Error") diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 9f60439bb8..46ee24b8fd 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -396,11 +396,11 @@ func NewTestProvider(providerURL *url.URL, emailAddress string) *TestProvider { } } -func (tp *TestProvider) GetEmailAddress(ctx context.Context, session *sessions.SessionState) (string, error) { +func (tp *TestProvider) GetEmailAddress(_ context.Context, _ *sessions.SessionState) (string, error) { return tp.EmailAddress, nil } -func (tp *TestProvider) ValidateSessionState(ctx context.Context, session *sessions.SessionState) bool { +func (tp *TestProvider) ValidateSessionState(_ context.Context, _ *sessions.SessionState) bool { return tp.ValidToken } @@ -468,7 +468,7 @@ func Test_enrichSession(t *testing.T) { t.Fatal(err) } - err = proxy.enrichSession(context.Background(), tc.session) + err = proxy.enrichSessionState(context.Background(), tc.session) assert.NoError(t, err) assert.Equal(t, tc.expectedUser, tc.session.User) assert.Equal(t, tc.expectedEmail, tc.session.Email) @@ -1955,7 +1955,7 @@ func TestClearSingleCookie(t *testing.T) { type NoOpKeySet struct { } -func (NoOpKeySet) VerifySignature(ctx context.Context, jwt string) (payload []byte, err error) { +func (NoOpKeySet) VerifySignature(_ context.Context, jwt string) (payload []byte, err error) { splitStrings := strings.Split(jwt, ".") payloadString := splitStrings[1] return base64.RawURLEncoding.DecodeString(payloadString) diff --git a/providers/provider_default.go b/providers/provider_default.go index 4e96f0e014..87bef08ec8 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -87,21 +87,29 @@ func (p *ProviderData) GetLoginURL(redirectURI, state string) string { } // GetEmailAddress returns the Account email address -func (p *ProviderData) GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) { +// DEPRECATED: Migrate to EnrichSessionState +func (p *ProviderData) GetEmailAddress(_ context.Context, _ *sessions.SessionState) (string, error) { return "", ErrNotImplemented } // GetUserName returns the Account username -func (p *ProviderData) GetUserName(ctx context.Context, s *sessions.SessionState) (string, error) { +// DEPRECATED: Migrate to EnrichSessionState +func (p *ProviderData) GetUserName(_ context.Context, _ *sessions.SessionState) (string, error) { return "", ErrNotImplemented } // ValidateGroup validates that the provided email exists in the configured provider // email group(s). -func (p *ProviderData) ValidateGroup(email string) bool { +func (p *ProviderData) ValidateGroup(_ string) bool { return true } +// EnrichSessionState is called after Redeem to allow providers to enrich session fields +// such as User, Email, Groups with provider specific API calls. +func (p *ProviderData) EnrichSessionState(_ context.Context, _ *sessions.SessionState) error { + return nil +} + // ValidateSessionState validates the AccessToken func (p *ProviderData) ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool { return validateToken(ctx, p, s.AccessToken, nil) @@ -109,12 +117,12 @@ func (p *ProviderData) ValidateSessionState(ctx context.Context, s *sessions.Ses // RefreshSessionIfNeeded should refresh the user's session if required and // do nothing if a refresh is not required -func (p *ProviderData) RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error) { +func (p *ProviderData) RefreshSessionIfNeeded(_ context.Context, _ *sessions.SessionState) (bool, error) { return false, nil } // CreateSessionStateFromBearerToken should be implemented to allow providers // to convert ID tokens into sessions -func (p *ProviderData) CreateSessionStateFromBearerToken(ctx context.Context, rawIDToken string, idToken *oidc.IDToken) (*sessions.SessionState, error) { +func (p *ProviderData) CreateSessionStateFromBearerToken(_ context.Context, _ string, _ *oidc.IDToken) (*sessions.SessionState, error) { return nil, ErrNotImplemented } diff --git a/providers/provider_default_test.go b/providers/provider_default_test.go index 8597ac6614..f04fe6073a 100644 --- a/providers/provider_default_test.go +++ b/providers/provider_default_test.go @@ -47,3 +47,9 @@ func TestAcrValuesConfigured(t *testing.T) { result := p.GetLoginURL("https://my.test.app/oauth", "") assert.Contains(t, result, "acr_values=testValue") } + +func TestEnrichSessionState(t *testing.T) { + p := &ProviderData{} + s := &sessions.SessionState{} + assert.NoError(t, p.EnrichSessionState(context.Background(), s)) +} diff --git a/providers/providers.go b/providers/providers.go index e92b3293f0..5e43613eae 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -10,10 +10,13 @@ import ( // Provider represents an upstream identity provider implementation type Provider interface { Data() *ProviderData + // DEPRECATED: Migrate to EnrichSessionState GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) + // DEPRECATED: Migrate to EnrichSessionState GetUserName(ctx context.Context, s *sessions.SessionState) (string, error) Redeem(ctx context.Context, redirectURI, code string) (*sessions.SessionState, error) ValidateGroup(string) bool + EnrichSessionState(ctx context.Context, s *sessions.SessionState) error ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool GetLoginURL(redirectURI, finalRedirect string) string RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error) From e51f5fe7c9b939a06abad4f32cd846322a40e068 Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Sun, 27 Sep 2020 13:35:06 -0700 Subject: [PATCH 5/8] Refactor GitHub to EnrichSessionState --- providers/github.go | 53 +++++++++------ providers/github_test.go | 142 +++++++++++++++++++-------------------- 2 files changed, 102 insertions(+), 93 deletions(-) diff --git a/providers/github.go b/providers/github.go index 40d30799c3..d1be571f7e 100644 --- a/providers/github.go +++ b/providers/github.go @@ -102,6 +102,20 @@ func (p *GitHubProvider) SetUsers(users []string) { p.Users = users } +// EnrichSessionState updates the User & Email after the initial Redeem +func (p *GitHubProvider) EnrichSessionState(ctx context.Context, s *sessions.SessionState) error { + err := p.getEmail(ctx, s) + if err != nil { + return err + } + return p.getUser(ctx, s) +} + +// ValidateSessionState validates the AccessToken +func (p *GitHubProvider) ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool { + return validateToken(ctx, p, s.AccessToken, makeGitHubHeader(s.AccessToken)) +} + func (p *GitHubProvider) hasOrg(ctx context.Context, accessToken string) (bool, error) { // https://developer.github.com/v3/orgs/#list-your-organizations @@ -364,8 +378,8 @@ func (p *GitHubProvider) isCollaborator(ctx context.Context, username, accessTok return true, nil } -// GetEmailAddress returns the Account email address -func (p *GitHubProvider) GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) { +// getEmail updates the SessionState Email +func (p *GitHubProvider) getEmail(ctx context.Context, s *sessions.SessionState) error { var emails []struct { Email string `json:"email"` @@ -379,11 +393,11 @@ func (p *GitHubProvider) GetEmailAddress(ctx context.Context, s *sessions.Sessio var err error verifiedUser, err = p.hasUser(ctx, s.AccessToken) if err != nil { - return "", err + return err } // org and repository options are not configured if !verifiedUser && p.Org == "" && p.Repo == "" { - return "", errors.New("missing github user") + return errors.New("missing github user") } } // If a user is verified by username options, skip the following restrictions @@ -391,16 +405,16 @@ func (p *GitHubProvider) GetEmailAddress(ctx context.Context, s *sessions.Sessio if p.Org != "" { if p.Team != "" { if ok, err := p.hasOrgAndTeam(ctx, s.AccessToken); err != nil || !ok { - return "", err + return err } } else { if ok, err := p.hasOrg(ctx, s.AccessToken); err != nil || !ok { - return "", err + return err } } } else if p.Repo != "" && p.Token == "" { // If we have a token we'll do the collaborator check in GetUserName if ok, err := p.hasRepo(ctx, s.AccessToken); err != nil || !ok { - return "", err + return err } } } @@ -416,24 +430,23 @@ func (p *GitHubProvider) GetEmailAddress(ctx context.Context, s *sessions.Sessio Do(). UnmarshalInto(&emails) if err != nil { - return "", err + return err } - returnEmail := "" for _, email := range emails { if email.Verified { - returnEmail = email.Email if email.Primary { - return returnEmail, nil + s.Email = email.Email + return nil } } } - return returnEmail, nil + return nil } -// GetUserName returns the Account user name -func (p *GitHubProvider) GetUserName(ctx context.Context, s *sessions.SessionState) (string, error) { +// getUser updates the SessionState User +func (p *GitHubProvider) getUser(ctx context.Context, s *sessions.SessionState) error { var user struct { Login string `json:"login"` Email string `json:"email"` @@ -451,22 +464,18 @@ func (p *GitHubProvider) GetUserName(ctx context.Context, s *sessions.SessionSta Do(). UnmarshalInto(&user) if err != nil { - return "", err + return err } // Now that we have the username we can check collaborator status if !p.isVerifiedUser(user.Login) && p.Org == "" && p.Repo != "" && p.Token != "" { if ok, err := p.isCollaborator(ctx, user.Login, p.Token); err != nil || !ok { - return "", err + return err } } - return user.Login, nil -} - -// ValidateSessionState validates the AccessToken -func (p *GitHubProvider) ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool { - return validateToken(ctx, p, s.AccessToken, makeGitHubHeader(s.AccessToken)) + s.User = user.Login + return nil } // isVerifiedUser diff --git a/providers/github_test.go b/providers/github_test.go index dba4bcf6d4..d19aaa37ae 100644 --- a/providers/github_test.go +++ b/providers/github_test.go @@ -107,7 +107,7 @@ func TestGitHubProviderOverrides(t *testing.T) { assert.Equal(t, "profile", p.Data().Scope) } -func TestGitHubProviderGetEmailAddress(t *testing.T) { +func TestGitHubProvider_getEmail(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, }) @@ -117,14 +117,14 @@ func TestGitHubProviderGetEmailAddress(t *testing.T) { p := testGitHubProvider(bURL.Host) session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "michael.bland@gsa.gov", email) + err := p.getEmail(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "michael.bland@gsa.gov", session.Email) } -func TestGitHubProviderGetEmailAddressNotVerified(t *testing.T) { +func TestGitHubProvider_getEmailNotVerified(t *testing.T) { b := testGitHubBackend(map[string][]string{ - "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, + "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": false, "primary": true} ]`}, }) defer b.Close() @@ -132,12 +132,12 @@ func TestGitHubProviderGetEmailAddressNotVerified(t *testing.T) { p := testGitHubProvider(bURL.Host) session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Empty(t, "", email) + err := p.getEmail(context.Background(), session) + assert.NoError(t, err) + assert.Empty(t, session.Email) } -func TestGitHubProviderGetEmailAddressWithOrg(t *testing.T) { +func TestGitHubProvider_getEmailWithOrg(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, "/user/orgs": { @@ -153,12 +153,12 @@ func TestGitHubProviderGetEmailAddressWithOrg(t *testing.T) { p.Org = "testorg1" session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "michael.bland@gsa.gov", email) + err := p.getEmail(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "michael.bland@gsa.gov", session.Email) } -func TestGitHubProviderGetEmailAddressWithWriteAccessToPublicRepo(t *testing.T) { +func TestGitHubProvider_getEmailWithWriteAccessToPublicRepo(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/repo/oauth2-proxy/oauth2-proxy": {`{"permissions": {"pull": true, "push": true}, "private": false}`}, "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, @@ -170,12 +170,12 @@ func TestGitHubProviderGetEmailAddressWithWriteAccessToPublicRepo(t *testing.T) p.SetRepo("oauth2-proxy/oauth2-proxy", "") session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "michael.bland@gsa.gov", email) + err := p.getEmail(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "michael.bland@gsa.gov", session.Email) } -func TestGitHubProviderGetEmailAddressWithReadOnlyAccessToPrivateRepo(t *testing.T) { +func TestGitHubProvider_getEmailWithReadOnlyAccessToPrivateRepo(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/repo/oauth2-proxy/oauth2-proxy": {`{"permissions": {"pull": true, "push": false}, "private": true}`}, "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, @@ -187,12 +187,12 @@ func TestGitHubProviderGetEmailAddressWithReadOnlyAccessToPrivateRepo(t *testing p.SetRepo("oauth2-proxy/oauth2-proxy", "") session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "michael.bland@gsa.gov", email) + err := p.getEmail(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "michael.bland@gsa.gov", session.Email) } -func TestGitHubProviderGetEmailAddressWithWriteAccessToPrivateRepo(t *testing.T) { +func TestGitHubProvider_getEmailWithWriteAccessToPrivateRepo(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/repo/oauth2-proxy/oauth2-proxy": {`{"permissions": {"pull": true, "push": true}, "private": true}`}, "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, @@ -204,14 +204,14 @@ func TestGitHubProviderGetEmailAddressWithWriteAccessToPrivateRepo(t *testing.T) p.SetRepo("oauth2-proxy/oauth2-proxy", "") session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "michael.bland@gsa.gov", email) + err := p.getEmail(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "michael.bland@gsa.gov", session.Email) } -func TestGitHubProviderGetEmailAddressWithNoAccessToPrivateRepo(t *testing.T) { +func TestGitHubProvider_getEmailWithNoAccessToPrivateRepo(t *testing.T) { b := testGitHubBackend(map[string][]string{ - "/repo/oauth2-proxy/oauth2-proxy": {}, + "/repo/oauth2-proxy/oauth2-proxy": {`{}`}, }) defer b.Close() @@ -220,12 +220,12 @@ func TestGitHubProviderGetEmailAddressWithNoAccessToPrivateRepo(t *testing.T) { p.SetRepo("oauth2-proxy/oauth2-proxy", "") session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.NotEqual(t, nil, err) - assert.Equal(t, "", email) + err := p.getEmail(context.Background(), session) + assert.NoError(t, err) + assert.Empty(t, session.Email) } -func TestGitHubProviderGetEmailAddressWithToken(t *testing.T) { +func TestGitHubProvider_getEmailWithToken(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, }) @@ -236,14 +236,14 @@ func TestGitHubProviderGetEmailAddressWithToken(t *testing.T) { p.SetRepo("oauth2-proxy/oauth2-proxy", "token") session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "michael.bland@gsa.gov", email) + err := p.getEmail(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "michael.bland@gsa.gov", session.Email) } // Note that trying to trigger the "failed building request" case is not // practical, since the only way it can fail is if the URL fails to parse. -func TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) { +func TestGitHubProvider_getEmailFailedRequest(t *testing.T) { b := testGitHubBackend(map[string][]string{}) defer b.Close() @@ -254,12 +254,12 @@ func TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) { // token. Alternatively, we could allow the parsing of the payload as // JSON to fail. session := &sessions.SessionState{AccessToken: "unexpected_access_token"} - email, err := p.GetEmailAddress(context.Background(), session) - assert.NotEqual(t, nil, err) - assert.Equal(t, "", email) + err := p.getEmail(context.Background(), session) + assert.Error(t, err) + assert.Empty(t, session.Email) } -func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) { +func TestGitHubProvider_getEmailNotPresentInPayload(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/user/emails": {`{"foo": "bar"}`}, }) @@ -269,12 +269,12 @@ func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) { p := testGitHubProvider(bURL.Host) session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.NotEqual(t, nil, err) - assert.Equal(t, "", email) + err := p.getEmail(context.Background(), session) + assert.Error(t, err) + assert.Empty(t, session.Email) } -func TestGitHubProviderGetUserName(t *testing.T) { +func TestGitHubProvider_getUser(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}, }) @@ -284,12 +284,12 @@ func TestGitHubProviderGetUserName(t *testing.T) { p := testGitHubProvider(bURL.Host) session := CreateAuthorizedSession() - email, err := p.GetUserName(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "mbland", email) + err := p.getUser(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "mbland", session.User) } -func TestGitHubProviderGetUserNameWithRepoAndToken(t *testing.T) { +func TestGitHubProvider_getUserWithRepoAndToken(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}, "/repos/oauth2-proxy/oauth2-proxy/collaborators/mbland": {""}, @@ -301,12 +301,12 @@ func TestGitHubProviderGetUserNameWithRepoAndToken(t *testing.T) { p.SetRepo("oauth2-proxy/oauth2-proxy", "token") session := CreateAuthorizedSession() - email, err := p.GetUserName(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "mbland", email) + err := p.getUser(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "mbland", session.User) } -func TestGitHubProviderGetUserNameWithRepoAndTokenWithoutPushAccess(t *testing.T) { +func TestGitHubProvider_getUserWithRepoAndTokenWithoutPushAccess(t *testing.T) { b := testGitHubBackend(map[string][]string{}) defer b.Close() @@ -315,12 +315,12 @@ func TestGitHubProviderGetUserNameWithRepoAndTokenWithoutPushAccess(t *testing.T p.SetRepo("oauth2-proxy/oauth2-proxy", "token") session := CreateAuthorizedSession() - email, err := p.GetUserName(context.Background(), session) - assert.NotEqual(t, nil, err) - assert.Equal(t, "", email) + err := p.getUser(context.Background(), session) + assert.Error(t, err) + assert.Empty(t, session.User) } -func TestGitHubProviderGetEmailAddressWithUsername(t *testing.T) { +func TestGitHubProvider_getEmailWithUsername(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}, "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, @@ -332,12 +332,12 @@ func TestGitHubProviderGetEmailAddressWithUsername(t *testing.T) { p.SetUsers([]string{"mbland", "octocat"}) session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "michael.bland@gsa.gov", email) + err := p.getEmail(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "michael.bland@gsa.gov", session.Email) } -func TestGitHubProviderGetEmailAddressWithNotAllowedUsername(t *testing.T) { +func TestGitHubProvider_getEmailWithNotAllowedUsername(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}, "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, @@ -349,12 +349,12 @@ func TestGitHubProviderGetEmailAddressWithNotAllowedUsername(t *testing.T) { p.SetUsers([]string{"octocat"}) session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.NotEqual(t, nil, err) - assert.Equal(t, "", email) + err := p.getEmail(context.Background(), session) + assert.Error(t, err) + assert.Empty(t, session.Email) } -func TestGitHubProviderGetEmailAddressWithUsernameAndNotBelongToOrg(t *testing.T) { +func TestGitHubProvider_getEmailWithUsernameAndNotBelongToOrg(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}, "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, @@ -371,16 +371,16 @@ func TestGitHubProviderGetEmailAddressWithUsernameAndNotBelongToOrg(t *testing.T p.SetUsers([]string{"mbland"}) session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "michael.bland@gsa.gov", email) + err := p.getEmail(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "michael.bland@gsa.gov", session.Email) } -func TestGitHubProviderGetEmailAddressWithUsernameAndNoAccessToPrivateRepo(t *testing.T) { +func TestGitHubProvider_getEmailWithUsernameAndNoAccessToPrivateRepo(t *testing.T) { b := testGitHubBackend(map[string][]string{ "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`}, "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`}, - "/repo/oauth2-proxy/oauth2-proxy": {}, + "/repo/oauth2-proxy/oauth2-proxy": {`{}`}, }) defer b.Close() @@ -390,7 +390,7 @@ func TestGitHubProviderGetEmailAddressWithUsernameAndNoAccessToPrivateRepo(t *te p.SetUsers([]string{"mbland"}) session := CreateAuthorizedSession() - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "michael.bland@gsa.gov", email) + err := p.getEmail(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "michael.bland@gsa.gov", session.Email) } From 0da45e97e163c2dd901a4d336c4fa14412c19406 Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Sun, 27 Sep 2020 15:15:33 -0700 Subject: [PATCH 6/8] Refactor GitLab to EnrichSessionState --- providers/gitlab.go | 43 ++++++----------------------- providers/gitlab_test.go | 59 ++++++++++------------------------------ 2 files changed, 23 insertions(+), 79 deletions(-) diff --git a/providers/gitlab.go b/providers/gitlab.go index ee15a48642..eb673a3d9f 100644 --- a/providers/gitlab.go +++ b/providers/gitlab.go @@ -3,7 +3,6 @@ package providers import ( "context" "fmt" - "strings" "time" oidc "github.com/coreos/go-oidc" @@ -168,20 +167,6 @@ func (p *GitLabProvider) verifyGroupMembership(userInfo *gitlabUserInfo) error { return fmt.Errorf("user is not a member of '%s'", p.Groups) } -func (p *GitLabProvider) verifyEmailDomain(userInfo *gitlabUserInfo) error { - if len(p.EmailDomains) == 0 || p.EmailDomains[0] == "*" { - return nil - } - - for _, domain := range p.EmailDomains { - if strings.HasSuffix(userInfo.Email, domain) { - return nil - } - } - - return fmt.Errorf("user email is not one of the valid domains '%v'", p.EmailDomains) -} - func (p *GitLabProvider) createSessionState(ctx context.Context, token *oauth2.Token) (*sessions.SessionState, error) { rawIDToken, ok := token.Extra("id_token").(string) if !ok { @@ -211,39 +196,27 @@ func (p *GitLabProvider) ValidateSessionState(ctx context.Context, s *sessions.S } // GetEmailAddress returns the Account email address -func (p *GitLabProvider) GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) { +func (p *GitLabProvider) EnrichSessionState(ctx context.Context, s *sessions.SessionState) error { // Retrieve user info userInfo, err := p.getUserInfo(ctx, s) if err != nil { - return "", fmt.Errorf("failed to retrieve user info: %v", err) + return fmt.Errorf("failed to retrieve user info: %v", err) } // Check if email is verified if !p.AllowUnverifiedEmail && !userInfo.EmailVerified { - return "", fmt.Errorf("user email is not verified") - } - - // Check if email has valid domain - err = p.verifyEmailDomain(userInfo) - if err != nil { - return "", fmt.Errorf("email domain check failed: %v", err) + return fmt.Errorf("user email is not verified") } // Check group membership + // TODO (@NickMeves) - Refactor to Authorize err = p.verifyGroupMembership(userInfo) if err != nil { - return "", fmt.Errorf("group membership check failed: %v", err) + return fmt.Errorf("group membership check failed: %v", err) } - return userInfo.Email, nil -} - -// GetUserName returns the Account user name -func (p *GitLabProvider) GetUserName(ctx context.Context, s *sessions.SessionState) (string, error) { - userInfo, err := p.getUserInfo(ctx, s) - if err != nil { - return "", fmt.Errorf("failed to retrieve user info: %v", err) - } + s.User = userInfo.Username + s.Email = userInfo.Email - return userInfo.Username, nil + return nil } diff --git a/providers/gitlab_test.go b/providers/gitlab_test.go index 21bd04f747..12b9d6f425 100644 --- a/providers/gitlab_test.go +++ b/providers/gitlab_test.go @@ -64,8 +64,8 @@ func TestGitLabProviderBadToken(t *testing.T) { p := testGitLabProvider(bURL.Host) session := &sessions.SessionState{AccessToken: "unexpected_gitlab_access_token"} - _, err := p.GetEmailAddress(context.Background(), session) - assert.NotEqual(t, nil, err) + err := p.EnrichSessionState(context.Background(), session) + assert.Error(t, err) } func TestGitLabProviderUnverifiedEmailDenied(t *testing.T) { @@ -76,8 +76,8 @@ func TestGitLabProviderUnverifiedEmailDenied(t *testing.T) { p := testGitLabProvider(bURL.Host) session := &sessions.SessionState{AccessToken: "gitlab_access_token"} - _, err := p.GetEmailAddress(context.Background(), session) - assert.NotEqual(t, nil, err) + err := p.EnrichSessionState(context.Background(), session) + assert.Error(t, err) } func TestGitLabProviderUnverifiedEmailAllowed(t *testing.T) { @@ -89,9 +89,9 @@ func TestGitLabProviderUnverifiedEmailAllowed(t *testing.T) { p.AllowUnverifiedEmail = true session := &sessions.SessionState{AccessToken: "gitlab_access_token"} - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "foo@bar.com", email) + err := p.EnrichSessionState(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "foo@bar.com", session.Email) } func TestGitLabProviderUsername(t *testing.T) { @@ -103,9 +103,9 @@ func TestGitLabProviderUsername(t *testing.T) { p.AllowUnverifiedEmail = true session := &sessions.SessionState{AccessToken: "gitlab_access_token"} - username, err := p.GetUserName(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "FooBar", username) + err := p.EnrichSessionState(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "FooBar", session.User) } func TestGitLabProviderGroupMembershipValid(t *testing.T) { @@ -118,9 +118,9 @@ func TestGitLabProviderGroupMembershipValid(t *testing.T) { p.Groups = []string{"foo"} session := &sessions.SessionState{AccessToken: "gitlab_access_token"} - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "foo@bar.com", email) + err := p.EnrichSessionState(context.Background(), session) + assert.NoError(t, err) + assert.Equal(t, "FooBar", session.User) } func TestGitLabProviderGroupMembershipMissing(t *testing.T) { @@ -133,35 +133,6 @@ func TestGitLabProviderGroupMembershipMissing(t *testing.T) { p.Groups = []string{"baz"} session := &sessions.SessionState{AccessToken: "gitlab_access_token"} - _, err := p.GetEmailAddress(context.Background(), session) - assert.NotEqual(t, nil, err) -} - -func TestGitLabProviderEmailDomainValid(t *testing.T) { - b := testGitLabBackend() - defer b.Close() - - bURL, _ := url.Parse(b.URL) - p := testGitLabProvider(bURL.Host) - p.AllowUnverifiedEmail = true - p.EmailDomains = []string{"bar.com"} - - session := &sessions.SessionState{AccessToken: "gitlab_access_token"} - email, err := p.GetEmailAddress(context.Background(), session) - assert.Equal(t, nil, err) - assert.Equal(t, "foo@bar.com", email) -} - -func TestGitLabProviderEmailDomainInvalid(t *testing.T) { - b := testGitLabBackend() - defer b.Close() - - bURL, _ := url.Parse(b.URL) - p := testGitLabProvider(bURL.Host) - p.AllowUnverifiedEmail = true - p.EmailDomains = []string{"baz.com"} - - session := &sessions.SessionState{AccessToken: "gitlab_access_token"} - _, err := p.GetEmailAddress(context.Background(), session) - assert.NotEqual(t, nil, err) + err := p.EnrichSessionState(context.Background(), session) + assert.Error(t, err) } From d9c141ae7c88af6e095c513dfcebeede52e95c27 Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Sun, 27 Sep 2020 15:18:12 -0700 Subject: [PATCH 7/8] Remove GetUserName method from Provider --- oauthproxy.go | 6 ------ providers/provider_default.go | 6 ------ providers/providers.go | 2 -- 3 files changed, 14 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index db18e660a9..4fd03a40b7 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -368,12 +368,6 @@ func (p *OAuthProxy) enrichSessionState(ctx context.Context, s *sessionsapi.Sess return err } } - if s.User == "" { - s.User, err = p.provider.GetUserName(ctx, s) - if err != nil && !errors.Is(err, providers.ErrNotImplemented) { - return err - } - } return p.provider.EnrichSessionState(ctx, s) } diff --git a/providers/provider_default.go b/providers/provider_default.go index 87bef08ec8..479e52c0da 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -92,12 +92,6 @@ func (p *ProviderData) GetEmailAddress(_ context.Context, _ *sessions.SessionSta return "", ErrNotImplemented } -// GetUserName returns the Account username -// DEPRECATED: Migrate to EnrichSessionState -func (p *ProviderData) GetUserName(_ context.Context, _ *sessions.SessionState) (string, error) { - return "", ErrNotImplemented -} - // ValidateGroup validates that the provided email exists in the configured provider // email group(s). func (p *ProviderData) ValidateGroup(_ string) bool { diff --git a/providers/providers.go b/providers/providers.go index 5e43613eae..6987cf6b87 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -12,8 +12,6 @@ type Provider interface { Data() *ProviderData // DEPRECATED: Migrate to EnrichSessionState GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) - // DEPRECATED: Migrate to EnrichSessionState - GetUserName(ctx context.Context, s *sessions.SessionState) (string, error) Redeem(ctx context.Context, redirectURI, code string) (*sessions.SessionState, error) ValidateGroup(string) bool EnrichSessionState(ctx context.Context, s *sessions.SessionState) error From 4a54c9421c505446168b2463aa9a5a0b24b752ad Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Sun, 27 Sep 2020 15:30:45 -0700 Subject: [PATCH 8/8] Remove EmailDomain verification from GitLab provider This is handled globally --- CHANGELOG.md | 3 +-- pkg/validation/options.go | 1 - providers/gitlab.go | 4 +--- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dfc03e4ee..cb496b3a2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,11 +26,10 @@ ## Changes since v6.1.1 - [#825](https://github.com/oauth2-proxy/oauth2-proxy/pull/825) Fix code coverage reporting on GitHub actions(@JoelSpeed) -- [#767](https://github.com/oauth2-proxy/oauth2-proxy/pull/796) Deprecate GetUserName & GetEmailAdress for EnrichSessionState (@NickMeves) +- [#796](https://github.com/oauth2-proxy/oauth2-proxy/pull/796) Deprecate GetUserName & GetEmailAdress for EnrichSessionState (@NickMeves) - [#705](https://github.com/oauth2-proxy/oauth2-proxy/pull/705) Add generic Header injectors for upstream request and response headers (@JoelSpeed) - [#753](https://github.com/oauth2-proxy/oauth2-proxy/pull/753) Pass resource parameter in login url (@codablock) - [#789](https://github.com/oauth2-proxy/oauth2-proxy/pull/789) Add `--skip-auth-route` configuration option for `METHOD=pathRegex` based allowlists (@NickMeves) -- [#767](https://github.com/oauth2-proxy/oauth2-proxy/pull/796) Deprecate GetUserName & GetEmailAdress for EnrichSessionState (@NickMeves) - [#575](https://github.com/oauth2-proxy/oauth2-proxy/pull/575) Stop accepting legacy SHA1 signed cookies (@NickMeves) - [#722](https://github.com/oauth2-proxy/oauth2-proxy/pull/722) Validate Redis configuration options at startup (@NickMeves) - [#791](https://github.com/oauth2-proxy/oauth2-proxy/pull/791) Remove GetPreferredUsername method from provider interface (@NickMeves) diff --git a/pkg/validation/options.go b/pkg/validation/options.go index 2ea35f9f5f..b54df3300c 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -273,7 +273,6 @@ func parseProviderInfo(o *options.Options, msgs []string) []string { case *providers.GitLabProvider: p.AllowUnverifiedEmail = o.InsecureOIDCAllowUnverifiedEmail p.Groups = o.GitLabGroup - p.EmailDomains = o.EmailDomains if o.GetOIDCVerifier() != nil { p.Verifier = o.GetOIDCVerifier() diff --git a/providers/gitlab.go b/providers/gitlab.go index eb673a3d9f..a04beca6ad 100644 --- a/providers/gitlab.go +++ b/providers/gitlab.go @@ -15,9 +15,7 @@ import ( type GitLabProvider struct { *ProviderData - Groups []string - EmailDomains []string - + Groups []string Verifier *oidc.IDTokenVerifier AllowUnverifiedEmail bool }