1
0
mirror of https://github.com/oauth2-proxy/oauth2-proxy.git synced 2025-01-10 04:18:14 +02:00

Refactor GitHub to EnrichSessionState

This commit is contained in:
Nick Meves 2020-09-27 13:35:06 -07:00
parent 2b9e1bbba0
commit e51f5fe7c9
No known key found for this signature in database
GPG Key ID: 93BA8A3CEDCDD1CF
2 changed files with 102 additions and 93 deletions

View File

@ -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

View File

@ -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)
}