feat: add optional SCIM provisioning support for Workbench

[timtalbot]

Description

Adds opt-in SCIM user provisioning for Workbench, allowing an external IdP (e.g. Okta, Entra ID) to manage users and groups via the Workbench SCIM v2 API without requiring first-login.

API — new scim block on Site.spec.workbench:

spec:
  workbench:
    scim:
      enabled: true
      tokenSecretName: my-secret  # optional — omit to auto-generate

Operator behaviour when scim.enabled: true:

• Generates a cryptographically random bearer token (32 bytes, base64url) and stores it in a managed Secret named <site-name>-workbench-scim-token in posit-team. The token is never rotated automatically; deleting the Secret triggers regeneration on the next reconcile.
• Alternatively, point tokenSecretName at a pre-existing Secret with a token key (BYO mode). The operator uses it as-is and does not manage its lifecycle.
• Mounts the token into the Workbench pod at /etc/rstudio/scim-token via a Secret volume.
• Sets WORKBENCH_USER_SERVICE_AUTH_TOKEN_PATH=/etc/rstudio/scim-token on the Workbench container so Workbench can locate the token at startup.

Retrieve the token to configure your IdP:

kubectl get secret <site-name>-workbench-scim-token \
  -n posit-team -o jsonpath='{.data.token}' | base64 -d

Configure your IdP SCIM endpoint as https://<workbench-hostname>/scim/v2 with Bearer token auth.

Code Flow

File	Change
api/core/v1beta1/site_types.go	WorkbenchSCIMConfig struct; SCIM field on InternalWorkbenchSpec
api/core/v1beta1/workbench_types.go	SCIM field on WorkbenchSpec
internal/controller/core/site_controller_workbench.go	Propagate site.Spec.Workbench.SCIM → targetWorkbench.Spec.SCIM
internal/controller/core/workbench.go	reconcileSCIMToken, buildSCIMTokenEnvVars, buildSCIMTokenVolumeMounts, buildSCIMTokenVolumes
internal/controller/core/workbench_test.go	4 new tests covering disabled, managed token, BYO token, and no-rotation cases
config/crd/bases/, client-go/, zz_generated.deepcopy.go	Regenerated

Category of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Build: a code change that affects the build system or external dependencies
• Performance: a code change that improves performance
• Refactor: a code change that neither fixes a bug nor adds a feature
• Documentation: documentation changes
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

• I have run just test and all tests pass
• I have reviewed my own diff and added inline comments on lines I want reviewers to focus on or that I am uncertain about

[claude]

Claude finished @timtalbot's task —— View job

---

Review: SCIM Provisioning for Workbench

• Read review guidelines
• Analyze PR diff and changed files
• Create and submit review

Well-structured feature with clear separation between managed and BYO modes. The reconciler logic is idempotent and the test coverage is solid. A few items to consider:

---

Security — BYO secret key validation (workbench.go:1362-1370)

The BYO secret is fetched but never validated for the token key. If a user provides a secret without a token key, the volume mount will succeed but produce an empty/missing file. Consider validating:

if _, ok := byoSecret.Data["token"]; !ok {
    l.Info("WARNING: BYO SCIM token secret exists but is missing the 'token' key", "secretName", w.Spec.SCIM.TokenSecretName)
}

Race condition on managed secret creation (workbench.go:1408-1410)

If two reconcile loops fire concurrently, both could pass the IsNotFound check and attempt Create. Handle IsAlreadyExists on the create call (same pattern as package_manager.go:265):

if err := r.Create(ctx, secret); err != nil {
    if kerrors.IsAlreadyExists(err) {
        l.Info("SCIM token secret was created concurrently, reusing", "secretName", managedSecretName)
        return managedSecretName, nil
    }
    return "", fmt.Errorf("error creating SCIM token secret %q: %w", managedSecretName, err)
}

One-way SCIM propagation (site_controller_workbench.go:458-461)

The Site→Workbench propagation only copies when SCIM != nil. If a user removes the scim block from the Site spec, the Workbench retains its previous config. For a security-sensitive feature, consider explicitly clearing it:

// Propagate SCIM config
targetWorkbench.Spec.SCIM = site.Spec.Workbench.SCIM

This makes the nil case propagate correctly as well.

API validation (site_types.go:503)

TokenSecretName has no +kubebuilder:validation annotation. Consider adding a pattern for valid Kubernetes names (^[a-z0-9]([-a-z0-9]*[a-z0-9])?$) to give users an early admission error rather than a reconcile-time failure.

Guard clause repetition (workbench.go:1348,1419,1432,1447)

The w.Spec.SCIM == nil || !w.Spec.SCIM.Enabled check appears 4 times with slightly different variants. A small helper like scimEnabled(w) or a method on the Workbench type would reduce repetition and guard divergence.

SubPath mount note (workbench.go:1439)

SubPath mounts are not auto-updated by kubelet. This is fine for the current "no auto-rotation" design, but worth noting for anyone who later considers rotation — they'd need to drop the SubPath.

Test coverage gaps (workbench_test.go)

The four tests cover the main cases well. Two gaps to consider:

• Disable-after-enable: Enable SCIM, reconcile, then set Enabled = false and reconcile again. Verify mounts/envs are removed from the deployment spec.
• Invalid BYO secret: If key validation is added, test a secret missing the token key.

Self-review

Per repo norms — no inline comments from the author on this PR. Consider annotating key decision points (e.g. the SubPath choice, the no-rotation design) to help reviewers focus.

---

[timtalbot]

@claude re-review

[claude]

Claude finished @timtalbot's task —— View job

---

Re-review: SCIM Provisioning for Workbench

• Read current state of changed files
• Compare against previous review feedback
• Submit updated review

All five actionable items from the first review have been addressed:

Previous Feedback	Status
BYO secret key validation	Fixed — returns hard error at workbench.go:1365-1367
Race condition on managed secret creation	Fixed — IsAlreadyExists handled at workbench.go:1407-1410
One-way SCIM propagation	Fixed — unconditional assignment at site_controller_workbench.go:459
API validation for TokenSecretName	Fixed — +kubebuilder:validation:Pattern added at site_types.go:503
Guard clause repetition	Fixed — extracted scimEnabled() helper at workbench.go:1418-1420

Test coverage has also been expanded (6 tests, up from 4):

• TestWorkbenchSCIM_DisableAfterEnable — verifies mounts/envs are removed after disabling
• TestWorkbenchSCIM_BYOTokenMissingKey — verifies hard error on missing token key

---

One remaining observation (non-blocking):

Managed secret cleanup on disable (workbench.go:1348): When SCIM is disabled after being enabled, the managed secret persists as an orphan. The OwnerReferences will GC it when the Workbench is deleted, but if SCIM is toggled off while the Workbench lives on, the secret remains. This is fine for now — the no-rotation design means leftover secrets are harmless — but worth documenting if you plan to add rotation later.

Otherwise, this looks good to merge.

---

View job