Add vice-users OAuth callback relay to vice-operator by johnworth · Pull Request #144 · cyverse-de/app-exposer

johnworth · 2026-05-14T16:57:48Z

Summary

Adds a fixed, unauthenticated GET /auth/callback endpoint to the vice-operator. This becomes the single static redirect_uri registered in Keycloak for the vice-users client, replacing the non-functional https://*.cyverse.run:4343/* wildcard (Keycloak only honors * as a trailing wildcard, so per-app VICE subdomains never matched — the "invalid parameter: redirect_uri" error in QA).
The handler is a stateless relay: it verifies the HMAC-signed state (github.com/cyverse-de/go-mod/viceauth), recovers the original app URL, validates that URL is a single-label subdomain of the VICE base domain (open-redirect guard), and bounces the browser back with the authorization code intact. It does no token exchange — vice-proxy holds the client secret and redeems the code itself.
New --public-url and --state-hmac-secret flags feed two new cluster-config Secret keys (OPERATOR_CALLBACK_URL, STATE_HMAC_SECRET) that vice-proxy consumes via EnvFrom. No changes needed in incluster/ — the Secret is forwarded wholesale.

Notes

--state-hmac-secret must be stable across operator restarts (treat like --keycloak-client-secret). A regenerated secret would diverge from the value baked into already-running vice-proxy pods until they are recreated.
The relay route is registered only when a state HMAC secret is configured.

Dependencies

Depends on Add viceauth module for signed OAuth state go-mod#11 (the viceauth module) — pinned to a pseudo-version of that branch; bump to viceauth/v1.0.0 once it merges.
Companion PR Route OAuth callback through the vice-operator relay vice-proxy#17 reshapes the vice-proxy OAuth handlers to use the relay.

Test plan

go build ./..., go vet ./..., gofmt
go test ./cmd/vice-operator/... — table-driven tests for isAllowedHost and handleViceUsersCallback (valid bounce, ?error=, missing code/state, bad signature, off-domain / nested-subdomain / non-https origin)
Register the QA operator's /auth/callback URL in Keycloak
End-to-end in QA: launch a VICE app, confirm login bounces operator → app subdomain and the session is established

🤖 Generated with Claude Code

vice-proxy can no longer send per-app VICE subdomains as the Keycloak redirect_uri — Keycloak only wildcard-matches a trailing *, so https://*.cyverse.run:4343/* never matches a real app. The operator now exposes a fixed /vice-users/callback endpoint that is registered in Keycloak as the single static redirect_uri for the vice-users client. The handler is a stateless relay: it verifies the HMAC-signed state (go-mod/viceauth), recovers the original app URL from it, checks that URL is a single-label subdomain of the VICE base domain (open-redirect guard), and bounces the browser back with the authorization code intact. vice-proxy holds the client secret and does the token exchange itself. New --public-url and --state-hmac-secret flags feed two new cluster-config keys (OPERATOR_CALLBACK_URL, STATE_HMAC_SECRET) consumed by vice-proxy via EnvFrom. The state HMAC secret must be stable across operator restarts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

socket-security · 2026-05-14T16:58:24Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	github.com/cyverse-de/go-mod/viceauth@v1.0.0

View full report

johnworth · 2026-05-14T17:21:33Z

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

- Build operatorCallbackURL with url.URL.JoinPath() instead of string concatenation, per the project URL-construction guideline; fail fast on an unparseable --public-url. Drops the now-unused strings import. - Fix the startup warning: the relay is not "disabled" when only --public-url is empty (it is gated on the state HMAC secret alone) — the accurate consequence is that vice-proxy pods fail to start without both values. - Harden the relay redirect target: reject origin URLs carrying userinfo (a redirect-spoofing vector) and strip any fragment before relaying. Adds a userinfo test case. Addresses code review feedback on PR #144. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

go-mod#11 merged and viceauth/v1.0.0 is tagged; move off the auth-relay pseudo-version to the released tag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

slr71

This looks reasonable to me. 👍

johnworth · 2026-05-14T17:59:52Z

