From 1ae65f324e7133fc6d63160f916da7e9ee6a477d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 14 Nov 2024 12:06:15 -0600 Subject: [PATCH 01/10] chore: add query to fetch top level idp claim fields Used for idp sync settings. Tests WIP --- coderd/database/dbauthz/dbauthz.go | 12 ++ coderd/database/dbmem/dbmem.go | 29 ++++ coderd/database/dbmetrics/querymetrics.go | 7 + coderd/database/dbmock/dbmock.go | 15 ++ coderd/database/modelqueries.go | 7 + coderd/database/oidcclaims_test.go | 186 ++++++++++++++++++++++ coderd/database/querier.go | 4 + coderd/database/queries.sql.go | 61 +++++++ coderd/database/queries/user_links.sql | 39 +++++ 9 files changed, 360 insertions(+) create mode 100644 coderd/database/oidcclaims_test.go diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 2a9c29b175dae..4845ff22288fe 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -3283,6 +3283,18 @@ func (q *querier) ListWorkspaceAgentPortShares(ctx context.Context, workspaceID return q.db.ListWorkspaceAgentPortShares(ctx, workspaceID) } +func (q *querier) OIDCClaimFields(ctx context.Context, organizationID uuid.UUID) ([]string, error) { + resource := rbac.ResourceIdpsyncSettings + if organizationID != uuid.Nil { + resource = resource.InOrg(organizationID) + } + + if err := q.authorizeContext(ctx, policy.ActionRead, resource); err != nil { + return nil, err + } + return q.db.OIDCClaimFields(ctx, organizationID) +} + func (q *querier) OrganizationMembers(ctx context.Context, arg database.OrganizationMembersParams) ([]database.OrganizationMembersRow, error) { return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.OrganizationMembers)(ctx, arg) } diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 64310cf36445a..24fb93f92ddfe 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -8409,6 +8409,35 @@ func (q *FakeQuerier) ListWorkspaceAgentPortShares(_ context.Context, workspaceI return shares, nil } +func (q *FakeQuerier) OIDCClaimFields(ctx context.Context, organizationID uuid.UUID) ([]string, error) { + orgMembers := q.getOrganizationMemberNoLock(organizationID) + + var fields []string + for _, link := range q.userLinks { + if organizationID != uuid.Nil { + inOrg := slices.ContainsFunc(orgMembers, func(organizationMember database.OrganizationMember) bool { + return organizationMember.UserID == link.UserID + }) + if !inOrg { + continue + } + } + + if link.LoginType != database.LoginTypeOIDC { + continue + } + + for k := range link.Claims.IDTokenClaims { + fields = append(fields, k) + } + for k := range link.Claims.UserInfoClaims { + fields = append(fields, k) + } + } + + return slice.Unique(fields), nil +} + func (q *FakeQuerier) OrganizationMembers(_ context.Context, arg database.OrganizationMembersParams) ([]database.OrganizationMembersRow, error) { if err := validateDatabaseType(arg); err != nil { return []database.OrganizationMembersRow{}, err diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index d03e481628991..32d3cce658525 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -2058,6 +2058,13 @@ func (m queryMetricsStore) ListWorkspaceAgentPortShares(ctx context.Context, wor return r0, r1 } +func (m queryMetricsStore) OIDCClaimFields(ctx context.Context, organizationID uuid.UUID) ([]string, error) { + start := time.Now() + r0, r1 := m.s.OIDCClaimFields(ctx, organizationID) + m.queryLatencies.WithLabelValues("OIDCClaimFields").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) OrganizationMembers(ctx context.Context, arg database.OrganizationMembersParams) ([]database.OrganizationMembersRow, error) { start := time.Now() r0, r1 := m.s.OrganizationMembers(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 04bf967caf26b..d6c34411f8208 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -4359,6 +4359,21 @@ func (mr *MockStoreMockRecorder) ListWorkspaceAgentPortShares(arg0, arg1 any) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListWorkspaceAgentPortShares", reflect.TypeOf((*MockStore)(nil).ListWorkspaceAgentPortShares), arg0, arg1) } +// OIDCClaimFields mocks base method. +func (m *MockStore) OIDCClaimFields(arg0 context.Context, arg1 uuid.UUID) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "OIDCClaimFields", arg0, arg1) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// OIDCClaimFields indicates an expected call of OIDCClaimFields. +func (mr *MockStoreMockRecorder) OIDCClaimFields(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "OIDCClaimFields", reflect.TypeOf((*MockStore)(nil).OIDCClaimFields), arg0, arg1) +} + // OrganizationMembers mocks base method. func (m *MockStore) OrganizationMembers(arg0 context.Context, arg1 database.OrganizationMembersParams) ([]database.OrganizationMembersRow, error) { m.ctrl.T.Helper() diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index e687994778017..99a03fe9b614b 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -3,6 +3,7 @@ package database import ( "context" "database/sql" + "encoding/json" "fmt" "strings" @@ -527,3 +528,9 @@ func insertAuthorizedFilter(query string, replaceWith string) (string, error) { filtered := strings.Replace(query, authorizedQueryPlaceholder, replaceWith, 1) return filtered, nil } + +// UpdateUserLinkRawJSON is a custom query for unit testing. Do not ever expose this +func (q *sqlQuerier) UpdateUserLinkRawJSON(ctx context.Context, userID uuid.UUID, data json.RawMessage) error { + _, err := q.sdb.Exec("INSERT INTO user_links (user_id, claims) VALUES ($1, $2)", userID, data) + return err +} diff --git a/coderd/database/oidcclaims_test.go b/coderd/database/oidcclaims_test.go new file mode 100644 index 0000000000000..b480ef39eb533 --- /dev/null +++ b/coderd/database/oidcclaims_test.go @@ -0,0 +1,186 @@ +package database_test + +import ( + "context" + "encoding/json" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbfake" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/testutil" +) + +type extraKeys struct { + database.UserLinkClaims + Foo string `json:"foo"` +} + +func TestOIDCClaims(t *testing.T) { + t.Parallel() + + toJSON := func(a any) json.RawMessage { + b, _ := json.Marshal(a) + return b + } + + db, _ := dbtestutil.NewDB(t) + g := userGenerator{t: t, db: db} + + // https://en.wikipedia.org/wiki/Alice_and_Bob#Cast_of_characters + alice := g.withLink(database.LoginTypeOIDC, toJSON(extraKeys{ + UserLinkClaims: database.UserLinkClaims{ + IDTokenClaims: map[string]interface{}{ + "sub": "alice", + }, + UserInfoClaims: map[string]interface{}{ + "sub": "alice", + }, + }, + // Always should be a no-op + Foo: "bar", + })) + bob := g.withLink(database.LoginTypeOIDC, toJSON(database.UserLinkClaims{ + IDTokenClaims: map[string]interface{}{ + "sub": "bob", + }, + UserInfoClaims: map[string]interface{}{ + "sub": "bob", + }, + })) + charlie := g.withLink(database.LoginTypeOIDC, toJSON(database.UserLinkClaims{ + IDTokenClaims: map[string]interface{}{ + "sub": "charlie", + }, + UserInfoClaims: map[string]interface{}{ + "sub": "charlie", + }, + })) + + // users that just try to cause problems, but should not affect the output of + // queries. + problematics := []database.User{ + g.withLink(database.LoginTypeOIDC, toJSON(database.UserLinkClaims{})), // null claims + g.withLink(database.LoginTypeOIDC, []byte(`{}`)), // empty claims + g.withLink(database.LoginTypeOIDC, []byte(`{"foo": "bar"}`)), // random keys + g.noLink(database.LoginTypeOIDC), // no link + + g.withLink(database.LoginTypeGithub, toJSON(database.UserLinkClaims{ + IDTokenClaims: map[string]interface{}{ + "not": "allowed", + }, + UserInfoClaims: map[string]interface{}{ + "do-not": "look", + }, + })), // github should be omitted + + // extra random users + g.noLink(database.LoginTypeGithub), + g.noLink(database.LoginTypePassword), + } + + // Insert some orgs, users, and links + orgA := dbfake.Organization(t, db).Members( + append(problematics, + alice, + bob)..., + ).Do() + orgB := dbfake.Organization(t, db).Members( + append(problematics, + charlie, + )..., + ).Do() + + // Verify the OIDC claim fields + requireClaims(t, db, orgA.Org.ID, []string{"sub"}) + requireClaims(t, db, orgB.Org.ID, []string{"sub"}) +} + +func requireClaims(t *testing.T, db database.Store, orgID uuid.UUID, want []string) { + t.Helper() + + ctx := testutil.Context(t, testutil.WaitMedium) + got, err := db.OIDCClaimFields(ctx, orgID) + require.NoError(t, err) + + require.ElementsMatch(t, want, got) +} + +type userGenerator struct { + t *testing.T + db database.Store +} + +func (g userGenerator) noLink(lt database.LoginType) database.User { + return g.user(lt, false, nil) +} + +func (g userGenerator) withLink(lt database.LoginType, rawJSON json.RawMessage) database.User { + return g.user(lt, true, rawJSON) +} + +func (g userGenerator) user(lt database.LoginType, createLink bool, rawJSON json.RawMessage) database.User { + t := g.t + db := g.db + + t.Helper() + + u, err := db.InsertUser(context.Background(), database.InsertUserParams{ + ID: uuid.New(), + Email: testutil.GetRandomName(t), + Username: testutil.GetRandomName(t), + Name: testutil.GetRandomName(t), + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + RBACRoles: []string{}, + LoginType: lt, + Status: string(database.UserStatusActive), + }) + require.NoError(t, err) + + if !createLink { + return u + } + + link, err := db.InsertUserLink(context.Background(), database.InsertUserLinkParams{ + UserID: u.ID, + LoginType: lt, + Claims: database.UserLinkClaims{}, + }) + require.NoError(t, err) + + if sql, ok := db.(rawUpdater); ok { + // The only way to put arbitrary json into the db for testing edge cases. + // Making this a public API would be a mistake. + err = sql.UpdateUserLinkRawJSON(context.Background(), u.ID, rawJSON) + require.NoError(t, err) + } else { + // no need to test the json key logic in dbmem. Everything is type safe. + var claims database.UserLinkClaims + err := json.Unmarshal(rawJSON, &claims) + require.NoError(t, err) + + _, err = db.UpdateUserLink(context.Background(), database.UpdateUserLinkParams{ + OAuthAccessToken: link.OAuthAccessToken, + OAuthAccessTokenKeyID: link.OAuthAccessTokenKeyID, + OAuthRefreshToken: link.OAuthRefreshToken, + OAuthRefreshTokenKeyID: link.OAuthRefreshTokenKeyID, + OAuthExpiry: link.OAuthExpiry, + UserID: link.UserID, + LoginType: link.LoginType, + // The new claims + Claims: claims, + }) + require.NoError(t, err) + } + + return u +} + +type rawUpdater interface { + UpdateUserLinkRawJSON(ctx context.Context, userID uuid.UUID, data json.RawMessage) error +} diff --git a/coderd/database/querier.go b/coderd/database/querier.go index b7652865447ad..75d9aca015628 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -413,6 +413,10 @@ type sqlcQuerier interface { ListProvisionerKeysByOrganization(ctx context.Context, organizationID uuid.UUID) ([]ProvisionerKey, error) ListProvisionerKeysByOrganizationExcludeReserved(ctx context.Context, organizationID uuid.UUID) ([]ProvisionerKey, error) ListWorkspaceAgentPortShares(ctx context.Context, workspaceID uuid.UUID) ([]WorkspaceAgentPortShare, error) + // OIDCClaimFields returns a list of distinct keys in both the id_token_claims and user_info_claims fields. + // This query is used to generate the list of available sync fields for idp sync settings. + // Merge with user_info claims. + OIDCClaimFields(ctx context.Context, organizationID uuid.UUID) ([]string, error) // Arguments are optional with uuid.Nil to ignore. // - Use just 'organization_id' to get all members of an org // - Use just 'user_id' to get all orgs a user is a member of diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 94f108886ea60..a65b167dada13 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9846,6 +9846,67 @@ func (q *sqlQuerier) InsertUserLink(ctx context.Context, arg InsertUserLinkParam return i, err } +const oIDCClaimFields = `-- name: OIDCClaimFields :many +SELECT + DISTINCT jsonb_object_keys(claims->'id_token_claims') +FROM + user_links +WHERE + -- Only return rows where the top level key exists + claims ? 'id_token_claims' AND + -- 'null' is the default value for the id_token_claims field + -- jsonb 'null' is not the same as SQL NULL. Strip these out. + jsonb_typeof(claims->'id_token_claims') != 'null' AND + login_type = 'oidc' + AND CASE WHEN $1 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + user_links.user_id = ANY(SELECT organization_members.user_id FROM public.organization_members WHERE organization_id = $1) + ELSE true + END + +UNION + + -- This query is identical to the one above, except for 'user_info_claims'. + -- There might be some way to do this more concisely at a cost of readability. +SELECT + DISTINCT jsonb_object_keys(claims->'user_info_claims') +FROM + user_links +WHERE + claims ? 'user_info_claims' AND + jsonb_typeof(claims->'user_info_claims') != 'null' AND + login_type = 'oidc' + AND CASE WHEN $1 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + user_links.user_id = ANY(SELECT organization_members.user_id FROM public.organization_members WHERE organization_id = $1) + ELSE true + END +` + +// OIDCClaimFields returns a list of distinct keys in both the id_token_claims and user_info_claims fields. +// This query is used to generate the list of available sync fields for idp sync settings. +// Merge with user_info claims. +func (q *sqlQuerier) OIDCClaimFields(ctx context.Context, organizationID uuid.UUID) ([]string, error) { + rows, err := q.db.QueryContext(ctx, oIDCClaimFields, organizationID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []string + for rows.Next() { + var jsonb_object_keys string + if err := rows.Scan(&jsonb_object_keys); err != nil { + return nil, err + } + items = append(items, jsonb_object_keys) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const updateUserLink = `-- name: UpdateUserLink :one UPDATE user_links diff --git a/coderd/database/queries/user_links.sql b/coderd/database/queries/user_links.sql index d0d52c3eac054..fe7d0d62b4f7a 100644 --- a/coderd/database/queries/user_links.sql +++ b/coderd/database/queries/user_links.sql @@ -57,3 +57,42 @@ SET claims = $6 WHERE user_id = $7 AND login_type = $8 RETURNING *; + + +-- name: OIDCClaimFields :many +-- OIDCClaimFields returns a list of distinct keys in both the id_token_claims and user_info_claims fields. +-- This query is used to generate the list of available sync fields for idp sync settings. +SELECT + DISTINCT jsonb_object_keys(claims->'id_token_claims') +FROM + user_links +WHERE + -- Only return rows where the top level key exists + claims ? 'id_token_claims' AND + -- 'null' is the default value for the id_token_claims field + -- jsonb 'null' is not the same as SQL NULL. Strip these out. + jsonb_typeof(claims->'id_token_claims') != 'null' AND + login_type = 'oidc' + AND CASE WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + user_links.user_id = ANY(SELECT organization_members.user_id FROM public.organization_members WHERE organization_id = @organization_id) + ELSE true + END + +-- Merge with user_info claims. +UNION + + -- This query is identical to the one above, except for 'user_info_claims'. + -- There might be some way to do this more concisely at a cost of readability. +SELECT + DISTINCT jsonb_object_keys(claims->'user_info_claims') +FROM + user_links +WHERE + claims ? 'user_info_claims' AND + jsonb_typeof(claims->'user_info_claims') != 'null' AND + login_type = 'oidc' + AND CASE WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + user_links.user_id = ANY(SELECT organization_members.user_id FROM public.organization_members WHERE organization_id = @organization_id) + ELSE true + END +; From c9df1652d7276c081861767fd5358dfcb05b9f33 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 14 Nov 2024 13:23:37 -0600 Subject: [PATCH 02/10] chore: add unit test to verify OIDCClaims query --- coderd/database/modelqueries.go | 2 +- coderd/database/oidcclaims_test.go | 60 +++++++++++++++++------------- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index 99a03fe9b614b..ff77012755fa2 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -531,6 +531,6 @@ func insertAuthorizedFilter(query string, replaceWith string) (string, error) { // UpdateUserLinkRawJSON is a custom query for unit testing. Do not ever expose this func (q *sqlQuerier) UpdateUserLinkRawJSON(ctx context.Context, userID uuid.UUID, data json.RawMessage) error { - _, err := q.sdb.Exec("INSERT INTO user_links (user_id, claims) VALUES ($1, $2)", userID, data) + _, err := q.sdb.ExecContext(ctx, "UPDATE user_links SET claims = $2 WHERE user_id = $1", userID, data) return err } diff --git a/coderd/database/oidcclaims_test.go b/coderd/database/oidcclaims_test.go index b480ef39eb533..48e6e112a6a61 100644 --- a/coderd/database/oidcclaims_test.go +++ b/coderd/database/oidcclaims_test.go @@ -10,8 +10,9 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbfake" + "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" - "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/util/slice" "github.com/coder/coder/v2/testutil" ) @@ -35,29 +36,41 @@ func TestOIDCClaims(t *testing.T) { alice := g.withLink(database.LoginTypeOIDC, toJSON(extraKeys{ UserLinkClaims: database.UserLinkClaims{ IDTokenClaims: map[string]interface{}{ - "sub": "alice", - }, - UserInfoClaims: map[string]interface{}{ - "sub": "alice", + "sub": "alice", + "alice-id": "from-bob", }, + UserInfoClaims: nil, }, // Always should be a no-op Foo: "bar", })) bob := g.withLink(database.LoginTypeOIDC, toJSON(database.UserLinkClaims{ IDTokenClaims: map[string]interface{}{ - "sub": "bob", + "sub": "bob", + "bob-id": "from-bob", + "array": []string{ + "a", "b", "c", + }, + "map": map[string]interface{}{ + "key": "value", + "foo": "bar", + }, + "nil": nil, }, UserInfoClaims: map[string]interface{}{ - "sub": "bob", + "sub": "bob", + "bob-info": []string{}, + "number": 42, }, })) charlie := g.withLink(database.LoginTypeOIDC, toJSON(database.UserLinkClaims{ IDTokenClaims: map[string]interface{}{ - "sub": "charlie", + "sub": "charlie", + "charlie-id": "charlie", }, UserInfoClaims: map[string]interface{}{ - "sub": "charlie", + "sub": "charlie", + "charlie-info": "charlie", }, })) @@ -87,17 +100,23 @@ func TestOIDCClaims(t *testing.T) { orgA := dbfake.Organization(t, db).Members( append(problematics, alice, - bob)..., + bob, + )..., ).Do() orgB := dbfake.Organization(t, db).Members( append(problematics, + bob, charlie, )..., ).Do() // Verify the OIDC claim fields - requireClaims(t, db, orgA.Org.ID, []string{"sub"}) - requireClaims(t, db, orgB.Org.ID, []string{"sub"}) + always := []string{"array", "map", "nil", "number"} + expectA := append([]string{"sub", "alice-id", "bob-id", "bob-info"}, always...) + expectB := append([]string{"sub", "bob-id", "bob-info", "charlie-id", "charlie-info"}, always...) + requireClaims(t, db, orgA.Org.ID, expectA) + requireClaims(t, db, orgB.Org.ID, expectB) + requireClaims(t, db, uuid.Nil, slice.Unique(append(expectA, expectB...))) } func requireClaims(t *testing.T, db database.Store, orgID uuid.UUID, want []string) { @@ -129,34 +148,23 @@ func (g userGenerator) user(lt database.LoginType, createLink bool, rawJSON json t.Helper() - u, err := db.InsertUser(context.Background(), database.InsertUserParams{ - ID: uuid.New(), - Email: testutil.GetRandomName(t), - Username: testutil.GetRandomName(t), - Name: testutil.GetRandomName(t), - CreatedAt: dbtime.Now(), - UpdatedAt: dbtime.Now(), - RBACRoles: []string{}, + u := dbgen.User(t, db, database.User{ LoginType: lt, - Status: string(database.UserStatusActive), }) - require.NoError(t, err) if !createLink { return u } - link, err := db.InsertUserLink(context.Background(), database.InsertUserLinkParams{ + link := dbgen.UserLink(t, db, database.UserLink{ UserID: u.ID, LoginType: lt, - Claims: database.UserLinkClaims{}, }) - require.NoError(t, err) if sql, ok := db.(rawUpdater); ok { // The only way to put arbitrary json into the db for testing edge cases. // Making this a public API would be a mistake. - err = sql.UpdateUserLinkRawJSON(context.Background(), u.ID, rawJSON) + err := sql.UpdateUserLinkRawJSON(context.Background(), u.ID, rawJSON) require.NoError(t, err) } else { // no need to test the json key logic in dbmem. Everything is type safe. From 1189fe7098ad602f942a0186b37e58010ac6d472 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 14 Nov 2024 13:48:52 -0600 Subject: [PATCH 03/10] add api endpoints --- coderd/database/oidcclaims_test.go | 2 ++ codersdk/idpsync.go | 28 +++++++++++++++++ enterprise/coderd/coderd.go | 10 ++++-- enterprise/coderd/idpsync.go | 50 ++++++++++++++++++++++++++++++ enterprise/coderd/userauth_test.go | 13 ++++++++ 5 files changed, 100 insertions(+), 3 deletions(-) diff --git a/coderd/database/oidcclaims_test.go b/coderd/database/oidcclaims_test.go index 48e6e112a6a61..7b2f05679f325 100644 --- a/coderd/database/oidcclaims_test.go +++ b/coderd/database/oidcclaims_test.go @@ -109,6 +109,7 @@ func TestOIDCClaims(t *testing.T) { charlie, )..., ).Do() + orgC := dbfake.Organization(t, db).Members().Do() // Verify the OIDC claim fields always := []string{"array", "map", "nil", "number"} @@ -116,6 +117,7 @@ func TestOIDCClaims(t *testing.T) { expectB := append([]string{"sub", "bob-id", "bob-info", "charlie-id", "charlie-info"}, always...) requireClaims(t, db, orgA.Org.ID, expectA) requireClaims(t, db, orgB.Org.ID, expectB) + requireClaims(t, db, orgC.Org.ID, []string{}) requireClaims(t, db, uuid.Nil, slice.Unique(append(expectA, expectB...))) } diff --git a/codersdk/idpsync.go b/codersdk/idpsync.go index 0226dc7f9eb5f..6d34714bc5833 100644 --- a/codersdk/idpsync.go +++ b/codersdk/idpsync.go @@ -137,3 +137,31 @@ func (c *Client) PatchOrganizationIDPSyncSettings(ctx context.Context, req Organ var resp OrganizationSyncSettings return resp, json.NewDecoder(res.Body).Decode(&resp) } + +func (c *Client) GetAvailableIDPSyncFields(ctx context.Context) ([]string, error) { + res, err := c.Request(ctx, http.MethodGet, "/api/v2/settings/idpsync/available-fields", nil) + if err != nil { + return nil, xerrors.Errorf("make request: %w", err) + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return nil, ReadBodyAsError(res) + } + var resp []string + return resp, json.NewDecoder(res.Body).Decode(&resp) +} + +func (c *Client) GetOrganizationAvailableIDPSyncFields(ctx context.Context, orgID string) ([]string, error) { + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/organizations/%s/settings/idpsync/available-fields", orgID), nil) + if err != nil { + return nil, xerrors.Errorf("make request: %w", err) + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return nil, ReadBodyAsError(res) + } + var resp []string + return resp, json.NewDecoder(res.Body).Decode(&resp) +} diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index b7642f4835c3b..b9356bc5b8f92 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -291,9 +291,12 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { r.Use( apiKeyMiddleware, ) - r.Route("/settings/idpsync/organization", func(r chi.Router) { - r.Get("/", api.organizationIDPSyncSettings) - r.Patch("/", api.patchOrganizationIDPSyncSettings) + r.Route("/settings/idpsync", func(r chi.Router) { + r.Route("/organization", func(r chi.Router) { + r.Get("/", api.organizationIDPSyncSettings) + r.Patch("/", api.patchOrganizationIDPSyncSettings) + }) + r.Get("/available-fields", api.deploymentIDPSyncClaimFields) }) }) @@ -303,6 +306,7 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { httpmw.ExtractOrganizationParam(api.Database), ) r.Route("/organizations/{organization}/settings", func(r chi.Router) { + r.Get("/idpsync/available-fields", api.organizationIDPSyncClaimFields) r.Get("/idpsync/groups", api.groupIDPSyncSettings) r.Patch("/idpsync/groups", api.patchGroupIDPSyncSettings) r.Get("/idpsync/roles", api.roleIDPSyncSettings) diff --git a/enterprise/coderd/idpsync.go b/enterprise/coderd/idpsync.go index 0df64ffb86b4b..2c6c7c30869ed 100644 --- a/enterprise/coderd/idpsync.go +++ b/enterprise/coderd/idpsync.go @@ -1,8 +1,11 @@ package coderd import ( + "fmt" "net/http" + "github.com/google/uuid" + "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" @@ -259,3 +262,50 @@ func (api *API) patchOrganizationIDPSyncSettings(rw http.ResponseWriter, r *http AssignDefault: settings.AssignDefault, }) } + +// @Summary Get the available idp sync claim fields +// @ID get-the-available-idp-sync-claim-fields +// @Security CoderSessionToken +// @Produce json +// @Tags Enterprise +// @Param organization path string true "Organization ID" format(uuid) +// @Success 200 {array} string +// @Router /organizations/{organization}/settings/idpsync/available-fields [get] +func (api *API) organizationIDPSyncClaimFields(rw http.ResponseWriter, r *http.Request) { + org := httpmw.OrganizationParam(r) + api.idpSyncClaimFields(org.ID, rw, r) +} + +// @Summary Get the available idp sync claim fields +// @ID get-the-available-idp-sync-claim-fields +// @Security CoderSessionToken +// @Produce json +// @Tags Enterprise +// @Param organization path string true "Organization ID" format(uuid) +// @Success 200 {array} string +// @Router /settings/idpsync/available-fields [get] +func (api *API) deploymentIDPSyncClaimFields(rw http.ResponseWriter, r *http.Request) { + // nil uuid implies all organizations + api.idpSyncClaimFields(uuid.Nil, rw, r) +} + +func (api *API) idpSyncClaimFields(orgID uuid.UUID, rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + fields, err := api.Database.OIDCClaimFields(ctx, orgID) + if httpapi.IsUnauthorizedError(err) { + // Give a helpful error. The user could read the org, so this does not + // leak anything. + httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ + Message: "You do not have permission to view the available IDP fields", + Detail: fmt.Sprintf("%s.read permission is required", rbac.ResourceIdpsyncSettings.Type), + }) + return + } + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, fields) +} diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index 47ed424055ece..28257078ebb36 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -165,6 +165,19 @@ func TestUserOIDC(t *testing.T) { user, err := userClient.User(ctx, codersdk.Me) require.NoError(t, err) + // Then: the available sync fields should be "email" and "organization" + fields, err := runner.AdminClient.GetAvailableIDPSyncFields(ctx) + require.NoError(t, err) + require.ElementsMatch(t, []string{ + "aud", "exp", "iss", // Always included from jwt + "email", "organization", + }, fields) + + // This should be the same as above + orgFields, err := runner.AdminClient.GetOrganizationAvailableIDPSyncFields(ctx, orgOne.ID.String()) + require.NoError(t, err) + require.ElementsMatch(t, fields, orgFields) + // When: they are manually added to the fourth organization, a new sync // should remove them. _, err = runner.AdminClient.PostOrganizationMember(ctx, orgThree.ID, "alice") From 91d0eba604bc89631bf3cd90a7aa42a30dd5c1b0 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 14 Nov 2024 13:50:22 -0600 Subject: [PATCH 04/10] make gen --- coderd/apidoc/docs.go | 76 ++++++++++++++++++++++++++++++++ coderd/apidoc/swagger.json | 68 ++++++++++++++++++++++++++++ docs/reference/api/enterprise.md | 74 +++++++++++++++++++++++++++++++ enterprise/coderd/idpsync.go | 4 +- 4 files changed, 220 insertions(+), 2 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 48b5618dec653..2c046dc8d997c 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -3132,6 +3132,44 @@ const docTemplate = `{ } } }, + "/organizations/{organization}/settings/idpsync/available-fields": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Enterprise" + ], + "summary": "Get the available organization idp sync claim fields", + "operationId": "get-the-available-organization-idp-sync-claim-fields", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Organization ID", + "name": "organization", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } + }, "/organizations/{organization}/settings/idpsync/groups": { "get": { "security": [ @@ -3800,6 +3838,44 @@ const docTemplate = `{ } } }, + "/settings/idpsync/available-fields": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Enterprise" + ], + "summary": "Get the available idp sync claim fields", + "operationId": "get-the-available-idp-sync-claim-fields", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Organization ID", + "name": "organization", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } + }, "/settings/idpsync/organization": { "get": { "security": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index d6aec54109316..4baae0c3568c3 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -2754,6 +2754,40 @@ } } }, + "/organizations/{organization}/settings/idpsync/available-fields": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Enterprise"], + "summary": "Get the available organization idp sync claim fields", + "operationId": "get-the-available-organization-idp-sync-claim-fields", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Organization ID", + "name": "organization", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } + }, "/organizations/{organization}/settings/idpsync/groups": { "get": { "security": [ @@ -3342,6 +3376,40 @@ } } }, + "/settings/idpsync/available-fields": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Enterprise"], + "summary": "Get the available idp sync claim fields", + "operationId": "get-the-available-idp-sync-claim-fields", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Organization ID", + "name": "organization", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } + }, "/settings/idpsync/organization": { "get": { "security": [ diff --git a/docs/reference/api/enterprise.md b/docs/reference/api/enterprise.md index 53b50a460f875..d5bf44192fc00 100644 --- a/docs/reference/api/enterprise.md +++ b/docs/reference/api/enterprise.md @@ -1778,6 +1778,43 @@ curl -X DELETE http://coder-server:8080/api/v2/organizations/{organization}/prov To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Get the available organization idp sync claim fields + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/settings/idpsync/available-fields \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /organizations/{organization}/settings/idpsync/available-fields` + +### Parameters + +| Name | In | Type | Required | Description | +| -------------- | ---- | ------------ | -------- | --------------- | +| `organization` | path | string(uuid) | true | Organization ID | + +### Example responses + +> 200 Response + +```json +["string"] +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | --------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of string | + +

Response Schema

+ +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Get group IdP Sync settings by organization ### Code samples @@ -2274,6 +2311,43 @@ curl -X PATCH http://coder-server:8080/api/v2/scim/v2/Users/{id} \ To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Get the available idp sync claim fields + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/settings/idpsync/available-fields \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /settings/idpsync/available-fields` + +### Parameters + +| Name | In | Type | Required | Description | +| -------------- | ---- | ------------ | -------- | --------------- | +| `organization` | path | string(uuid) | true | Organization ID | + +### Example responses + +> 200 Response + +```json +["string"] +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | --------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of string | + +

Response Schema

+ +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Get organization IdP Sync settings ### Code samples diff --git a/enterprise/coderd/idpsync.go b/enterprise/coderd/idpsync.go index 2c6c7c30869ed..087266462df26 100644 --- a/enterprise/coderd/idpsync.go +++ b/enterprise/coderd/idpsync.go @@ -263,8 +263,8 @@ func (api *API) patchOrganizationIDPSyncSettings(rw http.ResponseWriter, r *http }) } -// @Summary Get the available idp sync claim fields -// @ID get-the-available-idp-sync-claim-fields +// @Summary Get the available organization idp sync claim fields +// @ID get-the-available-organization-idp-sync-claim-fields // @Security CoderSessionToken // @Produce json // @Tags Enterprise From 406fabad70d3641e22e590204745b792e3285805 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 14 Nov 2024 14:34:35 -0600 Subject: [PATCH 05/10] dbauthz tests --- coderd/database/dbauthz/dbauthz_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 978478e4709c5..2eb75f8b738c4 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -626,6 +626,13 @@ func (s *MethodTestSuite) TestLicense() { } func (s *MethodTestSuite) TestOrganization() { + s.Run("Deployment/OIDCClaimFields", s.Subtest(func(db database.Store, check *expects) { + check.Args(uuid.Nil).Asserts(rbac.ResourceIdpsyncSettings, policy.ActionRead).Returns([]string{}) + })) + s.Run("Organization/OIDCClaimFields", s.Subtest(func(db database.Store, check *expects) { + id := uuid.New() + check.Args(id).Asserts(rbac.ResourceIdpsyncSettings.InOrg(id), policy.ActionRead).Returns([]string{}) + })) s.Run("ByOrganization/GetGroups", s.Subtest(func(db database.Store, check *expects) { o := dbgen.Organization(s.T(), db, database.Organization{}) a := dbgen.Group(s.T(), db, database.Group{OrganizationID: o.ID}) From 33c0bc4d8c57188d2c621d170c17aa54beeb10f2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 14 Nov 2024 14:42:52 -0600 Subject: [PATCH 06/10] linting --- coderd/database/dbmem/dbmem.go | 2 +- coderd/database/oidcclaims_test.go | 21 +++++++++------------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 24fb93f92ddfe..aed57e9284b3a 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -8409,7 +8409,7 @@ func (q *FakeQuerier) ListWorkspaceAgentPortShares(_ context.Context, workspaceI return shares, nil } -func (q *FakeQuerier) OIDCClaimFields(ctx context.Context, organizationID uuid.UUID) ([]string, error) { +func (q *FakeQuerier) OIDCClaimFields(_ context.Context, organizationID uuid.UUID) ([]string, error) { orgMembers := q.getOrganizationMemberNoLock(organizationID) var fields []string diff --git a/coderd/database/oidcclaims_test.go b/coderd/database/oidcclaims_test.go index 7b2f05679f325..75402fed363a5 100644 --- a/coderd/database/oidcclaims_test.go +++ b/coderd/database/oidcclaims_test.go @@ -137,14 +137,7 @@ type userGenerator struct { } func (g userGenerator) noLink(lt database.LoginType) database.User { - return g.user(lt, false, nil) -} - -func (g userGenerator) withLink(lt database.LoginType, rawJSON json.RawMessage) database.User { - return g.user(lt, true, rawJSON) -} -func (g userGenerator) user(lt database.LoginType, createLink bool, rawJSON json.RawMessage) database.User { t := g.t db := g.db @@ -153,13 +146,17 @@ func (g userGenerator) user(lt database.LoginType, createLink bool, rawJSON json u := dbgen.User(t, db, database.User{ LoginType: lt, }) + return u +} - if !createLink { - return u - } +func (g userGenerator) withLink(lt database.LoginType, rawJSON json.RawMessage) database.User { + t := g.t + db := g.db + + user := g.noLink(lt) link := dbgen.UserLink(t, db, database.UserLink{ - UserID: u.ID, + UserID: user.ID, LoginType: lt, }) @@ -188,7 +185,7 @@ func (g userGenerator) user(lt database.LoginType, createLink bool, rawJSON json require.NoError(t, err) } - return u + return user } type rawUpdater interface { From 2e957530e22bcd15fb2a44e480be985d0bdacdab Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 14 Nov 2024 14:44:54 -0600 Subject: [PATCH 07/10] fix public schema --- coderd/database/queries.sql.go | 4 ++-- coderd/database/queries/user_links.sql | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a65b167dada13..fdce74b48a885 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9859,7 +9859,7 @@ WHERE jsonb_typeof(claims->'id_token_claims') != 'null' AND login_type = 'oidc' AND CASE WHEN $1 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - user_links.user_id = ANY(SELECT organization_members.user_id FROM public.organization_members WHERE organization_id = $1) + user_links.user_id = ANY(SELECT organization_members.user_id FROM organization_members WHERE organization_id = $1) ELSE true END @@ -9876,7 +9876,7 @@ WHERE jsonb_typeof(claims->'user_info_claims') != 'null' AND login_type = 'oidc' AND CASE WHEN $1 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - user_links.user_id = ANY(SELECT organization_members.user_id FROM public.organization_members WHERE organization_id = $1) + user_links.user_id = ANY(SELECT organization_members.user_id FROM organization_members WHERE organization_id = $1) ELSE true END ` diff --git a/coderd/database/queries/user_links.sql b/coderd/database/queries/user_links.sql index fe7d0d62b4f7a..3214bd4e8839f 100644 --- a/coderd/database/queries/user_links.sql +++ b/coderd/database/queries/user_links.sql @@ -74,7 +74,7 @@ WHERE jsonb_typeof(claims->'id_token_claims') != 'null' AND login_type = 'oidc' AND CASE WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - user_links.user_id = ANY(SELECT organization_members.user_id FROM public.organization_members WHERE organization_id = @organization_id) + user_links.user_id = ANY(SELECT organization_members.user_id FROM organization_members WHERE organization_id = @organization_id) ELSE true END @@ -92,7 +92,7 @@ WHERE jsonb_typeof(claims->'user_info_claims') != 'null' AND login_type = 'oidc' AND CASE WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - user_links.user_id = ANY(SELECT organization_members.user_id FROM public.organization_members WHERE organization_id = @organization_id) + user_links.user_id = ANY(SELECT organization_members.user_id FROM organization_members WHERE organization_id = @organization_id) ELSE true END ; From 1648c2244b75040512933bd5d9e7f1faf920b6b3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 14 Nov 2024 14:59:16 -0600 Subject: [PATCH 08/10] fixup! fix public schema --- coderd/database/oidcclaims_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/oidcclaims_test.go b/coderd/database/oidcclaims_test.go index 75402fed363a5..34b4f988dcf49 100644 --- a/coderd/database/oidcclaims_test.go +++ b/coderd/database/oidcclaims_test.go @@ -163,7 +163,7 @@ func (g userGenerator) withLink(lt database.LoginType, rawJSON json.RawMessage) if sql, ok := db.(rawUpdater); ok { // The only way to put arbitrary json into the db for testing edge cases. // Making this a public API would be a mistake. - err := sql.UpdateUserLinkRawJSON(context.Background(), u.ID, rawJSON) + err := sql.UpdateUserLinkRawJSON(context.Background(), user.ID, rawJSON) require.NoError(t, err) } else { // no need to test the json key logic in dbmem. Everything is type safe. From 2dae39be7573174436c1512e7053ff0a3084fadb Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 14 Nov 2024 15:02:05 -0600 Subject: [PATCH 09/10] fmt --- coderd/database/oidcclaims_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/coderd/database/oidcclaims_test.go b/coderd/database/oidcclaims_test.go index 34b4f988dcf49..dbab7a7693c5b 100644 --- a/coderd/database/oidcclaims_test.go +++ b/coderd/database/oidcclaims_test.go @@ -137,7 +137,6 @@ type userGenerator struct { } func (g userGenerator) noLink(lt database.LoginType) database.User { - t := g.t db := g.db From 2ee2d62ed50257bea52014384e54f59fb64f3de4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 18 Nov 2024 12:16:59 -0600 Subject: [PATCH 10/10] fixup comment --- coderd/database/oidcclaims_test.go | 27 ++++++++++++++++++++++++++ coderd/database/querier.go | 3 +-- coderd/database/queries.sql.go | 26 ++++--------------------- coderd/database/queries/user_links.sql | 26 ++++--------------------- coderd/userauth.go | 7 ------- 5 files changed, 36 insertions(+), 53 deletions(-) diff --git a/coderd/database/oidcclaims_test.go b/coderd/database/oidcclaims_test.go index dbab7a7693c5b..85fd5b3df3812 100644 --- a/coderd/database/oidcclaims_test.go +++ b/coderd/database/oidcclaims_test.go @@ -40,6 +40,10 @@ func TestOIDCClaims(t *testing.T) { "alice-id": "from-bob", }, UserInfoClaims: nil, + MergedClaims: map[string]interface{}{ + "sub": "alice", + "alice-id": "from-bob", + }, }, // Always should be a no-op Foo: "bar", @@ -62,6 +66,20 @@ func TestOIDCClaims(t *testing.T) { "bob-info": []string{}, "number": 42, }, + MergedClaims: map[string]interface{}{ + "sub": "bob", + "bob-info": []string{}, + "number": 42, + "bob-id": "from-bob", + "array": []string{ + "a", "b", "c", + }, + "map": map[string]interface{}{ + "key": "value", + "foo": "bar", + }, + "nil": nil, + }, })) charlie := g.withLink(database.LoginTypeOIDC, toJSON(database.UserLinkClaims{ IDTokenClaims: map[string]interface{}{ @@ -72,6 +90,11 @@ func TestOIDCClaims(t *testing.T) { "sub": "charlie", "charlie-info": "charlie", }, + MergedClaims: map[string]interface{}{ + "sub": "charlie", + "charlie-id": "charlie", + "charlie-info": "charlie", + }, })) // users that just try to cause problems, but should not affect the output of @@ -89,6 +112,10 @@ func TestOIDCClaims(t *testing.T) { UserInfoClaims: map[string]interface{}{ "do-not": "look", }, + MergedClaims: map[string]interface{}{ + "not": "allowed", + "do-not": "look", + }, })), // github should be omitted // extra random users diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 75d9aca015628..49ba6fbf8496a 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -413,9 +413,8 @@ type sqlcQuerier interface { ListProvisionerKeysByOrganization(ctx context.Context, organizationID uuid.UUID) ([]ProvisionerKey, error) ListProvisionerKeysByOrganizationExcludeReserved(ctx context.Context, organizationID uuid.UUID) ([]ProvisionerKey, error) ListWorkspaceAgentPortShares(ctx context.Context, workspaceID uuid.UUID) ([]WorkspaceAgentPortShare, error) - // OIDCClaimFields returns a list of distinct keys in both the id_token_claims and user_info_claims fields. + // OIDCClaimFields returns a list of distinct keys in the the merged_claims fields. // This query is used to generate the list of available sync fields for idp sync settings. - // Merge with user_info claims. OIDCClaimFields(ctx context.Context, organizationID uuid.UUID) ([]string, error) // Arguments are optional with uuid.Nil to ignore. // - Use just 'organization_id' to get all members of an org diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index fdce74b48a885..09dd4c1fbc488 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9848,32 +9848,15 @@ func (q *sqlQuerier) InsertUserLink(ctx context.Context, arg InsertUserLinkParam const oIDCClaimFields = `-- name: OIDCClaimFields :many SELECT - DISTINCT jsonb_object_keys(claims->'id_token_claims') + DISTINCT jsonb_object_keys(claims->'merged_claims') FROM user_links WHERE -- Only return rows where the top level key exists - claims ? 'id_token_claims' AND + claims ? 'merged_claims' AND -- 'null' is the default value for the id_token_claims field -- jsonb 'null' is not the same as SQL NULL. Strip these out. - jsonb_typeof(claims->'id_token_claims') != 'null' AND - login_type = 'oidc' - AND CASE WHEN $1 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - user_links.user_id = ANY(SELECT organization_members.user_id FROM organization_members WHERE organization_id = $1) - ELSE true - END - -UNION - - -- This query is identical to the one above, except for 'user_info_claims'. - -- There might be some way to do this more concisely at a cost of readability. -SELECT - DISTINCT jsonb_object_keys(claims->'user_info_claims') -FROM - user_links -WHERE - claims ? 'user_info_claims' AND - jsonb_typeof(claims->'user_info_claims') != 'null' AND + jsonb_typeof(claims->'merged_claims') != 'null' AND login_type = 'oidc' AND CASE WHEN $1 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN user_links.user_id = ANY(SELECT organization_members.user_id FROM organization_members WHERE organization_id = $1) @@ -9881,9 +9864,8 @@ WHERE END ` -// OIDCClaimFields returns a list of distinct keys in both the id_token_claims and user_info_claims fields. +// OIDCClaimFields returns a list of distinct keys in the the merged_claims fields. // This query is used to generate the list of available sync fields for idp sync settings. -// Merge with user_info claims. func (q *sqlQuerier) OIDCClaimFields(ctx context.Context, organizationID uuid.UUID) ([]string, error) { rows, err := q.db.QueryContext(ctx, oIDCClaimFields, organizationID) if err != nil { diff --git a/coderd/database/queries/user_links.sql b/coderd/database/queries/user_links.sql index 3214bd4e8839f..274193b0c8bf6 100644 --- a/coderd/database/queries/user_links.sql +++ b/coderd/database/queries/user_links.sql @@ -60,36 +60,18 @@ WHERE -- name: OIDCClaimFields :many --- OIDCClaimFields returns a list of distinct keys in both the id_token_claims and user_info_claims fields. +-- OIDCClaimFields returns a list of distinct keys in the the merged_claims fields. -- This query is used to generate the list of available sync fields for idp sync settings. SELECT - DISTINCT jsonb_object_keys(claims->'id_token_claims') + DISTINCT jsonb_object_keys(claims->'merged_claims') FROM user_links WHERE -- Only return rows where the top level key exists - claims ? 'id_token_claims' AND + claims ? 'merged_claims' AND -- 'null' is the default value for the id_token_claims field -- jsonb 'null' is not the same as SQL NULL. Strip these out. - jsonb_typeof(claims->'id_token_claims') != 'null' AND - login_type = 'oidc' - AND CASE WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - user_links.user_id = ANY(SELECT organization_members.user_id FROM organization_members WHERE organization_id = @organization_id) - ELSE true - END - --- Merge with user_info claims. -UNION - - -- This query is identical to the one above, except for 'user_info_claims'. - -- There might be some way to do this more concisely at a cost of readability. -SELECT - DISTINCT jsonb_object_keys(claims->'user_info_claims') -FROM - user_links -WHERE - claims ? 'user_info_claims' AND - jsonb_typeof(claims->'user_info_claims') != 'null' AND + jsonb_typeof(claims->'merged_claims') != 'null' AND login_type = 'oidc' AND CASE WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN user_links.user_id = ANY(SELECT organization_members.user_id FROM organization_members WHERE organization_id = @organization_id) diff --git a/coderd/userauth.go b/coderd/userauth.go index 44b8c15a5dba5..c5e95e44998b2 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -1395,13 +1395,6 @@ func mergeClaims(a, b map[string]interface{}) map[string]interface{} { return c } -// OauthDebugContext provides helpful information for admins to debug -// OAuth login issues. -type OauthDebugContext struct { - IDTokenClaims map[string]interface{} `json:"id_token_claims"` - UserInfoClaims map[string]interface{} `json:"user_info_claims"` -} - type oauthLoginParams struct { User database.User Link database.UserLink