From 3ff441026ce41be659a2eb96752a1b74bead6fe2 Mon Sep 17 00:00:00 2001 From: Lukasz Leszczuk Date: Mon, 9 Sep 2019 19:25:56 +0200 Subject: [PATCH] Cleanup in comments, removal of GetEmailAddress, use groups as []string --- oauthproxy.go | 12 +++++------ pkg/apis/sessions/session_state.go | 33 ++++++++++++++++-------------- providers/azure.go | 8 +++++--- providers/facebook.go | 2 +- providers/github.go | 2 +- providers/gitlab.go | 2 +- providers/linkedin.go | 2 +- providers/provider_default.go | 7 +------ providers/providers.go | 1 - 9 files changed, 33 insertions(+), 36 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index 1b7b85d..97d5de8 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -673,11 +673,9 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { return } - groupNames := []string{} for groupName := range groups { - groupNames = append(groupNames, groupName) + session.Groups = append(session.Groups, groupName) } - session.Groups = strings.Join(groupNames, p.GroupsDelimiter) } if !p.IsValidRedirect(redirect) { @@ -849,8 +847,8 @@ func (p *OAuthProxy) addHeadersForProxying(rw http.ResponseWriter, req *http.Req } else { req.Header.Del("X-Forwarded-Email") } - if p.PassGroups && session.Groups != "" { - req.Header["X-Forwarded-Groups"] = []string{session.Groups} + if p.PassGroups && len(session.Groups) != 0 { + req.Header["X-Forwarded-Groups"] = []string{strings.Join(session.Groups, p.GroupsDelimiter)} } } @@ -878,8 +876,8 @@ func (p *OAuthProxy) addHeadersForProxying(rw http.ResponseWriter, req *http.Req rw.Header().Del("X-Auth-Request-Access-Token") } } - if p.PassGroups && session.Groups != "" { - rw.Header().Set("X-Auth-Request-Groups", session.Groups) + if p.PassGroups && len(session.Groups) != 0 { + rw.Header().Set("X-Auth-Request-Groups", strings.Join(session.Groups, p.GroupsDelimiter)) } } diff --git a/pkg/apis/sessions/session_state.go b/pkg/apis/sessions/session_state.go index 1f9818f..e5f6f27 100644 --- a/pkg/apis/sessions/session_state.go +++ b/pkg/apis/sessions/session_state.go @@ -12,15 +12,16 @@ import ( // SessionState is used to store information about the currently authenticated user session type SessionState struct { - AccessToken string `json:",omitempty"` - IDToken string `json:",omitempty"` - CreatedAt time.Time `json:"-"` - ExpiresOn time.Time `json:"-"` - RefreshToken string `json:",omitempty"` - Email string `json:",omitempty"` - User string `json:",omitempty"` - ID string `json:",omitempty"` - Groups string `json:",omitempty"` + AccessToken string `json:",omitempty"` + IDToken string `json:",omitempty"` + CreatedAt time.Time `json:"-"` + ExpiresOn time.Time `json:"-"` + RefreshToken string `json:",omitempty"` + Email string `json:",omitempty"` + User string `json:",omitempty"` + ID string `json:",omitempty"` + Groups []string `json:",omitempty"` + EncodedGroups string `json:",omitempty"` } // SessionStateJSON is used to encode SessionState into JSON without exposing time.Time zero value @@ -64,8 +65,8 @@ func (s *SessionState) String() string { if s.RefreshToken != "" { o += " refresh_token:true" } - if s.Groups != "" { - o += fmt.Sprintf(" group:%s", s.Groups) + if len(s.Groups) != 0 { + o += fmt.Sprintf(" group:%s", strings.Join(s.Groups, ",")) } return o + "}" } @@ -110,11 +111,12 @@ func (s *SessionState) EncodeSessionState(c *encryption.Cipher) (string, error) return "", err } } - if ss.Groups != "" { - ss.Groups, err = c.Encrypt(ss.Groups) + if len(ss.Groups) != 0 { + ss.EncodedGroups, err = c.Encrypt(strings.Join(ss.Groups, ",")) if err != nil { return "", err } + ss.Groups = nil } if ss.ID != "" { ss.ID, err = c.Encrypt(ss.ID) @@ -252,11 +254,12 @@ func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) { return nil, err } } - if ss.Groups != "" { - ss.Groups, err = c.Decrypt(ss.Groups) + if ss.EncodedGroups != "" { + groupsString, err := c.Decrypt(ss.EncodedGroups) if err != nil { return nil, err } + ss.Groups = strings.Split(groupsString, ",") } if ss.ID != "" { ss.ID, err = c.Decrypt(ss.ID) diff --git a/providers/azure.go b/providers/azure.go index febc2a6..43908d1 100644 --- a/providers/azure.go +++ b/providers/azure.go @@ -255,9 +255,11 @@ func (p *AzureProvider) ValidateGroupWithSession(s *sessions.SessionState) bool if len(p.PermittedGroups) == 0 { return true } - for _, groupID := range p.PermittedGroups { - if strings.Contains(s.Groups, groupID) { - return true + for _, group := range s.Groups { + for _, groupID := range p.PermittedGroups { + if strings.Contains(group, groupID) { + return true + } } } return false diff --git a/providers/facebook.go b/providers/facebook.go index 7a768c2..0afe4af 100644 --- a/providers/facebook.go +++ b/providers/facebook.go @@ -54,7 +54,7 @@ func getFacebookHeader(accessToken string) http.Header { return header } -// GetEmailAddress returns the Account email address +// GetUserDetails returns the Account email address func (p *FacebookProvider) GetUserDetails(s *sessions.SessionState) (*UserDetails, error) { if s.AccessToken == "" { return nil, errors.New("missing access token") diff --git a/providers/github.go b/providers/github.go index 01c90b6..52ef9c1 100644 --- a/providers/github.go +++ b/providers/github.go @@ -200,7 +200,7 @@ func (p *GitHubProvider) hasOrgAndTeam(accessToken string) (bool, error) { return false, nil } -// GetEmailAddress returns the Account email address +// GetUserDetails returns the Account email address func (p *GitHubProvider) GetUserDetails(s *sessions.SessionState) (*UserDetails, error) { var emails []struct { Email string `json:"email"` diff --git a/providers/gitlab.go b/providers/gitlab.go index 155097e..e9827da 100644 --- a/providers/gitlab.go +++ b/providers/gitlab.go @@ -219,7 +219,7 @@ func (p *GitLabProvider) ValidateSessionState(s *sessions.SessionState) bool { return true } -// GetEmailAddress returns the Account email address +// GetUserDetails returns the Account email address func (p *GitLabProvider) GetUserDetails(s *sessions.SessionState) (*UserDetails, error) { // Retrieve user info userInfo, err := p.getUserInfo(s) diff --git a/providers/linkedin.go b/providers/linkedin.go index 2040454..598a24d 100644 --- a/providers/linkedin.go +++ b/providers/linkedin.go @@ -50,7 +50,7 @@ func getLinkedInHeader(accessToken string) http.Header { return header } -// GetEmailAddress returns the Account email address +// GetUserDetails returns the Account email address func (p *LinkedInProvider) GetUserDetails(s *sessions.SessionState) (*UserDetails, error) { if s.AccessToken == "" { return nil, errors.New("missing access token") diff --git a/providers/provider_default.go b/providers/provider_default.go index 497a5fc..3b7e7fb 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -117,11 +117,6 @@ func (p *ProviderData) SessionFromCookie(v string, c *encryption.Cipher) (s *ses return sessions.DecodeSessionState(v, c) } -// GetEmailAddress returns the Account email address -func (p *ProviderData) GetEmailAddress(s *sessions.SessionState) (string, error) { - return "", errors.New("not implemented") -} - func (p *ProviderData) GetUserDetails(s *sessions.SessionState) (*UserDetails, error) { return nil, errors.New("not implemented") } @@ -141,7 +136,7 @@ func (p *ProviderData) ValidateGroup(email string) bool { return true } -// ValidateExemptions checks if we can allow user login dispite group membership returned failure +// ValidateExemptions checks if we can allow user login despite group membership returned failure func (p *ProviderData) ValidateExemptions(*sessions.SessionState) (bool, string) { return false, "" } diff --git a/providers/providers.go b/providers/providers.go index 79facf1..87632b0 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -8,7 +8,6 @@ import ( // Provider represents an upstream identity provider implementation type Provider interface { Data() *ProviderData - GetEmailAddress(*sessions.SessionState) (string, error) GetUserDetails(*sessions.SessionState) (*UserDetails, error) GetUserName(*sessions.SessionState) (string, error) GetGroups(*sessions.SessionState, string) (map[string]string, error)