TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2025-11-01 00:46:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: add email query to github idp if email empty (#10705)

Browse Source 
# Which Problems Are Solved

In the integration with Github, private emails are not returned with the
userinfo.

# How the Problems Are Solved

If the scope `user:email` is set in the Github IDP and the email is not
included in the userinfo, a request to Github's API is executed to query
the email of the user.

# Additional Changes

Additional tests.

# Additional Context

Closes #10098

---------

Co-authored-by: Marco A. <marco@zitadel.com>
Co-authored-by: Livio Spring <livio.a@gmail.com>
This commit is contained in:
Stefan Benz
2025-10-07 08:13:06 +02:00
committed by GitHub
parent 695db96745
commit a7e1bfb4a3
7 changed files with 458 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
internal/api/idp/idp.go
View File

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

6
internal/api/ui/login/external_provider_handler.go
View File

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

3
internal/command/idp_intent.go
View File

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

26
internal/command/idp_intent_test.go
View File

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

11
internal/idp/providers/github/github.go
View File

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

113
internal/idp/providers/github/session.go Normal file
View File

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

339
internal/idp/providers/github/session_test.go
View File

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