-func NewApp(op *operator.Operator, verifier *oidc.IDTokenVerifier, expectedClientID string, swaggerCfg *SwaggerAuthConfig, adminRole string, adminEntitlements []string) *App {
+// without authentication. When viceUsersCfg is non-nil, the unauthenticated
+// vice-users OAuth callback relay is registered.
+func NewApp(op *operator.Operator, verifier *oidc.IDTokenVerifier, expectedClientID string, swaggerCfg *SwaggerAuthConfig, adminRole string, adminEntitlements []string, viceUsersCfg *ViceUsersAuthConfig) *App {


The number of parameters here was already large and is getting larger. Might be time to add an init struct.

johnworth · 2026-05-14T18:04:42Z

+// viceUsersCallbackPath is the fixed path of the OAuth callback relay. It is
+// combined with --public-url to form OPERATOR_CALLBACK_URL, the single static
+// redirect_uri registered in Keycloak for the vice-users client.
+const viceUsersCallbackPath = "/vice-users/callback"


Let's rename the callback path to /auth/callback.

johnworth · 2026-05-14T18:10:02Z

+// handleViceUsersCallback returns the handler for GET /vice-users/callback.
+//
+// vice-proxy registers this operator's URL as the single static redirect_uri
+// for the Keycloak "vice-users" client, because Keycloak cannot wildcard-match


Let's not reference configurable values in a comment like this. They're fine to include in examples, but the way it's worded here makes it sound like it's set in stone that we're using a vice-user client. We could rename or recreate it later, making the comment stale.

slr71 · 2026-05-14T18:15:40Z

The changes you proposed, all sound reasonable to me. 👍

- NewApp now takes an AppConfig struct instead of a 7-parameter positional list. - Rename the OAuth callback path from /vice-users/callback to /auth/callback. - Reword the relay comments so they no longer state the Keycloak client name as fixed — the client is configurable and could be renamed or recreated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

johnworth · 2026-05-14T18:20:50Z

Thanks for the review, @slr71.

slr71

These changes seem reasonable to me.

slr71 · 2026-05-14T18:26:30Z

+// redirect_uri for the OAuth client vice-proxy authenticates against —
+// Keycloak cannot wildcard-match per-app VICE subdomains, so a single fixed
+// callback is needed. Keycloak delivers the authorization code here; the


I view this endpoint as essentially a relay that allows us to use a static callback URL in the Keycloak OIDC client for each VICE operator. This endpoint uses the state parameter to determine which vice-proxy to redirect the browser to and does some sanity checks on the request to ensure that the state parameter is signed with the appropriate key and that the browser is only redirected to authorized locations.

Rewording the comment may not be necessary, but I didn't develop a thorough understanding of how this works until I read the code. I'm not sure that the original comment helped me understand it (although I must admit that I'm not entirely sure that it didn't help me understand the endpoint either). 😆

John Wregglesworth and others added 2 commits May 14, 2026 10:30

Pin viceauth to v1.0.0

f26be6e

go-mod#11 merged and viceauth/v1.0.0 is tagged; move off the auth-relay pseudo-version to the released tag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

johnworth marked this pull request as ready for review May 14, 2026 17:58

slr71 approved these changes May 14, 2026

View reviewed changes

johnworth commented May 14, 2026

View reviewed changes

johnworth merged commit 3ab3e63 into main May 14, 2026
3 checks passed

johnworth deleted the auth-relay branch May 14, 2026 18:21

slr71 reviewed May 14, 2026

View reviewed changes

Navigation Menu

Uh oh!

Add vice-users OAuth callback relay to vice-operator - #144

Add vice-users OAuth callback relay to vice-operator#144
johnworth merged 4 commits into
mainfrom
auth-relay

johnworth commented May 14, 2026 •

edited

Loading

Uh oh!

socket-security Bot commented May 14, 2026 •

edited

Loading

Uh oh!

johnworth commented May 14, 2026

Uh oh!

slr71 left a comment

Uh oh!

johnworth May 14, 2026

Uh oh!

johnworth May 14, 2026

Uh oh!

johnworth May 14, 2026

Uh oh!

slr71 commented May 14, 2026

Uh oh!

johnworth commented May 14, 2026

Uh oh!

Uh oh!

slr71 left a comment

Uh oh!

slr71 May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

johnworth commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Notes

Dependencies

Test plan

Uh oh!

socket-security Bot commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnworth commented May 14, 2026

Code review

Uh oh!

slr71 left a comment

Choose a reason for hiding this comment

Uh oh!

johnworth May 14, 2026

Choose a reason for hiding this comment

Uh oh!

johnworth May 14, 2026

Choose a reason for hiding this comment

Uh oh!

johnworth May 14, 2026

Choose a reason for hiding this comment

Uh oh!

slr71 commented May 14, 2026

Uh oh!

johnworth commented May 14, 2026

Uh oh!

Uh oh!

slr71 left a comment

Choose a reason for hiding this comment

Uh oh!

slr71 May 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

johnworth commented May 14, 2026 •

edited

Loading

socket-security Bot commented May 14, 2026 •

edited

Loading