Skip to content

feat: add PPM OIDC/SSO web UI configuration#112

Open
ian-flores wants to merge 16 commits intomainfrom
ppm-auth-repos
Open

feat: add PPM OIDC/SSO web UI configuration#112
ian-flores wants to merge 16 commits intomainfrom
ppm-auth-repos

Conversation

@ian-flores
Copy link
Collaborator

@ian-flores ian-flores commented Mar 2, 2026

Description

Enable OIDC authentication for the Package Manager web UI. Adds first-class OIDC configuration types and full typed coverage of all PPM authentication config fields from the PPM configuration reference.

When auth.type: oidc is set on the Site's packageManager spec, the operator automatically configures the [OpenIDConnect] and [Authentication] gcfg sections and mounts the OIDC client secret from the vault.

Issue

  • PTDC-224: Configure SSO (OIDC) for Package Manager web UI

Follow-up

Connect/Workbench authenticated repos via Identity Federation is in #114.

Code Flow

Key Components

  1. PPM Authentication Config Types (package_manager_config.go): New PackageManagerAuthenticationConfig and PackageManagerOIDCConfig structs with full field coverage from the PPM configuration reference. Extended GenerateGcfg() to emit [Authentication] and [OpenIDConnect] sections.

  2. OIDC Client Secret Mount (packagemanager_types.go): Mounts the OIDC client secret at /etc/rstudio-pm/oidc-client-secret via SecretProviderClass (AWS) or K8s Secret volume (K8s).

  3. Site Controller Propagation (site_controller_package_manager.go): Propagates OIDC config from Site spec to PackageManager, setting ClientSecretFile, Issuer, ClientId, RequireLogin, and Scope.

  4. SecretProviderClass (package_manager.go): Adds OIDC client secret to the SecretProviderClass when OIDCClientSecretKey is configured.

SiteSpec Example

spec:
  packageManager:
    auth:
      type: oidc
      clientId: "ppm-client-id"
      issuer: "https://login.example.com"
    oidcClientSecretKey: "ppm-oidc-client-secret"

Testing

Tested end-to-end on ganso01-staging (main site) with adhoc image adhoc-ppm-auth-repos-v1.16.1-15-gedc6aaa.

Setup

  • Keycloak: Created posit realm with ppm client (confidential, authorization code grant type)
  • OIDC client secret: Stored in AWS Secrets Manager, mounted to PPM at /etc/rstudio-pm/oidc-client-secret

Results

Test Status
PPM gcfg renders [OpenIDConnect] section with ClientSecretFile
PPM gcfg renders [Authentication] section
PPM OIDC web login redirects to Keycloak (/__login__ → 302 to Keycloak auth endpoint)
OIDC client secret mounted at /etc/rstudio-pm/oidc-client-secret

Note on repo authentication

PPM's OIDC redirect only activates when repos are configured as "Authenticated". The [OpenIDConnect] section enables the OIDC provider, but individual repos must be marked as authenticated (via rspm edit repo --name=<repo> --authenticated or creating repos with --authenticated flag). This is expected PPM behavior — unauthenticated repos remain accessible without login.

Category of change

  • New feature (non-breaking change which adds functionality)

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

Enable Connect and Workbench to authenticate against PPM using
Kubernetes Identity Federation (RFC 8693 token exchange). Adds
OIDC and Identity Federation config types for PPM, shared token
exchange init container and sidecar helpers, and opt-in
AuthenticatedRepos flag on Connect/Workbench specs.
All 8 findings addressed. Build passes and tests pass. Here's the summary:
Changes:
- Install curl and jq in token exchange script (`apk add --no-cache`) so alpine:3 default image works
- Add null/empty token validation after jq extraction to fail fast instead of writing "null" as password
- Add `OIDCAudience` field to `SiteSpec` so OIDC audience is configurable (defaults to `sts.amazonaws.com` for backwards compatibility)
- Revert `AutomountServiceAccountToken` to `ptr.To(false)` in Connect — projected volume works independently
- Add `PPMAuthImage` to `InternalConnectSpec` and `InternalWorkbenchSpec` and propagate from Site controllers
- Add gcfg injection validation for `IdentityFederation` Name (reject `"`, `]`, newlines)
- Add `cleanupPPMAuthConfigMap` to delete the ConfigMap when authenticated repos feature is disabled
- Add `SanitizePPMUrl` helper to strip existing scheme before prepending `https://`, preventing double-prefix
@claude
Copy link

claude bot commented Mar 2, 2026

Claude finished @ian-flores's task —— View job


Review: feat: add PPM authenticated repos via Identity Federation

  • Read review guidelines
  • Analyze PR diff and changed files
  • Review API types and CRD changes
  • Review controller logic
  • Review PPM config generation
  • Review tests
  • Submit review

