diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index cd647d0537a93..7e4088ccb6212 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -506,11 +506,6 @@ OIDC OPTIONS: groups. This filter is applied after the group mapping and before the regex filter. - --oidc-organization-assign-default bool, $CODER_OIDC_ORGANIZATION_ASSIGN_DEFAULT (default: true) - If set to true, users will always be added to the default - organization. If organization sync is enabled, then the default org is - always added to the user's set of expectedorganizations. - --oidc-auth-url-params struct[map[string]string], $CODER_OIDC_AUTH_URL_PARAMS (default: {"access_type": "offline"}) OIDC auth URL parameters to pass to the upstream provider. @@ -557,14 +552,6 @@ OIDC OPTIONS: --oidc-name-field string, $CODER_OIDC_NAME_FIELD (default: name) OIDC claim field to use as the name. - --oidc-organization-field string, $CODER_OIDC_ORGANIZATION_FIELD - This field must be set if using the organization sync feature. Set to - the claim to be used for organizations. - - --oidc-organization-mapping struct[map[string][]uuid.UUID], $CODER_OIDC_ORGANIZATION_MAPPING (default: {}) - A map of OIDC claims and the organizations in Coder it should map to. - This is required because organization IDs must be used within Coder. - --oidc-group-regex-filter regexp, $CODER_OIDC_GROUP_REGEX_FILTER (default: .*) If provided any group name not matching the regex is ignored. This allows for filtering out groups that are not needed. This filter is diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 6c770c18232ac..983abb61169c9 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -3166,6 +3166,9 @@ const docTemplate = `{ "CoderSessionToken": [] } ], + "consumes": [ + "application/json" + ], "produces": [ "application/json" ], @@ -3182,6 +3185,15 @@ const docTemplate = `{ "name": "organization", "in": "path", "required": true + }, + { + "description": "New settings", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.GroupSyncSettings" + } } ], "responses": { @@ -3234,6 +3246,9 @@ const docTemplate = `{ "CoderSessionToken": [] } ], + "consumes": [ + "application/json" + ], "produces": [ "application/json" ], @@ -3250,6 +3265,15 @@ const docTemplate = `{ "name": "organization", "in": "path", "required": true + }, + { + "description": "New settings", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.RoleSyncSettings" + } } ], "responses": { @@ -3770,6 +3794,68 @@ const docTemplate = `{ } } }, + "/settings/idpsync/organization": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Enterprise" + ], + "summary": "Get organization IdP Sync settings", + "operationId": "get-organization-idp-sync-settings", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSyncSettings" + } + } + } + }, + "patch": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Enterprise" + ], + "summary": "Update organization IdP Sync settings", + "operationId": "update-organization-idp-sync-settings", + "parameters": [ + { + "description": "New settings", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSyncSettings" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSyncSettings" + } + } + } + } + }, "/tailnet": { "get": { "security": [ @@ -11807,6 +11893,29 @@ const docTemplate = `{ } } }, + "codersdk.OrganizationSyncSettings": { + "type": "object", + "properties": { + "field": { + "description": "Field selects the claim field to be used as the created user's\norganizations. If the field is the empty string, then no organization\nupdates will ever come from the OIDC provider.", + "type": "string" + }, + "mapping": { + "description": "Mapping maps from an OIDC claim --\u003e Coder organization uuid", + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "organization_assign_default": { + "description": "AssignDefault will ensure the default org is always included\nfor every user, regardless of their claims. This preserves legacy behavior.", + "type": "boolean" + } + } + }, "codersdk.PatchGroupRequest": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 4f5ca444f703e..67cc92c71331d 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -2784,6 +2784,7 @@ "CoderSessionToken": [] } ], + "consumes": ["application/json"], "produces": ["application/json"], "tags": ["Enterprise"], "summary": "Update group IdP Sync settings by organization", @@ -2796,6 +2797,15 @@ "name": "organization", "in": "path", "required": true + }, + { + "description": "New settings", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.GroupSyncSettings" + } } ], "responses": { @@ -2844,6 +2854,7 @@ "CoderSessionToken": [] } ], + "consumes": ["application/json"], "produces": ["application/json"], "tags": ["Enterprise"], "summary": "Update role IdP Sync settings by organization", @@ -2856,6 +2867,15 @@ "name": "organization", "in": "path", "required": true + }, + { + "description": "New settings", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.RoleSyncSettings" + } } ], "responses": { @@ -3316,6 +3336,58 @@ } } }, + "/settings/idpsync/organization": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Enterprise"], + "summary": "Get organization IdP Sync settings", + "operationId": "get-organization-idp-sync-settings", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSyncSettings" + } + } + } + }, + "patch": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "tags": ["Enterprise"], + "summary": "Update organization IdP Sync settings", + "operationId": "update-organization-idp-sync-settings", + "parameters": [ + { + "description": "New settings", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSyncSettings" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSyncSettings" + } + } + } + } + }, "/tailnet": { "get": { "security": [ @@ -10641,6 +10713,29 @@ } } }, + "codersdk.OrganizationSyncSettings": { + "type": "object", + "properties": { + "field": { + "description": "Field selects the claim field to be used as the created user's\norganizations. If the field is the empty string, then no organization\nupdates will ever come from the OIDC provider.", + "type": "string" + }, + "mapping": { + "description": "Mapping maps from an OIDC claim --\u003e Coder organization uuid", + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "organization_assign_default": { + "description": "AssignDefault will ensure the default org is always included\nfor every user, regardless of their claims. This preserves legacy behavior.", + "type": "boolean" + } + } + }, "codersdk.PatchGroupRequest": { "type": "object", "properties": { diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index c855d5a1984df..9afa8afcb01d9 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -33,9 +33,8 @@ var _ database.Store = (*querier)(nil) const wrapname = "dbauthz.querier" -// NoActorError wraps ErrNoRows for the api to return a 404. This is the correct -// response when the user is not authorized. -var NoActorError = xerrors.Errorf("no authorization actor in context: %w", sql.ErrNoRows) +// NoActorError is returned if no actor is present in the context. +var NoActorError = xerrors.Errorf("no authorization actor in context") // NotAuthorizedError is a sentinel error that unwraps to sql.ErrNoRows. // This allows the internal error to be read by the caller if needed. Otherwise diff --git a/coderd/idpsync/group.go b/coderd/idpsync/group.go index 672bcb66da4cf..c14b7655e7e20 100644 --- a/coderd/idpsync/group.go +++ b/coderd/idpsync/group.go @@ -20,12 +20,12 @@ import ( ) type GroupParams struct { - // SyncEnabled if false will skip syncing the user's groups - SyncEnabled bool + // SyncEntitled if false will skip syncing the user's groups + SyncEntitled bool MergedClaims jwt.MapClaims } -func (AGPLIDPSync) GroupSyncEnabled() bool { +func (AGPLIDPSync) GroupSyncEntitled() bool { // AGPL does not support syncing groups. return false } @@ -73,13 +73,13 @@ func (s AGPLIDPSync) GroupSyncSettings(ctx context.Context, orgID uuid.UUID, db func (s AGPLIDPSync) ParseGroupClaims(_ context.Context, _ jwt.MapClaims) (GroupParams, *HTTPError) { return GroupParams{ - SyncEnabled: s.GroupSyncEnabled(), + SyncEntitled: s.GroupSyncEntitled(), }, nil } func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user database.User, params GroupParams) error { // Nothing happens if sync is not enabled - if !params.SyncEnabled { + if !params.SyncEntitled { return nil } diff --git a/coderd/idpsync/group_test.go b/coderd/idpsync/group_test.go index 1275dd4e48503..2baafd53ff03c 100644 --- a/coderd/idpsync/group_test.go +++ b/coderd/idpsync/group_test.go @@ -41,7 +41,7 @@ func TestParseGroupClaims(t *testing.T) { params, err := s.ParseGroupClaims(ctx, jwt.MapClaims{}) require.Nil(t, err) - require.False(t, params.SyncEnabled) + require.False(t, params.SyncEntitled) }) // AllowList has no effect in AGPL @@ -61,7 +61,7 @@ func TestParseGroupClaims(t *testing.T) { params, err := s.ParseGroupClaims(ctx, jwt.MapClaims{}) require.Nil(t, err) - require.False(t, params.SyncEnabled) + require.False(t, params.SyncEntitled) }) } @@ -276,7 +276,7 @@ func TestGroupSyncTable(t *testing.T) { // Do the group sync! err := s.SyncGroups(ctx, db, user, idpsync.GroupParams{ - SyncEnabled: true, + SyncEntitled: true, MergedClaims: userClaims, }) require.NoError(t, err) @@ -363,7 +363,7 @@ func TestGroupSyncTable(t *testing.T) { // Do the group sync! err = s.SyncGroups(ctx, db, user, idpsync.GroupParams{ - SyncEnabled: true, + SyncEntitled: true, MergedClaims: userClaims, }) require.NoError(t, err) @@ -420,7 +420,7 @@ func TestSyncDisabled(t *testing.T) { // Do the group sync! err := s.SyncGroups(ctx, db, user, idpsync.GroupParams{ - SyncEnabled: false, + SyncEntitled: false, MergedClaims: jwt.MapClaims{ "groups": []string{"baz", "bop"}, }, diff --git a/coderd/idpsync/idpsync.go b/coderd/idpsync/idpsync.go index f2c9e49ecc900..e936bada73752 100644 --- a/coderd/idpsync/idpsync.go +++ b/coderd/idpsync/idpsync.go @@ -24,8 +24,13 @@ import ( // claims to the internal representation of a user in Coder. // TODO: Move group + role sync into this interface. type IDPSync interface { - AssignDefaultOrganization() bool - OrganizationSyncEnabled() bool + OrganizationSyncEntitled() bool + OrganizationSyncSettings(ctx context.Context, db database.Store) (*OrganizationSyncSettings, error) + UpdateOrganizationSettings(ctx context.Context, db database.Store, settings OrganizationSyncSettings) error + // OrganizationSyncEnabled returns true if all OIDC users are assigned + // to organizations via org sync settings. + // This is used to know when to disable manual org membership assignment. + OrganizationSyncEnabled(ctx context.Context, db database.Store) bool // ParseOrganizationClaims takes claims from an OIDC provider, and returns the // organization sync params for assigning users into organizations. ParseOrganizationClaims(ctx context.Context, mergedClaims jwt.MapClaims) (OrganizationParams, *HTTPError) @@ -33,7 +38,7 @@ type IDPSync interface { // provided params. SyncOrganizations(ctx context.Context, tx database.Store, user database.User, params OrganizationParams) error - GroupSyncEnabled() bool + GroupSyncEntitled() bool // ParseGroupClaims takes claims from an OIDC provider, and returns the params // for group syncing. Most of the logic happens in SyncGroups. ParseGroupClaims(ctx context.Context, mergedClaims jwt.MapClaims) (GroupParams, *HTTPError) @@ -147,8 +152,9 @@ func FromDeploymentValues(dv *codersdk.DeploymentValues) DeploymentSyncSettings type SyncSettings struct { DeploymentSyncSettings - Group runtimeconfig.RuntimeEntry[*GroupSyncSettings] - Role runtimeconfig.RuntimeEntry[*RoleSyncSettings] + Group runtimeconfig.RuntimeEntry[*GroupSyncSettings] + Role runtimeconfig.RuntimeEntry[*RoleSyncSettings] + Organization runtimeconfig.RuntimeEntry[*OrganizationSyncSettings] } func NewAGPLSync(logger slog.Logger, manager *runtimeconfig.Manager, settings DeploymentSyncSettings) *AGPLIDPSync { @@ -159,6 +165,7 @@ func NewAGPLSync(logger slog.Logger, manager *runtimeconfig.Manager, settings De DeploymentSyncSettings: settings, Group: runtimeconfig.MustNew[*GroupSyncSettings]("group-sync-settings"), Role: runtimeconfig.MustNew[*RoleSyncSettings]("role-sync-settings"), + Organization: runtimeconfig.MustNew[*OrganizationSyncSettings]("organization-sync-settings"), }, } } diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index 3e2a0f84d5e5e..66d8ab08495cc 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -3,6 +3,7 @@ package idpsync import ( "context" "database/sql" + "encoding/json" "github.com/golang-jwt/jwt/v4" "github.com/google/uuid" @@ -13,35 +14,59 @@ import ( "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/runtimeconfig" "github.com/coder/coder/v2/coderd/util/slice" ) type OrganizationParams struct { - // SyncEnabled if false will skip syncing the user's organizations. - SyncEnabled bool - // IncludeDefault is primarily for single org deployments. It will ensure - // a user is always inserted into the default org. - IncludeDefault bool - // Organizations is the list of organizations the user should be a member of - // assuming syncing is turned on. - Organizations []uuid.UUID + // SyncEntitled if false will skip syncing the user's organizations. + SyncEntitled bool + // MergedClaims are passed to the organization level for syncing + MergedClaims jwt.MapClaims } -func (AGPLIDPSync) OrganizationSyncEnabled() bool { +func (AGPLIDPSync) OrganizationSyncEntitled() bool { // AGPL does not support syncing organizations. return false } -func (s AGPLIDPSync) AssignDefaultOrganization() bool { - return s.OrganizationAssignDefault +func (AGPLIDPSync) OrganizationSyncEnabled(_ context.Context, _ database.Store) bool { + return false +} + +func (s AGPLIDPSync) UpdateOrganizationSettings(ctx context.Context, db database.Store, settings OrganizationSyncSettings) error { + rlv := s.Manager.Resolver(db) + err := s.SyncSettings.Organization.SetRuntimeValue(ctx, rlv, &settings) + if err != nil { + return xerrors.Errorf("update organization sync settings: %w", err) + } + + return nil +} + +func (s AGPLIDPSync) OrganizationSyncSettings(ctx context.Context, db database.Store) (*OrganizationSyncSettings, error) { + rlv := s.Manager.Resolver(db) + orgSettings, err := s.SyncSettings.Organization.Resolve(ctx, rlv) + if err != nil { + if !xerrors.Is(err, runtimeconfig.ErrEntryNotFound) { + return nil, xerrors.Errorf("resolve org sync settings: %w", err) + } + + // Default to the statically assigned settings if they exist. + orgSettings = &OrganizationSyncSettings{ + Field: s.DeploymentSyncSettings.OrganizationField, + Mapping: s.DeploymentSyncSettings.OrganizationMapping, + AssignDefault: s.DeploymentSyncSettings.OrganizationAssignDefault, + } + } + return orgSettings, nil } -func (s AGPLIDPSync) ParseOrganizationClaims(_ context.Context, _ jwt.MapClaims) (OrganizationParams, *HTTPError) { +func (s AGPLIDPSync) ParseOrganizationClaims(_ context.Context, claims jwt.MapClaims) (OrganizationParams, *HTTPError) { // For AGPL we only sync the default organization. return OrganizationParams{ - SyncEnabled: s.OrganizationSyncEnabled(), - IncludeDefault: s.OrganizationAssignDefault, - Organizations: []uuid.UUID{}, + SyncEntitled: s.OrganizationSyncEntitled(), + MergedClaims: claims, }, nil } @@ -49,21 +74,25 @@ func (s AGPLIDPSync) ParseOrganizationClaims(_ context.Context, _ jwt.MapClaims) // organizations. It will add and remove their membership to match the expected set. func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, user database.User, params OrganizationParams) error { // Nothing happens if sync is not enabled - if !params.SyncEnabled { + if !params.SyncEntitled { return nil } // nolint:gocritic // all syncing is done as a system user ctx = dbauthz.AsSystemRestricted(ctx) - // This is a bit hacky, but if AssignDefault is included, then always - // make sure to include the default org in the list of expected. - if s.OrganizationAssignDefault { - defaultOrg, err := tx.GetDefaultOrganization(ctx) - if err != nil { - return xerrors.Errorf("failed to get default organization: %w", err) - } - params.Organizations = append(params.Organizations, defaultOrg.ID) + orgSettings, err := s.OrganizationSyncSettings(ctx, tx) + if err != nil { + return xerrors.Errorf("failed to get org sync settings: %w", err) + } + + if orgSettings.Field == "" { + return nil // No sync configured, nothing to do + } + + expectedOrgs, err := orgSettings.ParseClaims(ctx, tx, params.MergedClaims) + if err != nil { + return xerrors.Errorf("organization claims: %w", err) } existingOrgs, err := tx.GetOrganizationsByUserID(ctx, user.ID) @@ -77,11 +106,10 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u // Find the difference in the expected and the existing orgs, and // correct the set of orgs the user is a member of. - add, remove := slice.SymmetricDifference(existingOrgIDs, params.Organizations) + add, remove := slice.SymmetricDifference(existingOrgIDs, expectedOrgs) notExists := make([]uuid.UUID, 0) for _, orgID := range add { - //nolint:gocritic // System actor being used to assign orgs - _, err := tx.InsertOrganizationMember(dbauthz.AsSystemRestricted(ctx), database.InsertOrganizationMemberParams{ + _, err := tx.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{ OrganizationID: orgID, UserID: user.ID, CreatedAt: dbtime.Now(), @@ -98,8 +126,7 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u } for _, orgID := range remove { - //nolint:gocritic // System actor being used to assign orgs - err := tx.DeleteOrganizationMember(dbauthz.AsSystemRestricted(ctx), database.DeleteOrganizationMemberParams{ + err := tx.DeleteOrganizationMember(ctx, database.DeleteOrganizationMemberParams{ OrganizationID: orgID, UserID: user.ID, }) @@ -117,3 +144,64 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u } return nil } + +type OrganizationSyncSettings struct { + // Field selects the claim field to be used as the created user's + // organizations. If the field is the empty string, then no organization updates + // will ever come from the OIDC provider. + Field string + // Mapping controls how organizations returned by the OIDC provider get mapped + Mapping map[string][]uuid.UUID + // AssignDefault will ensure all users that authenticate will be + // placed into the default organization. This is mostly a hack to support + // legacy deployments. + AssignDefault bool +} + +func (s *OrganizationSyncSettings) Set(v string) error { + return json.Unmarshal([]byte(v), s) +} + +func (s *OrganizationSyncSettings) String() string { + return runtimeconfig.JSONString(s) +} + +// ParseClaims will parse the claims and return the list of organizations the user +// should sync to. +func (s *OrganizationSyncSettings) ParseClaims(ctx context.Context, db database.Store, mergedClaims jwt.MapClaims) ([]uuid.UUID, error) { + userOrganizations := make([]uuid.UUID, 0) + + if s.AssignDefault { + // This is a bit hacky, but if AssignDefault is included, then always + // make sure to include the default org in the list of expected. + defaultOrg, err := db.GetDefaultOrganization(ctx) + if err != nil { + return nil, xerrors.Errorf("failed to get default organization: %w", err) + } + + // Always include default org. + userOrganizations = append(userOrganizations, defaultOrg.ID) + } + + organizationRaw, ok := mergedClaims[s.Field] + if !ok { + return userOrganizations, nil + } + + parsedOrganizations, err := ParseStringSliceClaim(organizationRaw) + if err != nil { + return userOrganizations, xerrors.Errorf("failed to parese organizations OIDC claims: %w", err) + } + + // add any mapped organizations + for _, parsedOrg := range parsedOrganizations { + if mappedOrganization, ok := s.Mapping[parsedOrg]; ok { + // parsedOrg is in the mapping, so add the mapped organizations to the + // user's organizations. + userOrganizations = append(userOrganizations, mappedOrganization...) + } + } + + // Deduplicate the organizations + return slice.Unique(userOrganizations), nil +} diff --git a/coderd/idpsync/organizations_test.go b/coderd/idpsync/organizations_test.go index 1670beaaedc75..51c8a7365d22b 100644 --- a/coderd/idpsync/organizations_test.go +++ b/coderd/idpsync/organizations_test.go @@ -16,27 +16,6 @@ import ( func TestParseOrganizationClaims(t *testing.T) { t.Parallel() - t.Run("SingleOrgDeployment", func(t *testing.T) { - t.Parallel() - - s := idpsync.NewAGPLSync(slogtest.Make(t, &slogtest.Options{}), - runtimeconfig.NewManager(), - idpsync.DeploymentSyncSettings{ - OrganizationField: "", - OrganizationMapping: nil, - OrganizationAssignDefault: true, - }) - - ctx := testutil.Context(t, testutil.WaitMedium) - - params, err := s.ParseOrganizationClaims(ctx, jwt.MapClaims{}) - require.Nil(t, err) - - require.Empty(t, params.Organizations) - require.True(t, params.IncludeDefault) - require.False(t, params.SyncEnabled) - }) - t.Run("AGPL", func(t *testing.T) { t.Parallel() @@ -56,8 +35,6 @@ func TestParseOrganizationClaims(t *testing.T) { params, err := s.ParseOrganizationClaims(ctx, jwt.MapClaims{}) require.Nil(t, err) - require.Empty(t, params.Organizations) - require.False(t, params.IncludeDefault) - require.False(t, params.SyncEnabled) + require.False(t, params.SyncEntitled) }) } diff --git a/coderd/members.go b/coderd/members.go index 7f2acd982631b..97950b19e9137 100644 --- a/coderd/members.go +++ b/coderd/members.go @@ -45,11 +45,7 @@ func (api *API) postOrganizationMember(rw http.ResponseWriter, r *http.Request) aReq.Old = database.AuditableOrganizationMember{} defer commitAudit() - if user.LoginType == database.LoginTypeOIDC && api.IDPSync.OrganizationSyncEnabled() { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Organization sync is enabled for OIDC users, meaning manual organization assignment is not allowed for this user.", - Detail: fmt.Sprintf("User %s is an OIDC user and organization sync is enabled. Ask an administrator to resolve this in your external IDP.", user.ID), - }) + if !api.manualOrganizationMembership(ctx, rw, user) { return } @@ -116,6 +112,14 @@ func (api *API) deleteOrganizationMember(rw http.ResponseWriter, r *http.Request aReq.Old = member.OrganizationMember.Auditable(member.Username) defer commitAudit() + // Note: we disallow adding OIDC users if organization sync is enabled. + // For removing members, do not have this same enforcement. As long as a user + // does not re-login, they will not be immediately removed from the organization. + // There might be an urgent need to revoke access. + // A user can re-login if they are removed in error. + // If we add a feature to force logout a user, then we can prevent manual + // member removal when organization sync is enabled, and use force logout instead. + if member.UserID == apiKey.UserID { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{Message: "cannot remove self from an organization"}) return @@ -272,7 +276,7 @@ func (api *API) allowChangingMemberRoles(ctx context.Context, rw http.ResponseWr } if orgSync { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Cannot modify roles for OIDC users when role sync is enabled. This organization member's roles are managed by the identity provider.", + Message: "Cannot modify roles for OIDC users when role sync is enabled. This organization member's roles are managed by the identity provider. Have the user re-login to refresh their roles.", Detail: "'User Role Field' is set in the organization settings. Ask an administrator to adjust or disable these settings.", }) return false @@ -372,3 +376,17 @@ func convertOrganizationMembersWithUserData(ctx context.Context, db database.Sto return converted, nil } + +// manualOrganizationMembership checks if the user is an OIDC user and if organization sync is enabled. +// If organization sync is enabled, manual organization assignment is not allowed, +// since all organization membership is controlled by the external IDP. +func (api *API) manualOrganizationMembership(ctx context.Context, rw http.ResponseWriter, user database.User) bool { + if user.LoginType == database.LoginTypeOIDC && api.IDPSync.OrganizationSyncEnabled(ctx, api.Database) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Organization sync is enabled for OIDC users, meaning manual organization assignment is not allowed for this user. Have the user re-login to refresh their organizations.", + Detail: fmt.Sprintf("User %s is an OIDC user and organization sync is enabled. Ask an administrator to resolve the membership in your external IDP.", user.Username), + }) + return false + } + return true +} diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 14700500266a1..a57bd071a8052 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -352,6 +352,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceOrganizationMember.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, ResourceGroup.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, ResourceGroupMember.Type: {policy.ActionRead}, + // Manage org membership based on OIDC claims + ResourceIdpsyncSettings.Type: {policy.ActionRead, policy.ActionUpdate}, }), Org: map[string][]Permission{}, User: []Permission{}, diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 954b5e9788c53..0172439829063 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -733,10 +733,25 @@ func TestRolePermissions(t *testing.T) { Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, Resource: rbac.ResourceIdpsyncSettings.InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, orgUserAdmin}, + true: {owner, orgAdmin, orgUserAdmin, userAdmin}, false: { orgMemberMe, otherOrgAdmin, - memberMe, userAdmin, templateAdmin, + memberMe, templateAdmin, + orgAuditor, orgTemplateAdmin, + otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, + }, + }, + }, + { + Name: "OrganizationIDPSyncSettings", + Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate}, + Resource: rbac.ResourceIdpsyncSettings, + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {owner, userAdmin}, + false: { + orgAdmin, orgUserAdmin, + orgMemberMe, otherOrgAdmin, + memberMe, templateAdmin, orgAuditor, orgTemplateAdmin, otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin, }, diff --git a/coderd/userauth.go b/coderd/userauth.go index f6cf0e5292db7..e7db9e9719c35 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -968,12 +968,10 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { Name: normName, DebugContext: OauthDebugContext{}, GroupSync: idpsync.GroupParams{ - SyncEnabled: false, + SyncEntitled: false, }, OrganizationSync: idpsync.OrganizationParams{ - SyncEnabled: false, - IncludeDefault: true, - Organizations: []uuid.UUID{}, + SyncEntitled: false, }, }).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) { return audit.InitRequest[database.User](rw, params) @@ -1513,14 +1511,6 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C // This can happen if a user is a built-in user but is signing in // with OIDC for the first time. if user.ID == uuid.Nil { - // Until proper multi-org support, all users will be added to the default organization. - // The default organization should always be present. - //nolint:gocritic - defaultOrganization, err := tx.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx)) - if err != nil { - return xerrors.Errorf("unable to fetch default organization: %w", err) - } - //nolint:gocritic _, err = tx.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{ Username: params.Username, @@ -1555,19 +1545,22 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C } } - // Even if org sync is disabled, single org deployments will always - // have this set to true. - orgIDs := []uuid.UUID{} - if params.OrganizationSync.IncludeDefault { - orgIDs = append(orgIDs, defaultOrganization.ID) + //nolint:gocritic + defaultOrganization, err := tx.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx)) + if err != nil { + return xerrors.Errorf("unable to fetch default organization: %w", err) } //nolint:gocritic user, err = api.CreateUser(dbauthz.AsSystemRestricted(ctx), tx, CreateUserRequest{ CreateUserRequestWithOrgs: codersdk.CreateUserRequestWithOrgs{ - Email: params.Email, - Username: params.Username, - OrganizationIDs: orgIDs, + Email: params.Email, + Username: params.Username, + // This is a kludge, but all users are defaulted into the default + // organization. This exists as the default behavior. + // If org sync is enabled and configured, the user's groups + // will change based on the org sync settings. + OrganizationIDs: []uuid.UUID{defaultOrganization.ID}, UserStatus: ptr.Ref(codersdk.UserStatusActive), }, LoginType: params.LoginType, diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 3ba09bd38d1a4..5959c35fa9d95 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1749,6 +1749,7 @@ when required by your organization's security policy.`, Value: &c.OIDC.OrganizationField, Group: &deploymentGroupOIDC, YAML: "organizationField", + Hidden: true, // Use db runtime config instead }, { Name: "OIDC Assign Default Organization", @@ -1762,6 +1763,7 @@ when required by your organization's security policy.`, Value: &c.OIDC.OrganizationAssignDefault, Group: &deploymentGroupOIDC, YAML: "organizationAssignDefault", + Hidden: true, // Use db runtime config instead }, { Name: "OIDC Organization Sync Mapping", @@ -1773,6 +1775,7 @@ when required by your organization's security policy.`, Value: &c.OIDC.OrganizationMapping, Group: &deploymentGroupOIDC, YAML: "organizationMapping", + Hidden: true, // Use db runtime config instead }, { Name: "OIDC Group Field", diff --git a/codersdk/idpsync.go b/codersdk/idpsync.go index 380b26336ad90..0226dc7f9eb5f 100644 --- a/codersdk/idpsync.go +++ b/codersdk/idpsync.go @@ -97,3 +97,43 @@ func (c *Client) PatchRoleIDPSyncSettings(ctx context.Context, orgID string, req var resp RoleSyncSettings return resp, json.NewDecoder(res.Body).Decode(&resp) } + +type OrganizationSyncSettings struct { + // Field selects the claim field to be used as the created user's + // organizations. If the field is the empty string, then no organization + // updates will ever come from the OIDC provider. + Field string `json:"field"` + // Mapping maps from an OIDC claim --> Coder organization uuid + Mapping map[string][]uuid.UUID `json:"mapping"` + // AssignDefault will ensure the default org is always included + // for every user, regardless of their claims. This preserves legacy behavior. + AssignDefault bool `json:"organization_assign_default"` +} + +func (c *Client) OrganizationIDPSyncSettings(ctx context.Context) (OrganizationSyncSettings, error) { + res, err := c.Request(ctx, http.MethodGet, "/api/v2/settings/idpsync/organization", nil) + if err != nil { + return OrganizationSyncSettings{}, xerrors.Errorf("make request: %w", err) + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return OrganizationSyncSettings{}, ReadBodyAsError(res) + } + var resp OrganizationSyncSettings + return resp, json.NewDecoder(res.Body).Decode(&resp) +} + +func (c *Client) PatchOrganizationIDPSyncSettings(ctx context.Context, req OrganizationSyncSettings) (OrganizationSyncSettings, error) { + res, err := c.Request(ctx, http.MethodPatch, "/api/v2/settings/idpsync/organization", req) + if err != nil { + return OrganizationSyncSettings{}, xerrors.Errorf("make request: %w", err) + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return OrganizationSyncSettings{}, ReadBodyAsError(res) + } + var resp OrganizationSyncSettings + return resp, json.NewDecoder(res.Body).Decode(&resp) +} diff --git a/docs/reference/api/enterprise.md b/docs/reference/api/enterprise.md index 57ffa5260edde..17f55afcddd81 100644 --- a/docs/reference/api/enterprise.md +++ b/docs/reference/api/enterprise.md @@ -1831,17 +1831,37 @@ To perform this operation, you must be authenticated. [Learn more](authenticatio ```shell # Example request using curl curl -X PATCH http://coder-server:8080/api/v2/organizations/{organization}/settings/idpsync/groups \ + -H 'Content-Type: application/json' \ -H 'Accept: application/json' \ -H 'Coder-Session-Token: API_KEY' ``` `PATCH /organizations/{organization}/settings/idpsync/groups` +> Body parameter + +```json +{ + "auto_create_missing_groups": true, + "field": "string", + "legacy_group_name_mapping": { + "property1": "string", + "property2": "string" + }, + "mapping": { + "property1": ["string"], + "property2": ["string"] + }, + "regex_filter": {} +} +``` + ### Parameters -| Name | In | Type | Required | Description | -| -------------- | ---- | ------------ | -------- | --------------- | -| `organization` | path | string(uuid) | true | Organization ID | +| Name | In | Type | Required | Description | +| -------------- | ---- | ------------------------------------------------------------------ | -------- | --------------- | +| `organization` | path | string(uuid) | true | Organization ID | +| `body` | body | [codersdk.GroupSyncSettings](schemas.md#codersdkgroupsyncsettings) | true | New settings | ### Example responses @@ -1919,17 +1939,31 @@ To perform this operation, you must be authenticated. [Learn more](authenticatio ```shell # Example request using curl curl -X PATCH http://coder-server:8080/api/v2/organizations/{organization}/settings/idpsync/roles \ + -H 'Content-Type: application/json' \ -H 'Accept: application/json' \ -H 'Coder-Session-Token: API_KEY' ``` `PATCH /organizations/{organization}/settings/idpsync/roles` +> Body parameter + +```json +{ + "field": "string", + "mapping": { + "property1": ["string"], + "property2": ["string"] + } +} +``` + ### Parameters -| Name | In | Type | Required | Description | -| -------------- | ---- | ------------ | -------- | --------------- | -| `organization` | path | string(uuid) | true | Organization ID | +| Name | In | Type | Required | Description | +| -------------- | ---- | ---------------------------------------------------------------- | -------- | --------------- | +| `organization` | path | string(uuid) | true | Organization ID | +| `body` | body | [codersdk.RoleSyncSettings](schemas.md#codersdkrolesyncsettings) | true | New settings | ### Example responses @@ -2239,6 +2273,98 @@ 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 organization IdP Sync settings + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/settings/idpsync/organization \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /settings/idpsync/organization` + +### Example responses + +> 200 Response + +```json +{ + "field": "string", + "mapping": { + "property1": ["string"], + "property2": ["string"] + }, + "organization_assign_default": true +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | -------------------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.OrganizationSyncSettings](schemas.md#codersdkorganizationsyncsettings) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + +## Update organization IdP Sync settings + +### Code samples + +```shell +# Example request using curl +curl -X PATCH http://coder-server:8080/api/v2/settings/idpsync/organization \ + -H 'Content-Type: application/json' \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`PATCH /settings/idpsync/organization` + +> Body parameter + +```json +{ + "field": "string", + "mapping": { + "property1": ["string"], + "property2": ["string"] + }, + "organization_assign_default": true +} +``` + +### Parameters + +| Name | In | Type | Required | Description | +| ------ | ---- | -------------------------------------------------------------------------------- | -------- | ------------ | +| `body` | body | [codersdk.OrganizationSyncSettings](schemas.md#codersdkorganizationsyncsettings) | true | New settings | + +### Example responses + +> 200 Response + +```json +{ + "field": "string", + "mapping": { + "property1": ["string"], + "property2": ["string"] + }, + "organization_assign_default": true +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | -------------------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.OrganizationSyncSettings](schemas.md#codersdkorganizationsyncsettings) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Get template ACLs ### Code samples diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index dab7703345b08..fe8db822aafb5 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -3941,6 +3941,28 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `user_id` | string | false | | | | `username` | string | false | | | +## codersdk.OrganizationSyncSettings + +```json +{ + "field": "string", + "mapping": { + "property1": ["string"], + "property2": ["string"] + }, + "organization_assign_default": true +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ----------------------------- | --------------- | -------- | ------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `field` | string | false | | Field selects the claim field to be used as the created user's organizations. If the field is the empty string, then no organization updates will ever come from the OIDC provider. | +| `mapping` | object | false | | Mapping maps from an OIDC claim --> Coder organization uuid | +| ยป `[any property]` | array of string | false | | | +| `organization_assign_default` | boolean | false | | Organization assign default will ensure the default org is always included for every user, regardless of their claims. This preserves legacy behavior. | + ## codersdk.PatchGroupRequest ```json diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 42ef7f7418b45..3b3d2376c9aab 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -559,38 +559,6 @@ OIDC auth URL parameters to pass to the upstream provider. Ignore the userinfo endpoint and only use the ID token for user information. -### --oidc-organization-field - -| | | -| ----------- | ------------------------------------------- | -| Type | string | -| Environment | $CODER_OIDC_ORGANIZATION_FIELD | -| YAML | oidc.organizationField | - -This field must be set if using the organization sync feature. Set to the claim to be used for organizations. - -### --oidc-organization-assign-default - -| | | -| ----------- | ---------------------------------------------------- | -| Type | bool | -| Environment | $CODER_OIDC_ORGANIZATION_ASSIGN_DEFAULT | -| YAML | oidc.organizationAssignDefault | -| Default | true | - -If set to true, users will always be added to the default organization. If organization sync is enabled, then the default org is always added to the user's set of expectedorganizations. - -### --oidc-organization-mapping - -| | | -| ----------- | --------------------------------------------- | -| Type | struct[map[string][]uuid.UUID] | -| Environment | $CODER_OIDC_ORGANIZATION_MAPPING | -| YAML | oidc.organizationMapping | -| Default | {} | - -A map of OIDC claims and the organizations in Coder it should map to. This is required because organization IDs must be used within Coder. - ### --oidc-group-field | | | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index a6398586fa972..aaa4725c65181 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -507,11 +507,6 @@ OIDC OPTIONS: groups. This filter is applied after the group mapping and before the regex filter. - --oidc-organization-assign-default bool, $CODER_OIDC_ORGANIZATION_ASSIGN_DEFAULT (default: true) - If set to true, users will always be added to the default - organization. If organization sync is enabled, then the default org is - always added to the user's set of expectedorganizations. - --oidc-auth-url-params struct[map[string]string], $CODER_OIDC_AUTH_URL_PARAMS (default: {"access_type": "offline"}) OIDC auth URL parameters to pass to the upstream provider. @@ -558,14 +553,6 @@ OIDC OPTIONS: --oidc-name-field string, $CODER_OIDC_NAME_FIELD (default: name) OIDC claim field to use as the name. - --oidc-organization-field string, $CODER_OIDC_ORGANIZATION_FIELD - This field must be set if using the organization sync feature. Set to - the claim to be used for organizations. - - --oidc-organization-mapping struct[map[string][]uuid.UUID], $CODER_OIDC_ORGANIZATION_MAPPING (default: {}) - A map of OIDC claims and the organizations in Coder it should map to. - This is required because organization IDs must be used within Coder. - --oidc-group-regex-filter regexp, $CODER_OIDC_GROUP_REGEX_FILTER (default: .*) If provided any group name not matching the regex is ignored. This allows for filtering out groups that are not needed. This filter is diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index dddf619b34058..03d535f6ffb69 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -287,6 +287,16 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { r.Delete("/organizations/{organization}/members/roles/{roleName}", api.deleteOrgRole) }) + r.Group(func(r chi.Router) { + r.Use( + apiKeyMiddleware, + ) + r.Route("/settings/idpsync/organization", func(r chi.Router) { + r.Get("/", api.organizationIDPSyncSettings) + r.Patch("/", api.patchOrganizationIDPSyncSettings) + }) + }) + r.Group(func(r chi.Router) { r.Use( apiKeyMiddleware, diff --git a/enterprise/coderd/enidpsync/groups.go b/enterprise/coderd/enidpsync/groups.go index dc8456fc6b1c9..7cabce412a1ea 100644 --- a/enterprise/coderd/enidpsync/groups.go +++ b/enterprise/coderd/enidpsync/groups.go @@ -10,7 +10,7 @@ import ( "github.com/coder/coder/v2/codersdk" ) -func (e EnterpriseIDPSync) GroupSyncEnabled() bool { +func (e EnterpriseIDPSync) GroupSyncEntitled() bool { return e.entitlements.Enabled(codersdk.FeatureTemplateRBAC) } @@ -20,7 +20,7 @@ func (e EnterpriseIDPSync) GroupSyncEnabled() bool { // GroupAllowList is implemented here to prevent login by unauthorized users. // TODO: GroupAllowList overlaps with the default organization group sync settings. func (e EnterpriseIDPSync) ParseGroupClaims(ctx context.Context, mergedClaims jwt.MapClaims) (idpsync.GroupParams, *idpsync.HTTPError) { - if !e.GroupSyncEnabled() { + if !e.GroupSyncEntitled() { return e.AGPLIDPSync.ParseGroupClaims(ctx, mergedClaims) } @@ -64,7 +64,7 @@ func (e EnterpriseIDPSync) ParseGroupClaims(ctx context.Context, mergedClaims jw } return idpsync.GroupParams{ - SyncEnabled: true, + SyncEntitled: true, MergedClaims: mergedClaims, }, nil } diff --git a/enterprise/coderd/enidpsync/groups_test.go b/enterprise/coderd/enidpsync/groups_test.go index 278b647f29f14..652432c73f503 100644 --- a/enterprise/coderd/enidpsync/groups_test.go +++ b/enterprise/coderd/enidpsync/groups_test.go @@ -39,7 +39,7 @@ func TestEnterpriseParseGroupClaims(t *testing.T) { params, err := s.ParseGroupClaims(ctx, jwt.MapClaims{}) require.Nil(t, err) - require.False(t, params.SyncEnabled) + require.False(t, params.SyncEntitled) }) t.Run("NotInAllowList", func(t *testing.T) { @@ -90,7 +90,7 @@ func TestEnterpriseParseGroupClaims(t *testing.T) { } params, err := s.ParseGroupClaims(ctx, claims) require.Nil(t, err) - require.True(t, params.SyncEnabled) + require.True(t, params.SyncEntitled) require.Equal(t, claims, params.MergedClaims) }) } diff --git a/enterprise/coderd/enidpsync/organizations.go b/enterprise/coderd/enidpsync/organizations.go index 2c7520fc412ee..313d90fac8a9f 100644 --- a/enterprise/coderd/enidpsync/organizations.go +++ b/enterprise/coderd/enidpsync/organizations.go @@ -2,72 +2,39 @@ package enidpsync import ( "context" - "net/http" "github.com/golang-jwt/jwt/v4" - "github.com/google/uuid" - "cdr.dev/slog" - "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/idpsync" - "github.com/coder/coder/v2/coderd/util/slice" "github.com/coder/coder/v2/codersdk" ) -func (e EnterpriseIDPSync) OrganizationSyncEnabled() bool { - return e.entitlements.Enabled(codersdk.FeatureMultipleOrganizations) && e.OrganizationField != "" +func (e EnterpriseIDPSync) OrganizationSyncEntitled() bool { + return e.entitlements.Enabled(codersdk.FeatureMultipleOrganizations) } -func (e EnterpriseIDPSync) ParseOrganizationClaims(ctx context.Context, mergedClaims jwt.MapClaims) (idpsync.OrganizationParams, *idpsync.HTTPError) { - if !e.OrganizationSyncEnabled() { - // Default to agpl if multi-org is not enabled - return e.AGPLIDPSync.ParseOrganizationClaims(ctx, mergedClaims) +func (e EnterpriseIDPSync) OrganizationSyncEnabled(ctx context.Context, db database.Store) bool { + if !e.OrganizationSyncEntitled() { + return false } - // nolint:gocritic // all syncing is done as a system user - ctx = dbauthz.AsSystemRestricted(ctx) - userOrganizations := make([]uuid.UUID, 0) - - // Pull extra organizations from the claims. - if e.OrganizationField != "" { - organizationRaw, ok := mergedClaims[e.OrganizationField] - if ok { - parsedOrganizations, err := idpsync.ParseStringSliceClaim(organizationRaw) - if err != nil { - return idpsync.OrganizationParams{}, &idpsync.HTTPError{ - Code: http.StatusBadRequest, - Msg: "Failed to sync organizations from the OIDC claims", - Detail: err.Error(), - RenderStaticPage: false, - RenderDetailMarkdown: false, - } - } - - // Keep track of which claims are not mapped for debugging purposes. - var ignored []string - for _, parsedOrg := range parsedOrganizations { - if mappedOrganization, ok := e.OrganizationMapping[parsedOrg]; ok { - // parsedOrg is in the mapping, so add the mapped organizations to the - // user's organizations. - userOrganizations = append(userOrganizations, mappedOrganization...) - } else { - ignored = append(ignored, parsedOrg) - } - } + settings, err := e.OrganizationSyncSettings(ctx, db) + if err == nil && settings.Field != "" { + return true + } + return false +} - e.Logger.Debug(ctx, "parsed organizations from claim", - slog.F("len", len(parsedOrganizations)), - slog.F("ignored", ignored), - slog.F("organizations", parsedOrganizations), - ) - } +func (e EnterpriseIDPSync) ParseOrganizationClaims(ctx context.Context, mergedClaims jwt.MapClaims) (idpsync.OrganizationParams, *idpsync.HTTPError) { + if !e.OrganizationSyncEntitled() { + // Default to agpl if multi-org is not enabled + return e.AGPLIDPSync.ParseOrganizationClaims(ctx, mergedClaims) } return idpsync.OrganizationParams{ - // If the field is not set, then sync is not enabled. - SyncEnabled: e.OrganizationField != "", - IncludeDefault: e.OrganizationAssignDefault, - // Do not return duplicates - Organizations: slice.Unique(userOrganizations), + // Return true if entitled + SyncEntitled: true, + MergedClaims: mergedClaims, }, nil } diff --git a/enterprise/coderd/enidpsync/organizations_test.go b/enterprise/coderd/enidpsync/organizations_test.go index 6be2f597e382f..36dbedf3a466d 100644 --- a/enterprise/coderd/enidpsync/organizations_test.go +++ b/enterprise/coderd/enidpsync/organizations_test.go @@ -34,17 +34,19 @@ type Expectations struct { Name string Claims jwt.MapClaims // Parse - ParseError func(t *testing.T, httpErr *idpsync.HTTPError) - ExpectedParams idpsync.OrganizationParams + ParseError func(t *testing.T, httpErr *idpsync.HTTPError) + ExpectedParams idpsync.OrganizationParams + ExpectedEnabled bool // Mutate allows mutating the user before syncing Mutate func(t *testing.T, db database.Store, user database.User) Sync ExpectedUser } type OrganizationSyncTestCase struct { - Settings idpsync.DeploymentSyncSettings - Entitlements *entitlements.Set - Exps []Expectations + Settings idpsync.DeploymentSyncSettings + RuntimeSettings *idpsync.OrganizationSyncSettings + Entitlements *entitlements.Set + Exps []Expectations } func TestOrganizationSync(t *testing.T) { @@ -100,10 +102,9 @@ func TestOrganizationSync(t *testing.T) { Name: "NoOrganizations", Claims: jwt.MapClaims{}, ExpectedParams: idpsync.OrganizationParams{ - SyncEnabled: false, - IncludeDefault: true, - Organizations: []uuid.UUID{}, + SyncEntitled: true, }, + ExpectedEnabled: false, Sync: ExpectedUser{ Organizations: []uuid.UUID{}, }, @@ -112,10 +113,9 @@ func TestOrganizationSync(t *testing.T) { Name: "AlreadyInOrgs", Claims: jwt.MapClaims{}, ExpectedParams: idpsync.OrganizationParams{ - SyncEnabled: false, - IncludeDefault: true, - Organizations: []uuid.UUID{}, + SyncEntitled: true, }, + ExpectedEnabled: false, Mutate: func(t *testing.T, db database.Store, user database.User) { dbgen.OrganizationMember(t, db, database.OrganizationMember{ UserID: user.ID, @@ -157,10 +157,9 @@ func TestOrganizationSync(t *testing.T) { Name: "NoOrganizations", Claims: jwt.MapClaims{}, ExpectedParams: idpsync.OrganizationParams{ - SyncEnabled: true, - IncludeDefault: true, - Organizations: []uuid.UUID{}, + SyncEntitled: true, }, + ExpectedEnabled: true, Sync: ExpectedUser{ Organizations: []uuid.UUID{def.ID}, }, @@ -171,10 +170,9 @@ func TestOrganizationSync(t *testing.T) { "organizations": []string{"second", "extra"}, }, ExpectedParams: idpsync.OrganizationParams{ - SyncEnabled: true, - IncludeDefault: true, - Organizations: []uuid.UUID{two.ID}, + SyncEntitled: true, }, + ExpectedEnabled: true, Mutate: func(t *testing.T, db database.Store, user database.User) { dbgen.OrganizationMember(t, db, database.OrganizationMember{ UserID: user.ID, @@ -196,12 +194,9 @@ func TestOrganizationSync(t *testing.T) { "organizations": []string{"second", "extra", "first", "third", "second", "second"}, }, ExpectedParams: idpsync.OrganizationParams{ - SyncEnabled: true, - IncludeDefault: true, - Organizations: []uuid.UUID{ - two.ID, one.ID, three.ID, - }, + SyncEntitled: true, }, + ExpectedEnabled: true, Mutate: func(t *testing.T, db database.Store, user database.User) { dbgen.OrganizationMember(t, db, database.OrganizationMember{ UserID: user.ID, @@ -220,6 +215,72 @@ func TestOrganizationSync(t *testing.T) { } }, }, + { + Name: "DynamicSettings", + Case: func(t *testing.T, db database.Store) OrganizationSyncTestCase { + def, _ := db.GetDefaultOrganization(context.Background()) + one := dbgen.Organization(t, db, database.Organization{}) + two := dbgen.Organization(t, db, database.Organization{}) + three := dbgen.Organization(t, db, database.Organization{}) + return OrganizationSyncTestCase{ + Entitlements: entitled, + Settings: idpsync.DeploymentSyncSettings{ + OrganizationField: "organizations", + OrganizationMapping: map[string][]uuid.UUID{ + "first": {one.ID}, + "second": {two.ID}, + "third": {three.ID}, + }, + OrganizationAssignDefault: true, + }, + // Override + RuntimeSettings: &idpsync.OrganizationSyncSettings{ + Field: "dynamic", + Mapping: map[string][]uuid.UUID{ + "third": {three.ID}, + }, + AssignDefault: false, + }, + Exps: []Expectations{ + { + Name: "NoOrganizations", + Claims: jwt.MapClaims{}, + ExpectedParams: idpsync.OrganizationParams{ + SyncEntitled: true, + }, + ExpectedEnabled: true, + Sync: ExpectedUser{ + Organizations: []uuid.UUID{}, + }, + }, + { + Name: "AlreadyInOrgs", + Claims: jwt.MapClaims{ + "organizations": []string{"second", "extra"}, + "dynamic": []string{"third"}, + }, + ExpectedParams: idpsync.OrganizationParams{ + SyncEntitled: true, + }, + ExpectedEnabled: true, + Mutate: func(t *testing.T, db database.Store, user database.User) { + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: def.ID, + }) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: one.ID, + }) + }, + Sync: ExpectedUser{ + Organizations: []uuid.UUID{three.ID}, + }, + }, + }, + } + }, + }, } for _, tc := range testCases { @@ -238,6 +299,11 @@ func TestOrganizationSync(t *testing.T) { // Create a new sync object sync := enidpsync.NewSync(logger, runtimeconfig.NewManager(), caseData.Entitlements, caseData.Settings) + if caseData.RuntimeSettings != nil { + err := sync.UpdateOrganizationSettings(ctx, rdb, *caseData.RuntimeSettings) + require.NoError(t, err) + } + for _, exp := range caseData.Exps { t.Run(exp.Name, func(t *testing.T) { params, httpErr := sync.ParseOrganizationClaims(ctx, exp.Claims) @@ -247,12 +313,8 @@ func TestOrganizationSync(t *testing.T) { } require.Nil(t, httpErr, "no parse error") - require.Equal(t, exp.ExpectedParams.SyncEnabled, params.SyncEnabled, "match enabled") - require.Equal(t, exp.ExpectedParams.IncludeDefault, params.IncludeDefault, "match include default") - if exp.ExpectedParams.Organizations == nil { - exp.ExpectedParams.Organizations = []uuid.UUID{} - } - require.ElementsMatch(t, exp.ExpectedParams.Organizations, params.Organizations, "match organizations") + require.Equal(t, exp.ExpectedParams.SyncEntitled, params.SyncEntitled, "match enabled") + require.Equal(t, exp.ExpectedEnabled, sync.OrganizationSyncEnabled(context.Background(), rdb)) user := dbgen.User(t, db, database.User{}) if exp.Mutate != nil { diff --git a/enterprise/coderd/idpsync.go b/enterprise/coderd/idpsync.go index 096209ffe8292..a3e2e184e9598 100644 --- a/enterprise/coderd/idpsync.go +++ b/enterprise/coderd/idpsync.go @@ -44,8 +44,10 @@ func (api *API) groupIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { // @ID update-group-idp-sync-settings-by-organization // @Security CoderSessionToken // @Produce json +// @Accept json // @Tags Enterprise // @Param organization path string true "Organization ID" format(uuid) +// @Param request body codersdk.GroupSyncSettings true "New settings" // @Success 200 {object} codersdk.GroupSyncSettings // @Router /organizations/{organization}/settings/idpsync/groups [patch] func (api *API) patchGroupIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { @@ -57,7 +59,7 @@ func (api *API) patchGroupIDPSyncSettings(rw http.ResponseWriter, r *http.Reques return } - var req idpsync.GroupSyncSettings + var req codersdk.GroupSyncSettings if !httpapi.Read(ctx, rw, r, &req) { return } @@ -78,7 +80,13 @@ func (api *API) patchGroupIDPSyncSettings(rw http.ResponseWriter, r *http.Reques //nolint:gocritic // Requires system context to update runtime config sysCtx := dbauthz.AsSystemRestricted(ctx) - err := api.IDPSync.UpdateGroupSettings(sysCtx, org.ID, api.Database, req) + err := api.IDPSync.UpdateGroupSettings(sysCtx, org.ID, api.Database, idpsync.GroupSyncSettings{ + Field: req.Field, + Mapping: req.Mapping, + RegexFilter: req.RegexFilter, + AutoCreateMissing: req.AutoCreateMissing, + LegacyNameMapping: req.LegacyNameMapping, + }) if err != nil { httpapi.InternalServerError(rw, err) return @@ -90,7 +98,13 @@ func (api *API) patchGroupIDPSyncSettings(rw http.ResponseWriter, r *http.Reques return } - httpapi.Write(ctx, rw, http.StatusOK, settings) + httpapi.Write(ctx, rw, http.StatusOK, codersdk.GroupSyncSettings{ + Field: settings.Field, + Mapping: settings.Mapping, + RegexFilter: settings.RegexFilter, + AutoCreateMissing: settings.AutoCreateMissing, + LegacyNameMapping: settings.LegacyNameMapping, + }) } // @Summary Get role IdP Sync settings by organization @@ -125,8 +139,10 @@ func (api *API) roleIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { // @ID update-role-idp-sync-settings-by-organization // @Security CoderSessionToken // @Produce json +// @Accept json // @Tags Enterprise // @Param organization path string true "Organization ID" format(uuid) +// @Param request body codersdk.RoleSyncSettings true "New settings" // @Success 200 {object} codersdk.RoleSyncSettings // @Router /organizations/{organization}/settings/idpsync/roles [patch] func (api *API) patchRoleIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { @@ -138,14 +154,17 @@ func (api *API) patchRoleIDPSyncSettings(rw http.ResponseWriter, r *http.Request return } - var req idpsync.RoleSyncSettings + var req codersdk.RoleSyncSettings if !httpapi.Read(ctx, rw, r, &req) { return } //nolint:gocritic // Requires system context to update runtime config sysCtx := dbauthz.AsSystemRestricted(ctx) - err := api.IDPSync.UpdateRoleSettings(sysCtx, org.ID, api.Database, req) + err := api.IDPSync.UpdateRoleSettings(sysCtx, org.ID, api.Database, idpsync.RoleSyncSettings{ + Field: req.Field, + Mapping: req.Mapping, + }) if err != nil { httpapi.InternalServerError(rw, err) return @@ -157,5 +176,82 @@ func (api *API) patchRoleIDPSyncSettings(rw http.ResponseWriter, r *http.Request return } + httpapi.Write(ctx, rw, http.StatusOK, codersdk.RoleSyncSettings{ + Field: settings.Field, + Mapping: settings.Mapping, + }) +} + +// @Summary Get organization IdP Sync settings +// @ID get-organization-idp-sync-settings +// @Security CoderSessionToken +// @Produce json +// @Tags Enterprise +// @Success 200 {object} codersdk.OrganizationSyncSettings +// @Router /settings/idpsync/organization [get] +func (api *API) organizationIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + if !api.Authorize(r, policy.ActionRead, rbac.ResourceIdpsyncSettings) { + httpapi.Forbidden(rw) + return + } + + //nolint:gocritic // Requires system context to read runtime config + sysCtx := dbauthz.AsSystemRestricted(ctx) + settings, err := api.IDPSync.OrganizationSyncSettings(sysCtx, api.Database) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + httpapi.Write(ctx, rw, http.StatusOK, settings) } + +// @Summary Update organization IdP Sync settings +// @ID update-organization-idp-sync-settings +// @Security CoderSessionToken +// @Produce json +// @Accept json +// @Tags Enterprise +// @Success 200 {object} codersdk.OrganizationSyncSettings +// @Param request body codersdk.OrganizationSyncSettings true "New settings" +// @Router /settings/idpsync/organization [patch] +func (api *API) patchOrganizationIDPSyncSettings(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + if !api.Authorize(r, policy.ActionUpdate, rbac.ResourceIdpsyncSettings) { + httpapi.Forbidden(rw) + return + } + + var req codersdk.OrganizationSyncSettings + if !httpapi.Read(ctx, rw, r, &req) { + return + } + + //nolint:gocritic // Requires system context to update runtime config + sysCtx := dbauthz.AsSystemRestricted(ctx) + err := api.IDPSync.UpdateOrganizationSettings(sysCtx, api.Database, idpsync.OrganizationSyncSettings{ + Field: req.Field, + // We do not check if the mappings point to actual organizations. + Mapping: req.Mapping, + AssignDefault: req.AssignDefault, + }) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + settings, err := api.IDPSync.OrganizationSyncSettings(sysCtx, api.Database) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, codersdk.OrganizationSyncSettings{ + Field: settings.Field, + Mapping: settings.Mapping, + AssignDefault: settings.AssignDefault, + }) +} diff --git a/enterprise/coderd/scim.go b/enterprise/coderd/scim.go index 439e6ca3225de..01d04626a6948 100644 --- a/enterprise/coderd/scim.go +++ b/enterprise/coderd/scim.go @@ -281,7 +281,13 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { // the default org, regardless if sync is enabled or not. // This is to preserve single org deployment behavior. organizations := []uuid.UUID{} - if api.IDPSync.AssignDefaultOrganization() { + //nolint:gocritic // SCIM operations are a system user + orgSync, err := api.IDPSync.OrganizationSyncSettings(dbauthz.AsSystemRestricted(ctx), api.Database) + if err != nil { + _ = handlerutil.WriteError(rw, xerrors.Errorf("failed to get organization sync settings: %w", err)) + return + } + if orgSync.AssignDefault { //nolint:gocritic // SCIM operations are a system user defaultOrganization, err := api.Database.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx)) if err != nil { diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index 538904cd5b428..47ed424055ece 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -102,70 +102,89 @@ func TestUserOIDC(t *testing.T) { t.Run("MultiOrgWithDefault", func(t *testing.T) { t.Parallel() - // Chicken and egg problem. Config is at startup, but orgs are - // created at runtime. We should add a runtime configuration of - // this. - second := uuid.New() - third := uuid.New() - // Given: 4 organizations: default, second, third, and fourth runner := setupOIDCTest(t, oidcTestConfig{ Config: func(cfg *coderd.OIDCConfig) { cfg.AllowSignups = true }, DeploymentValues: func(dv *codersdk.DeploymentValues) { - dv.OIDC.OrganizationAssignDefault = true + // Will be overwritten by dynamic value + dv.OIDC.OrganizationAssignDefault = false dv.OIDC.OrganizationField = "organization" dv.OIDC.OrganizationMapping = serpent.Struct[map[string][]uuid.UUID]{ - Value: map[string][]uuid.UUID{ - "second": {second}, - "third": {third}, - }, + Value: map[string][]uuid.UUID{}, } }, }) - dbgen.Organization(t, runner.API.Database, database.Organization{ - ID: second, + + ctx := testutil.Context(t, testutil.WaitMedium) + orgOne, err := runner.AdminClient.CreateOrganization(ctx, codersdk.CreateOrganizationRequest{ + Name: "one", + DisplayName: "One", + Description: "", + Icon: "", }) - dbgen.Organization(t, runner.API.Database, database.Organization{ - ID: third, + require.NoError(t, err) + + orgTwo, err := runner.AdminClient.CreateOrganization(ctx, codersdk.CreateOrganizationRequest{ + Name: "two", + DisplayName: "two", + Description: "", + Icon: "", }) - fourth := dbgen.Organization(t, runner.API.Database, database.Organization{}) + require.NoError(t, err) + + orgThree, err := runner.AdminClient.CreateOrganization(ctx, codersdk.CreateOrganizationRequest{ + Name: "three", + DisplayName: "three", + }) + require.NoError(t, err) + + expectedSettings := codersdk.OrganizationSyncSettings{ + Field: "organization", + Mapping: map[string][]uuid.UUID{ + "first": {orgOne.ID}, + "second": {orgTwo.ID}, + }, + AssignDefault: true, + } + settings, err := runner.AdminClient.PatchOrganizationIDPSyncSettings(ctx, expectedSettings) + require.NoError(t, err) + require.Equal(t, expectedSettings.Field, settings.Field) - ctx := testutil.Context(t, testutil.WaitMedium) claims := jwt.MapClaims{ "email": "alice@coder.com", - "organization": []string{"second", "third"}, + "organization": []string{"first", "second"}, } // Then: a new user logs in with claims "second" and "third", they // should belong to [default, second, third]. userClient, resp := runner.Login(t, claims) require.Equal(t, http.StatusOK, resp.StatusCode) - runner.AssertOrganizations(t, "alice", true, []uuid.UUID{second, third}) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{orgOne.ID, orgTwo.ID}) user, err := userClient.User(ctx, codersdk.Me) require.NoError(t, err) // When: they are manually added to the fourth organization, a new sync // should remove them. - _, err = runner.AdminClient.PostOrganizationMember(ctx, fourth.ID, "alice") + _, err = runner.AdminClient.PostOrganizationMember(ctx, orgThree.ID, "alice") require.ErrorContains(t, err, "Organization sync is enabled") - runner.AssertOrganizations(t, "alice", true, []uuid.UUID{second, third}) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{orgOne.ID, orgTwo.ID}) // Go around the block to add the user to see if they are removed. dbgen.OrganizationMember(t, runner.API.Database, database.OrganizationMember{ UserID: user.ID, - OrganizationID: fourth.ID, + OrganizationID: orgThree.ID, }) - runner.AssertOrganizations(t, "alice", true, []uuid.UUID{second, third, fourth.ID}) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{orgOne.ID, orgTwo.ID, orgThree.ID}) // Then: Log in again will resync the orgs to their updated // claims. runner.Login(t, jwt.MapClaims{ "email": "alice@coder.com", - "organization": []string{"third"}, + "organization": []string{"second"}, }) - runner.AssertOrganizations(t, "alice", true, []uuid.UUID{third}) + runner.AssertOrganizations(t, "alice", true, []uuid.UUID{orgTwo.ID}) }) t.Run("MultiOrgWithoutDefault", func(t *testing.T) { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index f89d9eada822c..bd00dbda353c3 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -974,6 +974,13 @@ export interface OrganizationMemberWithUserData extends OrganizationMember { readonly global_roles: Readonly>; } +// From codersdk/idpsync.go +export interface OrganizationSyncSettings { + readonly field: string; + readonly mapping: Record>>; + readonly organization_assign_default: boolean; +} + // From codersdk/pagination.go export interface Pagination { readonly after_id?: string;