diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 7b85bd3323df2..19063d63b1c05 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -13961,7 +13961,7 @@ const docTemplate = `{ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/coderd.SCIMUser" + "$ref": "#/definitions/legacyscim.SCIMUser" } } ], @@ -13969,7 +13969,7 @@ const docTemplate = `{ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/coderd.SCIMUser" + "$ref": "#/definitions/legacyscim.SCIMUser" } } }, @@ -14035,7 +14035,7 @@ const docTemplate = `{ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/coderd.SCIMUser" + "$ref": "#/definitions/legacyscim.SCIMUser" } } ], @@ -14077,7 +14077,7 @@ const docTemplate = `{ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/coderd.SCIMUser" + "$ref": "#/definitions/legacyscim.SCIMUser" } } ], @@ -14288,71 +14288,6 @@ const docTemplate = `{ "ReinitializeReasonPrebuildClaimed" ] }, - "coderd.SCIMUser": { - "type": "object", - "properties": { - "active": { - "description": "Active is a ptr to prevent the empty value from being interpreted as false.", - "type": "boolean" - }, - "emails": { - "type": "array", - "items": { - "type": "object", - "properties": { - "display": { - "type": "string" - }, - "primary": { - "type": "boolean" - }, - "type": { - "type": "string" - }, - "value": { - "type": "string", - "format": "email" - } - } - } - }, - "groups": { - "type": "array", - "items": {} - }, - "id": { - "type": "string" - }, - "meta": { - "type": "object", - "properties": { - "resourceType": { - "type": "string" - } - } - }, - "name": { - "type": "object", - "properties": { - "familyName": { - "type": "string" - }, - "givenName": { - "type": "string" - } - } - }, - "schemas": { - "type": "array", - "items": { - "type": "string" - } - }, - "userName": { - "type": "string" - } - } - }, "coderd.cspViolation": { "type": "object", "properties": { @@ -18789,6 +18724,9 @@ const docTemplate = `{ "scim_api_key": { "type": "string" }, + "scim_use_legacy": { + "type": "boolean" + }, "session_lifetime": { "$ref": "#/definitions/codersdk.SessionLifetime" }, @@ -27406,6 +27344,71 @@ const docTemplate = `{ "key.NodePublic": { "type": "object" }, + "legacyscim.SCIMUser": { + "type": "object", + "properties": { + "active": { + "description": "Active is a ptr to prevent the empty value from being interpreted as false.", + "type": "boolean" + }, + "emails": { + "type": "array", + "items": { + "type": "object", + "properties": { + "display": { + "type": "string" + }, + "primary": { + "type": "boolean" + }, + "type": { + "type": "string" + }, + "value": { + "type": "string", + "format": "email" + } + } + } + }, + "groups": { + "type": "array", + "items": {} + }, + "id": { + "type": "string" + }, + "meta": { + "type": "object", + "properties": { + "resourceType": { + "type": "string" + } + } + }, + "name": { + "type": "object", + "properties": { + "familyName": { + "type": "string" + }, + "givenName": { + "type": "string" + } + } + }, + "schemas": { + "type": "array", + "items": { + "type": "string" + } + }, + "userName": { + "type": "string" + } + } + }, "netcheck.Report": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 0bff17f6dc572..55928186492cb 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -12389,7 +12389,7 @@ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/coderd.SCIMUser" + "$ref": "#/definitions/legacyscim.SCIMUser" } } ], @@ -12397,7 +12397,7 @@ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/coderd.SCIMUser" + "$ref": "#/definitions/legacyscim.SCIMUser" } } }, @@ -12455,7 +12455,7 @@ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/coderd.SCIMUser" + "$ref": "#/definitions/legacyscim.SCIMUser" } } ], @@ -12493,7 +12493,7 @@ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/coderd.SCIMUser" + "$ref": "#/definitions/legacyscim.SCIMUser" } } ], @@ -12692,71 +12692,6 @@ "enum": ["prebuild_claimed"], "x-enum-varnames": ["ReinitializeReasonPrebuildClaimed"] }, - "coderd.SCIMUser": { - "type": "object", - "properties": { - "active": { - "description": "Active is a ptr to prevent the empty value from being interpreted as false.", - "type": "boolean" - }, - "emails": { - "type": "array", - "items": { - "type": "object", - "properties": { - "display": { - "type": "string" - }, - "primary": { - "type": "boolean" - }, - "type": { - "type": "string" - }, - "value": { - "type": "string", - "format": "email" - } - } - } - }, - "groups": { - "type": "array", - "items": {} - }, - "id": { - "type": "string" - }, - "meta": { - "type": "object", - "properties": { - "resourceType": { - "type": "string" - } - } - }, - "name": { - "type": "object", - "properties": { - "familyName": { - "type": "string" - }, - "givenName": { - "type": "string" - } - } - }, - "schemas": { - "type": "array", - "items": { - "type": "string" - } - }, - "userName": { - "type": "string" - } - } - }, "coderd.cspViolation": { "type": "object", "properties": { @@ -17060,6 +16995,9 @@ "scim_api_key": { "type": "string" }, + "scim_use_legacy": { + "type": "boolean" + }, "session_lifetime": { "$ref": "#/definitions/codersdk.SessionLifetime" }, @@ -25271,6 +25209,71 @@ "key.NodePublic": { "type": "object" }, + "legacyscim.SCIMUser": { + "type": "object", + "properties": { + "active": { + "description": "Active is a ptr to prevent the empty value from being interpreted as false.", + "type": "boolean" + }, + "emails": { + "type": "array", + "items": { + "type": "object", + "properties": { + "display": { + "type": "string" + }, + "primary": { + "type": "boolean" + }, + "type": { + "type": "string" + }, + "value": { + "type": "string", + "format": "email" + } + } + } + }, + "groups": { + "type": "array", + "items": {} + }, + "id": { + "type": "string" + }, + "meta": { + "type": "object", + "properties": { + "resourceType": { + "type": "string" + } + } + }, + "name": { + "type": "object", + "properties": { + "familyName": { + "type": "string" + }, + "givenName": { + "type": "string" + } + } + }, + "schemas": { + "type": "array", + "items": { + "type": "string" + } + }, + "userName": { + "type": "string" + } + } + }, "netcheck.Report": { "type": "object", "properties": { diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index e0d5617c1a8dc..c6c73bb85ebcc 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -741,6 +741,29 @@ var ( }), Scope: rbac.ScopeAll, }.WithCachedASTValue() + + subjectSCIM = rbac.Subject{ + Type: rbac.SubjectSCIMProvisioner, + FriendlyName: "SCIM Provisioner", + ID: uuid.Nil.String(), + Roles: rbac.Roles([]rbac.Role{ + { + Identifier: rbac.RoleIdentifier{Name: "scim"}, + DisplayName: "SCIM", + Site: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceSystem.Type: {policy.ActionRead}, // Required for idp config reads, this should be fixed + rbac.ResourceAssignRole.Type: rbac.ResourceAssignRole.AvailableActions(), + rbac.ResourceAssignOrgRole.Type: rbac.ResourceAssignOrgRole.AvailableActions(), + rbac.ResourceUser.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionRead, policy.ActionUpdatePersonal}, + rbac.ResourceOrganization.Type: {policy.ActionRead}, + rbac.ResourceOrganizationMember.Type: {policy.ActionRead, policy.ActionCreate, policy.ActionUpdate}, + }), + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, + }, + }), + Scope: rbac.ScopeAll, + }.WithCachedASTValue() ) // AsProvisionerd returns a context with an actor that has permissions required @@ -871,6 +894,12 @@ func AsAIProviderMetadataReader(ctx context.Context) context.Context { return As(ctx, subjectAIProviderMetadataReader) } +// AsSubAgentAPI returns a context with an actor that has permissions required for +// handling the /scim/v2 routes and provisioning users via SCIM. +func AsSCIMProvisioner(ctx context.Context) context.Context { + return As(ctx, subjectSCIM) +} + var AsRemoveActor = rbac.Subject{ ID: "remove-actor", } @@ -4634,7 +4663,8 @@ func (q *querier) GetUserCodeDiffDisplayMode(ctx context.Context, userID uuid.UU } func (q *querier) GetUserCount(ctx context.Context, includeSystem bool) (int64, error) { - if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { + // If you can read every user, then you can read the count of users. + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceUser); err != nil { return 0, err } return q.db.GetUserCount(ctx, includeSystem) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 354453da40000..582af77796dac 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -4546,7 +4546,7 @@ func (s *MethodTestSuite) TestSystemFunctions() { })) s.Run("GetUserCount", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { dbm.EXPECT().GetUserCount(gomock.Any(), false).Return(int64(0), nil).AnyTimes() - check.Args(false).Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(int64(0)) + check.Args(false).Asserts(rbac.ResourceUser, policy.ActionRead).Returns(int64(0)) })) s.Run("GetTemplates", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { dbm.EXPECT().GetTemplates(gomock.Any()).Return([]database.Template{}, nil).AnyTimes() diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index f33c047c942cc..f7befa7cb1b70 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -413,6 +413,8 @@ func (q *sqlQuerier) GetAuthorizedUsers(ctx context.Context, arg GetUsersParams, arg.AfterID, arg.Search, arg.Name, + arg.Exactusername, + arg.Exactemail, pq.Array(arg.Status), pq.Array(arg.RbacRole), arg.LastSeenBefore, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 58f3bd5fc7b3f..3394539430ba8 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -27765,65 +27765,77 @@ WHERE name ILIKE concat('%', $3, '%') ELSE true END + -- Filter by exact username + AND CASE + WHEN $4 :: text != '' THEN + lower(username) = lower($4) + ELSE true + END + -- Filter by exact email + AND CASE + WHEN $5 :: text != '' THEN + lower(email) = lower($5) + ELSE true + END -- Filter by status AND CASE -- @status needs to be a text because it can be empty, If it was -- user_status enum, it would not. - WHEN cardinality($4 :: user_status[]) > 0 THEN - status = ANY($4 :: user_status[]) + WHEN cardinality($6 :: user_status[]) > 0 THEN + status = ANY($6 :: user_status[]) ELSE true END -- Filter by rbac_roles AND CASE -- @rbac_role allows filtering by rbac roles. If 'member' is included, show everyone, as -- everyone is a member. - WHEN cardinality($5 :: text[]) > 0 AND 'member' != ANY($5 :: text[]) THEN - rbac_roles && $5 :: text[] + WHEN cardinality($7 :: text[]) > 0 AND 'member' != ANY($7 :: text[]) THEN + rbac_roles && $7 :: text[] ELSE true END -- Filter by last_seen AND CASE - WHEN $6 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN - last_seen_at <= $6 + WHEN $8 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + last_seen_at <= $8 ELSE true END AND CASE - WHEN $7 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN - last_seen_at >= $7 + WHEN $9 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + last_seen_at >= $9 ELSE true END -- Filter by created_at AND CASE - WHEN $8 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN - created_at <= $8 + WHEN $10 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + created_at <= $10 ELSE true END AND CASE - WHEN $9 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN - created_at >= $9 + WHEN $11 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + created_at >= $11 ELSE true END -- Filter by system type AND CASE - WHEN $10::bool THEN TRUE + WHEN $12::bool THEN TRUE ELSE is_system = false END -- Filter by github.com user ID AND CASE - WHEN $11 :: bigint != 0 THEN - github_com_user_id = $11 + WHEN $13 :: bigint != 0 THEN + github_com_user_id = $13 ELSE true END -- Filter by login_type AND CASE - WHEN cardinality($12 :: login_type[]) > 0 THEN - login_type = ANY($12 :: login_type[]) + WHEN cardinality($14 :: login_type[]) > 0 THEN + login_type = ANY($14 :: login_type[]) ELSE true END -- Filter by service account. AND CASE - WHEN $13 :: boolean IS NOT NULL THEN - is_service_account = $13 :: boolean + WHEN $15 :: boolean IS NOT NULL THEN + is_service_account = $15 :: boolean ELSE true END -- End of filters @@ -27832,16 +27844,18 @@ WHERE -- @authorize_filter ORDER BY -- Deterministic and consistent ordering of all users. This is to ensure consistent pagination. - LOWER(username) ASC OFFSET $14 + LOWER(username) ASC OFFSET $16 LIMIT -- A null limit means "no limit", so 0 means return all - NULLIF($15 :: int, 0) + NULLIF($17 :: int, 0) ` type GetUsersParams struct { AfterID uuid.UUID `db:"after_id" json:"after_id"` Search string `db:"search" json:"search"` Name string `db:"name" json:"name"` + Exactusername string `db:"exactusername" json:"exactusername"` + Exactemail string `db:"exactemail" json:"exactemail"` Status []UserStatus `db:"status" json:"status"` RbacRole []string `db:"rbac_role" json:"rbac_role"` LastSeenBefore time.Time `db:"last_seen_before" json:"last_seen_before"` @@ -27886,6 +27900,8 @@ func (q *sqlQuerier) GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUse arg.AfterID, arg.Search, arg.Name, + arg.Exactusername, + arg.Exactemail, pq.Array(arg.Status), pq.Array(arg.RbacRole), arg.LastSeenBefore, diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index 03f403e145c91..fd4a65385227d 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -486,6 +486,18 @@ WHERE name ILIKE concat('%', @name, '%') ELSE true END + -- Filter by exact username + AND CASE + WHEN @exactUsername :: text != '' THEN + lower(username) = lower(@exactUsername) + ELSE true + END + -- Filter by exact email + AND CASE + WHEN @exactEmail :: text != '' THEN + lower(email) = lower(@exactEmail) + ELSE true + END -- Filter by status AND CASE -- @status needs to be a text because it can be empty, If it was diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index 5d40e59cc6fac..155b1907a498e 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -85,6 +85,7 @@ const ( SubjectTypeWorkspaceBuilder SubjectType = "workspace_builder" SubjectTypeChatd SubjectType = "chatd" SubjectTypeAIProviderMetadataReader SubjectType = "ai_provider_metadata_reader" + SubjectSCIMProvisioner SubjectType = "scim_provisioner" ) const ( diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 97dcd6e27d72e..823833c77a9f4 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -638,6 +638,7 @@ type DeploymentValues struct { AgentFallbackTroubleshootingURL serpent.URL `json:"agent_fallback_troubleshooting_url,omitempty" typescript:",notnull"` BrowserOnly serpent.Bool `json:"browser_only,omitempty" typescript:",notnull"` SCIMAPIKey serpent.String `json:"scim_api_key,omitempty" typescript:",notnull"` + UseLegacySCIM serpent.Bool `json:"scim_use_legacy,omitempty" typescript:",notnull"` ExternalTokenEncryptionKeys serpent.StringArray `json:"external_token_encryption_keys,omitempty" typescript:",notnull"` Provisioner ProvisionerConfig `json:"provisioner,omitempty" typescript:",notnull"` RateLimit RateLimitConfig `json:"rate_limit,omitempty" typescript:",notnull"` @@ -3437,6 +3438,18 @@ func (c *DeploymentValues) Options() serpent.OptionSet { Annotations: serpent.Annotations{}.Mark(annotationEnterpriseKey, "true").Mark(annotationSecretKey, "true"), Value: &c.SCIMAPIKey, }, + { + Name: "SCIM Use Legacy", + // The legacy SCIM is a weird mix of SCIM 1.0 and SCIM 2.0 + Description: "Use the legacy SCIM implementation instead of the SCIM 2.0 handler. This is provided for backward compatibility for existing users.", + Flag: "scim-use-legacy", + Env: "CODER_SCIM_USE_LEGACY", + Hidden: true, + // TODO: When SCIM 2.0 has been tested more, flip this to false to default to the new scim + Default: "true", + Annotations: serpent.Annotations{}.Mark(annotationEnterpriseKey, "true"), + Value: &c.UseLegacySCIM, + }, { Name: "External Token Encryption Keys", Description: "Encrypt OIDC and Git authentication tokens with AES-256-GCM in the database. The value must be a comma-separated list of base64-encoded keys. Each key, when base64-decoded, must be exactly 32 bytes in length. The first key will be used to encrypt new values. Subsequent keys will be used as a fallback when decrypting. During normal operation it is recommended to only set one key unless you are in the process of rotating keys with the `coder server dbcrypt rotate` command.", diff --git a/docs/reference/api/enterprise.md b/docs/reference/api/enterprise.md index def5c219a2e24..eca72025c0e0d 100644 --- a/docs/reference/api/enterprise.md +++ b/docs/reference/api/enterprise.md @@ -4520,9 +4520,9 @@ curl -X POST http://coder-server:8080/scim/v2/Users \ ### Parameters -| Name | In | Type | Required | Description | -|--------|------|----------------------------------------------|----------|-------------| -| `body` | body | [coderd.SCIMUser](schemas.md#coderdscimuser) | true | New user | +| Name | In | Type | Required | Description | +|--------|------|------------------------------------------------------|----------|-------------| +| `body` | body | [legacyscim.SCIMUser](schemas.md#legacyscimscimuser) | true | New user | ### Example responses @@ -4559,9 +4559,9 @@ curl -X POST http://coder-server:8080/scim/v2/Users \ ### Responses -| Status | Meaning | Description | Schema | -|--------|---------------------------------------------------------|-------------|----------------------------------------------| -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [coderd.SCIMUser](schemas.md#coderdscimuser) | +| Status | Meaning | Description | Schema | +|--------|---------------------------------------------------------|-------------|------------------------------------------------------| +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [legacyscim.SCIMUser](schemas.md#legacyscimscimuser) | To perform this operation, you must be authenticated. [Learn more](authentication.md). @@ -4638,10 +4638,10 @@ curl -X PUT http://coder-server:8080/scim/v2/Users/{id} \ ### Parameters -| Name | In | Type | Required | Description | -|--------|------|----------------------------------------------|----------|----------------------| -| `id` | path | string(uuid) | true | User ID | -| `body` | body | [coderd.SCIMUser](schemas.md#coderdscimuser) | true | Replace user request | +| Name | In | Type | Required | Description | +|--------|------|------------------------------------------------------|----------|----------------------| +| `id` | path | string(uuid) | true | User ID | +| `body` | body | [legacyscim.SCIMUser](schemas.md#legacyscimscimuser) | true | Replace user request | ### Example responses @@ -4730,10 +4730,10 @@ curl -X PATCH http://coder-server:8080/scim/v2/Users/{id} \ ### Parameters -| Name | In | Type | Required | Description | -|--------|------|----------------------------------------------|----------|---------------------| -| `id` | path | string(uuid) | true | User ID | -| `body` | body | [coderd.SCIMUser](schemas.md#coderdscimuser) | true | Update user request | +| Name | In | Type | Required | Description | +|--------|------|------------------------------------------------------|----------|---------------------| +| `id` | path | string(uuid) | true | User ID | +| `body` | body | [legacyscim.SCIMUser](schemas.md#legacyscimscimuser) | true | Update user request | ### Example responses diff --git a/docs/reference/api/general.md b/docs/reference/api/general.md index 57e564aee31bb..90715c620c84b 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -538,6 +538,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "workspace_agent_logs": 0 }, "scim_api_key": "string", + "scim_use_legacy": true, "session_lifetime": { "default_duration": 0, "default_token_lifetime": 0, diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index c2cffd13d441b..92b379bbfc82a 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -220,57 +220,6 @@ |--------------------| | `prebuild_claimed` | -## coderd.SCIMUser - -```json -{ - "active": true, - "emails": [ - { - "display": "string", - "primary": true, - "type": "string", - "value": "user@example.com" - } - ], - "groups": [ - null - ], - "id": "string", - "meta": { - "resourceType": "string" - }, - "name": { - "familyName": "string", - "givenName": "string" - }, - "schemas": [ - "string" - ], - "userName": "string" -} -``` - -### Properties - -| Name | Type | Required | Restrictions | Description | -|------------------|--------------------|----------|--------------|-----------------------------------------------------------------------------| -| `active` | boolean | false | | Active is a ptr to prevent the empty value from being interpreted as false. | -| `emails` | array of object | false | | | -| `» display` | string | false | | | -| `» primary` | boolean | false | | | -| `» type` | string | false | | | -| `» value` | string | false | | | -| `groups` | array of undefined | false | | | -| `id` | string | false | | | -| `meta` | object | false | | | -| `» resourceType` | string | false | | | -| `name` | object | false | | | -| `» familyName` | string | false | | | -| `» givenName` | string | false | | | -| `schemas` | array of string | false | | | -| `userName` | string | false | | | - ## coderd.cspViolation ```json @@ -6059,6 +6008,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "workspace_agent_logs": 0 }, "scim_api_key": "string", + "scim_use_legacy": true, "session_lifetime": { "default_duration": 0, "default_token_lifetime": 0, @@ -6658,6 +6608,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "workspace_agent_logs": 0 }, "scim_api_key": "string", + "scim_use_legacy": true, "session_lifetime": { "default_duration": 0, "default_token_lifetime": 0, @@ -6818,6 +6769,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `redirect_to_access_url` | boolean | false | | | | `retention` | [codersdk.RetentionConfig](#codersdkretentionconfig) | false | | | | `scim_api_key` | string | false | | | +| `scim_use_legacy` | boolean | false | | | | `session_lifetime` | [codersdk.SessionLifetime](#codersdksessionlifetime) | false | | | | `ssh_keygen_algorithm` | string | false | | | | `stats_collection` | [codersdk.StatsCollectionConfig](#codersdkstatscollectionconfig) | false | | | @@ -17916,6 +17868,57 @@ Zero means unspecified. There might be a limit, but the client need not try to r None +## legacyscim.SCIMUser + +```json +{ + "active": true, + "emails": [ + { + "display": "string", + "primary": true, + "type": "string", + "value": "user@example.com" + } + ], + "groups": [ + null + ], + "id": "string", + "meta": { + "resourceType": "string" + }, + "name": { + "familyName": "string", + "givenName": "string" + }, + "schemas": [ + "string" + ], + "userName": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|------------------|--------------------|----------|--------------|-----------------------------------------------------------------------------| +| `active` | boolean | false | | Active is a ptr to prevent the empty value from being interpreted as false. | +| `emails` | array of object | false | | | +| `» display` | string | false | | | +| `» primary` | boolean | false | | | +| `» type` | string | false | | | +| `» value` | string | false | | | +| `groups` | array of undefined | false | | | +| `id` | string | false | | | +| `meta` | object | false | | | +| `» resourceType` | string | false | | | +| `name` | object | false | | | +| `» familyName` | string | false | | | +| `» givenName` | string | false | | | +| `schemas` | array of string | false | | | +| `userName` | string | false | | | + ## netcheck.Report ```json diff --git a/enterprise/cli/server.go b/enterprise/cli/server.go index 0ffc730a37a1a..29f05b814e203 100644 --- a/enterprise/cli/server.go +++ b/enterprise/cli/server.go @@ -96,6 +96,7 @@ func (r *RootCmd) Server(_ func()) *serpent.Command { ConnectionLogging: true, BrowserOnly: options.DeploymentValues.BrowserOnly.Value(), SCIMAPIKey: []byte(options.DeploymentValues.SCIMAPIKey.Value()), + UseLegacySCIM: options.DeploymentValues.UseLegacySCIM.Value(), RBAC: true, DERPServerRelayAddress: options.DeploymentValues.DERP.Server.RelayURL.String(), DERPServerRegionID: int(options.DeploymentValues.DERP.Server.RegionID.Value()), diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 82fdd4d27f405..2a55c6218f00f 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -622,40 +622,12 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { }) }) - if len(options.SCIMAPIKey) != 0 { - api.AGPL.RootHandler.Route("/scim/v2", func(r chi.Router) { - r.Use( - api.RequireFeatureMW(codersdk.FeatureSCIM), - ) - r.Get("/ServiceProviderConfig", api.scimServiceProviderConfig) - r.Post("/Users", api.scimPostUser) - r.Route("/Users", func(r chi.Router) { - r.Get("/", api.scimGetUsers) - r.Post("/", api.scimPostUser) - r.Get("/{id}", api.scimGetUser) - r.Patch("/{id}", api.scimPatchUser) - r.Put("/{id}", api.scimPutUser) - }) - r.NotFound(func(w http.ResponseWriter, r *http.Request) { - u := r.URL.String() - httpapi.Write(r.Context(), w, http.StatusNotFound, codersdk.Response{ - Message: fmt.Sprintf("SCIM endpoint %s not found", u), - Detail: "This endpoint is not implemented. If it is correct and required, please contact support.", - }) - }) - }) - } else { - // Show a helpful 404 error. Because this is not under the /api/v2 routes, - // the frontend is the fallback. A html page is not a helpful error for - // a SCIM provider. This JSON has a call to action that __may__ resolve - // the issue. - // Using Mount to cover all subroute possibilities. - api.AGPL.RootHandler.Mount("/scim/v2", http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - httpapi.Write(r.Context(), w, http.StatusNotFound, codersdk.Response{ - Message: "SCIM is disabled, please contact your administrator if you believe this is an error", - Detail: "SCIM endpoints are disabled if no SCIM is configured. Configure 'CODER_SCIM_AUTH_HEADER' to enable.", - }) - }))) + var mountScimError error + api.AGPL.RootHandler.Route("/scim", func(r chi.Router) { + mountScimError = api.mountScimRoute(options, r) + }) + if mountScimError != nil { + return nil, xerrors.Errorf("mount scim routes: %w", mountScimError) } // We always want to run the replica manager even if we don't have DERP @@ -754,6 +726,11 @@ type Options struct { // Whether to block non-browser connections. BrowserOnly bool SCIMAPIKey []byte + // UseLegacySCIM opts into the legacy SCIM handler implementation + // (imulab/go-scim based). This is provided for backward compatibility + // during the transition to the new elimity-com/scim implementation. + // It will be removed in a future release. + UseLegacySCIM bool ExternalTokenEncryption []dbcrypt.Cipher diff --git a/enterprise/coderd/coderdenttest/coderdenttest.go b/enterprise/coderd/coderdenttest/coderdenttest.go index a7efd1b3023c5..1115ba12118c7 100644 --- a/enterprise/coderd/coderdenttest/coderdenttest.go +++ b/enterprise/coderd/coderdenttest/coderdenttest.go @@ -67,6 +67,7 @@ type Options struct { BrowserOnly bool EntitlementsUpdateInterval time.Duration SCIMAPIKey []byte + UseLegacySCIM bool UserWorkspaceQuota int ProxyHealthInterval time.Duration LicenseOptions *LicenseOptions @@ -108,6 +109,7 @@ func NewWithAPI(t *testing.T, options *Options) ( AuditLogging: options.AuditLogging, BrowserOnly: options.BrowserOnly, SCIMAPIKey: options.SCIMAPIKey, + UseLegacySCIM: options.UseLegacySCIM, DERPServerRelayAddress: serverURL.String(), DERPServerRegionID: int(oop.DeploymentValues.DERP.Server.RegionID.Value()), ReplicaSyncUpdateInterval: options.ReplicaSyncUpdateInterval, diff --git a/enterprise/coderd/scim.go b/enterprise/coderd/legacyscim/legacyscim.go similarity index 65% rename from enterprise/coderd/scim.go rename to enterprise/coderd/legacyscim/legacyscim.go index 5d0b248abdc65..d92965f3bd814 100644 --- a/enterprise/coderd/scim.go +++ b/enterprise/coderd/legacyscim/legacyscim.go @@ -1,4 +1,12 @@ -package coderd +// Package legacyscim contains the legacy SCIM 1.1-era (alleged 2.0) handler +// implementation. For backwards compatibility during the transition, this package +// is preserved in May 2026. Remove this package in it's entirely and rely on the +// new Scim 2.0 implementation in enterprise/coderd/scim after the transition is complete. +// +// It can be enabled via the UseLegacySCIM option. +// +// Deprecated: Use the new enterprise/coderd/scim package instead. +package legacyscim import ( "bytes" @@ -6,6 +14,8 @@ import ( "database/sql" "encoding/json" "net/http" + "net/url" + "sync/atomic" "time" "github.com/go-chi/chi/v5" @@ -16,17 +26,64 @@ import ( "github.com/imulab/go-scim/pkg/v2/spec" "golang.org/x/xerrors" + "cdr.dev/slog/v3" agpl "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/coderd/idpsync" "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/enterprise/coderd/scim" ) -func (api *API) scimVerifyAuthHeader(r *http.Request) bool { +// LegacyServer is the old SCIM handler implementation, kept for backward +// compatibility. It uses the imulab/go-scim library and custom JSON handling. +type LegacyServer struct { + Logger slog.Logger + Database database.Store + IDPSync idpsync.IDPSync + AGPL *agpl.API + AccessURL *url.URL + SCIMAPIKey []byte + Auditor *atomic.Pointer[audit.Auditor] +} + +// Handler returns an http.Handler that serves the legacy SCIM endpoints. +// It should be mounted at /scim/v2. +func (s *LegacyServer) Handler() http.Handler { + r := chi.NewRouter() + r.Get("/ServiceProviderConfig", s.scimServiceProviderConfig) + r.Post("/Users", s.scimPostUser) + r.Route("/Users", func(r chi.Router) { + r.Get("/", s.scimGetUsers) + r.Post("/", s.scimPostUser) + r.Get("/{id}", s.scimGetUser) + r.Patch("/{id}", s.scimPatchUser) + r.Put("/{id}", s.scimPutUser) + }) + r.NotFound(func(w http.ResponseWriter, r *http.Request) { + u := r.URL.String() + httpapi.Write(r.Context(), w, http.StatusNotFound, codersdk.Response{ + Message: "SCIM endpoint not found: " + u, + Detail: "This endpoint is not implemented. If it is correct and required, please contact support.", + }) + }) + return r +} + +// AuthMiddleware verifies the SCIM Bearer token. +func (s *LegacyServer) AuthMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + if !s.scimVerifyAuthHeader(r) { + scimUnauthorized(rw) + return + } + next.ServeHTTP(rw, r) + }) +} + +func (s *LegacyServer) scimVerifyAuthHeader(r *http.Request) bool { bearer := []byte("bearer ") hdr := []byte(r.Header.Get("Authorization")) @@ -35,11 +92,11 @@ func (api *API) scimVerifyAuthHeader(r *http.Request) bool { hdr = hdr[len(bearer):] } - return len(api.SCIMAPIKey) != 0 && subtle.ConstantTimeCompare(hdr, api.SCIMAPIKey) == 1 + return len(s.SCIMAPIKey) != 0 && subtle.ConstantTimeCompare(hdr, s.SCIMAPIKey) == 1 } func scimUnauthorized(rw http.ResponseWriter) { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusUnauthorized, "invalidAuthorization", xerrors.New("invalid authorization"))) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusUnauthorized, "invalidAuthorization", xerrors.New("invalid authorization"))) } // scimServiceProviderConfig returns a static SCIM service provider configuration. @@ -50,7 +107,7 @@ func scimUnauthorized(rw http.ResponseWriter) { // @Tags Enterprise // @Success 200 // @Router /scim/v2/ServiceProviderConfig [get] -func (api *API) scimServiceProviderConfig(rw http.ResponseWriter, _ *http.Request) { +func (s *LegacyServer) scimServiceProviderConfig(rw http.ResponseWriter, _ *http.Request) { // No auth needed to query this endpoint. rw.Header().Set("Content-Type", spec.ApplicationScimJson) @@ -60,35 +117,35 @@ func (api *API) scimServiceProviderConfig(rw http.ResponseWriter, _ *http.Reques // Increment this time if you make any changes to the provider config. providerUpdated := time.Date(2024, 10, 25, 17, 0, 0, 0, time.UTC) var location string - locURL, err := api.AccessURL.Parse("/scim/v2/ServiceProviderConfig") + locURL, err := s.AccessURL.Parse("/scim/v2/ServiceProviderConfig") if err == nil { location = locURL.String() } enc := json.NewEncoder(rw) enc.SetEscapeHTML(true) - _ = enc.Encode(scim.ServiceProviderConfig{ + _ = enc.Encode(ServiceProviderConfig{ Schemas: []string{"urn:ietf:params:scim:schemas:core:2.0:ServiceProviderConfig"}, DocURI: "https://coder.com/docs/admin/users/oidc-auth#scim", - Patch: scim.Supported{ + Patch: Supported{ Supported: true, }, - Bulk: scim.BulkSupported{ + Bulk: BulkSupported{ Supported: false, }, - Filter: scim.FilterSupported{ + Filter: FilterSupported{ Supported: false, }, - ChangePassword: scim.Supported{ + ChangePassword: Supported{ Supported: false, }, - Sort: scim.Supported{ + Sort: Supported{ Supported: false, }, - ETag: scim.Supported{ + ETag: Supported{ Supported: false, }, - AuthSchemes: []scim.AuthenticationScheme{ + AuthSchemes: []AuthenticationScheme{ { Type: "oauthbearertoken", Name: "HTTP Header Authentication", @@ -96,7 +153,7 @@ func (api *API) scimServiceProviderConfig(rw http.ResponseWriter, _ *http.Reques DocURI: "https://coder.com/docs/admin/users/oidc-auth#scim", }, }, - Meta: scim.ServiceProviderMeta{ + Meta: ServiceProviderMeta{ Created: providerUpdated, LastModified: providerUpdated, Location: location, @@ -118,8 +175,8 @@ func (api *API) scimServiceProviderConfig(rw http.ResponseWriter, _ *http.Reques // @Router /scim/v2/Users [get] // //nolint:revive -func (api *API) scimGetUsers(rw http.ResponseWriter, r *http.Request) { - if !api.scimVerifyAuthHeader(r) { +func (s *LegacyServer) scimGetUsers(rw http.ResponseWriter, r *http.Request) { + if !s.scimVerifyAuthHeader(r) { scimUnauthorized(rw) return } @@ -146,13 +203,13 @@ func (api *API) scimGetUsers(rw http.ResponseWriter, r *http.Request) { // @Router /scim/v2/Users/{id} [get] // //nolint:revive -func (api *API) scimGetUser(rw http.ResponseWriter, r *http.Request) { - if !api.scimVerifyAuthHeader(r) { +func (s *LegacyServer) scimGetUser(rw http.ResponseWriter, r *http.Request) { + if !s.scimVerifyAuthHeader(r) { scimUnauthorized(rw) return } - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusNotFound, spec.ErrNotFound.Type, xerrors.New("endpoint will always return 404"))) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusNotFound, spec.ErrNotFound.Type, xerrors.New("endpoint will always return 404"))) } // We currently use our own struct instead of using the SCIM package. This was @@ -193,20 +250,20 @@ var SCIMAuditAdditionalFields = map[string]string{ // @Security Authorization // @Produce json // @Tags Enterprise -// @Param request body coderd.SCIMUser true "New user" -// @Success 200 {object} coderd.SCIMUser +// @Param request body legacyscim.SCIMUser true "New user" +// @Success 200 {object} legacyscim.SCIMUser // @Router /scim/v2/Users [post] -func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { +func (s *LegacyServer) scimPostUser(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - if !api.scimVerifyAuthHeader(r) { + if !s.scimVerifyAuthHeader(r) { scimUnauthorized(rw) return } - auditor := *api.AGPL.Auditor.Load() + auditor := *s.Auditor.Load() aReq, commitAudit := audit.InitRequest[database.User](rw, &audit.RequestParams{ Audit: auditor, - Log: api.Logger, + Log: s.Logger, Request: r, Action: database.AuditActionCreate, AdditionalFields: SCIMAuditAdditionalFields, @@ -216,12 +273,12 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { var sUser SCIMUser err := json.NewDecoder(r.Body).Decode(&sUser) if err != nil { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusBadRequest, "invalidRequest", err)) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusBadRequest, "invalidRequest", err)) return } if sUser.Active == nil { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusBadRequest, "invalidRequest", xerrors.New("active field is required"))) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusBadRequest, "invalidRequest", xerrors.New("active field is required"))) return } @@ -234,12 +291,12 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { } if email == "" { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusBadRequest, "invalidEmail", xerrors.New("no primary email provided"))) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusBadRequest, "invalidEmail", xerrors.New("no primary email provided"))) return } //nolint:gocritic - dbUser, err := api.Database.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{ + dbUser, err := s.Database.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{ Email: email, Username: sUser.UserName, }) @@ -253,7 +310,7 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { if *sUser.Active && dbUser.Status == database.UserStatusSuspended { //nolint:gocritic - newUser, err := api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{ + newUser, err := s.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{ ID: dbUser.ID, // The user will get transitioned to Active after logging in. Status: database.UserStatusDormant, @@ -295,23 +352,23 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { // This is to preserve single org deployment behavior. organizations := []uuid.UUID{} //nolint:gocritic // SCIM operations are a system user - orgSync, err := api.IDPSync.OrganizationSyncSettings(dbauthz.AsSystemRestricted(ctx), api.Database) + orgSync, err := s.IDPSync.OrganizationSyncSettings(dbauthz.AsSystemRestricted(ctx), s.Database) if err != nil { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusInternalServerError, "internalError", xerrors.Errorf("failed to get organization sync settings: %w", err))) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusInternalServerError, "internalError", 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)) + defaultOrganization, err := s.Database.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx)) if err != nil { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusInternalServerError, "internalError", xerrors.Errorf("failed to get default organization: %w", err))) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusInternalServerError, "internalError", xerrors.Errorf("failed to get default organization: %w", err))) return } organizations = append(organizations, defaultOrganization.ID) } //nolint:gocritic // needed for SCIM - dbUser, err = api.AGPL.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, agpl.CreateUserRequest{ + dbUser, err = s.AGPL.CreateUser(dbauthz.AsSystemRestricted(ctx), s.Database, agpl.CreateUserRequest{ CreateUserRequestWithOrgs: codersdk.CreateUserRequestWithOrgs{ Username: sUser.UserName, Email: email, @@ -322,7 +379,7 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { SkipNotifications: true, }) if err != nil { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusInternalServerError, "internalError", xerrors.Errorf("failed to create user: %w", err))) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusInternalServerError, "internalError", xerrors.Errorf("failed to create user: %w", err))) return } aReq.New = dbUser @@ -342,20 +399,20 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { // @Produce application/scim+json // @Tags Enterprise // @Param id path string true "User ID" format(uuid) -// @Param request body coderd.SCIMUser true "Update user request" +// @Param request body legacyscim.SCIMUser true "Update user request" // @Success 200 {object} codersdk.User // @Router /scim/v2/Users/{id} [patch] -func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) { +func (s *LegacyServer) scimPatchUser(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - if !api.scimVerifyAuthHeader(r) { + if !s.scimVerifyAuthHeader(r) { scimUnauthorized(rw) return } - auditor := *api.AGPL.Auditor.Load() + auditor := *s.Auditor.Load() aReq, commitAudit := audit.InitRequestWithCancel[database.User](rw, &audit.RequestParams{ Audit: auditor, - Log: api.Logger, + Log: s.Logger, Request: r, Action: database.AuditActionWrite, }) @@ -367,19 +424,19 @@ func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) { var sUser SCIMUser err := json.NewDecoder(r.Body).Decode(&sUser) if err != nil { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusBadRequest, "invalidRequest", err)) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusBadRequest, "invalidRequest", err)) return } sUser.ID = id uid, err := uuid.Parse(id) if err != nil { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusBadRequest, "invalidId", xerrors.Errorf("id must be a uuid: %w", err))) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusBadRequest, "invalidId", xerrors.Errorf("id must be a uuid: %w", err))) return } //nolint:gocritic // needed for SCIM - dbUser, err := api.Database.GetUserByID(dbauthz.AsSystemRestricted(ctx), uid) + dbUser, err := s.Database.GetUserByID(dbauthz.AsSystemRestricted(ctx), uid) if err != nil { _ = handlerutil.WriteError(rw, err) // internal error return @@ -388,14 +445,14 @@ func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) { aReq.UserID = dbUser.ID if sUser.Active == nil { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusBadRequest, "invalidRequest", xerrors.New("active field is required"))) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusBadRequest, "invalidRequest", xerrors.New("active field is required"))) return } newStatus := scimUserStatus(dbUser, *sUser.Active) if dbUser.Status != newStatus { //nolint:gocritic // needed for SCIM - userNew, err := api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{ + userNew, err := s.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{ ID: dbUser.ID, Status: newStatus, UpdatedAt: dbtime.Now(), @@ -426,20 +483,20 @@ func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) { // @Produce application/scim+json // @Tags Enterprise // @Param id path string true "User ID" format(uuid) -// @Param request body coderd.SCIMUser true "Replace user request" +// @Param request body legacyscim.SCIMUser true "Replace user request" // @Success 200 {object} codersdk.User // @Router /scim/v2/Users/{id} [put] -func (api *API) scimPutUser(rw http.ResponseWriter, r *http.Request) { +func (s *LegacyServer) scimPutUser(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - if !api.scimVerifyAuthHeader(r) { + if !s.scimVerifyAuthHeader(r) { scimUnauthorized(rw) return } - auditor := *api.AGPL.Auditor.Load() + auditor := *s.Auditor.Load() aReq, commitAudit := audit.InitRequestWithCancel[database.User](rw, &audit.RequestParams{ Audit: auditor, - Log: api.Logger, + Log: s.Logger, Request: r, Action: database.AuditActionWrite, }) @@ -451,23 +508,23 @@ func (api *API) scimPutUser(rw http.ResponseWriter, r *http.Request) { var sUser SCIMUser err := json.NewDecoder(r.Body).Decode(&sUser) if err != nil { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusBadRequest, "invalidRequest", err)) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusBadRequest, "invalidRequest", err)) return } sUser.ID = id if sUser.Active == nil { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusBadRequest, "invalidRequest", xerrors.New("active field is required"))) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusBadRequest, "invalidRequest", xerrors.New("active field is required"))) return } uid, err := uuid.Parse(id) if err != nil { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusBadRequest, "invalidId", xerrors.Errorf("id must be a uuid: %w", err))) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusBadRequest, "invalidId", xerrors.Errorf("id must be a uuid: %w", err))) return } //nolint:gocritic // needed for SCIM - dbUser, err := api.Database.GetUserByID(dbauthz.AsSystemRestricted(ctx), uid) + dbUser, err := s.Database.GetUserByID(dbauthz.AsSystemRestricted(ctx), uid) if err != nil { _ = handlerutil.WriteError(rw, err) // internal error return @@ -484,14 +541,14 @@ func (api *API) scimPutUser(rw http.ResponseWriter, r *http.Request) { // TODO: Currently ignoring a lot of the SCIM fields. Coder's SCIM implementation // is very basic and only supports active status changes. if immutabilityViolation(dbUser.Username, sUser.UserName) { - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusBadRequest, "mutability", xerrors.Errorf("username is currently an immutable field, and cannot be changed. Current: %s, New: %s", dbUser.Username, sUser.UserName))) + _ = handlerutil.WriteError(rw, NewHTTPError(http.StatusBadRequest, "mutability", xerrors.Errorf("username is currently an immutable field, and cannot be changed. Current: %s, New: %s", dbUser.Username, sUser.UserName))) return } newStatus := scimUserStatus(dbUser, *sUser.Active) if dbUser.Status != newStatus { //nolint:gocritic // needed for SCIM - userNew, err := api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{ + userNew, err := s.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{ ID: dbUser.ID, Status: newStatus, UpdatedAt: dbtime.Now(), diff --git a/enterprise/coderd/scim/scimtypes.go b/enterprise/coderd/legacyscim/scimtypes.go similarity index 99% rename from enterprise/coderd/scim/scimtypes.go rename to enterprise/coderd/legacyscim/scimtypes.go index 39e022aa24e05..c96044befbc30 100644 --- a/enterprise/coderd/scim/scimtypes.go +++ b/enterprise/coderd/legacyscim/scimtypes.go @@ -1,4 +1,4 @@ -package scim +package legacyscim import ( "encoding/json" diff --git a/enterprise/coderd/scim/expression.go b/enterprise/coderd/scim/expression.go new file mode 100644 index 0000000000000..a0518c9ba10ef --- /dev/null +++ b/enterprise/coderd/scim/expression.go @@ -0,0 +1,39 @@ +package scim + +import ( + "github.com/scim2/filter-parser/v2" + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/database" +) + +// userQuery only supports queries of a singular attribute expression. +// Everything else is rejected. Okta just uses username. +// Eg: username eq "alice" +func userQuery(expr filter.Expression) (database.GetUsersParams, error) { + if expr == nil { + return database.GetUsersParams{}, nil + } + + attrExpr, ok := expr.(*filter.AttributeExpression) + if !ok { + return database.GetUsersParams{}, xerrors.Errorf("expected attribute expression") + } + + attrValue, ok := attrExpr.CompareValue.(string) + if !ok { + return database.GetUsersParams{}, xerrors.Errorf("expected string compare value") + } + + var getUsers database.GetUsersParams + switch attrExpr.AttributePath.AttributeName { + case "userName": + getUsers.Exactusername = attrValue + case "email": + getUsers.Exactemail = attrValue + default: + return database.GetUsersParams{}, xerrors.Errorf("unsupported filter attribute: %s", attrExpr.AttributePath.AttributeName) + } + + return getUsers, nil +} diff --git a/enterprise/coderd/scim/scim.go b/enterprise/coderd/scim/scim.go new file mode 100644 index 0000000000000..ee9c18205eea7 --- /dev/null +++ b/enterprise/coderd/scim/scim.go @@ -0,0 +1,138 @@ +package scim + +import ( + "bytes" + "crypto/subtle" + "encoding/json" + "net/http" + "sync/atomic" + + "github.com/elimity-com/scim" + scimErrors "github.com/elimity-com/scim/errors" + "github.com/elimity-com/scim/optional" + "github.com/elimity-com/scim/schema" + + "cdr.dev/slog/v3" + agpl "github.com/coder/coder/v2/coderd" + "github.com/coder/coder/v2/coderd/audit" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/idpsync" +) + +// Server wraps the elimity-com/scim library's Server to implement +// SCIM 2.0 endpoints. The library auto-serves /Schemas, /ResourceTypes, +// and /ServiceProviderConfig from schema definitions. +type Server struct { + opts *Options + srv *scim.Server +} + +// Options holds all the dependencies needed by SCIM resource handlers. +type Options struct { + DB database.Store + Auditor *atomic.Pointer[audit.Auditor] + IDPSync idpsync.IDPSync + Logger slog.Logger + + // AGPL is needed for CreateUser. + AGPL *agpl.API + + // SCIMAPIKey is the bearer token used to authenticate SCIM requests. + SCIMAPIKey []byte +} + +func New(opts *Options) (*Server, error) { + userHandler := &ResourceUser{ + store: opts.DB, + opts: opts, + } + + args := &scim.ServerArgs{ + ServiceProviderConfig: &scim.ServiceProviderConfig{ + DocumentationURI: optional.NewString("https://coder.com/docs/admin/users/oidc-auth#scim"), + AuthenticationSchemes: []scim.AuthenticationScheme{ + { + Type: scim.AuthenticationTypeOauthBearerToken, + Name: "HTTP Header Authentication", + Description: "Authentication scheme using the Authorization header with the shared token", + // TODO: Add documentation links for these specific docs once they exist. + SpecURI: optional.String{}, + DocumentationURI: optional.String{}, + Primary: true, + }, + }, + MaxResults: 0, + // SupportFiltering is set to false, as all filtering operations are not + // supported. A minimal filtering syntax is supported because Okta seems to + // ignore this field and attempt to filter anyway. + SupportFiltering: false, + SupportPatch: true, + }, + ResourceTypes: []scim.ResourceType{ + { + ID: optional.NewString("User"), + Name: "User", + Description: optional.NewString("User Account"), + Endpoint: "/Users", + Schema: schema.CoreUserSchema(), + Handler: userHandler, + SchemaExtensions: nil, + }, + }, + } + + srv, err := scim.NewServer(args) + if err != nil { + return nil, err + } + + return &Server{ + opts: opts, + srv: &srv, + }, nil +} + +func (s *Server) AuthMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + if !s.verifyAuthHeader(r) { + scimUnauthorized(rw) + return + } + + // All authenticated requests are treated as coming from the SCIM provisioner + //nolint:gocritic // auth header authenticates as this identity + ctx := dbauthz.AsSCIMProvisioner(r.Context()) + r = r.WithContext(ctx) + + next.ServeHTTP(rw, r) + }) +} + +func (s *Server) Handler() http.Handler { + return s.AuthMiddleware(s.srv) +} + +func (s *Server) verifyAuthHeader(r *http.Request) bool { + bearer := []byte("bearer ") + hdr := []byte(r.Header.Get("Authorization")) + + // Case-insensitive comparison of the "Bearer " prefix. + if len(hdr) >= len(bearer) && subtle.ConstantTimeCompare(bytes.ToLower(hdr[:len(bearer)]), bearer) == 1 { + hdr = hdr[len(bearer):] + } + + return len(s.opts.SCIMAPIKey) != 0 && subtle.ConstantTimeCompare(hdr, s.opts.SCIMAPIKey) == 1 +} + +func scimUnauthorized(rw http.ResponseWriter) { + rw.Header().Set("Content-Type", "application/scim+json") + rw.WriteHeader(http.StatusUnauthorized) + // scim error spec: + // https://datatracker.ietf.org/doc/html/rfc7644#section-3.12 + _ = json.NewEncoder(rw).Encode(scimErrors.ScimError{ + ScimType: "", // No scimType exists for unauthorized errors. + Detail: "invalid authorization", + Status: http.StatusUnauthorized, + }) +} diff --git a/enterprise/coderd/scim/users.go b/enterprise/coderd/scim/users.go new file mode 100644 index 0000000000000..282c17002d9d2 --- /dev/null +++ b/enterprise/coderd/scim/users.go @@ -0,0 +1,584 @@ +package scim + +import ( + "context" + "database/sql" + "encoding/json" + "fmt" + "net/http" + "strconv" + "strings" + + "github.com/elimity-com/scim" + scimErrors "github.com/elimity-com/scim/errors" + "github.com/elimity-com/scim/optional" + "github.com/google/uuid" + "golang.org/x/xerrors" + + agpl "github.com/coder/coder/v2/coderd" + "github.com/coder/coder/v2/coderd/audit" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/util/ptr" + "github.com/coder/coder/v2/codersdk" +) + +var _ scim.ResourceHandler = (*ResourceUser)(nil) + +// scimAudit emits an audit log for a SCIM operation. This uses +// BackgroundAudit instead of InitRequest because the elimity-com/scim +// library owns the http.ResponseWriter and does not expose it to +// resource handlers. +func (ru *ResourceUser) scimAudit(ctx context.Context, r *http.Request, action database.AuditAction, old, changed database.User) { + raw, _ := json.Marshal(map[string]string{ + "automatic_actor": "coder", + "automatic_subsystem": "scim", + }) + auditor := *ru.opts.Auditor.Load() + + // This is a best effort + // TODO: Check X-Forwarded-For and others for proxied requests + ip := r.RemoteAddr + + audit.BackgroundAudit(ctx, &audit.BackgroundAuditParams[database.User]{ + Audit: auditor, + Log: ru.opts.Logger, + UserID: uuid.Nil, // SCIM provisioner, not a real user + Action: action, + Old: old, + New: changed, + IP: ip, + UserAgent: r.UserAgent(), + AdditionalFields: raw, + Status: http.StatusOK, + }) +} + +type ResourceUser struct { + store database.Store + opts *Options +} + +// Create implements scim.ResourceHandler. Creates a new Coder user from +// SCIM attributes, or returns the existing user if a duplicate is found. +func (ru *ResourceUser) Create(r *http.Request, attributes scim.ResourceAttributes) (scim.Resource, error) { + ctx := r.Context() + + // Extract fields from the SCIM attributes. + // Do our best to match what the OIDC signup flow also does. + username, _ := AttributeAsString(attributes, "userName") + email := primaryEmail(attributes) + if email == "" { + // email is required + return scim.Resource{}, scimErrors.ScimErrorBadRequest("no primary email provided") + } + + // This comes from userOIDC + // TODO: Ideally this code would be shared between the two places. + usernameValidErr := codersdk.NameValid(username) + if usernameValidErr != nil { + if username == "" { + username = email + } + username = codersdk.UsernameFrom(username) + } + + // TODO: OIDC has optional configuration like `EmailDomain` to reject emails outside a specific domain. + // We should consider whether we want to support that for SCIM as well, and if so, apply that validation here. + + active := true + if a, ok := Attribute(attributes, "active"); ok { + v, err := BooleanValue(a) + if err != nil { + return scim.Resource{}, scimErrors.ScimErrorBadRequest( + fmt.Sprintf("invalid boolean value for 'active' field: %v", a)) + } + active = v + } + + // Check for existing user by email or username. + dbUser, err := ru.store.GetUserByEmailOrUsername(ctx, database.GetUserByEmailOrUsernameParams{ + Email: email, + Username: username, + }) + if err == nil { + // SCIM spec says to return a StatusConflict if the user already exists. + // However, Coder never deletes a user. So suspended ~= deleted. + // If the user is not suspended, we return a conflict. + if dbUser.Status != database.UserStatusSuspended { + return scim.Resource{}, scimErrors.ScimError{ + ScimType: scimErrors.ScimTypeUniqueness, + Detail: fmt.Sprintf("user with email (%q) or username (%q)", email, username), + Status: http.StatusConflict, + } + } + + // If the user is suspended, then they might be deleted on the SCIM side. + // We can just update their status and return the user as they exist. + status := scimUserStatus(dbUser, &active) + dbUser, err = ru.updateUserStatus(ctx, r, dbUser, status) + if err != nil { + return scim.Resource{}, err + } + return userResource(dbUser), nil + } + + if !xerrors.Is(err, sql.ErrNoRows) { + // Internal DB errors should be returned. + // ErrNoRows is expected if the user does not exist. + return scim.Resource{}, err + } + + // OIDC login runs org, group, and role sync. SCIM does not have (or not yet) these + // claims. We only need to sync the default organization if that is enabled. + // + // When the user eventually logs in via OIDC, the regular sync will run. + // However, since org sync can be disabled. We need to assign the default org if + // that is how we are configured. + organizations := []uuid.UUID{} + orgSync, err := ru.opts.IDPSync.OrganizationSyncSettings(ctx, ru.store) + if err != nil { + return scim.Resource{}, xerrors.Errorf("get organization sync settings: %w", err) + } + if orgSync.AssignDefault { + // Technically, we could just always assign this. When they eventually log in, + // the org would be removed if necessary. But to avoid confusion of the user + // being in the org before they log in, we apply some intelligence to this guess + // of "Do they belong in the default org". + defaultOrganization, err := ru.store.GetDefaultOrganization(ctx) + if err != nil { + return scim.Resource{}, xerrors.Errorf("get default organization: %w", err) + } + organizations = append(organizations, defaultOrganization.ID) + } + + // CreateUser does InsertOrganizationMember internally, and InsertUser + // implicitly assigns the member role at site scope. The SCIM provisioner + // role cannot assign either, so escalate to a system context for this + // specific call, matching the legacy SCIM handler. + //nolint:gocritic // SCIM bearer token authenticates as the SCIM provisioner; user creation needs broader rights to assign default roles. + dbUser, err = ru.opts.AGPL.CreateUser(dbauthz.AsSystemRestricted(ctx), ru.store, agpl.CreateUserRequest{ + CreateUserRequestWithOrgs: codersdk.CreateUserRequestWithOrgs{ + Username: username, + Email: email, + OrganizationIDs: organizations, + }, + LoginType: database.LoginTypeOIDC, + // Do not send notifications to user admins; SCIM may call this + // sequentially for many users. + // TODO: Maybe we should spam them anyway? + SkipNotifications: true, + }) + if err != nil { + return scim.Resource{}, xerrors.Errorf("create user: %w", err) + } + + ru.scimAudit(ctx, r, database.AuditActionCreate, database.User{}, dbUser) + return userResource(dbUser), nil +} + +// Get implements scim.ResourceHandler. Returns a single user by ID. +func (ru *ResourceUser) Get(r *http.Request, idStr string) (scim.Resource, error) { + ctx := r.Context() + usr, err := ru.user(ctx, idStr) + if err != nil { + return scim.Resource{}, err + } + + return userResource(usr), nil +} + +// GetAll implements scim.ResourceHandler. Returns a paginated list of users. +func (ru *ResourceUser) GetAll(r *http.Request, params scim.ListRequestParams) (scim.Page, error) { + ctx := r.Context() + + var qry database.GetUsersParams + if params.FilterValidator != nil { + var err error + qry, err = userQuery(params.FilterValidator.GetFilter()) + if err != nil { + return scim.Page{}, scimErrors.ScimErrorBadRequest(fmt.Sprintf("invalid filter: %v", err)) + } + } + + qry.LimitOpt = int32(params.Count) //nolint:gosec + qry.OffsetOpt = int32(params.StartIndex - 1) //nolint:gosec + + if qry.LimitOpt < 0 { + qry.LimitOpt = 100 + } + + users, err := ru.store.GetUsers(ctx, qry) + if err != nil { + return scim.Page{}, err + } + + totalCount := int64(len(users)) + if len(users) == int(qry.LimitOpt) { + // If the limit is not reached, that is the count + // TODO: If there is a query and the limit is reached, this is inaccurate. + totalCount, err = ru.store.GetUserCount(ctx, false) + if err != nil { + return scim.Page{}, err + } + } + + resources := make([]scim.Resource, 0, len(users)) + for _, u := range users { + resources = append(resources, userResourceFromGetUsersRow(u)) + } + + return scim.Page{ + TotalResults: int(totalCount), + Resources: resources, + }, nil +} + +// Replace implements scim.ResourceHandler (PUT). Replaces user attributes. +// Currently only supports changing the active status per existing behavior. +func (ru *ResourceUser) Replace(r *http.Request, idStr string, attributes scim.ResourceAttributes) (scim.Resource, error) { + ctx := r.Context() + + dbUser, err := ru.user(ctx, idStr) + if err != nil { + return scim.Resource{}, err + } + + // All of our fields except for active are immutable. + if !AttributeEqual(dbUser.Username, attributes, "userName") { + return scim.Resource{}, scimErrors.ScimErrorBadRequest(fmt.Sprintf("changing the 'userName' field is not supported (current value: %q)", dbUser.Username)) + } + + // TODO: Check if the primary email has changed. If it has, should we do something? + + activeInterface, ok := Attribute(attributes, "active") + if !ok { + return scim.Resource{}, scimErrors.ScimErrorBadRequest("missing required 'active' field") + } + + active, err := BooleanValue(activeInterface) + if err != nil { + return scim.Resource{}, scimErrors.ScimErrorBadRequest(fmt.Sprintf("invalid boolean value for 'active' field: %v", activeInterface)) + } + + newStatus := scimUserStatus(dbUser, &active) + dbUser, err = ru.updateUserStatus(ctx, r, dbUser, newStatus) + if err != nil { + return scim.Resource{}, err + } + + return userResource(dbUser), nil +} + +// Delete implements scim.ResourceHandler. Suspends the user (Coder does +// not hard-delete users). +func (ru *ResourceUser) Delete(r *http.Request, idStr string) error { + ctx := r.Context() + + dbUser, err := ru.user(ctx, idStr) + if err != nil { + return err + } + + _, err = ru.updateUserStatus(ctx, r, dbUser, database.UserStatusSuspended) + if err != nil { + return err + } + + return nil +} + +// Patch implements scim.ResourceHandler. Updates user attributes based on +// SCIM PatchOp operations. Currently, supports changing the active status. +func (ru *ResourceUser) Patch(r *http.Request, idStr string, operations []scim.PatchOperation) (scim.Resource, error) { + ctx := r.Context() + + uid, err := uuid.Parse(idStr) + if err != nil { + return scim.Resource{}, BadUUID(idStr, err) + } + + dbUser, err := ru.store.GetUserByID(ctx, uid) + if err != nil { + if xerrors.Is(err, sql.ErrNoRows) { + return scim.Resource{}, scimErrors.ScimErrorResourceNotFound(idStr) + } + return scim.Resource{}, err + } + + // Process operations. Currently, we only handle the "active" attribute. + var activeSet *bool + for _, op := range operations { + switch op.Op { + case "add": + case "remove": + // TODO: If the path is unspecified, we should fail with the status code 400. + // Today, we only accept the 'active' field and silently drop the rest. + if strings.EqualFold(op.Path.String(), "active") { + activeSet = ptr.Ref(false) + } + case "replace": + // TODO: Honor mutability rules of fields like `userName` and `email`. + // Should scim be able to change those fields? + + // SCIM PATCH replace can come in two forms: + // 1. Path set: {"op":"replace","path":"active","value":false} + // 2. No path, value is a map: {"op":"replace","value":{"active":false}} + if op.Path != nil && strings.EqualFold(op.Path.String(), "active") { + v, err := BooleanValue(op.Value) + if err != nil { + return scim.Resource{}, scimErrors.ScimErrorBadRequest(fmt.Sprintf("invalid boolean value for 'active' field: %v", op.Value)) + } + activeSet = &v + } else if m, ok := op.Value.(map[string]interface{}); ok { + if actV, ok := Attribute(m, "active"); ok { + v, err := BooleanValue(actV) + if err != nil { + return scim.Resource{}, scimErrors.ScimErrorBadRequest(fmt.Sprintf("invalid boolean value for 'active' field: %v", actV)) + } + activeSet = &v + } + } + default: + } + } + + newStatus := scimUserStatus(dbUser, activeSet) + dbUser, err = ru.updateUserStatus(ctx, r, dbUser, newStatus) + if err != nil { + return scim.Resource{}, err + } + + return userResource(dbUser), nil +} + +func (ru *ResourceUser) user(ctx context.Context, idStr string) (database.User, error) { + id, err := uuid.Parse(idStr) + if err != nil { + return database.User{}, BadUUID(idStr, err) + } + + usr, err := ru.store.GetUserByID(ctx, id) + if err != nil { + if xerrors.Is(err, sql.ErrNoRows) { + return database.User{}, scimErrors.ScimErrorResourceNotFound(idStr) + } + return database.User{}, err + } + + return usr, nil +} + +// updateUserStatus is a no-op if the status did not change. +func (ru *ResourceUser) updateUserStatus(ctx context.Context, r *http.Request, u database.User, status database.UserStatus) (database.User, error) { + if u.Status == status { + return u, nil + } + newUser, err := ru.store.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ + ID: u.ID, Status: status, UpdatedAt: dbtime.Now(), UserIsSeen: false, + }) + if err != nil { + return database.User{}, err + } + ru.scimAudit(ctx, r, database.AuditActionWrite, u, newUser) + return newUser, nil +} + +// scimUserStatus maps the SCIM "active" boolean to Coder's internal user status. +// It preserves the active/dormant distinction: active users stay active, +// dormant or suspended users become dormant when re-activated (they become +// active after their next login). +// +//nolint:revive // active is not a control flag +func scimUserStatus(user database.User, active *bool) database.UserStatus { + if active == nil { + return user.Status + } + + if !(*active) { + // SCIM "active: false" means the user should be suspended + return database.UserStatusSuspended + } + + switch user.Status { + case database.UserStatusActive: + // Active users stay active + return database.UserStatusActive + case database.UserStatusDormant, database.UserStatusSuspended: + // Dormant or suspended users become dormant when re-activated + // The user can then become active by doing something in the product. + return database.UserStatusDormant + default: + return database.UserStatusDormant + } +} + +// userResource converts a database.User into a SCIM Resource. +func userResource(u database.User) scim.Resource { + return scim.Resource{ + ID: u.ID.String(), + ExternalID: optional.String{}, + Attributes: scim.ResourceAttributes{ + "userName": u.Username, + "name": map[string]interface{}{ + "formatted": u.Name, + }, + "emails": []map[string]interface{}{ + { + "primary": true, + "value": u.Email, + }, + }, + "active": u.Status == database.UserStatusActive || + u.Status == database.UserStatusDormant, + }, + Meta: scim.Meta{ + Created: &u.CreatedAt, + LastModified: &u.UpdatedAt, + }, + } +} + +// userResourceFromGetUsersRow converts a database.GetUsersRow into a SCIM Resource. +func userResourceFromGetUsersRow(u database.GetUsersRow) scim.Resource { + return scim.Resource{ + ID: u.ID.String(), + ExternalID: optional.String{}, + Attributes: scim.ResourceAttributes{ + "userName": u.Username, + "name": map[string]interface{}{ + "formatted": u.Name, + }, + "emails": []map[string]interface{}{ + { + "primary": true, + "value": u.Email, + }, + }, + "active": u.Status == database.UserStatusActive || + u.Status == database.UserStatusDormant, + }, + Meta: scim.Meta{ + Created: &u.CreatedAt, + LastModified: &u.UpdatedAt, + }, + } +} + +func AttributeAsBool(attrs scim.ResourceAttributes, key string) (value bool, exists bool) { + val, ok := Attribute(attrs, key) + if !ok { + return false, false + } + + switch v := val.(type) { + case string: + pv, err := strconv.ParseBool(v) + return pv, err == nil + case bool: + return v, true + default: + return false, false + } +} + +func AttributeAsString(attrs scim.ResourceAttributes, key string) (string, bool) { + val, ok := Attribute(attrs, key) + if !ok { + return "", false + } + + switch v := val.(type) { + case string: + return v, true + case bool: + return strconv.FormatBool(v), true + default: + return "", false + } +} + +func Attribute(attrs scim.ResourceAttributes, key string) (interface{}, bool) { + // attribute names are case-insensitive per SCIM spec + val, ok := attrs[key] + if ok { + return val, true + } + + // This is terrible, but we need to iterate the map to find the key in a case-insensitive way. + // The scim Spec says attribute names are case-insensitive. + for k, v := range attrs { + if k == key { + return v, true + } + if len(k) == len(key) && strings.EqualFold(k, key) { + return v, true + } + } + + return nil, false +} + +// BadUUID returns a 404 not-found error for non-UUID identifiers. +// SCIM clients may send arbitrary strings as IDs; returning 404 +// (rather than 400) signals that no resource matches. +func BadUUID(idStr string, _ error) scimErrors.ScimError { + return scimErrors.ScimErrorResourceNotFound(idStr) +} + +func BooleanValue(v interface{}) (bool, error) { + switch b := v.(type) { + case bool: + return b, nil + case string: + return strconv.ParseBool(b) + default: + return false, xerrors.Errorf("expected boolean or string value, got %T", v) + } +} + +func AttributeEqual[T comparable](existing T, attrs scim.ResourceAttributes, key string) bool { + found, ok := Attribute(attrs, key) + if !ok { + return true // No change if the attribute is not present in the request + } + + sameType, ok := found.(T) + if !ok { + return false // Type mismatch, consider it a change + } + + return existing == sameType +} + +// primaryEmail extracts the primary email from SCIM resource attributes. +func primaryEmail(attributes scim.ResourceAttributes) string { + emailsRaw, ok := Attribute(attributes, "emails") + if !ok { + return "" + } + + emails, ok := emailsRaw.([]interface{}) + if !ok { + return "" + } + + var fallback string + for _, e := range emails { + emailMap, ok := e.(map[string]interface{}) + if !ok { + continue + } + val, ok := AttributeAsString(emailMap, "value") + if !ok { + continue + } + if primary, _ := AttributeAsBool(emailMap, "primary"); primary { + return val + } + fallback = val + } + + return fallback +} diff --git a/enterprise/coderd/scim/users_internal_test.go b/enterprise/coderd/scim/users_internal_test.go new file mode 100644 index 0000000000000..3f3c110a624f4 --- /dev/null +++ b/enterprise/coderd/scim/users_internal_test.go @@ -0,0 +1,760 @@ +package scim + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/elimity-com/scim" + scimErrors "github.com/elimity-com/scim/errors" + "github.com/google/uuid" + filter "github.com/scim2/filter-parser/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "cdr.dev/slog/v3" + "cdr.dev/slog/v3/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/audit" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbmock" + "github.com/coder/coder/v2/coderd/database/dbtestutil" +) + +// setupSCIM creates a ResourceUser backed by a real database for testing. +// The returned mock auditor can be inspected for emitted audit logs. +func setupSCIM(t *testing.T) (*ResourceUser, database.Store, *audit.MockAuditor) { + t.Helper() + + db, _ := dbtestutil.NewDB(t) + mockAudit := audit.NewMock() + auditorPtr := atomic.Pointer[audit.Auditor]{} + var a audit.Auditor = mockAudit + auditorPtr.Store(&a) + + ru := &ResourceUser{ + store: db, + opts: &Options{ + DB: db, + Auditor: &auditorPtr, + Logger: slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug), + }, + } + return ru, db, mockAudit +} + +// scimRequest builds an *http.Request with scim provisioner context, +// simulating the auth context that the SCIM middleware normally sets. +func scimRequest(t *testing.T) *http.Request { + t.Helper() + ctx := dbauthz.AsSCIMProvisioner(context.Background()) + return httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx) +} + +// seedUser creates a user in the database for testing. +func seedUser(t *testing.T, db database.Store, opts database.User) database.User { + t.Helper() + return dbgen.User(t, db, opts) +} + +// setupSCIMMock creates a ResourceUser backed by a gomock store for tests +// that only need to verify call patterns (e.g. audit emission) without +// real SQL. +func setupSCIMMock(t *testing.T) (*ResourceUser, *dbmock.MockStore, *audit.MockAuditor) { + t.Helper() + + ctrl := gomock.NewController(t) + mockStore := dbmock.NewMockStore(ctrl) + mockAudit := audit.NewMock() + auditorPtr := atomic.Pointer[audit.Auditor]{} + var a audit.Auditor = mockAudit + auditorPtr.Store(&a) + + ru := &ResourceUser{ + store: mockStore, + opts: &Options{ + DB: mockStore, + Auditor: &auditorPtr, + Logger: slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug), + }, + } + return ru, mockStore, mockAudit +} + +// --- Pure function tests (no DB) --- + +func TestScimUserStatus(t *testing.T) { + t.Parallel() + + boolPtr := func(b bool) *bool { return &b } + + tests := []struct { + name string + status database.UserStatus + active *bool + expected database.UserStatus + }{ + {"active+true=active", database.UserStatusActive, boolPtr(true), database.UserStatusActive}, + {"active+false=suspended", database.UserStatusActive, boolPtr(false), database.UserStatusSuspended}, + {"suspended+true=dormant", database.UserStatusSuspended, boolPtr(true), database.UserStatusDormant}, + {"suspended+false=suspended", database.UserStatusSuspended, boolPtr(false), database.UserStatusSuspended}, + {"dormant+true=dormant", database.UserStatusDormant, boolPtr(true), database.UserStatusDormant}, + {"dormant+false=suspended", database.UserStatusDormant, boolPtr(false), database.UserStatusSuspended}, + {"active+nil=active", database.UserStatusActive, nil, database.UserStatusActive}, + {"suspended+nil=suspended", database.UserStatusSuspended, nil, database.UserStatusSuspended}, + {"dormant+nil=dormant", database.UserStatusDormant, nil, database.UserStatusDormant}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + user := database.User{Status: tt.status} + got := scimUserStatus(user, tt.active) + assert.Equal(t, tt.expected, got) + }) + } +} + +func TestPrimaryEmail(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + attrs scim.ResourceAttributes + expected string + }{ + { + name: "primary email", + attrs: scim.ResourceAttributes{ + "emails": []interface{}{ + map[string]interface{}{"value": "a@b.com", "primary": true}, + }, + }, + expected: "a@b.com", + }, + { + name: "fallback to first when no primary", + attrs: scim.ResourceAttributes{ + "emails": []interface{}{ + map[string]interface{}{"value": "first@b.com"}, + }, + }, + expected: "first@b.com", + }, + { + name: "picks primary over first", + attrs: scim.ResourceAttributes{ + "emails": []interface{}{ + map[string]interface{}{"value": "first@b.com"}, + map[string]interface{}{"value": "primary@b.com", "primary": true}, + }, + }, + expected: "primary@b.com", + }, + { + name: "polluted", + attrs: scim.ResourceAttributes{ + "emails": []interface{}{ + // Try and cause a panic + "not-a-map", + true, + 7, + map[int]interface{}{ + 1: "bad", + }, + map[string]interface{}{ + "value": 123, // value is not a string + }, + map[string]interface{}{}, + map[string]interface{}{"value": "first@b.com"}, + map[string]interface{}{"value": "primary@b.com", "primary": true}, + }, + }, + expected: "primary@b.com", + }, + { + name: "no emails key", + attrs: scim.ResourceAttributes{}, + expected: "", + }, + { + name: "empty emails", + attrs: scim.ResourceAttributes{"emails": []interface{}{}}, + expected: "", + }, + { + name: "wrong type", + attrs: scim.ResourceAttributes{"emails": "not-a-list"}, + expected: "", + }, + { + name: "case-insensitive top-level key", + attrs: scim.ResourceAttributes{ + "Emails": []interface{}{ + map[string]interface{}{"value": "a@b.com", "primary": true}, + }, + }, + expected: "a@b.com", + }, + { + name: "case-insensitive inner keys", + attrs: scim.ResourceAttributes{ + "emails": []interface{}{ + map[string]interface{}{"Value": "a@b.com", "Primary": true}, + }, + }, + expected: "a@b.com", + }, + { + name: "all caps keys", + attrs: scim.ResourceAttributes{ + "EMAILS": []interface{}{ + map[string]interface{}{"VALUE": "a@b.com", "PRIMARY": true}, + }, + }, + expected: "a@b.com", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := primaryEmail(tt.attrs) + assert.Equal(t, tt.expected, got) + }) + } +} + +func TestBooleanValue(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input interface{} + want bool + wantErr bool + }{ + {"bool true", true, true, false}, + {"bool false", false, false, false}, + {"string true", "true", true, false}, + {"string false", "false", false, false}, + {"string True", "True", true, false}, + {"int", 42, false, true}, + {"nil", nil, false, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := BooleanValue(tt.input) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tt.want, got) + } + }) + } +} + +func TestAttribute(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + attrs scim.ResourceAttributes + key string + wantVal interface{} + wantOK bool + }{ + {"exact match", scim.ResourceAttributes{"active": true}, "active", true, true}, + {"capital first", scim.ResourceAttributes{"active": true}, "Active", true, true}, + {"all caps", scim.ResourceAttributes{"active": true}, "ACTIVE", true, true}, + {"camelCase key", scim.ResourceAttributes{"userName": "alice"}, "username", "alice", true}, + {"camelCase swapped", scim.ResourceAttributes{"username": "alice"}, "userName", "alice", true}, + {"missing key", scim.ResourceAttributes{"active": true}, "missing", nil, false}, + {"empty map", scim.ResourceAttributes{}, "active", nil, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + val, ok := Attribute(tt.attrs, tt.key) + assert.Equal(t, tt.wantOK, ok) + assert.Equal(t, tt.wantVal, val) + }) + } +} + +func TestAttributeAsBool(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + attrs scim.ResourceAttributes + key string + want bool + wantOK bool + }{ + {"exact key bool", scim.ResourceAttributes{"active": true}, "active", true, true}, + {"mixed case bool", scim.ResourceAttributes{"active": false}, "Active", false, true}, + {"all caps bool", scim.ResourceAttributes{"active": true}, "ACTIVE", true, true}, + {"mixed case string true", scim.ResourceAttributes{"active": "true"}, "Active", true, true}, + {"mixed case string false", scim.ResourceAttributes{"active": "false"}, "ACTIVE", false, true}, + {"missing key", scim.ResourceAttributes{}, "active", false, false}, + {"non-convertible", scim.ResourceAttributes{"active": 42}, "active", false, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, ok := AttributeAsBool(tt.attrs, tt.key) + assert.Equal(t, tt.wantOK, ok) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestAttributeAsString(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + attrs scim.ResourceAttributes + key string + want string + wantOK bool + }{ + {"exact key string", scim.ResourceAttributes{"userName": "alice"}, "userName", "alice", true}, + {"mixed case string", scim.ResourceAttributes{"userName": "alice"}, "UserName", "alice", true}, + {"lower case lookup", scim.ResourceAttributes{"userName": "alice"}, "username", "alice", true}, + {"bool to string", scim.ResourceAttributes{"active": true}, "active", "true", true}, + {"mixed case bool to string", scim.ResourceAttributes{"active": false}, "Active", "false", true}, + {"missing key", scim.ResourceAttributes{}, "userName", "", false}, + {"non-convertible", scim.ResourceAttributes{"count": 42}, "count", "", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, ok := AttributeAsString(tt.attrs, tt.key) + assert.Equal(t, tt.wantOK, ok) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestAttributeEqual(t *testing.T) { + t.Parallel() + + t.Run("exact match same value", func(t *testing.T) { + t.Parallel() + attrs := scim.ResourceAttributes{"userName": "alice"} + assert.True(t, AttributeEqual("alice", attrs, "userName")) + }) + + t.Run("mixed case same value", func(t *testing.T) { + t.Parallel() + attrs := scim.ResourceAttributes{"userName": "alice"} + assert.True(t, AttributeEqual("alice", attrs, "UserName")) + }) + + t.Run("mixed case different value", func(t *testing.T) { + t.Parallel() + attrs := scim.ResourceAttributes{"userName": "bob"} + assert.False(t, AttributeEqual("alice", attrs, "USERNAME")) + }) + + t.Run("missing key means no change", func(t *testing.T) { + t.Parallel() + attrs := scim.ResourceAttributes{} + assert.True(t, AttributeEqual("alice", attrs, "userName")) + }) + + t.Run("type mismatch", func(t *testing.T) { + t.Parallel() + attrs := scim.ResourceAttributes{"userName": 42} + assert.False(t, AttributeEqual("alice", attrs, "userName")) + }) +} + +// --- Handler tests (with DB) --- + +func TestResourceUser_CaseInsensitive(t *testing.T) { + t.Parallel() + + ru, db, _ := setupSCIM(t) + + // Seed an active user. + user := seedUser(t, db, database.User{ + Status: database.UserStatusActive, + LoginType: database.LoginTypeOIDC, + }) + + r := scimRequest(t) + + // Replace with "Active" (capital A) instead of "active". + res, err := ru.Replace(r, user.ID.String(), scim.ResourceAttributes{ + "userName": user.Username, + "Active": false, + }) + require.NoError(t, err) + assert.Equal(t, false, res.Attributes["active"]) + + // Confirm suspended via Get. + res, err = ru.Get(r, user.ID.String()) + require.NoError(t, err) + assert.Equal(t, false, res.Attributes["active"]) + + // Patch back with map-style replace using "Active" key. + res, err = ru.Patch(r, user.ID.String(), []scim.PatchOperation{ + {Op: "replace", Value: map[string]interface{}{"Active": true}}, + }) + require.NoError(t, err) + assert.Equal(t, true, res.Attributes["active"]) + + // Confirm reactivated via Get. + res, err = ru.Get(r, user.ID.String()) + require.NoError(t, err) + assert.Equal(t, true, res.Attributes["active"]) +} + +func TestResourceUser_Create(t *testing.T) { + t.Parallel() + + // Coder does not hard-delete users. A SCIM Delete suspends the user, so + // when an IdP later re-creates the same user, the handler should match + // them by email/username and reactivate the existing row instead of + // returning 409 Conflict. See commit b3e6e0aa06. + + t.Run("duplicate-active-conflict", func(t *testing.T) { + t.Parallel() + ru, db, _ := setupSCIM(t) + + existing := seedUser(t, db, database.User{ + Status: database.UserStatusActive, + LoginType: database.LoginTypeOIDC, + }) + + _, err := ru.Create(scimRequest(t), scim.ResourceAttributes{ + "userName": existing.Username, + "emails": []interface{}{ + map[string]interface{}{"value": existing.Email, "primary": true}, + }, + "active": true, + }) + require.Error(t, err) + var scimErr scimErrors.ScimError + require.ErrorAs(t, err, &scimErr) + assert.Equal(t, http.StatusConflict, scimErr.Status) + }) + + t.Run("suspended-user-reactivates", func(t *testing.T) { + t.Parallel() + ru, db, mockAudit := setupSCIM(t) + + existing := seedUser(t, db, database.User{ + Status: database.UserStatusSuspended, + LoginType: database.LoginTypeOIDC, + }) + + res, err := ru.Create(scimRequest(t), scim.ResourceAttributes{ + "userName": existing.Username, + "emails": []interface{}{ + map[string]interface{}{"value": existing.Email, "primary": true}, + }, + "active": true, + }) + require.NoError(t, err) + assert.Equal(t, existing.ID.String(), res.ID, "response should reference the existing user, not a new one") + + // The SCIM response must reflect the post-update state so the IdP + // sees active=true after the recreate. + assert.Equal(t, true, res.Attributes["active"], "response should report the reactivated state") + + // Suspended + active=true reactivates to Dormant (not Active) per scimUserStatus. + got, err := db.GetUserByID(dbauthz.AsSCIMProvisioner(context.Background()), existing.ID) + require.NoError(t, err) + assert.Equal(t, database.UserStatusDormant, got.Status, "suspended user should be marked dormant on recreate") + + // Reactivation should emit one audit log for the status change. + assert.Len(t, mockAudit.AuditLogs(), 1) + }) + + t.Run("suspended-user-stays-suspended-when-active-false", func(t *testing.T) { + t.Parallel() + ru, db, mockAudit := setupSCIM(t) + + existing := seedUser(t, db, database.User{ + Status: database.UserStatusSuspended, + LoginType: database.LoginTypeOIDC, + }) + + res, err := ru.Create(scimRequest(t), scim.ResourceAttributes{ + "userName": existing.Username, + "emails": []interface{}{ + map[string]interface{}{"value": existing.Email, "primary": true}, + }, + "active": false, + }) + require.NoError(t, err) + assert.Equal(t, existing.ID.String(), res.ID) + assert.Equal(t, false, res.Attributes["active"]) + + got, err := db.GetUserByID(dbauthz.AsSCIMProvisioner(context.Background()), existing.ID) + require.NoError(t, err) + assert.Equal(t, database.UserStatusSuspended, got.Status) + + // No status change → no audit log. + assert.Empty(t, mockAudit.AuditLogs()) + }) +} + +func TestResourceUser_Lifecycle(t *testing.T) { + t.Parallel() + + ru, db, _ := setupSCIM(t) + + // Seed an active user. + user := seedUser(t, db, database.User{ + Status: database.UserStatusActive, + LoginType: database.LoginTypeOIDC, + }) + + r := scimRequest(t) + + // Step 1: Get the user. Verify fields match. + res, err := ru.Get(r, user.ID.String()) + require.NoError(t, err) + assert.Equal(t, user.ID.String(), res.ID) + assert.Equal(t, user.Username, res.Attributes["userName"]) + assert.Equal(t, true, res.Attributes["active"]) + + // Step 2: Replace with active=false → suspended. + res, err = ru.Replace(r, user.ID.String(), scim.ResourceAttributes{ + "userName": user.Username, + "active": false, + }) + require.NoError(t, err) + assert.Equal(t, false, res.Attributes["active"]) + + // Step 3: Get → confirm inactive. + res, err = ru.Get(r, user.ID.String()) + require.NoError(t, err) + assert.Equal(t, false, res.Attributes["active"]) + + // Step 4: Patch active=true → dormant (shown as active in SCIM). + res, err = ru.Patch(r, user.ID.String(), []scim.PatchOperation{ + {Op: "replace", Path: mustPath("active"), Value: true}, + }) + require.NoError(t, err) + assert.Equal(t, true, res.Attributes["active"]) + + // Step 5: Get → confirm active again. + res, err = ru.Get(r, user.ID.String()) + require.NoError(t, err) + assert.Equal(t, true, res.Attributes["active"]) + + // Step 6: Delete → suspended. + err = ru.Delete(r, user.ID.String()) + require.NoError(t, err) + + // Step 7: Get → confirm inactive after delete. + res, err = ru.Get(r, user.ID.String()) + require.NoError(t, err) + assert.Equal(t, false, res.Attributes["active"]) +} + +func TestResourceUser_GetAll(t *testing.T) { + t.Parallel() + + ru, db, _ := setupSCIM(t) + + // Seed 3 users. + for i := 0; i < 3; i++ { + seedUser(t, db, database.User{ + LoginType: database.LoginTypeOIDC, + }) + } + + r := scimRequest(t) + + // Get all with large count. + page, err := ru.GetAll(r, scim.ListRequestParams{Count: 100, StartIndex: 1}) + require.NoError(t, err) + assert.GreaterOrEqual(t, page.TotalResults, 3) + assert.GreaterOrEqual(t, len(page.Resources), 3) + + // Paginate: startIndex=2, count=1. + page, err = ru.GetAll(r, scim.ListRequestParams{Count: 1, StartIndex: 2}) + require.NoError(t, err) + assert.Len(t, page.Resources, 1) + assert.GreaterOrEqual(t, page.TotalResults, 3) +} + +func TestResourceUser_Errors(t *testing.T) { + t.Parallel() + + ru, _, _ := setupSCIM(t) + r := scimRequest(t) + missingUUID := uuid.New().String() + + tests := []struct { + name string + run func() error + wantStatus int + }{ + { + name: "Get/non-UUID", + run: func() error { _, err := ru.Get(r, "not-a-uuid"); return err }, + wantStatus: http.StatusNotFound, + }, + { + name: "Get/missing", + run: func() error { _, err := ru.Get(r, missingUUID); return err }, + wantStatus: http.StatusNotFound, + }, + { + name: "Replace/non-UUID", + run: func() error { _, err := ru.Replace(r, "bad", scim.ResourceAttributes{}); return err }, + wantStatus: http.StatusNotFound, + }, + { + name: "Replace/missing", + run: func() error { _, err := ru.Replace(r, missingUUID, scim.ResourceAttributes{}); return err }, + wantStatus: http.StatusNotFound, + }, + { + name: "Replace/immutable-userName", + run: func() error { + // Need a real user for this test. + user := seedUser(t, ru.store, database.User{LoginType: database.LoginTypeOIDC}) + _, err := ru.Replace(r, user.ID.String(), scim.ResourceAttributes{ + "userName": "different-name", + }) + return err + }, + wantStatus: http.StatusBadRequest, + }, + { + name: "Patch/non-UUID", + run: func() error { _, err := ru.Patch(r, "bad", nil); return err }, + wantStatus: http.StatusNotFound, + }, + { + name: "Patch/missing", + run: func() error { _, err := ru.Patch(r, missingUUID, nil); return err }, + wantStatus: http.StatusNotFound, + }, + { + name: "Delete/non-UUID", + run: func() error { return ru.Delete(r, "bad") }, + wantStatus: http.StatusNotFound, + }, + { + name: "Delete/missing", + run: func() error { return ru.Delete(r, missingUUID) }, + wantStatus: http.StatusNotFound, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := tt.run() + require.Error(t, err) + var scimErr scimErrors.ScimError + require.ErrorAs(t, err, &scimErr) + assert.Equal(t, tt.wantStatus, scimErr.Status) + }) + } +} + +func TestResourceUser_AuditLogs(t *testing.T) { + t.Parallel() + + // These tests use dbmock instead of a real database because they only + // verify audit emission logic (does an audit log fire when status + // changes?), not SQL correctness. The handlers call just GetUserByID + // and UpdateUserStatus, both trivially mockable. + + makeUser := func(status database.UserStatus) (database.User, database.User) { + id := uuid.New() + user := database.User{ + ID: id, + Username: "testuser", + Status: status, + LoginType: database.LoginTypeOIDC, + } + suspended := user + suspended.Status = database.UserStatusSuspended + return user, suspended + } + + t.Run("Replace/status-change-emits-audit", func(t *testing.T) { + t.Parallel() + ru, mockStore, mockAudit := setupSCIMMock(t) + activeUser, suspendedUser := makeUser(database.UserStatusActive) + + mockStore.EXPECT().GetUserByID(gomock.Any(), activeUser.ID).Return(activeUser, nil) + mockStore.EXPECT().UpdateUserStatus(gomock.Any(), gomock.Any()).Return(suspendedUser, nil) + + _, err := ru.Replace(scimRequest(t), activeUser.ID.String(), scim.ResourceAttributes{ + "userName": activeUser.Username, + "active": false, + }) + require.NoError(t, err) + assert.Len(t, mockAudit.AuditLogs(), 1) + }) + + t.Run("Replace/no-change-skips-audit", func(t *testing.T) { + t.Parallel() + ru, mockStore, mockAudit := setupSCIMMock(t) + activeUser, _ := makeUser(database.UserStatusActive) + + mockStore.EXPECT().GetUserByID(gomock.Any(), activeUser.ID).Return(activeUser, nil) + // No UpdateUserStatus expected: active=true on an already active user is a no-op. + + _, err := ru.Replace(scimRequest(t), activeUser.ID.String(), scim.ResourceAttributes{ + "userName": activeUser.Username, + "active": true, + }) + require.NoError(t, err) + assert.Empty(t, mockAudit.AuditLogs()) + }) + + t.Run("Delete/active-user-emits-audit", func(t *testing.T) { + t.Parallel() + ru, mockStore, mockAudit := setupSCIMMock(t) + activeUser, suspendedUser := makeUser(database.UserStatusActive) + + mockStore.EXPECT().GetUserByID(gomock.Any(), activeUser.ID).Return(activeUser, nil) + mockStore.EXPECT().UpdateUserStatus(gomock.Any(), gomock.Any()).Return(suspendedUser, nil) + + err := ru.Delete(scimRequest(t), activeUser.ID.String()) + require.NoError(t, err) + assert.Len(t, mockAudit.AuditLogs(), 1) + }) + + t.Run("Delete/suspended-user-skips-audit", func(t *testing.T) { + t.Parallel() + ru, mockStore, mockAudit := setupSCIMMock(t) + _, suspendedUser := makeUser(database.UserStatusSuspended) + + mockStore.EXPECT().GetUserByID(gomock.Any(), suspendedUser.ID).Return(suspendedUser, nil) + // No UpdateUserStatus expected: already suspended. + + err := ru.Delete(scimRequest(t), suspendedUser.ID.String()) + require.NoError(t, err) + assert.Empty(t, mockAudit.AuditLogs()) + }) +} + +// mustPath parses a SCIM attribute path string into a *filter.Path +// for use in PatchOperation test data. +func mustPath(attr string) *filter.Path { + p, err := filter.ParsePath([]byte(attr)) + if err != nil { + panic(fmt.Sprintf("mustPath(%q): %v", attr, err)) + } + return &p +} diff --git a/enterprise/coderd/scim_test.go b/enterprise/coderd/scim_test.go index e33c49e2a4834..0aeb61d8e0221 100644 --- a/enterprise/coderd/scim_test.go +++ b/enterprise/coderd/scim_test.go @@ -4,13 +4,10 @@ import ( "context" "encoding/json" "fmt" - "io" "net/http" "net/http/httptest" "testing" - "github.com/golang-jwt/jwt/v4" - "github.com/google/uuid" "github.com/imulab/go-scim/pkg/v2/handlerutil" "github.com/imulab/go-scim/pkg/v2/spec" "github.com/stretchr/testify/assert" @@ -19,25 +16,22 @@ import ( "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/coderdtest" - "github.com/coder/coder/v2/coderd/coderdtest/oidctest" - "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/notifications/notificationstest" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/cryptorand" - "github.com/coder/coder/v2/enterprise/coderd" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" + "github.com/coder/coder/v2/enterprise/coderd/legacyscim" "github.com/coder/coder/v2/enterprise/coderd/license" - "github.com/coder/coder/v2/enterprise/coderd/scim" "github.com/coder/coder/v2/testutil" ) //nolint:revive -func makeScimUser(t testing.TB) coderd.SCIMUser { +func makeScimUser(t testing.TB) legacyscim.SCIMUser { rstr, err := cryptorand.String(10) require.NoError(t, err) - return coderd.SCIMUser{ + return legacyscim.SCIMUser{ UserName: rstr, Name: struct { GivenName string `json:"givenName"` @@ -64,807 +58,651 @@ func setScimAuth(key []byte) func(*http.Request) { } } -func setScimAuthBearer(key []byte) func(*http.Request) { - return func(r *http.Request) { - // Do strange casing to ensure it's case-insensitive - r.Header.Set("Authorization", "beAreR "+string(key)) - } -} - +// TestLegacyScim tests the legacy SCIM handler (imulab/go-scim based). +// This is a reduced set of integration tests verifying HTTP routing, auth, +// and core CRUD. Detailed handler logic is covered by the unit tests in +// enterprise/coderd/scim/scimusers_test.go. +// //nolint:gocritic // SCIM authenticates via a special header and bypasses internal RBAC. -func TestScim(t *testing.T) { +func TestLegacyScim(t *testing.T) { t.Parallel() - t.Run("postUser", func(t *testing.T) { + t.Run("disabled", func(t *testing.T) { t.Parallel() - - t.Run("disabled", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - SCIMAPIKey: []byte("hi"), - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 0, - }, - }, - }) - - res, err := client.Request(ctx, "POST", "/scim/v2/Users", struct{}{}) - require.NoError(t, err) - defer res.Body.Close() - assert.Equal(t, http.StatusForbidden, res.StatusCode) - }) - - t.Run("noAuth", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - SCIMAPIKey: []byte("hi"), - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - }, - }, - }) - - res, err := client.Request(ctx, "POST", "/scim/v2/Users", struct{}{}) - require.NoError(t, err) - defer res.Body.Close() - assert.Equal(t, http.StatusUnauthorized, res.StatusCode) - }) - - t.Run("OK", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - // given - scimAPIKey := []byte("hi") - mockAudit := audit.NewMock() - notifyEnq := ¬ificationstest.FakeEnqueuer{} - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - Options: &coderdtest.Options{ - Auditor: mockAudit, - NotificationsEnqueuer: notifyEnq, - }, - SCIMAPIKey: scimAPIKey, - AuditLogging: true, - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - codersdk.FeatureAuditLog: 1, - }, - }, - }) - mockAudit.ResetLogs() - - // verify scim is enabled - res, err := client.Request(ctx, http.MethodGet, "/scim/v2/ServiceProviderConfig", nil) - require.NoError(t, err) - defer res.Body.Close() - require.Equal(t, http.StatusOK, res.StatusCode) - - // when - sUser := makeScimUser(t) - res, err = client.Request(ctx, http.MethodPost, "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - defer res.Body.Close() - require.Equal(t, http.StatusOK, res.StatusCode) - - // then - // Expect audit logs - aLogs := mockAudit.AuditLogs() - require.Len(t, aLogs, 1) - af := map[string]string{} - err = json.Unmarshal([]byte(aLogs[0].AdditionalFields), &af) - require.NoError(t, err) - assert.Equal(t, coderd.SCIMAuditAdditionalFields, af) - assert.Equal(t, database.AuditActionCreate, aLogs[0].Action) - - // Expect users exposed over API - userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value}) - require.NoError(t, err) - require.Len(t, userRes.Users, 1) - assert.Equal(t, sUser.Emails[0].Value, userRes.Users[0].Email) - assert.Equal(t, sUser.UserName, userRes.Users[0].Username) - assert.Len(t, userRes.Users[0].OrganizationIDs, 1) - - // Expect zero notifications (SkipNotifications = true) - require.Empty(t, notifyEnq.Sent()) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + SCIMAPIKey: []byte("hi"), + UseLegacySCIM: true, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{codersdk.FeatureSCIM: 0}, + }, }) - t.Run("OK_Bearer", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - // given - scimAPIKey := []byte("hi") - mockAudit := audit.NewMock() - notifyEnq := ¬ificationstest.FakeEnqueuer{} - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - Options: &coderdtest.Options{ - Auditor: mockAudit, - NotificationsEnqueuer: notifyEnq, - }, - SCIMAPIKey: scimAPIKey, - AuditLogging: true, - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - codersdk.FeatureAuditLog: 1, - }, - }, - }) - mockAudit.ResetLogs() - - // when - sUser := makeScimUser(t) - res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuthBearer(scimAPIKey)) - require.NoError(t, err) - defer res.Body.Close() - require.Equal(t, http.StatusOK, res.StatusCode) - - // then - // Expect audit logs - aLogs := mockAudit.AuditLogs() - require.Len(t, aLogs, 1) - af := map[string]string{} - err = json.Unmarshal([]byte(aLogs[0].AdditionalFields), &af) - require.NoError(t, err) - assert.Equal(t, coderd.SCIMAuditAdditionalFields, af) - assert.Equal(t, database.AuditActionCreate, aLogs[0].Action) - - // Expect users exposed over API - userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value}) - require.NoError(t, err) - require.Len(t, userRes.Users, 1) - assert.Equal(t, sUser.Emails[0].Value, userRes.Users[0].Email) - assert.Equal(t, sUser.UserName, userRes.Users[0].Username) - assert.Len(t, userRes.Users[0].OrganizationIDs, 1) - - // Expect zero notifications (SkipNotifications = true) - require.Empty(t, notifyEnq.Sent()) - }) - - t.Run("OKNoDefault", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - // given - scimAPIKey := []byte("hi") - mockAudit := audit.NewMock() - notifyEnq := ¬ificationstest.FakeEnqueuer{} - dv := coderdtest.DeploymentValues(t) - dv.OIDC.OrganizationAssignDefault = false - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - Options: &coderdtest.Options{ - Auditor: mockAudit, - NotificationsEnqueuer: notifyEnq, - DeploymentValues: dv, - }, - SCIMAPIKey: scimAPIKey, - AuditLogging: true, - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - codersdk.FeatureAuditLog: 1, - }, - }, - }) - mockAudit.ResetLogs() - - // when - sUser := makeScimUser(t) - res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - defer res.Body.Close() - require.Equal(t, http.StatusOK, res.StatusCode) - - // then - // Expect audit logs - aLogs := mockAudit.AuditLogs() - require.Len(t, aLogs, 1) - af := map[string]string{} - err = json.Unmarshal([]byte(aLogs[0].AdditionalFields), &af) - require.NoError(t, err) - assert.Equal(t, coderd.SCIMAuditAdditionalFields, af) - assert.Equal(t, database.AuditActionCreate, aLogs[0].Action) - - // Expect users exposed over API - userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value}) - require.NoError(t, err) - require.Len(t, userRes.Users, 1) - assert.Equal(t, sUser.Emails[0].Value, userRes.Users[0].Email) - assert.Equal(t, sUser.UserName, userRes.Users[0].Username) - assert.Len(t, userRes.Users[0].OrganizationIDs, 0) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", struct{}{}) + require.NoError(t, err) + defer res.Body.Close() + assert.Equal(t, http.StatusForbidden, res.StatusCode) + }) - // Expect zero notifications (SkipNotifications = true) - require.Empty(t, notifyEnq.Sent()) + t.Run("noAuth", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + SCIMAPIKey: []byte("hi"), + UseLegacySCIM: true, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{codersdk.FeatureSCIM: 1}, + }, }) - t.Run("Duplicate", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() + res, err := client.Request(ctx, "POST", "/scim/v2/Users", struct{}{}) + require.NoError(t, err) + defer res.Body.Close() + assert.Equal(t, http.StatusUnauthorized, res.StatusCode) + }) - scimAPIKey := []byte("hi") - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - SCIMAPIKey: scimAPIKey, - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - }, + t.Run("postUser", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + scimAPIKey := []byte("hi") + mockAudit := audit.NewMock() + notifyEnq := ¬ificationstest.FakeEnqueuer{} + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + Auditor: mockAudit, + NotificationsEnqueuer: notifyEnq, + }, + SCIMAPIKey: scimAPIKey, + UseLegacySCIM: true, + AuditLogging: true, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + codersdk.FeatureAuditLog: 1, + codersdk.FeatureMultipleOrganizations: 1, }, - }) - - sUser := makeScimUser(t) - for i := 0; i < 3; i++ { - res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - _ = res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) - } - - userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value}) - require.NoError(t, err) - require.Len(t, userRes.Users, 1) - - assert.Equal(t, sUser.Emails[0].Value, userRes.Users[0].Email) - assert.Equal(t, sUser.UserName, userRes.Users[0].Username) + }, }) - t.Run("Unsuspend", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() + sUser := makeScimUser(t) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + defer res.Body.Close() + assert.Equal(t, http.StatusOK, res.StatusCode) + + var createdUser legacyscim.SCIMUser + err = json.NewDecoder(res.Body).Decode(&createdUser) + require.NoError(t, err) + assert.NotEmpty(t, createdUser.ID) + assert.Equal(t, sUser.UserName, createdUser.UserName) + + // Verify user exists. + userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: createdUser.UserName}) + require.NoError(t, err) + require.Len(t, userRes.Users, 1) + assert.Equal(t, codersdk.LoginTypeOIDC, userRes.Users[0].LoginType) + + // Verify audit log. + require.True(t, len(mockAudit.AuditLogs()) > 0) + + // Verify no user admin notification (SCIM skips notifications). + require.Empty(t, notifyEnq.Sent()) + }) - scimAPIKey := []byte("hi") - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - SCIMAPIKey: scimAPIKey, - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - }, + t.Run("Duplicate", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + scimAPIKey := []byte("hi") + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + SCIMAPIKey: scimAPIKey, + UseLegacySCIM: true, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + codersdk.FeatureMultipleOrganizations: 1, }, - }) - - sUser := makeScimUser(t) - res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - defer res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) - err = json.NewDecoder(res.Body).Decode(&sUser) - require.NoError(t, err) - - sUser.Active = ptr.Ref(false) - res, err = client.Request(ctx, "PATCH", "/scim/v2/Users/"+sUser.ID, sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - _, _ = io.Copy(io.Discard, res.Body) - _ = res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) - - sUser.Active = ptr.Ref(true) - res, err = client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - _, _ = io.Copy(io.Discard, res.Body) - _ = res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) - - userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value}) - require.NoError(t, err) - require.Len(t, userRes.Users, 1) - - assert.Equal(t, sUser.Emails[0].Value, userRes.Users[0].Email) - assert.Equal(t, sUser.UserName, userRes.Users[0].Username) - assert.Equal(t, codersdk.UserStatusDormant, userRes.Users[0].Status) + }, }) - t.Run("DomainStrips", func(t *testing.T) { - t.Parallel() + sUser := makeScimUser(t) - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - scimAPIKey := []byte("hi") - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - SCIMAPIKey: scimAPIKey, - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - }, - }, - }) - - sUser := makeScimUser(t) - sUser.UserName = sUser.UserName + "@coder.com" + // Create same user 3 times. + for i := 0; i < 3; i++ { res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) require.NoError(t, err) - _, _ = io.Copy(io.Discard, res.Body) _ = res.Body.Close() assert.Equal(t, http.StatusOK, res.StatusCode) + } - userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value}) - require.NoError(t, err) - require.Len(t, userRes.Users, 1) - - assert.Equal(t, sUser.Emails[0].Value, userRes.Users[0].Email) - // Username should be the same as the given name. They all use the - // same string before we modified it above. - assert.Equal(t, sUser.Name.GivenName, userRes.Users[0].Username) - }) + // Only 1 user should exist. + userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.UserName}) + require.NoError(t, err) + require.Len(t, userRes.Users, 1) }) t.Run("patchUser", func(t *testing.T) { t.Parallel() - - t.Run("disabled", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - SCIMAPIKey: []byte("hi"), - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 0, - }, + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + scimAPIKey := []byte("hi") + mockAudit := audit.NewMock() + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{Auditor: mockAudit}, + SCIMAPIKey: scimAPIKey, + UseLegacySCIM: true, + AuditLogging: true, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + codersdk.FeatureAuditLog: 1, + codersdk.FeatureMultipleOrganizations: 1, }, - }) - - res, err := client.Request(ctx, "PATCH", "/scim/v2/Users/bob", struct{}{}) - require.NoError(t, err) - _, _ = io.Copy(io.Discard, res.Body) - _ = res.Body.Close() - assert.Equal(t, http.StatusForbidden, res.StatusCode) + }, }) - t.Run("noAuth", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() + // Create user first. + sUser := makeScimUser(t) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + defer res.Body.Close() + require.Equal(t, http.StatusOK, res.StatusCode) + + var createdUser legacyscim.SCIMUser + err = json.NewDecoder(res.Body).Decode(&createdUser) + require.NoError(t, err) + + // Suspend via PATCH. + mockAudit.ResetLogs() + sUser.Active = ptr.Ref(false) + res, err = client.Request(ctx, "PATCH", "/scim/v2/Users/"+createdUser.ID, sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + defer res.Body.Close() + assert.Equal(t, http.StatusOK, res.StatusCode) + + // Verify suspended. + userRes, err := client.User(ctx, createdUser.ID) + require.NoError(t, err) + assert.Equal(t, codersdk.UserStatusSuspended, userRes.Status) + }) - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - SCIMAPIKey: []byte("hi"), - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - }, + t.Run("putUser", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + scimAPIKey := []byte("hi") + mockAudit := audit.NewMock() + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{Auditor: mockAudit}, + SCIMAPIKey: scimAPIKey, + UseLegacySCIM: true, + AuditLogging: true, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + codersdk.FeatureAuditLog: 1, + codersdk.FeatureMultipleOrganizations: 1, }, - }) - - res, err := client.Request(ctx, "PATCH", "/scim/v2/Users/bob", struct{}{}) - require.NoError(t, err) - _, _ = io.Copy(io.Discard, res.Body) - _ = res.Body.Close() - assert.Equal(t, http.StatusUnauthorized, res.StatusCode) + }, }) - t.Run("OK", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - scimAPIKey := []byte("hi") - mockAudit := audit.NewMock() - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - Options: &coderdtest.Options{Auditor: mockAudit}, - SCIMAPIKey: scimAPIKey, - AuditLogging: true, - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - codersdk.FeatureAuditLog: 1, - }, - }, - }) - mockAudit.ResetLogs() + // Create user first. + sUser := makeScimUser(t) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + defer res.Body.Close() + require.Equal(t, http.StatusOK, res.StatusCode) + + var createdUser legacyscim.SCIMUser + err = json.NewDecoder(res.Body).Decode(&createdUser) + require.NoError(t, err) + + // Suspend via PUT. + mockAudit.ResetLogs() + sUser.Active = ptr.Ref(false) + res, err = client.Request(ctx, "PUT", "/scim/v2/Users/"+createdUser.ID, sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + defer res.Body.Close() + assert.Equal(t, http.StatusOK, res.StatusCode) + + // Verify suspended. + userRes, err := client.User(ctx, createdUser.ID) + require.NoError(t, err) + assert.Equal(t, codersdk.UserStatusSuspended, userRes.Status) + }) +} - sUser := makeScimUser(t) - res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - defer res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) - mockAudit.ResetLogs() +// scim2User is a minimal struct for decoding SCIM 2.0 user responses +// returned by the elimity-com/scim library. +type scim2User struct { + ID string `json:"id"` + UserName string `json:"userName"` + Active bool `json:"active"` +} - err = json.NewDecoder(res.Body).Decode(&sUser) - require.NoError(t, err) +// scim2UserBody is the request body for SCIM 2.0 POST/PUT calls. +// Unlike the legacy handler, the elimity-com/scim library validates the +// "schemas" attribute against the core User schema URI and rejects bodies +// that omit it. +type scim2UserBody struct { + Schemas []string `json:"schemas"` + UserName string `json:"userName"` + Name struct { + GivenName string `json:"givenName"` + FamilyName string `json:"familyName"` + } `json:"name"` + Emails []struct { + Primary bool `json:"primary"` + Value string `json:"value"` + } `json:"emails"` + Active *bool `json:"active,omitempty"` +} - sUser.Active = ptr.Ref(false) +func makeScim2User(t testing.TB) scim2UserBody { + rstr, err := cryptorand.String(10) + require.NoError(t, err) - res, err = client.Request(ctx, "PATCH", "/scim/v2/Users/"+sUser.ID, sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - _, _ = io.Copy(io.Discard, res.Body) - _ = res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) + b := scim2UserBody{ + Schemas: []string{"urn:ietf:params:scim:schemas:core:2.0:User"}, + UserName: rstr, + Active: ptr.Ref(true), + } + b.Name.GivenName = rstr + b.Name.FamilyName = rstr + b.Emails = []struct { + Primary bool `json:"primary"` + Value string `json:"value"` + }{{Primary: true, Value: fmt.Sprintf("%s@coder.com", rstr)}} + return b +} - aLogs := mockAudit.AuditLogs() - require.Len(t, aLogs, 1) - assert.Equal(t, database.AuditActionWrite, aLogs[0].Action) +// TestScim exercises the SCIM 2.0 handler through real HTTP routes. It +// mirrors TestLegacyScim's structure (disabled/noAuth/post/patch/put) and +// adds coverage for behavior unique to the v2 implementation: discovery +// endpoints, 409 Conflict on duplicate active users, suspended-user +// reactivation, GET by id, and DELETE. +// +//nolint:gocritic // SCIM authenticates via a special header and bypasses internal RBAC. +func TestScim(t *testing.T) { + t.Parallel() - userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value}) - require.NoError(t, err) - require.Len(t, userRes.Users, 1) - assert.Equal(t, codersdk.UserStatusSuspended, userRes.Users[0].Status) + t.Run("disabled", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + SCIMAPIKey: []byte("hi"), + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{codersdk.FeatureSCIM: 0}, + }, }) - // Create a user via SCIM, which starts as dormant. - // Log in as the user, making them active. - // Then patch the user again and the user should still be active. - t.Run("ActiveIsActive", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - scimAPIKey := []byte("hi") - - mockAudit := audit.NewMock() - fake := oidctest.NewFakeIDP(t, oidctest.WithServing()) - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - Options: &coderdtest.Options{ - Auditor: mockAudit, - OIDCConfig: fake.OIDCConfig(t, []string{}), - }, - SCIMAPIKey: scimAPIKey, - AuditLogging: true, - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - codersdk.FeatureAuditLog: 1, - }, - }, - }) - mockAudit.ResetLogs() - - // User is dormant on create - sUser := makeScimUser(t) - res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - defer res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", struct{}{}) + require.NoError(t, err) + defer res.Body.Close() + assert.Equal(t, http.StatusForbidden, res.StatusCode) + }) - err = json.NewDecoder(res.Body).Decode(&sUser) - require.NoError(t, err) + t.Run("noAuth", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + SCIMAPIKey: []byte("hi"), + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{codersdk.FeatureSCIM: 1}, + }, + }) - // Check the audit log - aLogs := mockAudit.AuditLogs() - require.Len(t, aLogs, 1) - assert.Equal(t, database.AuditActionCreate, aLogs[0].Action) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", struct{}{}) + require.NoError(t, err) + defer res.Body.Close() + assert.Equal(t, http.StatusUnauthorized, res.StatusCode) + }) - // Verify the user is dormant - scimUser, err := client.User(ctx, sUser.UserName) - require.NoError(t, err) - require.Equal(t, codersdk.UserStatusDormant, scimUser.Status, "user starts as dormant") - - // Log in as the user, making them active - //nolint:bodyclose - scimUserClient, _ := fake.Login(t, client, jwt.MapClaims{ - "email": sUser.Emails[0].Value, - "sub": uuid.NewString(), - }) - scimUser, err = scimUserClient.User(ctx, codersdk.Me) - require.NoError(t, err) - require.Equal(t, codersdk.UserStatusActive, scimUser.Status, "user should now be active") + t.Run("discovery", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + scimAPIKey := []byte("hi") + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + SCIMAPIKey: scimAPIKey, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{codersdk.FeatureSCIM: 1}, + }, + }) - // Patch the user - mockAudit.ResetLogs() - res, err = client.Request(ctx, "PATCH", "/scim/v2/Users/"+sUser.ID, sUser, setScimAuth(scimAPIKey)) + for _, path := range []string{ + "/scim/v2/ServiceProviderConfig", + "/scim/v2/ResourceTypes", + "/scim/v2/Schemas", + } { + res, err := client.Request(ctx, "GET", path, nil, setScimAuth(scimAPIKey)) require.NoError(t, err) - _, _ = io.Copy(io.Discard, res.Body) _ = res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) - - // Should be no audit logs since there is no diff - aLogs = mockAudit.AuditLogs() - require.Len(t, aLogs, 0) - - // Verify the user is still active. - scimUser, err = client.User(ctx, sUser.UserName) - require.NoError(t, err) - require.Equal(t, codersdk.UserStatusActive, scimUser.Status, "user is still active") - }) + assert.Equal(t, http.StatusOK, res.StatusCode, "discovery endpoint %s", path) + } }) - t.Run("putUser", func(t *testing.T) { + t.Run("postUser", func(t *testing.T) { t.Parallel() - - t.Run("disabled", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - SCIMAPIKey: []byte("hi"), - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 0, - }, + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + scimAPIKey := []byte("hi") + mockAudit := audit.NewMock() + notifyEnq := ¬ificationstest.FakeEnqueuer{} + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + Auditor: mockAudit, + NotificationsEnqueuer: notifyEnq, + }, + SCIMAPIKey: scimAPIKey, + AuditLogging: true, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + codersdk.FeatureAuditLog: 1, + codersdk.FeatureMultipleOrganizations: 1, }, - }) - - res, err := client.Request(ctx, http.MethodPut, "/scim/v2/Users/bob", struct{}{}) - require.NoError(t, err) - _, _ = io.Copy(io.Discard, res.Body) - _ = res.Body.Close() - assert.Equal(t, http.StatusForbidden, res.StatusCode) + }, }) - t.Run("noAuth", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() + sUser := makeScim2User(t) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + defer res.Body.Close() + require.Equal(t, http.StatusCreated, res.StatusCode) + + var created scim2User + require.NoError(t, json.NewDecoder(res.Body).Decode(&created)) + assert.NotEmpty(t, created.ID) + assert.Equal(t, sUser.UserName, created.UserName) + assert.True(t, created.Active) + + // Verify user exists. + userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: created.UserName}) + require.NoError(t, err) + require.Len(t, userRes.Users, 1) + assert.Equal(t, codersdk.LoginTypeOIDC, userRes.Users[0].LoginType) + + // Verify audit log. + require.True(t, len(mockAudit.AuditLogs()) > 0) + + // Verify no user admin notification (SCIM skips notifications). + require.Empty(t, notifyEnq.Sent()) + }) - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - SCIMAPIKey: []byte("hi"), - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - }, + t.Run("postUserConflict", func(t *testing.T) { + // SCIM 2.0 returns 409 Conflict on duplicate active user, unlike the + // legacy handler which returned 200 with the existing user. + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + scimAPIKey := []byte("hi") + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + SCIMAPIKey: scimAPIKey, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + codersdk.FeatureMultipleOrganizations: 1, }, - }) - - res, err := client.Request(ctx, http.MethodPut, "/scim/v2/Users/bob", struct{}{}) - require.NoError(t, err) - _, _ = io.Copy(io.Discard, res.Body) - _ = res.Body.Close() - assert.Equal(t, http.StatusUnauthorized, res.StatusCode) + }, }) - t.Run("MissingActiveField", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - scimAPIKey := []byte("hi") - mockAudit := audit.NewMock() - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - Options: &coderdtest.Options{Auditor: mockAudit}, - SCIMAPIKey: scimAPIKey, - AuditLogging: true, - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - codersdk.FeatureAuditLog: 1, - }, - }, - }) - mockAudit.ResetLogs() - - sUser := makeScimUser(t) - res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - defer res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) - mockAudit.ResetLogs() - - err = json.NewDecoder(res.Body).Decode(&sUser) - require.NoError(t, err) - - sUser.Active = nil + sUser := makeScim2User(t) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + _ = res.Body.Close() + require.Equal(t, http.StatusCreated, res.StatusCode) - res, err = client.Request(ctx, http.MethodPut, "/scim/v2/Users/"+sUser.ID, sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - defer res.Body.Close() - assert.Equal(t, http.StatusBadRequest, res.StatusCode) + res, err = client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + _ = res.Body.Close() + assert.Equal(t, http.StatusConflict, res.StatusCode) - data, err := io.ReadAll(res.Body) - require.NoError(t, err) - require.Contains(t, string(data), "active field is required") - mockAudit.ResetLogs() - }) + userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.UserName}) + require.NoError(t, err) + require.Len(t, userRes.Users, 1) + }) - t.Run("ImmutabilityViolation", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - scimAPIKey := []byte("hi") - mockAudit := audit.NewMock() - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - Options: &coderdtest.Options{Auditor: mockAudit}, - SCIMAPIKey: scimAPIKey, - AuditLogging: true, - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - codersdk.FeatureAuditLog: 1, - }, + t.Run("postUserReactivatesSuspended", func(t *testing.T) { + // When the SCIM client deletes a user (which only suspends in Coder), + // posting the same user again should reactivate the existing row. + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + scimAPIKey := []byte("hi") + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + SCIMAPIKey: scimAPIKey, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + codersdk.FeatureMultipleOrganizations: 1, }, - }) - mockAudit.ResetLogs() - - sUser := makeScimUser(t) - res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - defer res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) - mockAudit.ResetLogs() - - err = json.NewDecoder(res.Body).Decode(&sUser) - require.NoError(t, err) - - sUser.UserName += "changed" - - res, err = client.Request(ctx, http.MethodPut, "/scim/v2/Users/"+sUser.ID, sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - defer res.Body.Close() - assert.Equal(t, http.StatusBadRequest, res.StatusCode) - mockAudit.ResetLogs() - - data, err := io.ReadAll(res.Body) - require.NoError(t, err) - require.Contains(t, string(data), "mutability") - require.NoError(t, err) + }, }) - t.Run("OK", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - scimAPIKey := []byte("hi") - mockAudit := audit.NewMock() - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - Options: &coderdtest.Options{Auditor: mockAudit}, - SCIMAPIKey: scimAPIKey, - AuditLogging: true, - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - codersdk.FeatureAuditLog: 1, - }, - }, - }) - mockAudit.ResetLogs() - - sUser := makeScimUser(t) - res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - defer res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) - mockAudit.ResetLogs() - - err = json.NewDecoder(res.Body).Decode(&sUser) - require.NoError(t, err) - - sUser.Active = ptr.Ref(false) - - res, err = client.Request(ctx, http.MethodPatch, "/scim/v2/Users/"+sUser.ID, sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - _, _ = io.Copy(io.Discard, res.Body) - _ = res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) - - aLogs := mockAudit.AuditLogs() - require.Len(t, aLogs, 1) - assert.Equal(t, database.AuditActionWrite, aLogs[0].Action) + sUser := makeScim2User(t) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + var created scim2User + require.NoError(t, json.NewDecoder(res.Body).Decode(&created)) + _ = res.Body.Close() + require.Equal(t, http.StatusCreated, res.StatusCode) + require.NotEmpty(t, created.ID) + + // Delete (suspends) the user. + res, err = client.Request(ctx, "DELETE", "/scim/v2/Users/"+created.ID, nil, setScimAuth(scimAPIKey)) + require.NoError(t, err) + _ = res.Body.Close() + assert.Equal(t, http.StatusNoContent, res.StatusCode) + + userRes, err := client.User(ctx, created.ID) + require.NoError(t, err) + assert.Equal(t, codersdk.UserStatusSuspended, userRes.Status) + + // Re-create. The handler should reactivate the existing row. + res, err = client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + var recreated scim2User + require.NoError(t, json.NewDecoder(res.Body).Decode(&recreated)) + _ = res.Body.Close() + require.Equal(t, http.StatusCreated, res.StatusCode) + assert.Equal(t, created.ID, recreated.ID, "recreate should reactivate the existing row, not create a new one") + assert.True(t, recreated.Active, "recreated user should be active in the SCIM response") + + // The DB user moves from suspended → dormant on reactivate; the SCIM + // response reports both Active and Dormant as active=true. + userRes, err = client.User(ctx, created.ID) + require.NoError(t, err) + assert.Equal(t, codersdk.UserStatusDormant, userRes.Status) + }) - userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value}) - require.NoError(t, err) - require.Len(t, userRes.Users, 1) - assert.Equal(t, codersdk.UserStatusSuspended, userRes.Users[0].Status) + t.Run("getUser", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + scimAPIKey := []byte("hi") + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + SCIMAPIKey: scimAPIKey, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + codersdk.FeatureMultipleOrganizations: 1, + }, + }, }) - // Create a user via SCIM, which starts as dormant. - // Log in as the user, making them active. - // Then patch the user again and the user should still be active. - t.Run("ActiveIsActive", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - scimAPIKey := []byte("hi") + sUser := makeScim2User(t) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + var created scim2User + require.NoError(t, json.NewDecoder(res.Body).Decode(&created)) + _ = res.Body.Close() + require.Equal(t, http.StatusCreated, res.StatusCode) + + res, err = client.Request(ctx, "GET", "/scim/v2/Users/"+created.ID, nil, setScimAuth(scimAPIKey)) + require.NoError(t, err) + defer res.Body.Close() + require.Equal(t, http.StatusOK, res.StatusCode) + + var got scim2User + require.NoError(t, json.NewDecoder(res.Body).Decode(&got)) + assert.Equal(t, created.ID, got.ID) + assert.Equal(t, sUser.UserName, got.UserName) + }) - mockAudit := audit.NewMock() - fake := oidctest.NewFakeIDP(t, oidctest.WithServing()) - client, _ := coderdenttest.New(t, &coderdenttest.Options{ - Options: &coderdtest.Options{ - Auditor: mockAudit, - OIDCConfig: fake.OIDCConfig(t, []string{}), - }, - SCIMAPIKey: scimAPIKey, - AuditLogging: true, - LicenseOptions: &coderdenttest.LicenseOptions{ - AccountID: "coolin", - Features: license.Features{ - codersdk.FeatureSCIM: 1, - codersdk.FeatureAuditLog: 1, - }, + t.Run("patchUser", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + scimAPIKey := []byte("hi") + mockAudit := audit.NewMock() + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{Auditor: mockAudit}, + SCIMAPIKey: scimAPIKey, + AuditLogging: true, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + codersdk.FeatureAuditLog: 1, + codersdk.FeatureMultipleOrganizations: 1, }, - }) - mockAudit.ResetLogs() - - // User is dormant on create - sUser := makeScimUser(t) - res, err := client.Request(ctx, http.MethodPost, "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - defer res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) - - err = json.NewDecoder(res.Body).Decode(&sUser) - require.NoError(t, err) - - // Check the audit log - aLogs := mockAudit.AuditLogs() - require.Len(t, aLogs, 1) - assert.Equal(t, database.AuditActionCreate, aLogs[0].Action) + }, + }) - // Verify the user is dormant - scimUser, err := client.User(ctx, sUser.UserName) - require.NoError(t, err) - require.Equal(t, codersdk.UserStatusDormant, scimUser.Status, "user starts as dormant") - - // Log in as the user, making them active - //nolint:bodyclose - scimUserClient, _ := fake.Login(t, client, jwt.MapClaims{ - "email": sUser.Emails[0].Value, - "sub": uuid.NewString(), - }) - scimUser, err = scimUserClient.User(ctx, codersdk.Me) - require.NoError(t, err) - require.Equal(t, codersdk.UserStatusActive, scimUser.Status, "user should now be active") + sUser := makeScim2User(t) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + var created scim2User + require.NoError(t, json.NewDecoder(res.Body).Decode(&created)) + _ = res.Body.Close() + require.Equal(t, http.StatusCreated, res.StatusCode) + + // PATCH with replace op setting active=false. + mockAudit.ResetLogs() + patchBody := map[string]interface{}{ + "schemas": []string{"urn:ietf:params:scim:api:messages:2.0:PatchOp"}, + "Operations": []map[string]interface{}{ + {"op": "replace", "path": "active", "value": false}, + }, + } + res, err = client.Request(ctx, "PATCH", "/scim/v2/Users/"+created.ID, patchBody, setScimAuth(scimAPIKey)) + require.NoError(t, err) + _ = res.Body.Close() + assert.Equal(t, http.StatusOK, res.StatusCode) + + userRes, err := client.User(ctx, created.ID) + require.NoError(t, err) + assert.Equal(t, codersdk.UserStatusSuspended, userRes.Status) + }) - // Patch the user - mockAudit.ResetLogs() - res, err = client.Request(ctx, http.MethodPut, "/scim/v2/Users/"+sUser.ID, sUser, setScimAuth(scimAPIKey)) - require.NoError(t, err) - _, _ = io.Copy(io.Discard, res.Body) - _ = res.Body.Close() - assert.Equal(t, http.StatusOK, res.StatusCode) + t.Run("putUser", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + scimAPIKey := []byte("hi") + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + SCIMAPIKey: scimAPIKey, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + codersdk.FeatureMultipleOrganizations: 1, + }, + }, + }) - // Should be no audit logs since there is no diff - aLogs = mockAudit.AuditLogs() - require.Len(t, aLogs, 0) + sUser := makeScim2User(t) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + var created scim2User + require.NoError(t, json.NewDecoder(res.Body).Decode(&created)) + _ = res.Body.Close() + require.Equal(t, http.StatusCreated, res.StatusCode) + + // PUT with active=false. + sUser.Active = ptr.Ref(false) + res, err = client.Request(ctx, "PUT", "/scim/v2/Users/"+created.ID, sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + _ = res.Body.Close() + assert.Equal(t, http.StatusOK, res.StatusCode) + + userRes, err := client.User(ctx, created.ID) + require.NoError(t, err) + assert.Equal(t, codersdk.UserStatusSuspended, userRes.Status) + }) - // Verify the user is still active. - scimUser, err = client.User(ctx, sUser.UserName) - require.NoError(t, err) - require.Equal(t, codersdk.UserStatusActive, scimUser.Status, "user is still active") + t.Run("deleteUser", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + scimAPIKey := []byte("hi") + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + SCIMAPIKey: scimAPIKey, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + codersdk.FeatureMultipleOrganizations: 1, + }, + }, }) + + sUser := makeScim2User(t) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + var created scim2User + require.NoError(t, json.NewDecoder(res.Body).Decode(&created)) + _ = res.Body.Close() + require.Equal(t, http.StatusCreated, res.StatusCode) + + res, err = client.Request(ctx, "DELETE", "/scim/v2/Users/"+created.ID, nil, setScimAuth(scimAPIKey)) + require.NoError(t, err) + _ = res.Body.Close() + assert.Equal(t, http.StatusNoContent, res.StatusCode) + + // Coder does not hard-delete users. The user should remain but be suspended. + userRes, err := client.User(ctx, created.ID) + require.NoError(t, err) + assert.Equal(t, codersdk.UserStatusSuspended, userRes.Status) }) } -func TestScimError(t *testing.T) { +func TestLegacyScimError(t *testing.T) { t.Parallel() // Demonstrates that we cannot use the standard errors @@ -876,7 +714,7 @@ func TestScimError(t *testing.T) { // Our error wrapper works rw = httptest.NewRecorder() - _ = handlerutil.WriteError(rw, scim.NewHTTPError(http.StatusNotFound, spec.ErrNotFound.Type, xerrors.New("not found"))) + _ = handlerutil.WriteError(rw, legacyscim.NewHTTPError(http.StatusNotFound, spec.ErrNotFound.Type, xerrors.New("not found"))) resp = rw.Result() defer resp.Body.Close() require.Equal(t, http.StatusNotFound, resp.StatusCode) diff --git a/enterprise/coderd/scimroutes.go b/enterprise/coderd/scimroutes.go new file mode 100644 index 0000000000000..891b760e2f412 --- /dev/null +++ b/enterprise/coderd/scimroutes.go @@ -0,0 +1,74 @@ +package coderd + +import ( + "net/http" + + "github.com/go-chi/chi/v5" + "github.com/go-chi/chi/v5/middleware" + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/legacyscim" + "github.com/coder/coder/v2/enterprise/coderd/scim" +) + +func (api *API) mountScimRoute(opt *Options, r chi.Router) error { + if len(opt.SCIMAPIKey) == 0 { + // Show a helpful 404 error. Because this is not under the /api/v2 routes, + // the frontend is the fallback. A html page is not a helpful error for + // a SCIM provider. This JSON has a call to action that __may__ resolve + // the issue. + // + // Using mount to cover all subroute possibilities + r.Mount("/", http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + httpapi.Write(r.Context(), w, http.StatusNotFound, codersdk.Response{ + Message: "SCIM is disabled, please contact your administrator if you believe this is an error", + Detail: "SCIM endpoints are disabled if no SCIM is configured. Configure 'CODER_SCIM_AUTH_HEADER' to enable.", + }) + }))) + return nil + } + + if opt.UseLegacySCIM { + // Legacy SCIM handler (imulab/go-scim based). Opt-in for + // backward compatibility during the transition period. + legacySrv := &legacyscim.LegacyServer{ + Logger: opt.Logger, + Database: opt.Database, + IDPSync: opt.IDPSync, + AGPL: api.AGPL, + AccessURL: api.AccessURL, + SCIMAPIKey: opt.SCIMAPIKey, + Auditor: &api.AGPL.Auditor, + } + r.Mount("/v2", chi.Chain( + api.RequireFeatureMW(codersdk.FeatureSCIM), + legacySrv.AuthMiddleware, + ).Handler(legacySrv.Handler())) + return nil + } + + // SCIM 2.0 handler (elimity-com/scim based). + scimSrv, err := scim.New(&scim.Options{ + DB: opt.Database, + Auditor: &api.AGPL.Auditor, + IDPSync: opt.IDPSync, + Logger: opt.Logger, + AGPL: api.AGPL, + SCIMAPIKey: opt.SCIMAPIKey, + }) + if err != nil { + return xerrors.Errorf("create scim server: %w", err) + } + + // The elimity-com/scim library reads r.URL.Path and strips "/v2" + // internally. Chi's Route/Mount modifies its own routing context + // but not r.URL.Path, so we use http.StripPrefix to ensure the + // library sees paths like "/v2/Users" instead of "/scim/v2/Users". + r.Mount("/", chi.Chain( + api.RequireFeatureMW(codersdk.FeatureSCIM), + middleware.StripPrefix("/scim"), + ).Handler(scimSrv.Handler())) + return nil +} diff --git a/go.mod b/go.mod index e0c11bb5fb7a5..7104a57e533a6 100644 --- a/go.mod +++ b/go.mod @@ -512,11 +512,13 @@ require ( github.com/danieljoos/wincred v1.2.3 github.com/dgraph-io/ristretto/v2 v2.4.0 github.com/elazarl/goproxy v1.8.0 + github.com/elimity-com/scim v0.0.0-20260506142751-830e1caafcc3 github.com/fsnotify/fsnotify v1.10.1 github.com/go-git/go-git/v5 v5.19.1 github.com/invopop/jsonschema v0.14.0 github.com/mark3labs/mcp-go v0.38.0 github.com/openai/openai-go/v3 v3.28.0 + github.com/scim2/filter-parser/v2 v2.2.0 github.com/shopspring/decimal v1.4.0 github.com/smallstep/pkcs7 v0.2.1 github.com/sony/gobreaker/v2 v2.4.0 @@ -571,6 +573,8 @@ require ( github.com/cpuguy83/go-md2man/v2 v2.0.7 // indirect github.com/daixiang0/gci v0.13.7 // indirect github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.0 // indirect + github.com/di-wu/parser v0.2.2 // indirect + github.com/di-wu/xsd-datetime v1.0.0 // indirect github.com/distribution/reference v0.6.0 // indirect github.com/envoyproxy/go-control-plane/envoy v1.37.0 // indirect github.com/envoyproxy/protoc-gen-validate v1.3.3 // indirect diff --git a/go.sum b/go.sum index 644911e5ec82b..8f3d128fb098a 100644 --- a/go.sum +++ b/go.sum @@ -418,6 +418,10 @@ github.com/dgryski/trifles v0.0.0-20230903005119-f50d829f2e54 h1:SG7nF6SRlWhcT7c github.com/dgryski/trifles v0.0.0-20230903005119-f50d829f2e54/go.mod h1:if7Fbed8SFyPtHLHbg49SI7NAdJiC5WIA09pe59rfAA= github.com/dhui/dktest v0.4.6 h1:+DPKyScKSEp3VLtbMDHcUq6V5Lm5zfZZVb0Sk7Ahom4= github.com/dhui/dktest v0.4.6/go.mod h1:JHTSYDtKkvFNFHJKqCzVzqXecyv+tKt8EzceOmQOgbU= +github.com/di-wu/parser v0.2.2 h1:I9oHJ8spBXOeL7Wps0ffkFFFiXJf/pk7NX9lcAMqRMU= +github.com/di-wu/parser v0.2.2/go.mod h1:SLp58pW6WamdmznrVRrw2NTyn4wAvT9rrEFynKX7nYo= +github.com/di-wu/xsd-datetime v1.0.0 h1:vZoGNkbzpBNoc+JyfVLEbutNDNydYV8XwHeV7eUJoxI= +github.com/di-wu/xsd-datetime v1.0.0/go.mod h1:i3iEhrP3WchwseOBeIdW/zxeoleXTOzx1WyDXgdmOww= github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= github.com/dlclark/regexp2 v1.4.0/go.mod h1:2pZnwuY/m+8K6iRw6wQdMtk+rH5tNGR1i55kozfMjCc= @@ -448,6 +452,8 @@ github.com/elastic/go-windows v1.0.0 h1:qLURgZFkkrYyTTkvYpsZIgf83AUsdIHfvlJaqaZ7 github.com/elastic/go-windows v1.0.0/go.mod h1:TsU0Nrp7/y3+VwE82FoZF8gC/XFg/Elz6CcloAxnPgU= github.com/elazarl/goproxy v1.8.0 h1:dt561rX7UAYMeFRLtzFx6uQGl2TpL1dr6uCG23nFQSY= github.com/elazarl/goproxy v1.8.0/go.mod h1:b5xm6W48AUHNpRTCvlnd0YVh+JafCCtsLsJZvvNTz+E= +github.com/elimity-com/scim v0.0.0-20260506142751-830e1caafcc3 h1:P+JJLBS2QNe5aWBpNoDWqmGwNv/DKP+WZpU/mPIS+28= +github.com/elimity-com/scim v0.0.0-20260506142751-830e1caafcc3/go.mod h1:JkjcmqbLW+khwt2fmBPJFBhx2zGZ8XobRZ+O0VhlwWo= github.com/emersion/go-sasl v0.0.0-20200509203442-7bfe0ed36a21 h1:OJyUGMJTzHTd1XQp98QTaHernxMYzRaOasRir9hUlFQ= github.com/emersion/go-sasl v0.0.0-20200509203442-7bfe0ed36a21/go.mod h1:iL2twTeMvZnrg54ZoPDNfJaJaqy0xIQFuBdrLsmspwQ= github.com/emersion/go-smtp v0.21.2 h1:OLDgvZKuofk4em9fT5tFG5j4jE1/hXnX75UMvcrL4AA= @@ -1063,6 +1069,8 @@ github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEV github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= github.com/satori/go.uuid v1.2.1-0.20181028125025-b2ce2384e17b h1:gQZ0qzfKHQIybLANtM3mBXNUtOfsCFXeTsnBqCsx1KM= github.com/satori/go.uuid v1.2.1-0.20181028125025-b2ce2384e17b/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0= +github.com/scim2/filter-parser/v2 v2.2.0 h1:QGadEcsmypxg8gYChRSM2j1edLyE/2j72j+hdmI4BJM= +github.com/scim2/filter-parser/v2 v2.2.0/go.mod h1:jWnkDToqX/Y0ugz0P5VvpVEUKcWcyHHj+X+je9ce5JA= github.com/secure-systems-lab/go-securesystemslib v0.10.0 h1:l+H5ErcW0PAehBNrBxoGv1jjNpGYdZ9RcheFkB2WI14= github.com/secure-systems-lab/go-securesystemslib v0.10.0/go.mod h1:MRKONWmRoFzPNQ9USRF9i1mc7MvAVvF1LlW8X5VWDvk= github.com/segmentio/asm v1.2.1 h1:DTNbBqs57ioxAD4PrArqftgypG4/qNpXoJx8TVXxPR0= diff --git a/scripts/scimverify/.gitignore b/scripts/scimverify/.gitignore new file mode 100644 index 0000000000000..53752db253e3b --- /dev/null +++ b/scripts/scimverify/.gitignore @@ -0,0 +1 @@ +output diff --git a/scripts/scimverify/README.md b/scripts/scimverify/README.md new file mode 100644 index 0000000000000..f2644d2024185 --- /dev/null +++ b/scripts/scimverify/README.md @@ -0,0 +1,198 @@ +# SCIM Verify + +> This test harness was set up by an AI agent ([Mux](https://mux.coder.com/)) +> during the SCIM 2.0 refactor. It discovered `scimverify`, figured out +> how to run it (the Docker image is a web UI, not the test CLI; use +> `npx scimverify` instead), worked through config quirks (schema detection +> crashes, plural vs singular resource type names, `id: AUTO` targeting the +> admin user), and iterated the config until 14/16 tests passed against a +> live Coder dev server. + +Run SCIM 2.0 compliance tests against a live Coder instance using [scimverify](https://verify.scim.dev/). + +## Prerequisites + +- **Node.js** (npx must be on PATH) +- A running Coder instance with SCIM enabled (`CODER_SCIM_AUTH_HEADER` set) +- An enterprise license (SCIM is an enterprise feature) + +## Quick start + +```bash +# Start a dev server with SCIM enabled +CODER_SCIM_AUTH_HEADER=my-secret-token ./scripts/develop.sh + +# In another terminal, run the tests +./scripts/scimverify/run.sh --token my-secret-token +``` + +## Usage + +```text +./scripts/scimverify/run.sh [--base-url URL] [--token TOKEN] + +Options: + --base-url URL SCIM endpoint base URL (default: http://localhost:3000/scim/v2) + --token TOKEN Bearer token matching CODER_SCIM_AUTH_HEADER (required) + --help Show help + +Environment variables (alternatives to flags): + SCIM_BASE_URL Same as --base-url + SCIM_AUTH_TOKEN Same as --token +``` + +## Reading the output + +The tool outputs [TAP](https://testanything.org/) (Test Anything Protocol) format: + +```text +TAP version 13 +# Subtest: ResourceTypes + ok 1 - Retrieves resource types <-- individual test +ok 1 - ResourceTypes <-- suite result +# Subtest: Schemas + ok 1 - Retrieves schemas +ok 2 - Schemas +# Subtest: Basic tests + ok 1 - Base URL should not contain any query parameters + ok 2 - Base URL should be reachable + ok 3 - Authentication should be required for /Users +ok 3 - Basic tests +# Subtest: Users + ok 1 - userSchema contains attribute userName ... + ok 2 - Retrieves a list of users + not ok 3 - Filters users by userName <-- FAILURE (details follow) + --- + error: |- + User should have matching userName + 'member' !== 'admin' + expected: 'admin' + actual: 'member' + ... + ok 4 - Creates a new user - Alternative 1 +ok 4 - Users +# tests 16 <-- summary +# pass 14 +# fail 1 +# skipped 1 +``` + +Key patterns: + +- `ok N - description` means the test passed +- `not ok N - description` means the test failed (error details follow in YAML block) +- `ok N - description # SKIP reason` means the test was skipped +- Suite-level `not ok` means at least one subtest in the suite failed +- The summary at the bottom shows total pass/fail/skip counts + +### Quick filter for results only + +```bash +# Just pass/fail lines +./scripts/scimverify/run.sh --token TOKEN 2>&1 | grep -E " ok| not ok" + +# Summary only +./scripts/scimverify/run.sh --token TOKEN 2>&1 | grep "^# " +``` + +## HAR file + +Each run writes an [HAR file](https://en.wikipedia.org/wiki/HAR_(file_format)) +to `scripts/scimverify/output/output.har` containing every HTTP request and +response. This is useful for debugging failures: + +```bash +# Show all requests with errors +python3 -c " +import json +with open('scripts/scimverify/output/output.har') as f: + har = json.load(f) +for entry in har['log']['entries']: + req = entry['request'] + resp = entry['response'] + if resp['status'] >= 400: + body = resp.get('content', {}).get('text', '') + print(f'{req[\"method\"]} {req[\"url\"]} -> {resp[\"status\"]}') + if body: print(f' {body[:200]}') + print() +" +``` + +The `output/` directory is gitignored. + +## What the tests cover + +The test suite is configured via `config.yaml`. Currently it tests: + +| Category | Tests | What it checks | +|-------------------|-------|---------------------------------------------------------------------------------| +| **ResourceTypes** | 1 | `GET /ResourceTypes` returns 200 with valid resource type definitions | +| **Schemas** | 1 | `GET /Schemas` returns 200 with valid schema definitions | +| **Basic** | 3 | URL format, reachability, and authentication enforcement | +| **Users: Read** | 6 | List users, get single user, non-existent user returns 404, attribute filtering | +| **Users: Create** | 5 | Standard, minimal body, `active: false` initial state, and multi-email payloads | +| **Users: PUT** | 2 | Rename via PUT, then suspend via PUT | +| **Users: PATCH** | 3 | Suspend with path, reactivate via path-less op, suspend again | +| **Users: DELETE** | 1 | Delete suspends the user (Coder does not hard-delete) | + +After the scimverify suite finishes, `run.sh` does one extra curl-based check +that the suite itself does not cover: re-`POST`ing the deleted user should +reactivate the existing row (same ID) instead of creating a duplicate or +returning 409 Conflict. This exercises Coder's suspend-equals-delete semantics. + +### PUT, PATCH, and DELETE + +The `run.sh` script works around scimverify's `id: AUTO` limitation (which +resolves to the first user, typically the admin) by pre-creating a sacrificial +test user via `POST /Users`, capturing its UUID, and generating a temporary +config with the real ID hardcoded for PUT, PATCH, and DELETE tests. The test +user has a random name like `scimverify-a1b2c3d4` and is deleted by the +DELETE test at the end of the run. + +## Configuration + +Edit `config.yaml` to change what gets tested. Key settings: + +```yaml +detectSchema: true # Test /Schemas endpoint +detectResourceTypes: true # Test /ResourceTypes endpoint +verifyPagination: false # Skip pagination tests +verifySorting: false # Skip sorting tests +requireAuthentication: true # Test that unauthenticated requests are rejected + +users: + enabled: true + operations: [GET, POST, PUT, PATCH, DELETE] # Which HTTP methods to test + post_tests: # User creation payloads + - request: { ... } + put_tests: [] # Empty = skip (see note above about id: AUTO) + patch_tests: [] # Empty = skip + delete_tests: [] # Empty = skip + +groups: + enabled: false # Coder does not support SCIM groups +``` + +### Adding test users + +To add more POST test cases, append to `post_tests`. Each entry needs a valid +SCIM User payload. Important Coder-specific notes: + +- `userName` must be a valid Coder username (lowercase, no `@`, no spaces). + If invalid, Coder normalizes it (e.g., `user@example.com` becomes `user`). +- `emails` must include at least one entry with `"primary": true`. +- `active` defaults to `true` if omitted. + +## Expected results + +On a correctly functioning Coder instance with the SCIM 2.0 refactor: + +```text +# tests 22 +# pass 22 +# fail 0 +# skipped 0 +``` + +The `run.sh` recreate-after-delete check then prints +`ok - POST after DELETE reactivated existing user ()`. diff --git a/scripts/scimverify/config.yaml b/scripts/scimverify/config.yaml new file mode 100644 index 0000000000000..c90d573aaa10a --- /dev/null +++ b/scripts/scimverify/config.yaml @@ -0,0 +1,78 @@ +# The elimity-com/scim library auto-serves /Schemas and /ResourceTypes +# from the schema definitions, so we can detect them. +detectSchema: true +detectResourceTypes: true +verifyPagination: false +verifySorting: false +requireAuthentication: true + +users: + enabled: true + operations: + - GET + - POST + - PUT + - PATCH + - DELETE + + post_tests: + - request: + { + "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], + "userName": "scimtest1", + "name": { "givenName": "SCIM", "familyName": "TestOne" }, + "emails": [{ "value": "scimtest1@example.com", "primary": true }], + "active": true, + } + - request: + { + "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], + "userName": "scimtest2", + "name": { "givenName": "SCIM", "familyName": "TestTwo" }, + "emails": [{ "value": "scimtest2@example.com", "primary": true }], + "active": true, + } + # Minimal body: only userName + primary email; "active" defaults to true. + - request: + { + "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], + "userName": "scimtest3", + "emails": [{ "value": "scimtest3@example.com", "primary": true }], + } + # Initial state "active": false. Coder creates the user as dormant and the + # SCIM response reports active=false. + - request: + { + "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], + "userName": "scimtest4", + "name": { "givenName": "SCIM", "familyName": "TestFour" }, + "emails": [{ "value": "scimtest4@example.com", "primary": true }], + "active": false, + } + # Multiple emails; the primary one (second entry) is what Coder picks. + - request: + { + "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], + "userName": "scimtest5", + "name": { "givenName": "SCIM", "familyName": "TestFive" }, + "emails": + [ + { "value": "scimtest5-alt@example.com" }, + { "value": "scimtest5@example.com", "primary": true }, + ], + "active": true, + } + + # PUT/PATCH/DELETE tests use id: AUTO which resolves to the first user + # from GET /Users?count=1. This is typically the admin user, not a test + # user. Suspending or deleting the admin breaks the dev server. + # + # These operations are tested safely in the Go integration tests + # (enterprise/coderd/scim_test.go) where user IDs are controlled. + # The scimverify config only tests operations safe against a live server. + put_tests: [] + patch_tests: [] + delete_tests: [] + +groups: + enabled: false diff --git a/scripts/scimverify/run.sh b/scripts/scimverify/run.sh new file mode 100755 index 0000000000000..518e53dae2ae5 --- /dev/null +++ b/scripts/scimverify/run.sh @@ -0,0 +1,275 @@ +#!/usr/bin/env bash + +# Run the SCIM Verify compliance test suite against a running Coder instance. +# +# This script creates a temporary test user, generates a config with the real +# user ID (avoiding scimverify's "AUTO" which targets the first user, typically +# the admin), runs the full test suite, and cleans up. +# +# Usage: +# ./scripts/scimverify/run.sh [--base-url URL] [--token TOKEN] +# +# Environment variables: +# SCIM_BASE_URL Base URL of the SCIM endpoint (default: http://localhost:3000/scim/v2) +# SCIM_AUTH_TOKEN Bearer token for SCIM authentication (required) +# +# Prerequisites: +# - Node.js / npx available on PATH +# - A running Coder instance with SCIM enabled (CODER_SCIM_AUTH_HEADER set) +# +# The tool outputs TAP-format test results to stdout and writes a HAR file +# capturing all request/response pairs for debugging. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +BASE_CONFIG="${SCRIPT_DIR}/config.yaml" +OUTPUT_DIR="${SCRIPT_DIR}/output" + +# Defaults +BASE_URL="${SCIM_BASE_URL:-http://localhost:3000/scim/v2}" +AUTH_TOKEN="${SCIM_AUTH_TOKEN:-}" + +# Parse CLI args (override env vars) +while [[ $# -gt 0 ]]; do + case "$1" in + --base-url) + BASE_URL="$2" + shift 2 + ;; + --token) + AUTH_TOKEN="$2" + shift 2 + ;; + --help | -h) + echo "Usage: $0 [--base-url URL] [--token TOKEN]" + echo "" + echo "Environment variables:" + echo " SCIM_BASE_URL Base URL (default: http://localhost:3000/scim/v2)" + echo " SCIM_AUTH_TOKEN Bearer token (required)" + exit 0 + ;; + *) + echo "Unknown option: $1" >&2 + exit 1 + ;; + esac +done + +if [[ -z "${AUTH_TOKEN}" ]]; then + echo "Error: SCIM auth token is required." >&2 + echo "Set SCIM_AUTH_TOKEN or pass --token TOKEN." >&2 + exit 1 +fi + +if ! command -v npx &>/dev/null; then + echo "Error: npx is not installed. Install Node.js to continue." >&2 + exit 1 +fi + +mkdir -p "${OUTPUT_DIR}" + +AUTH_HEADER="Authorization: Bearer ${AUTH_TOKEN}" + +# --- Step 1: Create a sacrificial test user for PUT/PATCH/DELETE tests --- +# scimverify's "id: AUTO" resolves to the first user from GET /Users?count=1, +# which is usually the admin. Instead, we pre-create a test user and hardcode +# its UUID into a temporary config. + +RAND_SUFFIX="$(head -c 4 /dev/urandom | od -An -tx1 | tr -d ' \n')" +TEST_USERNAME="scimverify-${RAND_SUFFIX}" +TEST_EMAIL="${TEST_USERNAME}@scimverify.test" + +echo "=== SCIM Verify ===" +echo "Base URL: ${BASE_URL}" +echo "Test user: ${TEST_USERNAME}" +echo "HAR out: ${OUTPUT_DIR}/output.har" +echo "" + +echo "Creating test user for PUT/PATCH/DELETE..." +CREATE_RESPONSE=$(curl -s -X POST \ + -H "${AUTH_HEADER}" \ + -H "Content-Type: application/scim+json" \ + "${BASE_URL}/Users" \ + -d "{ + \"schemas\": [\"urn:ietf:params:scim:schemas:core:2.0:User\"], + \"userName\": \"${TEST_USERNAME}\", + \"name\": {\"givenName\": \"Verify\", \"familyName\": \"Test\"}, + \"emails\": [{\"value\": \"${TEST_EMAIL}\", \"primary\": true}], + \"active\": true + }") + +TEST_USER_ID=$(echo "${CREATE_RESPONSE}" | python3 -c "import sys,json; print(json.load(sys.stdin).get('id',''))" 2>/dev/null || true) + +if [[ -z "${TEST_USER_ID}" ]]; then + echo "Warning: Failed to create test user. PUT/PATCH/DELETE tests will use AUTO (may target admin)." >&2 + echo "Response: ${CREATE_RESPONSE}" >&2 + echo "" + # Fall back to the base config (empty PUT/PATCH/DELETE) + CONFIG_FILE="${BASE_CONFIG}" +else + echo "Created test user: ${TEST_USERNAME} (${TEST_USER_ID})" + echo "" + + # --- Step 2: Generate a temporary config with the real user ID --- + CONFIG_FILE=$(mktemp "${OUTPUT_DIR}/config-XXXXXX.yaml") + trap 'rm -f "${CONFIG_FILE}"' EXIT + + cat >"${CONFIG_FILE}" </dev/null || true) + + if [[ "${RECREATE_STATUS}" == "201" || "${RECREATE_STATUS}" == "200" ]] && [[ "${RECREATE_ID}" == "${TEST_USER_ID}" ]]; then + echo "ok - POST after DELETE reactivated existing user (${TEST_USER_ID})" + else + echo "not ok - POST after DELETE should reactivate, got status=${RECREATE_STATUS} id=${RECREATE_ID} (expected ${TEST_USER_ID})" >&2 + echo " body: ${RECREATE_BODY}" >&2 + exit 1 + fi +fi diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index af46758f9fd89..280465a18a41d 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -4083,6 +4083,7 @@ export interface DeploymentValues { readonly agent_fallback_troubleshooting_url?: string; readonly browser_only?: boolean; readonly scim_api_key?: string; + readonly scim_use_legacy?: boolean; readonly external_token_encryption_keys?: string; readonly provisioner?: ProvisionerConfig; readonly rate_limit?: RateLimitConfig;