Skip to content

Commit

Permalink
[OIDC] IDP: use workspace.context.normalizedContextUrl to fill the "s…
Browse files Browse the repository at this point in the history
…ub" and "context" claims of the IDToken (#20111)

The motivation is to have more uniform URL shapes to match against e.g. across different IDEs.
  • Loading branch information
geropl authored Aug 13, 2024
1 parent 2d67254 commit 4524c19
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 24 deletions.
13 changes: 11 additions & 2 deletions components/public-api-server/pkg/apiv1/identityprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (srv *IdentityProviderService) GetIDToken(ctx context.Context, req *connect
userInfo.SetName(user.Name)
userInfo.AppendClaims("user_id", user.ID)
userInfo.AppendClaims("org_id", workspace.Workspace.OrganizationId)
userInfo.AppendClaims("context", workspace.Workspace.ContextURL)
userInfo.AppendClaims("context", getContext(workspace))
userInfo.AppendClaims("workspace_id", workspaceID)

if req.Msg.GetScope() != "" {
Expand Down Expand Up @@ -127,7 +127,7 @@ func (srv *IdentityProviderService) getOIDCSubject(ctx context.Context, userInfo
UserID: user.ID,
TeamID: workspace.Workspace.OrganizationId,
})
subject := workspace.Workspace.ContextURL
subject := getContext(workspace)
if len(claimKeys) != 0 {
subArr := []string{}
for _, key := range claimKeys {
Expand All @@ -141,3 +141,12 @@ func (srv *IdentityProviderService) getOIDCSubject(ctx context.Context, userInfo
}
return subject
}

func getContext(workspace *protocol.WorkspaceInfo) string {
context := "no-context"
if workspace.Workspace.Context != nil && workspace.Workspace.Context.NormalizedContextURL != "" {
// using Workspace.Context.NormalizedContextURL to not include prefixes (like "referrer:jetbrains-gateway", or other prefix contexts)
context = workspace.Workspace.Context.NormalizedContextURL
}
return context
}
81 changes: 59 additions & 22 deletions components/public-api-server/pkg/apiv1/identityprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func TestGetIDToken(t *testing.T) {
Repository: &protocol.Repository{
CloneURL: "https://github.com/gitpod-io/gitpod.git",
},
NormalizedContextURL: "https://github.com/gitpod-io/gitpod",
},
},
},
Expand Down Expand Up @@ -112,6 +113,7 @@ func TestGetIDToken(t *testing.T) {
Repository: &protocol.Repository{
CloneURL: "https://github.com/gitpod-io/gitpod.git",
},
NormalizedContextURL: "https://github.com/gitpod-io/gitpod",
},
},
},
Expand Down Expand Up @@ -174,6 +176,9 @@ func TestGetIDToken(t *testing.T) {
&protocol.WorkspaceInfo{
Workspace: &protocol.Workspace{
ContextURL: "https://github.com/gitpod-io/gitpod",
Context: &protocol.WorkspaceContext{
NormalizedContextURL: "https://github.com/gitpod-io/gitpod",
},
},
},
nil,
Expand Down Expand Up @@ -226,6 +231,7 @@ func TestGetIDToken(t *testing.T) {
Repository: &protocol.Repository{
CloneURL: "https://github.com/gitpod-io/gitpod.git",
},
NormalizedContextURL: "https://github.com/gitpod-io/gitpod",
},
},
},
Expand Down Expand Up @@ -268,6 +274,9 @@ func TestGetIDToken(t *testing.T) {
&protocol.WorkspaceInfo{
Workspace: &protocol.Workspace{
ContextURL: "https://github.com/gitpod-io/gitpod",
Context: &protocol.WorkspaceContext{
NormalizedContextURL: "https://github.com/gitpod-io/gitpod",
},
},
},
nil,
Expand Down Expand Up @@ -344,36 +353,64 @@ func (f functionIDTokenSource) IDToken(ctx context.Context, org string, audience
}

func TestGetOIDCSubject(t *testing.T) {
contextUrl := "https://github.com/gitpod-io/gitpod"
normalizedContextUrl := "https://github.com/gitpod-io/gitpod"
defaultWorkspace := &protocol.Workspace{
ContextURL: "SOME_ENV=test/" + normalizedContextUrl,
Context: &protocol.WorkspaceContext{
NormalizedContextURL: normalizedContextUrl,
}}
tests := []struct {
Name string
Keys string
Claims map[string]interface{}
Subject string
Name string
Keys string
Claims map[string]interface{}
Subject string
Workspace *protocol.Workspace
}{
{
Name: "happy path",
Keys: "",
Claims: map[string]interface{}{},
Subject: contextUrl,
Name: "happy path",
Keys: "",
Claims: map[string]interface{}{},
Subject: normalizedContextUrl,
Workspace: defaultWorkspace,
},
{
Name: "happy path 2",
Keys: "undefined",
Claims: map[string]interface{}{},
Subject: contextUrl,
Name: "happy path 2",
Keys: "undefined",
Claims: map[string]interface{}{},
Subject: normalizedContextUrl,
Workspace: defaultWorkspace,
},
{
Name: "with custom keys",
Keys: "key1,key3,key2",
Claims: map[string]interface{}{"key1": 1, "key2": "hello"},
Subject: "key1:1:key3::key2:hello",
Name: "with custom keys",
Keys: "key1,key3,key2",
Claims: map[string]interface{}{"key1": 1, "key2": "hello"},
Subject: "key1:1:key3::key2:hello",
Workspace: defaultWorkspace,
},
{
Name: "with custom keys",
Keys: "key1,key3,key2",
Claims: map[string]interface{}{"key1": 1, "key3": errors.New("test")},
Subject: "key1:1:key3:test:key2:",
Name: "with custom keys",
Keys: "key1,key3,key2",
Claims: map[string]interface{}{"key1": 1, "key3": errors.New("test")},
Subject: "key1:1:key3:test:key2:",
Workspace: defaultWorkspace,
},
{
Name: "happy path with strange prefix",
Keys: "",
Claims: map[string]interface{}{},
Subject: normalizedContextUrl,
Workspace: &protocol.Workspace{ContextURL: "referrer:jetbrains-gateway:intellij/" + normalizedContextUrl, Context: &protocol.WorkspaceContext{
NormalizedContextURL: normalizedContextUrl,
}},
},
{
Name: "happy path without NormalizedContextURL",
Keys: "",
Claims: map[string]interface{}{},
Subject: "no-context",
Workspace: &protocol.Workspace{ContextURL: "referrer:jetbrains-gateway:intellij/" + normalizedContextUrl, Context: &protocol.WorkspaceContext{
NormalizedContextURL: "",
}},
},
}

Expand All @@ -389,7 +426,7 @@ func TestGetOIDCSubject(t *testing.T) {
userinfo.AppendClaims(k, v)
}
act := svc.getOIDCSubject(context.Background(), userinfo, &protocol.User{}, &protocol.WorkspaceInfo{
Workspace: &protocol.Workspace{ContextURL: contextUrl},
Workspace: test.Workspace,
})
if diff := cmp.Diff(test.Subject, act); diff != "" {
t.Errorf("getOIDCSubject() mismatch (-want +got):\n%s", diff)
Expand Down

0 comments on commit 4524c19

Please sign in to comment.