From 3371284a3698378147eff32e073b350218b586da Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Wed, 23 Sep 2020 15:55:22 -0700 Subject: [PATCH 1/2] Remove GetPreferredUsername method from Provider interface It isn't used in any providers and we have future plans to remove the specialness of PreferredUsername and make it an optional field in the session. User, Email & Groups will eventually be the only first class fields on the session that are always set. --- oauthproxy.go | 7 ------- providers/provider_default.go | 5 ----- providers/providers.go | 1 - 3 files changed, 13 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index 17d106d5..b3b3cbe6 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -310,13 +310,6 @@ func (p *OAuthProxy) redeemCode(ctx context.Context, host, code string) (s *sess s.Email, err = p.provider.GetEmailAddress(ctx, s) } - if s.PreferredUsername == "" { - s.PreferredUsername, err = p.provider.GetPreferredUsername(ctx, s) - if err != nil && err.Error() == "not implemented" { - err = nil - } - } - if s.User == "" { s.User, err = p.provider.GetUserName(ctx, s) if err != nil && err.Error() == "not implemented" { diff --git a/providers/provider_default.go b/providers/provider_default.go index 58e1b4fc..ba05a96c 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -104,11 +104,6 @@ func (p *ProviderData) GetUserName(ctx context.Context, s *sessions.SessionState return "", errors.New("not implemented") } -// GetPreferredUsername returns the Account preferred username -func (p *ProviderData) GetPreferredUsername(ctx context.Context, s *sessions.SessionState) (string, error) { - return "", errors.New("not implemented") -} - // ValidateGroup validates that the provided email exists in the configured provider // email group(s). func (p *ProviderData) ValidateGroup(email string) bool { diff --git a/providers/providers.go b/providers/providers.go index a52eff90..eefc5994 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -12,7 +12,6 @@ type Provider interface { Data() *ProviderData GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) GetUserName(ctx context.Context, s *sessions.SessionState) (string, error) - GetPreferredUsername(ctx context.Context, s *sessions.SessionState) (string, error) Redeem(ctx context.Context, redirectURI, code string) (*sessions.SessionState, error) ValidateGroup(string) bool ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool From e0d915cc0347c8d026baa6bad205d7cc5564d932 Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Wed, 23 Sep 2020 16:04:54 -0700 Subject: [PATCH 2/2] Stop shadowing GetEmailAddress errors in redeemCode --- CHANGELOG.md | 1 + oauthproxy.go | 16 ++++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4919b34a..5e4ec5bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - [#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) - [#764](https://github.com/oauth2-proxy/oauth2-proxy/pull/764) Document bcrypt encryption for htpasswd (and hide SHA) (@lentzi90) - [#616](https://github.com/oauth2-proxy/oauth2-proxy/pull/616) Add support to ensure user belongs in required groups when using the OIDC provider (@stefansedich) diff --git a/oauthproxy.go b/oauthproxy.go index b3b3cbe6..0f69caab 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -296,27 +296,31 @@ func (p *OAuthProxy) GetRedirectURI(host string) string { return u.String() } -func (p *OAuthProxy) redeemCode(ctx context.Context, host, code string) (s *sessionsapi.SessionState, err error) { +func (p *OAuthProxy) redeemCode(ctx context.Context, host, code string) (*sessionsapi.SessionState, error) { if code == "" { return nil, errors.New("missing code") } redirectURI := p.GetRedirectURI(host) - s, err = p.provider.Redeem(ctx, redirectURI, code) + s, err := p.provider.Redeem(ctx, redirectURI, code) if err != nil { - return + return nil, err } if s.Email == "" { s.Email, err = p.provider.GetEmailAddress(ctx, s) + if err != nil && err.Error() != "not implemented" { + return nil, err + } } if s.User == "" { s.User, err = p.provider.GetUserName(ctx, s) - if err != nil && err.Error() == "not implemented" { - err = nil + if err != nil && err.Error() != "not implemented" { + return nil, err } } - return + + return s, nil } // MakeCSRFCookie creates a cookie for CSRF