diff --git a/internal/api/idp/idp.go b/internal/api/idp/idp.go index 8b1c24134a0..888933855fe 100644 --- a/internal/api/idp/idp.go +++ b/internal/api/idp/idp.go @@ -550,7 +550,7 @@ func (h *Handler) fetchIDPUserFromCode(ctx context.Context, identityProvider idp case *azuread.Provider: session = azuread.NewSession(provider, code) case *github.Provider: - session = oauth.NewSession(provider.Provider, code, idpArguments) + session = github.NewSession(provider, code, idpArguments) case *gitlab.Provider: session = openid.NewSession(provider.Provider, code, idpArguments) case *google.Provider: diff --git a/internal/api/ui/login/external_provider_handler.go b/internal/api/ui/login/external_provider_handler.go index 461af46b517..04414c28609 100644 --- a/internal/api/ui/login/external_provider_handler.go +++ b/internal/api/ui/login/external_provider_handler.go @@ -321,14 +321,14 @@ func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Reque l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - session = oauth.NewSession(provider.Provider, data.Code, authReq.SelectedIDPConfigArgs) + session = github.NewSession(provider, data.Code, authReq.SelectedIDPConfigArgs) case domain.IDPTypeGitHubEnterprise: provider, err := l.githubEnterpriseProvider(r.Context(), identityProvider) if err != nil { l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } - session = oauth.NewSession(provider.Provider, data.Code, authReq.SelectedIDPConfigArgs) + session = github.NewSession(provider, data.Code, authReq.SelectedIDPConfigArgs) case domain.IDPTypeGitLab: provider, err := l.gitlabProvider(r.Context(), identityProvider) if err != nil { @@ -1324,6 +1324,8 @@ func tokens(session idp.Session) *oidc.Tokens[*oidc.IDTokenClaims] { return s.Tokens case *oauth.Session: return s.Tokens + case *github.Session: + return s.Tokens() case *azuread.Session: return s.Tokens() case *apple.Session: diff --git a/internal/command/idp_intent.go b/internal/command/idp_intent.go index 9690117edd0..36948a6fd52 100644 --- a/internal/command/idp_intent.go +++ b/internal/command/idp_intent.go @@ -17,6 +17,7 @@ import ( "github.com/zitadel/zitadel/internal/idp" "github.com/zitadel/zitadel/internal/idp/providers/apple" "github.com/zitadel/zitadel/internal/idp/providers/azuread" + "github.com/zitadel/zitadel/internal/idp/providers/github" "github.com/zitadel/zitadel/internal/idp/providers/jwt" "github.com/zitadel/zitadel/internal/idp/providers/ldap" "github.com/zitadel/zitadel/internal/idp/providers/oauth" @@ -299,6 +300,8 @@ func tokensForSucceededIDPIntent(session idp.Session, encryptionAlg crypto.Encry tokens = s.Tokens case *jwt.Session: tokens = s.Tokens + case *github.Session: + tokens = s.Tokens() case *azuread.Session: tokens = s.Tokens() case *apple.Session: diff --git a/internal/command/idp_intent_test.go b/internal/command/idp_intent_test.go index df976559465..000a48025f3 100644 --- a/internal/command/idp_intent_test.go +++ b/internal/command/idp_intent_test.go @@ -24,6 +24,7 @@ import ( "github.com/zitadel/zitadel/internal/id/mock" "github.com/zitadel/zitadel/internal/idp" "github.com/zitadel/zitadel/internal/idp/providers/azuread" + "github.com/zitadel/zitadel/internal/idp/providers/github" "github.com/zitadel/zitadel/internal/idp/providers/jwt" "github.com/zitadel/zitadel/internal/idp/providers/ldap" "github.com/zitadel/zitadel/internal/idp/providers/oauth" @@ -1533,6 +1534,31 @@ func Test_tokensForSucceededIDPIntent(t *testing.T) { err: nil, }, }, + { + "github tokens", + args{ + &github.Session{ + OAuthSession: &oauth.Session{ + Tokens: &oidc.Tokens[*oidc.IDTokenClaims]{ + Token: &oauth2.Token{ + AccessToken: "accessToken", + }, + }, + }, + }, + crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + res{ + accessToken: &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("accessToken"), + }, + idToken: "", + err: nil, + }, + }, { "azure tokens", args{ diff --git a/internal/idp/providers/github/github.go b/internal/idp/providers/github/github.go index 64e02941bc5..4559e576f85 100644 --- a/internal/idp/providers/github/github.go +++ b/internal/idp/providers/github/github.go @@ -13,10 +13,11 @@ import ( ) const ( - authURL = "https://github.com/login/oauth/authorize" - tokenURL = "https://github.com/login/oauth/access_token" - profileURL = "https://api.github.com/user" - name = "GitHub" + authURL = "https://github.com/login/oauth/authorize" + tokenURL = "https://github.com/login/oauth/access_token" + profileURL = "https://api.github.com/user" + emailURLSuffix = "/emails" + name = "GitHub" ) var _ idp.Provider = (*Provider)(nil) @@ -43,12 +44,14 @@ func NewCustomURL(name, clientID, secret, callbackURL, authURL, tokenURL, profil } return &Provider{ Provider: rp, + emailURL: profileURL + emailURLSuffix, }, nil } // Provider is the [idp.Provider] implementation for GitHub type Provider struct { *oauth.Provider + emailURL string } func newConfig(clientID, secret, callbackURL, authURL, tokenURL string, scopes []string) *oauth2.Config { diff --git a/internal/idp/providers/github/session.go b/internal/idp/providers/github/session.go new file mode 100644 index 00000000000..b1b7ac90518 --- /dev/null +++ b/internal/idp/providers/github/session.go @@ -0,0 +1,113 @@ +package github + +import ( + "context" + "net/http" + "strings" + "time" + + httphelper "github.com/zitadel/oidc/v3/pkg/http" + "github.com/zitadel/oidc/v3/pkg/oidc" + + "github.com/zitadel/zitadel/internal/domain" + "github.com/zitadel/zitadel/internal/idp" + "github.com/zitadel/zitadel/internal/idp/providers/oauth" +) + +var _ idp.Session = (*Session)(nil) + +// Session extends the [oauth.Session] to be able to handle private email addresses. +type Session struct { + *Provider + Code string + IDPArguments map[string]any + + OAuthSession *oauth.Session +} + +func NewSession(provider *Provider, code string, idpArguments map[string]any) *Session { + return &Session{Provider: provider, Code: code, IDPArguments: idpArguments} +} + +// GetAuth implements the [idp.Provider] interface by calling the wrapped [oauth.Session]. +func (s *Session) GetAuth(ctx context.Context) (idp.Auth, error) { + return s.oauth().GetAuth(ctx) +} + +// PersistentParameters implements the [idp.Session] interface by calling the wrapped [oauth.Session]. +func (s *Session) PersistentParameters() map[string]any { + return s.oauth().PersistentParameters() +} + +// FetchUser implements the [idp.Session] interface. +// It will execute an OAuth 2.0 code exchange if needed to retrieve the access token, +// call the specified userEndpoint and map the received information into an [idp.User]. +// In case of a specific TenantID as [TenantType] it will additionally extract the id_token and validate it. +func (s *Session) FetchUser(ctx context.Context) (user idp.User, err error) { + user, err = s.oauth().FetchUser(ctx) + if err != nil { + return nil, err + } + if user.GetEmail() == "" { + userInternal, ok := user.(*User) + if !ok { + return user, nil + } + for _, scope := range s.Provider.Provider.OAuthConfig().Scopes { + if strings.TrimSpace(scope) == "user:email" { + email, err := s.getPrivateMail() + if err != nil { + return user, nil + } + userInternal.Email = domain.EmailAddress(email) + return userInternal, nil + } + } + } + return user, nil +} + +func (s *Session) ExpiresAt() time.Time { + if s.OAuthSession == nil { + return time.Time{} + } + return s.OAuthSession.ExpiresAt() +} + +// Tokens returns the [oidc.Tokens] of the underlying [oauth.Session]. +func (s *Session) Tokens() *oidc.Tokens[*oidc.IDTokenClaims] { + return s.oauth().Tokens +} + +func (s *Session) oauth() *oauth.Session { + if s.OAuthSession != nil { + return s.OAuthSession + } + s.OAuthSession = oauth.NewSession(s.Provider.Provider, s.Code, s.IDPArguments) + return s.OAuthSession +} + +type Email struct { + Email string `json:"email"` + Primary bool `json:"primary"` + Verified bool `json:"verified"` +} + +func (s *Session) getPrivateMail() (email string, err error) { + req, err := http.NewRequest("GET", s.Provider.emailURL, nil) + if err != nil { + return "", err + } + req.Header.Set("authorization", s.oauth().Tokens.TokenType+" "+s.oauth().Tokens.AccessToken) + + emailList := make([]Email, 0) + if err := httphelper.HttpRequest(s.Provider.HttpClient(), req, &emailList); err != nil { + return "", err + } + for _, v := range emailList { + if v.Primary && v.Verified { + return v.Email, nil + } + } + return email, nil +} diff --git a/internal/idp/providers/github/session_test.go b/internal/idp/providers/github/session_test.go index 247ef35c681..da4d40a6f18 100644 --- a/internal/idp/providers/github/session_test.go +++ b/internal/idp/providers/github/session_test.go @@ -21,15 +21,16 @@ import ( func TestSession_FetchUser(t *testing.T) { type fields struct { - clientID string - clientSecret string - redirectURI string - httpMock func() - authURL string - code string - tokens *oidc.Tokens[*oidc.IDTokenClaims] - scopes []string - options []oauth.ProviderOpts + clientID string + clientSecret string + redirectURI string + httpMock func() + httpEmailMock func() + authURL string + code string + tokens *oidc.Tokens[*oidc.IDTokenClaims] + scopes []string + options []oauth.ProviderOpts } type args struct { session idp.Session @@ -73,7 +74,7 @@ func TestSession_FetchUser(t *testing.T) { tokens: nil, }, args: args{ - &oauth.Session{}, + &Session{}, }, want: want{ err: func(err error) bool { @@ -101,7 +102,7 @@ func TestSession_FetchUser(t *testing.T) { }, }, args: args{ - &oauth.Session{}, + &Session{}, }, want: want{ err: func(err error) bool { @@ -130,7 +131,7 @@ func TestSession_FetchUser(t *testing.T) { }, }, args: args{ - &oauth.Session{}, + &Session{}, }, want: want{ user: &User{ @@ -159,44 +160,277 @@ func TestSession_FetchUser(t *testing.T) { profile: "htmlURL", }, }, + { + name: "successful fetch, no verified email", + fields: fields{ + clientID: "clientID", + clientSecret: "clientSecret", + redirectURI: "redirectURI", + httpMock: func() { + gock.New("https://api.github.com"). + Get("/user"). + Reply(200). + JSON(userinfoWithoutEmail()) + }, + httpEmailMock: func() { + gock.New("https://api.github.com"). + Get("/user/emails"). + Reply(200). + JSON(emailList()) + }, + authURL: "https://github.com/login/oauth/authorize?client_id=clientID&redirect_uri=redirectURI&response_type=code&state=testState", + tokens: &oidc.Tokens[*oidc.IDTokenClaims]{ + Token: &oauth2.Token{ + AccessToken: "accessToken", + TokenType: oidc.BearerToken, + }, + }, + }, + args: args{ + &Session{}, + }, + want: want{ + user: &User{ + Login: "login", + ID: 1, + AvatarUrl: "avatarURL", + GravatarId: "gravatarID", + Name: "name", + Email: "", + HtmlUrl: "htmlURL", + CreatedAt: time.Date(2023, 01, 10, 11, 10, 35, 0, time.UTC), + UpdatedAt: time.Date(2023, 01, 10, 11, 10, 35, 0, time.UTC), + }, + id: "1", + firstName: "", + lastName: "", + displayName: "name", + nickName: "login", + preferredUsername: "login", + email: "", + isEmailVerified: true, + phone: "", + isPhoneVerified: false, + preferredLanguage: language.Und, + avatarURL: "avatarURL", + profile: "htmlURL", + }, + }, + { + name: "successful fetch, no user scope provided", + fields: fields{ + clientID: "clientID", + clientSecret: "clientSecret", + redirectURI: "redirectURI", + httpMock: func() { + gock.New("https://api.github.com"). + Get("/user"). + Reply(200). + JSON(userinfoWithoutEmail()) + }, + httpEmailMock: func() { + gock.New("https://api.github.com"). + Get("/user/email"). + Reply(200). + JSON(emailListVerified()) + }, + authURL: "https://github.com/login/oauth/authorize?client_id=clientID&redirect_uri=redirectURI&response_type=code&state=testState", + tokens: &oidc.Tokens[*oidc.IDTokenClaims]{ + Token: &oauth2.Token{ + AccessToken: "accessToken", + TokenType: oidc.BearerToken, + }, + }, + }, + args: args{ + &Session{}, + }, + want: want{ + user: &User{ + Login: "login", + ID: 1, + AvatarUrl: "avatarURL", + GravatarId: "gravatarID", + Name: "name", + Email: "", + HtmlUrl: "htmlURL", + CreatedAt: time.Date(2023, 01, 10, 11, 10, 35, 0, time.UTC), + UpdatedAt: time.Date(2023, 01, 10, 11, 10, 35, 0, time.UTC), + }, + id: "1", + firstName: "", + lastName: "", + displayName: "name", + nickName: "login", + preferredUsername: "login", + email: "", + isEmailVerified: true, + phone: "", + isPhoneVerified: false, + preferredLanguage: language.Und, + avatarURL: "avatarURL", + profile: "htmlURL", + }, + }, + { + name: "successful fetch, private email", + fields: fields{ + clientID: "clientID", + clientSecret: "clientSecret", + redirectURI: "redirectURI", + httpMock: func() { + gock.New("https://api.github.com"). + Get("/user"). + Reply(200). + JSON(userinfoWithoutEmail()) + }, + httpEmailMock: func() { + gock.New("https://api.github.com"). + Get("/user/email"). + Reply(200). + JSON(emailListVerified()) + }, + authURL: "https://github.com/login/oauth/authorize?client_id=clientID&redirect_uri=redirectURI&response_type=code&state=testState", + tokens: &oidc.Tokens[*oidc.IDTokenClaims]{ + Token: &oauth2.Token{ + AccessToken: "accessToken", + TokenType: oidc.BearerToken, + }, + }, + scopes: []string{"user:email"}, + }, + args: args{ + &Session{}, + }, + want: want{ + user: &User{ + Login: "login", + ID: 1, + AvatarUrl: "avatarURL", + GravatarId: "gravatarID", + Name: "name", + Email: "primaryandverfied@example.com", + HtmlUrl: "htmlURL", + CreatedAt: time.Date(2023, 01, 10, 11, 10, 35, 0, time.UTC), + UpdatedAt: time.Date(2023, 01, 10, 11, 10, 35, 0, time.UTC), + }, + id: "1", + firstName: "", + lastName: "", + displayName: "name", + nickName: "login", + preferredUsername: "login", + email: "primaryandverfied@example.com", + isEmailVerified: true, + phone: "", + isPhoneVerified: false, + preferredLanguage: language.Und, + avatarURL: "avatarURL", + profile: "htmlURL", + }, + }, + { + name: "successful fetch, unsuccessful email", + fields: fields{ + clientID: "clientID", + clientSecret: "clientSecret", + redirectURI: "redirectURI", + httpMock: func() { + gock.New("https://api.github.com"). + Get("/user"). + Reply(200). + JSON(userinfoWithoutEmail()) + }, + httpEmailMock: func() { + gock.New("https://api.github.com"). + Get("/user/email"). + Reply(400) + }, + authURL: "https://github.com/login/oauth/authorize?client_id=clientID&redirect_uri=redirectURI&response_type=code&state=testState", + tokens: &oidc.Tokens[*oidc.IDTokenClaims]{ + Token: &oauth2.Token{ + AccessToken: "accessToken", + TokenType: oidc.BearerToken, + }, + }, + scopes: []string{"user"}, + }, + args: args{ + &Session{}, + }, + want: want{ + user: &User{ + Login: "login", + ID: 1, + AvatarUrl: "avatarURL", + GravatarId: "gravatarID", + Name: "name", + Email: "", + HtmlUrl: "htmlURL", + CreatedAt: time.Date(2023, 01, 10, 11, 10, 35, 0, time.UTC), + UpdatedAt: time.Date(2023, 01, 10, 11, 10, 35, 0, time.UTC), + }, + id: "1", + firstName: "", + lastName: "", + displayName: "name", + nickName: "login", + preferredUsername: "login", + email: "", + isEmailVerified: true, + phone: "", + isPhoneVerified: false, + preferredLanguage: language.Und, + avatarURL: "avatarURL", + profile: "htmlURL", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { defer gock.Off() tt.fields.httpMock() + if tt.fields.httpEmailMock != nil { + tt.fields.httpEmailMock() + } a := assert.New(t) provider, err := New(tt.fields.clientID, tt.fields.clientSecret, tt.fields.redirectURI, tt.fields.scopes, tt.fields.options...) require.NoError(t, err) - session := &oauth.Session{ - AuthURL: tt.fields.authURL, + session := &Session{ + OAuthSession: &oauth.Session{ + AuthURL: tt.fields.authURL, + Tokens: tt.fields.tokens, + Provider: provider.Provider, + Code: tt.fields.code, + }, Code: tt.fields.code, - Tokens: tt.fields.tokens, - Provider: provider.Provider, + Provider: provider, } user, err := session.FetchUser(context.Background()) - if tt.want.err != nil && !tt.want.err(err) { - a.Fail("invalid error", err) - } - if tt.want.err == nil { - a.NoError(err) - a.Equal(tt.want.user, user) - a.Equal(tt.want.id, user.GetID()) - a.Equal(tt.want.firstName, user.GetFirstName()) - a.Equal(tt.want.lastName, user.GetLastName()) - a.Equal(tt.want.displayName, user.GetDisplayName()) - a.Equal(tt.want.nickName, user.GetNickname()) - a.Equal(tt.want.preferredUsername, user.GetPreferredUsername()) - a.Equal(domain.EmailAddress(tt.want.email), user.GetEmail()) - a.Equal(tt.want.isEmailVerified, user.IsEmailVerified()) - a.Equal(domain.PhoneNumber(tt.want.phone), user.GetPhone()) - a.Equal(tt.want.isPhoneVerified, user.IsPhoneVerified()) - a.Equal(tt.want.preferredLanguage, user.GetPreferredLanguage()) - a.Equal(tt.want.avatarURL, user.GetAvatarURL()) - a.Equal(tt.want.profile, user.GetProfile()) + if tt.want.err != nil { + a.True(tt.want.err(err), "invalid err") + return } + + a.NoError(err) + a.Equal(tt.want.user, user) + a.Equal(tt.want.id, user.GetID()) + a.Equal(tt.want.firstName, user.GetFirstName()) + a.Equal(tt.want.lastName, user.GetLastName()) + a.Equal(tt.want.displayName, user.GetDisplayName()) + a.Equal(tt.want.nickName, user.GetNickname()) + a.Equal(tt.want.preferredUsername, user.GetPreferredUsername()) + a.Equal(domain.EmailAddress(tt.want.email), user.GetEmail()) + a.Equal(tt.want.isEmailVerified, user.IsEmailVerified()) + a.Equal(domain.PhoneNumber(tt.want.phone), user.GetPhone()) + a.Equal(tt.want.isPhoneVerified, user.IsPhoneVerified()) + a.Equal(tt.want.preferredLanguage, user.GetPreferredLanguage()) + a.Equal(tt.want.avatarURL, user.GetAvatarURL()) + a.Equal(tt.want.profile, user.GetProfile()) + }) } } @@ -214,3 +448,38 @@ func userinfo() *User { UpdatedAt: time.Date(2023, 01, 10, 11, 10, 35, 0, time.UTC), } } + +func userinfoWithoutEmail() *User { + userinfo := userinfo() + userinfo.Email = "" + return userinfo +} + +func emailListVerified() []Email { + return append(emailList(), + Email{ + Email: "primaryandverfied@example.com", + Primary: true, + Verified: true, + }) +} + +func emailList() []Email { + return []Email{ + { + Email: "notverified@example.com", + Primary: false, + Verified: false, + }, + { + Email: "verfied@example.com", + Primary: false, + Verified: true, + }, + { + Email: "primary@example.com", + Primary: true, + Verified: false, + }, + } +}