Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[idp] Add email to ID token #17678

Merged
merged 3 commits into from
May 19, 2023
+48 −17
Merged

[idp] Add email to ID token #17678

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
91fe278
[idp] Add email to ID token
csweichel May 19, 2023
69e25fb
[idp] Claim that the email is verified
csweichel May 19, 2023
a399933
Incorporate review feedback
csweichel May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions components/public-api-server/pkg/apiv1/identityprovider.go
View file
Open in desktop
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 != "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could drop the if, it would set it to the empty string, which is the default value in the struct anyway - it's omitempty in the JSON struct

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. I like the explicitness of this code though :)

userInfo.SetEmail(email, false)
easyCZ marked this conversation as resolved.
Show resolved Hide resolved
}

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
View file
Open in desktop
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.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(
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]"},
{PrimaryEmail: "[email protected]"},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{PrimaryEmail: "[email protected]"},
{Deleted: false, PrimaryEmail: "[email protected]"},

I know it's the default, but visually grepping the tests the symmetry makes it easier

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