From 91fe27849032792f6ffb07271934fc6567dc0d7c Mon Sep 17 00:00:00 2001 From: csweichel Date: Fri, 19 May 2023 12:30:13 +0000 Subject: [PATCH 1/3] [idp] Add email to ID token --- .../pkg/apiv1/identityprovider.go | 12 +++++ .../pkg/apiv1/identityprovider_test.go | 53 +++++++++++++------ 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/components/public-api-server/pkg/apiv1/identityprovider.go b/components/public-api-server/pkg/apiv1/identityprovider.go index b31ec59a186105..346edbd9dcc2cf 100644 --- a/components/public-api-server/pkg/apiv1/identityprovider.go +++ b/components/public-api-server/pkg/apiv1/identityprovider.go @@ -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")) @@ -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, false) + } token, err := srv.idTokenSource.IDToken(ctx, "gitpod", req.Msg.Audience, userInfo) if err != nil { diff --git a/components/public-api-server/pkg/apiv1/identityprovider_test.go b/components/public-api-server/pkg/apiv1/identityprovider_test.go index 5df49aa5be1741..babb833fd96fbe 100644 --- a/components/public-api-server/pkg/apiv1/identityprovider_test.go +++ b/components/public-api-server/pkg/apiv1/identityprovider_test.go @@ -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" ) @@ -31,7 +32,7 @@ func TestGetIDToken(t *testing.T) { } tests := []struct { Name string - TokenSource IDTokenSource + TokenSource func(t *testing.T) IDTokenSource ServerSetup func(*protocol.MockAPIInterface) Request *v1.GetIDTokenRequest @@ -39,9 +40,14 @@ func TestGetIDToken(t *testing.T) { }{ { 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, "correct@gitpod.io", userInfo.GetEmail()) + require.False(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( @@ -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: "nonsense@gitpod.io"}, + {PrimaryEmail: "correct@gitpod.io"}, + }, }, nil, ) @@ -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( @@ -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( @@ -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, }, @@ -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( @@ -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) From 69e25fb332fef5162810a74f684b800506f4a52a Mon Sep 17 00:00:00 2001 From: csweichel Date: Fri, 19 May 2023 13:47:52 +0000 Subject: [PATCH 2/3] [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. --- components/public-api-server/pkg/apiv1/identityprovider.go | 2 +- components/public-api-server/pkg/apiv1/identityprovider_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/public-api-server/pkg/apiv1/identityprovider.go b/components/public-api-server/pkg/apiv1/identityprovider.go index 346edbd9dcc2cf..c79f5d6b4f975a 100644 --- a/components/public-api-server/pkg/apiv1/identityprovider.go +++ b/components/public-api-server/pkg/apiv1/identityprovider.go @@ -89,7 +89,7 @@ func (srv *IdentityProviderService) GetIDToken(ctx context.Context, req *connect userInfo.SetName(user.Name) userInfo.SetSubject(subject) if email != "" { - userInfo.SetEmail(email, false) + userInfo.SetEmail(email, true) } token, err := srv.idTokenSource.IDToken(ctx, "gitpod", req.Msg.Audience, userInfo) diff --git a/components/public-api-server/pkg/apiv1/identityprovider_test.go b/components/public-api-server/pkg/apiv1/identityprovider_test.go index babb833fd96fbe..2bcbd02f7eeb13 100644 --- a/components/public-api-server/pkg/apiv1/identityprovider_test.go +++ b/components/public-api-server/pkg/apiv1/identityprovider_test.go @@ -43,7 +43,7 @@ func TestGetIDToken(t *testing.T) { TokenSource: func(t *testing.T) IDTokenSource { return functionIDTokenSource(func(ctx context.Context, org string, audience []string, userInfo oidc.UserInfo) (string, error) { require.Equal(t, "correct@gitpod.io", userInfo.GetEmail()) - require.False(t, userInfo.IsEmailVerified()) + require.True(t, userInfo.IsEmailVerified()) return "foobar", nil }) From a3999338495498a53c06adabfee31b966fa6b93b Mon Sep 17 00:00:00 2001 From: csweichel Date: Fri, 19 May 2023 14:03:29 +0000 Subject: [PATCH 3/3] Incorporate review feedback --- components/public-api-server/pkg/apiv1/identityprovider_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/public-api-server/pkg/apiv1/identityprovider_test.go b/components/public-api-server/pkg/apiv1/identityprovider_test.go index 2bcbd02f7eeb13..4b1fcb44e6af95 100644 --- a/components/public-api-server/pkg/apiv1/identityprovider_test.go +++ b/components/public-api-server/pkg/apiv1/identityprovider_test.go @@ -64,7 +64,7 @@ func TestGetIDToken(t *testing.T) { Identities: []*protocol.Identity{ nil, {Deleted: true, PrimaryEmail: "nonsense@gitpod.io"}, - {PrimaryEmail: "correct@gitpod.io"}, + {Deleted: false, PrimaryEmail: "correct@gitpod.io"}, }, }, nil,