Skip to content

Commit

Permalink
[idp] Add email to ID token (#17678)
Browse files Browse the repository at this point in the history
* [idp] Add email to ID token

* [idp] Claim that the email is verified

which is a valid claim because it's verified by the original IDP,
and this is not the user-editable value.

* Incorporate review feedback
  • Loading branch information
csweichel authored May 19, 2023
1 parent 1a7c50a commit 36905ac
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 17 deletions.
12 changes: 12 additions & 0 deletions components/public-api-server/pkg/apiv1/identityprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ func (srv *IdentityProviderService) GetIDToken(ctx context.Context, req *connect
return nil, proxy.ConvertError(err)
}

var email string
for _, id := range user.Identities {
if id == nil || id.Deleted || id.PrimaryEmail == "" {
continue
}
email = id.PrimaryEmail
break
}

if workspace.Workspace == nil {
log.Extract(ctx).WithError(err).Error("Server did not return a workspace.")
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("workspace not found"))
Expand All @@ -79,6 +88,9 @@ func (srv *IdentityProviderService) GetIDToken(ctx context.Context, req *connect
userInfo := oidc.NewUserInfo()
userInfo.SetName(user.Name)
userInfo.SetSubject(subject)
if email != "" {
userInfo.SetEmail(email, true)
}

token, err := srv.idTokenSource.IDToken(ctx, "gitpod", req.Msg.Audience, userInfo)
if err != nil {
Expand Down
53 changes: 36 additions & 17 deletions components/public-api-server/pkg/apiv1/identityprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/sourcegraph/jsonrpc2"
"github.com/stretchr/testify/require"
"github.com/zitadel/oidc/pkg/oidc"
)

Expand All @@ -31,17 +32,22 @@ func TestGetIDToken(t *testing.T) {
}
tests := []struct {
Name string
TokenSource IDTokenSource
TokenSource func(t *testing.T) IDTokenSource
ServerSetup func(*protocol.MockAPIInterface)
Request *v1.GetIDTokenRequest

Expectation Expectation
}{
{
Name: "happy path",
TokenSource: functionIDTokenSource(func(ctx context.Context, org string, audience []string, userInfo oidc.UserInfo) (string, error) {
return "foobar", nil
}),
TokenSource: func(t *testing.T) IDTokenSource {
return functionIDTokenSource(func(ctx context.Context, org string, audience []string, userInfo oidc.UserInfo) (string, error) {
require.Equal(t, "[email protected]", userInfo.GetEmail())
require.True(t, userInfo.IsEmailVerified())

return "foobar", nil
})
},
ServerSetup: func(ma *protocol.MockAPIInterface) {
ma.EXPECT().GetIDToken(gomock.Any()).MinTimes(1).Return(nil)
ma.EXPECT().GetWorkspace(gomock.Any(), workspaceID).MinTimes(1).Return(
Expand All @@ -55,6 +61,11 @@ func TestGetIDToken(t *testing.T) {
ma.EXPECT().GetLoggedInUser(gomock.Any()).Return(
&protocol.User{
Name: "foobar",
Identities: []*protocol.Identity{
nil,
{Deleted: true, PrimaryEmail: "[email protected]"},
{Deleted: false, PrimaryEmail: "[email protected]"},
},
},
nil,
)
Expand All @@ -71,9 +82,11 @@ func TestGetIDToken(t *testing.T) {
},
{
Name: "workspace not found",
TokenSource: functionIDTokenSource(func(ctx context.Context, org string, audience []string, userInfo oidc.UserInfo) (string, error) {
return "foobar", nil
}),
TokenSource: func(t *testing.T) IDTokenSource {
return functionIDTokenSource(func(ctx context.Context, org string, audience []string, userInfo oidc.UserInfo) (string, error) {
return "foobar", nil
})
},
ServerSetup: func(ma *protocol.MockAPIInterface) {
ma.EXPECT().GetIDToken(gomock.Any()).MinTimes(1).Return(nil)
ma.EXPECT().GetWorkspace(gomock.Any(), workspaceID).MinTimes(1).Return(
Expand All @@ -91,9 +104,11 @@ func TestGetIDToken(t *testing.T) {
},
{
Name: "no logged in user",
TokenSource: functionIDTokenSource(func(ctx context.Context, org string, audience []string, userInfo oidc.UserInfo) (string, error) {
return "foobar", nil
}),
TokenSource: func(t *testing.T) IDTokenSource {
return functionIDTokenSource(func(ctx context.Context, org string, audience []string, userInfo oidc.UserInfo) (string, error) {
return "foobar", nil
})
},
ServerSetup: func(ma *protocol.MockAPIInterface) {
ma.EXPECT().GetIDToken(gomock.Any()).MinTimes(1).Return(nil)
ma.EXPECT().GetWorkspace(gomock.Any(), workspaceID).MinTimes(1).Return(
Expand All @@ -119,9 +134,11 @@ func TestGetIDToken(t *testing.T) {
},
{
Name: "no audience",
TokenSource: functionIDTokenSource(func(ctx context.Context, org string, audience []string, userInfo oidc.UserInfo) (string, error) {
return "foobar", nil
}),
TokenSource: func(t *testing.T) IDTokenSource {
return functionIDTokenSource(func(ctx context.Context, org string, audience []string, userInfo oidc.UserInfo) (string, error) {
return "foobar", nil
})
},
Request: &v1.GetIDTokenRequest{
WorkspaceId: workspaceID,
},
Expand All @@ -131,9 +148,11 @@ func TestGetIDToken(t *testing.T) {
},
{
Name: "token source error",
TokenSource: functionIDTokenSource(func(ctx context.Context, org string, audience []string, userInfo oidc.UserInfo) (string, error) {
return "", fmt.Errorf("cannot produce token")
}),
TokenSource: func(t *testing.T) IDTokenSource {
return functionIDTokenSource(func(ctx context.Context, org string, audience []string, userInfo oidc.UserInfo) (string, error) {
return "", fmt.Errorf("cannot produce token")
})
},
ServerSetup: func(ma *protocol.MockAPIInterface) {
ma.EXPECT().GetIDToken(gomock.Any()).MinTimes(1).Return(nil)
ma.EXPECT().GetWorkspace(gomock.Any(), workspaceID).MinTimes(1).Return(
Expand Down Expand Up @@ -170,7 +189,7 @@ func TestGetIDToken(t *testing.T) {
test.ServerSetup(serverMock)
}

svc := NewIdentityProviderService(&FakeServerConnPool{api: serverMock}, test.TokenSource)
svc := NewIdentityProviderService(&FakeServerConnPool{api: serverMock}, test.TokenSource(t))
_, handler := v1connect.NewIdentityProviderServiceHandler(svc, connect.WithInterceptors(auth.NewServerInterceptor()))
srv := httptest.NewServer(handler)
t.Cleanup(srv.Close)
Expand Down

0 comments on commit 36905ac

Please sign in to comment.