Overall

Well-structured feature. The separation of concerns (shared helpers in ppm_auth.go, product integration in connect.go/workbench.go, site-level orchestration in site_controller*.go) follows existing patterns. The token exchange flow via init container + sidecar is a solid approach.

Below are findings organized by severity.


Critical: Audience mismatch between projected SA token and Identity Federation config

internal/controller/core/ppm_auth.go:170 — The projected SA token uses audience = ppmURL (e.g., https://ppm.example.com).

internal/controller/core/site_controller_package_manager.go:139-141 — The IdentityFederation entry uses site.Spec.OIDCAudience (defaulting to "sts.amazonaws.com").

These must match for the token exchange to succeed. If the PPM-side IdentityFederation.Audience is "sts.amazonaws.com" but the projected token's aud claim is "https://ppm.example.com", PPM will reject the token. Either:

  • Pass the OIDC audience through to PPMAuthVolumes() instead of using the PPM URL, or
  • Document explicitly why these are intentionally different values and explain how the PPM token endpoint resolves this.

Important: Shell script installs packages at runtime

internal/controller/core/ppm_auth.go:36apk add --no-cache curl jq runs on every container start.

  1. Air-gapped environments: This will always fail if the Alpine mirror is unreachable.
  2. Supply chain risk: Pulls unsigned packages at runtime on each pod start.
  3. Startup latency: Adds network dependency to pod initialization.

Consider using a Posit-owned image with curl and jq pre-installed as the default. The PPMAuthImage override field is already there, so this is mainly about a better default. At minimum, document this requirement.


Important: Sidecar exits on transient token refresh failure

internal/controller/core/ppm_auth.go:74-79 — The script uses set -e, so if exchange_token fails during a sidecar refresh cycle (e.g., PPM temporarily unavailable), the entire sidecar dies. The token will expire and auth will break with no recovery.

Consider:

if [ "${MODE}" = "sidecar" ]; then
    while true; do
        sleep "$REFRESH_INTERVAL"
        exchange_token || echo "WARNING: token refresh failed, will retry" >&2
    done
fi

Important: Connect sets AutomountServiceAccountToken: false

internal/controller/core/connect.go:653 — Connect explicitly sets AutomountServiceAccountToken: ptr.To(false). The PPM auth flow uses a projected SA token volume (not the default automounted one), so this should still work. However, if the projected volume projection depends on the SA token being available at the kubelet level, verify this works correctly in testing. Workbench sets it to true (line 815), so there's no issue there.


Important: PPM URL computed without https:// prefix in site controller

internal/controller/core/site_controller_connect.go:159 and site_controller_workbench.go:261PPMUrl is set to prefixDomain(...) which returns a bare domain like ppm.example.com (no scheme). This gets passed to SanitizePPMUrl() in the product reconcilers which prepends https://. This works but is fragile — the PPM URL meaning shifts between layers. Consider adding the scheme at the site controller level for clarity.


Suggestion: Remove committed .claude/tsc-cache/ files

.claude/tsc-cache/3a39c930-cf12-4be4-a2f2-eae1467b53c6/ — These are local roborev tool artifacts (the edited-files.log contains macOS temp paths). They should be removed from this PR and .claude/tsc-cache/ added to .gitignore.


Suggestion: Unused constant

internal/controller/core/ppm_auth.go:20ppmAuthCurlrcPath is defined but never referenced in Go code (the shell script uses its own CURLRC_PATH variable). Remove to avoid confusion.


Suggestion: Default audience assumption

internal/controller/core/site_controller_package_manager.go:141 — Defaulting audience to "sts.amazonaws.com" is AWS-specific. The field comment mentions AKS, but the default is EKS-only. Consider requiring this field when AuthenticatedRepos is enabled instead of silently defaulting to an AWS value.


API Checklist

  • Kubebuilder annotations present (+optional)
  • New fields have sensible defaults (booleans default to false, strings to empty)
  • No validation rules for PPMUrl or OIDCIssuerURL — consider adding URL format validation
  • Non-breaking (additive only)

Controller Checklist

  • Reconciliation is idempotent (ConfigMap uses CreateOrUpdate, cleanup handles NotFound)
  • Error handling reports status
  • Config flows from Site → Product correctly
  • Unit tests for ppm_auth.go helpers and package_manager_config.go
  • No integration/controller tests for the end-to-end auth flow (ConfigMap creation, product reconciliation with AuthenticatedRepos=true)

Helm Chart

  • CRDs are up to date (both config/crd and dist/chart match)

Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review posted via comment due to tool permissions. See the Claude comment on this PR for detailed feedback.

- Fix audience mismatch: thread OIDCAudience from site spec through
  Connect/Workbench to projected SA token volumes
- Remove runtime apk add from token exchange script (image must
  have curl+jq pre-installed)
- Add sidecar resilience: catch refresh failures instead of dying
- Remove unused ppmAuthCurlrcPath constant
- Clarify AutomountServiceAccountToken comment re: projected volumes
- Add .claude/tsc-cache to .gitignore
Add typed support for all PPM authentication configuration fields:
- New [Authentication] section (APITokenAuth, DeviceAuthType, session
  lifetime, etc.)
- 12 new [OpenIDConnect] fields (ClientSecretFile, PKCE, claims
  customization, token lifetime, etc.)
- 9 new [IdentityFederation] fields (claims, groups, roles, logging)
alpine:3 runs as root by default, which fails pod security contexts
that require runAsNonRoot. Set RunAsUser=65534 (nobody) on both init
and sidecar containers.

PPM requires Scope, RoleClaim, or GroupToScopeMapping when OIDC is
configured. Default to "repos:read:*" in the site controller.
alpine:3 doesn't include curl or jq, and apk install requires root.
Replace curl with wget (BusyBox built-in) and jq with a sed-based
JSON field extractor for zero-dependency operation.
The field was tagged json:"-" which prevented it from being stored
in the PackageManager CR. The site controller sets it in memory but
it was lost when the PM controller read the CR back from the API.
The Name field was tagged json:"-" so it was lost during Kubernetes
API serialization. This caused empty section names in the generated
gcfg: [IdentityFederation ""] instead of [IdentityFederation "connect"].
PPM treats ClientSecret as a literal value, not a file path. Use
ClientSecretFile instead so PPM reads the secret from the mounted file.

K8s SA tokens don't have a preferred_username claim. Set UniqueIdClaim
and UsernameClaim to "sub" for Identity Federation entries.
All builds pass and tests are green. Findings #1 (PPMAuthImage propagation), #2 (cache files), and #3 (apk add stderr) were already addressed in prior commits. I fixed the two remaining issues.
Changes:
- `SanitizePPMUrl` now returns empty string for empty input instead of invalid `"https://"`
- Added edge case tests for `SanitizePPMUrl`: empty string and URL with port/path
Comment on lines +27 to +30
// PPMAuthTokenExchangeScript returns the shell script content for the token exchange
// init container and sidecar. The script exchanges a K8s service account token for a
// PPM API token via RFC 8693 token exchange, then writes a netrc file and a curlrc
// file (so R's libcurl can also authenticate via --netrc-file).
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern is where I'd like more input on the review

@ian-flores ian-flores marked this pull request as ready for review March 5, 2026 14:46
@ian-flores ian-flores requested a review from stevenolen March 5, 2026 14:46
@stevenolen
Copy link
Collaborator

This is a pretty big PR. Looking at the parts that relate to PTDC-224, that looks pretty standard/good. Is it possible to put a separate PR for that piece in first and then follow up with some discussion on connect+workbench integration to focus our discussion on the pros/cons of that part?

Remove Connect/Workbench authenticated repos (Identity Federation)
code to a separate PR per review feedback. This PR now contains
only the PPM OIDC/SSO web UI configuration (PTDC-224).
@ian-flores ian-flores changed the title feat: add PPM authenticated repos via Identity Federation feat: add PPM OIDC/SSO web UI configuration Mar 5, 2026
@ian-flores
Copy link
Collaborator Author

Done! I've split this PR to only contain the OIDC/SSO web UI config (PTDC-224). The Connect/Workbench authenticated repos piece is now in #114 as a separate draft PR — happy to discuss the approach there.

@stevenolen
Copy link
Collaborator

This is a pretty interesting idea overall. Do you have any suggestions or paths for RStudio or Positron desktop user support with oidc+package manager (and do you think the magic-ness of relegates those users in desktop environments?

I'd also be super curious for some input from @jstruzik + team on the use of this pattern for integrating ppm oidc with workbench and connect. is this a direction that feels natural and worth of documentation outside of team-operator-land? do you have a preferred means of integration on the way that we should maybe make use of instead?

@ian-flores
Copy link
Collaborator Author

Do you have any suggestions or paths for RStudio or Positron desktop user support with oidc+package manager (and do you think the magic-ness of relegates those users in desktop environments?

Team Operator users won't have that problem though...

I'd also be super curious for some input from @jstruzik + team on the use of this pattern for integrating ppm oidc with workbench and connect. is this a direction that feels natural and worth of documentation outside of team-operator-land? do you have a preferred means of integration on the way that we should maybe make use of instead?

I think this is for PR #114?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants