diff --git a/providers/github.go b/providers/github.go index 40d30799..d1be571f 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 dba4bcf6..d19aaa37 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) }