diff --git a/auth-service/handler.go b/auth-service/handler.go index c608bd2c1..b8c14ff70 100644 --- a/auth-service/handler.go +++ b/auth-service/handler.go @@ -18,6 +18,7 @@ import ( "github.com/hmchangw/chat/pkg/errcode" "github.com/hmchangw/chat/pkg/errcode/errhttp" pkgoidc "github.com/hmchangw/chat/pkg/oidc" + "github.com/hmchangw/chat/pkg/subject" ) // TokenValidator validates an SSO token and returns OIDC claims. @@ -149,16 +150,17 @@ func (h *AuthHandler) HandleAuth(c *gin.Context) { return } - account := claims.PreferredUsername - if account == "" { - account = claims.Name - } + account := claims.Account() if account == "" { // Blank account would mint a JWT with chat.user..> permissions — refuse. errhttp.Write(ctx, c, errcode.Unauthenticated("token missing account claim", errcode.WithReason(errcode.AuthInvalidToken))) return } + if !subject.IsValidAccountToken(account) { + errhttp.Write(ctx, c, errcode.BadRequest("account must be a single NATS subject token (no '.', '*', '>' or whitespace)")) + return + } ctx = errcode.WithLogValues(ctx, "account", account) natsJWT, err := h.signNATSJWT(req.NATSPublicKey, account) @@ -204,6 +206,10 @@ func (h *AuthHandler) handleDevAuth(c *gin.Context) { return } + if !subject.IsValidAccountToken(req.Account) { + errhttp.Write(ctx, c, errcode.BadRequest("account must be a single NATS subject token (no '.', '*', '>' or whitespace)")) + return + } ctx = errcode.WithLogValues(ctx, "account", req.Account) natsJWT, err := h.signNATSJWT(req.NATSPublicKey, req.Account) diff --git a/auth-service/handler_test.go b/auth-service/handler_test.go index ce2fea0ca..2713fa27f 100644 --- a/auth-service/handler_test.go +++ b/auth-service/handler_test.go @@ -419,3 +419,100 @@ func TestSignNATSJWT_LifetimeJitter(t *testing.T) { }) } } + +func TestHandleAuth_MissingAccountClaim(t *testing.T) { + // Prod-mode guard: a token with no usable account claim must be refused + // before minting — the JWT would otherwise grant chat.user..> permissions. + handler := NewAuthHandler(&fakeValidator{}, mustAccountKP(t), 2*time.Hour, false) + router := setupRouter(t, handler) + + body := `{"ssoToken":"valid-token","natsPublicKey":"` + mustUserNKey(t) + `"}` + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/auth", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusUnauthorized, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeUnauthenticated) + errtest.AssertReason(t, w.Body.Bytes(), errcode.AuthInvalidToken) +} + +func TestHandleAuth_InvalidAccountFormat(t *testing.T) { + // The account becomes a NATS subject token (chat.user.{account}.>): dots + // nest namespaces, wildcards broaden grants — refuse before gate and sign. + signingKP := mustAccountKP(t) + cases := []struct{ name, account string }{ + {"dotted account nests subjects", "john.doe"}, + {"single-token wildcard", "mal*ory"}, + {"multi-token wildcard", "mal>ory"}, + {"whitespace", "mal ory"}, + } + for _, tt := range cases { + t.Run("prod: "+tt.name, func(t *testing.T) { + handler := NewAuthHandler(&fakeValidator{account: tt.account}, signingKP, 2*time.Hour, false) + router := setupRouter(t, handler) + + body := `{"ssoToken":"valid-token","natsPublicKey":"` + mustUserNKey(t) + `"}` + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/auth", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeBadRequest) + }) + t.Run("dev: "+tt.name, func(t *testing.T) { + handler := NewAuthHandler(nil, signingKP, 2*time.Hour, true) + router := setupRouter(t, handler) + + payload, err := json.Marshal(map[string]string{"account": tt.account, "natsPublicKey": mustUserNKey(t)}) + require.NoError(t, err) + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/auth", strings.NewReader(string(payload))) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeBadRequest) + }) + } + + // Any account the routing layer can serve must pass the mint gate too: + // the rule is exactly pkg/subject's token invariant, not an ASCII allowlist. + for _, account := range []string{"alice@corp", "júlio"} { + t.Run("routable account accepted: "+account, func(t *testing.T) { + handler := NewAuthHandler(nil, signingKP, 2*time.Hour, true) + router := setupRouter(t, handler) + + payload, err := json.Marshal(map[string]string{"account": account, "natsPublicKey": mustUserNKey(t)}) + require.NoError(t, err) + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/auth", strings.NewReader(string(payload))) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + }) + } +} + +func TestHandleAuth_TokenGenerationFailure(t *testing.T) { + // Prod-mode twin of the dev-mode test: a user key cannot sign, so the prod path returns 500. + userKP, err := nkeys.CreateUser() + require.NoError(t, err, "create user key") + + validator := &fakeValidator{account: "alice", subject: "uuid-alice"} + handler := NewAuthHandler(validator, userKP, 2*time.Hour, false) + router := setupRouter(t, handler) + + userPub := mustUserNKey(t) + body := `{"ssoToken":"valid-token","natsPublicKey":"` + userPub + `"}` + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/auth", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + + require.Equal(t, http.StatusInternalServerError, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeInternal) + assert.NotContains(t, w.Body.String(), "generating NATS token") +} diff --git a/auth-service/integration_test.go b/auth-service/integration_test.go index da30b5fc0..d1f77d665 100644 --- a/auth-service/integration_test.go +++ b/auth-service/integration_test.go @@ -16,6 +16,8 @@ import ( "github.com/nats-io/nkeys" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/hmchangw/chat/pkg/testutil" ) // fakeValidator is defined in handler_test.go (same package). The integration @@ -63,3 +65,5 @@ func TestAuthHandler_Integration(t *testing.T) { assert.Contains(t, []string(claims.Pub.Allow), "chat.user.testuser.>") assert.Contains(t, []string(claims.Sub.Allow), "chat.room.>") } + +func TestMain(m *testing.M) { testutil.RunTests(m) } diff --git a/auth-service/main.go b/auth-service/main.go index d7c643cd5..5219f2e31 100644 --- a/auth-service/main.go +++ b/auth-service/main.go @@ -12,6 +12,7 @@ import ( "github.com/gin-gonic/gin" "github.com/nats-io/nkeys" + "github.com/hmchangw/chat/pkg/ginutil" pkgoidc "github.com/hmchangw/chat/pkg/oidc" "github.com/hmchangw/chat/pkg/shutdown" ) @@ -51,11 +52,13 @@ func run() error { ctx := context.Background() + opts := []Option{WithJitter(cfg.NATSJWTExpiryJitter)} + var handler *AuthHandler if cfg.DevMode { - slog.Info("dev mode enabled — OIDC validation disabled") - handler = NewAuthHandler(nil, signingKP, cfg.NATSJWTExpiry, true, WithJitter(cfg.NATSJWTExpiryJitter)) + slog.Warn("dev mode enabled — OIDC validation disabled") + handler = NewAuthHandler(nil, signingKP, cfg.NATSJWTExpiry, true, opts...) } else { if cfg.OIDCIssuerURL == "" || len(cfg.OIDCAudiences) == 0 { return fmt.Errorf("OIDC_ISSUER_URL and OIDC_AUDIENCES are required when DEV_MODE is false") @@ -71,15 +74,15 @@ func run() error { return fmt.Errorf("create oidc validator: %w", err) } slog.Info("oidc validator initialized", "issuer", cfg.OIDCIssuerURL) - handler = NewAuthHandler(oidcValidator, signingKP, cfg.NATSJWTExpiry, false, WithJitter(cfg.NATSJWTExpiryJitter)) + handler = NewAuthHandler(oidcValidator, signingKP, cfg.NATSJWTExpiry, false, opts...) } gin.SetMode(gin.ReleaseMode) r := gin.New() r.Use(gin.Recovery()) - r.Use(requestIDMiddleware()) - r.Use(accessLogMiddleware()) - r.Use(corsMiddleware()) + r.Use(ginutil.RequestID()) + r.Use(ginutil.AccessLog()) + r.Use(ginutil.CORS()) registerRoutes(r, handler) addr := fmt.Sprintf(":%s", cfg.Port) diff --git a/chat-frontend/CLAUDE.md b/chat-frontend/CLAUDE.md index e32dead4e..c8290b909 100644 --- a/chat-frontend/CLAUDE.md +++ b/chat-frontend/CLAUDE.md @@ -120,7 +120,8 @@ Reasons emitted today (full catalog in [`docs/client-api.md`](../docs/client-api - `max_room_size_reached`, `not_room_member`, `not_room_owner`, `last_owner_cannot_leave`, `bot_in_channel`, `bot_not_available`, `user_not_found`, `invalid_org`, `self_dm`, `last_member_cannot_remove`, `target_not_member`, `already_owner`, `cannot_demote_last_owner`, `promote_requires_individual` — room-service / room-worker - `large_room_post_restricted`, `not_subscribed`, `outside_access_window` — message-gatekeeper / history-service - `sso_token_expired`, `invalid_sso_token` — auth-service (drive a redirect-to-relogin) -- `invalid_request`, `invalid_nkey`, `missing_fields` — auth-service (form-validation surface; rarely actionable by the UI today) +- `invalid_request`, `invalid_nkey`, `missing_fields` — auth-service / portal-service (form-validation surface; rarely actionable by the UI today) +- `account_not_ready` — portal-service lookup (account absent from the HR directory cache, or present there but not provisioned in the users collection; show "contact your administrator" copy) When adding a new client-facing branch in the UI, prefer matching a reason over a message substring. If the case you need isn't in the catalog, ask backend to add a `Reason` constant in `pkg/errcode/codes_.go` rather than substring-matching the english text. diff --git a/chat-frontend/deploy/30-render-config.sh b/chat-frontend/deploy/30-render-config.sh index 83d74e232..c920262e4 100644 --- a/chat-frontend/deploy/30-render-config.sh +++ b/chat-frontend/deploy/30-render-config.sh @@ -1,17 +1,16 @@ #!/bin/sh -# Renders /config.js from env vars at container start. +# Renders /config.js from env vars at container start. PORTAL_URL has no +# default: a misconfigured deploy must fail here, not send browsers to localhost. set -eu -: "${AUTH_URL:=http://localhost:8080}" -: "${NATS_URL:=ws://localhost:9222}" -: "${DEFAULT_SITE_ID:=site-local}" +: "${PORTAL_URL:?PORTAL_URL is required (portal-service base URL)}" : "${DEV_MODE:=false}" : "${OIDC_ISSUER_URL:=}" : "${OIDC_CLIENT_ID:=nats-chat}" -export AUTH_URL NATS_URL DEFAULT_SITE_ID DEV_MODE OIDC_ISSUER_URL OIDC_CLIENT_ID +export PORTAL_URL DEV_MODE OIDC_ISSUER_URL OIDC_CLIENT_ID -envsubst '${AUTH_URL} ${NATS_URL} ${DEFAULT_SITE_ID} ${DEV_MODE} ${OIDC_ISSUER_URL} ${OIDC_CLIENT_ID}' \ +envsubst '${PORTAL_URL} ${DEV_MODE} ${OIDC_ISSUER_URL} ${OIDC_CLIENT_ID}' \ < /etc/config.js.template \ > /usr/share/nginx/html/config.js -echo "rendered /config.js AUTH_URL=$AUTH_URL NATS_URL=$NATS_URL DEFAULT_SITE_ID=$DEFAULT_SITE_ID DEV_MODE=$DEV_MODE OIDC_ISSUER_URL=$OIDC_ISSUER_URL OIDC_CLIENT_ID=$OIDC_CLIENT_ID" +echo "rendered /config.js PORTAL_URL=$PORTAL_URL DEV_MODE=$DEV_MODE OIDC_ISSUER_URL=$OIDC_ISSUER_URL OIDC_CLIENT_ID=$OIDC_CLIENT_ID" diff --git a/chat-frontend/deploy/config.js.template b/chat-frontend/deploy/config.js.template index d7d41113c..e26b599c5 100644 --- a/chat-frontend/deploy/config.js.template +++ b/chat-frontend/deploy/config.js.template @@ -1,8 +1,6 @@ // Generated at container start — edit template, not output. window.__APP_CONFIG__ = { - AUTH_URL: "${AUTH_URL}", - NATS_URL: "${NATS_URL}", - DEFAULT_SITE_ID: "${DEFAULT_SITE_ID}", + PORTAL_URL: "${PORTAL_URL}", DEV_MODE: "${DEV_MODE}", OIDC_ISSUER_URL: "${OIDC_ISSUER_URL}", OIDC_CLIENT_ID: "${OIDC_CLIENT_ID}" diff --git a/chat-frontend/deploy/docker-compose.yml b/chat-frontend/deploy/docker-compose.yml index fd0ca5a32..23c092afe 100644 --- a/chat-frontend/deploy/docker-compose.yml +++ b/chat-frontend/deploy/docker-compose.yml @@ -6,9 +6,7 @@ services: context: ../.. dockerfile: chat-frontend/deploy/Dockerfile environment: - AUTH_URL: ${AUTH_URL:-http://localhost:8080} - NATS_URL: ${NATS_URL:-ws://localhost:9222} - DEFAULT_SITE_ID: ${DEFAULT_SITE_ID:-site-local} + PORTAL_URL: ${PORTAL_URL:-http://localhost:8081} # OIDC values are only consumed when DEV_MODE=false. The Keycloak # realm at OIDC_ISSUER_URL must serve OIDC_CLIENT_ID with a redirect # URI matching this frontend's /oidc-callback. diff --git a/chat-frontend/src/api/_transport/asyncJob.test.js b/chat-frontend/src/api/_transport/asyncJob.test.js index bb2f73097..a20940daa 100644 --- a/chat-frontend/src/api/_transport/asyncJob.test.js +++ b/chat-frontend/src/api/_transport/asyncJob.test.js @@ -298,6 +298,14 @@ describe('formatAsyncJobError', () => { expect(formatAsyncJobError(err)).toBe('This room is at capacity.') }) + it('maps account_not_ready to the contact-administrator copy', () => { + const err = new AsyncJobError('account not ready for chat', ASYNC_JOB_ERROR_KINDS.SyncError, { + code: 'forbidden', + reason: 'account_not_ready', + }) + expect(formatAsyncJobError(err)).toBe("Your account isn't ready for chat yet — contact your administrator.") + }) + it('returns the humanized copy for not_room_member', () => { const err = new AsyncJobError('only room members can do that', ASYNC_JOB_ERROR_KINDS.AsyncError, { reason: 'not_room_member', diff --git a/chat-frontend/src/api/_transport/asyncJob.ts b/chat-frontend/src/api/_transport/asyncJob.ts index de2536328..6b3afdaa6 100644 --- a/chat-frontend/src/api/_transport/asyncJob.ts +++ b/chat-frontend/src/api/_transport/asyncJob.ts @@ -112,6 +112,7 @@ const REASON_COPY: Record = { pin_disabled: 'Pinning is turned off for this site.', pin_limit_reached: 'This room has reached its pin limit — unpin a message first.', pin_room_too_large: 'This room is too large for non-admins to pin.', + account_not_ready: "Your account isn't ready for chat yet — contact your administrator.", } /** diff --git a/chat-frontend/src/api/auth/oidcClient.js b/chat-frontend/src/api/auth/oidcClient.js index 5cbb8fa16..9edc5897f 100644 --- a/chat-frontend/src/api/auth/oidcClient.js +++ b/chat-frontend/src/api/auth/oidcClient.js @@ -45,11 +45,10 @@ export function isSSOTokenInvalidError(err) { export async function redirectToReloginOnTokenInvalid() { try { - // Clear oidc-client-ts's stashed user + the LoginPage's siteId stash. + // Clear oidc-client-ts's stashed user state. if (manager) { try { await manager.removeUser() } catch { /* best-effort */ } } - window.sessionStorage.removeItem('oidc.siteId') const mgr = getOidcManager() await mgr.signinRedirect() } catch { diff --git a/chat-frontend/src/context/NatsContext/NatsContext.jsx b/chat-frontend/src/context/NatsContext/NatsContext.jsx index 5d96876cf..d4a842cf4 100644 --- a/chat-frontend/src/context/NatsContext/NatsContext.jsx +++ b/chat-frontend/src/context/NatsContext/NatsContext.jsx @@ -1,7 +1,7 @@ import { createContext, useContext, useRef, useState, useCallback, useEffect, useMemo } from 'react' import { connect as natsConnect, StringCodec, headers as natsHeaders } from 'nats.ws' import { createUser } from 'nkeys.js' -import { AUTH_URL, NATS_URL } from '@/lib/runtimeConfig' +import { PORTAL_URL } from '@/lib/runtimeConfig' import { useDebug } from '@/context/DebugContext' import { useJwtRefresh } from './useJwtRefresh' import { @@ -14,14 +14,31 @@ export const NatsContext = createContext(null) const sc = StringCodec() +// Both services emit the errcode envelope {code, reason?, error, metadata?}. +// Legacy deployments may return {error} only — callers fall back to message. +async function throwEnvelopeError(resp, fallbackMsg) { + const errBody = await resp.json().catch(() => ({})) + throw new AsyncJobError( + errBody.error || `${fallbackMsg}: ${resp.status}`, + ASYNC_JOB_ERROR_KINDS.SyncError, + { code: errBody.code, reason: errBody.reason, metadata: errBody.metadata }, + ) +} + export function NatsProvider({ children }) { const ncRef = useRef(null) + // Bumped on every connect and disconnect so a superseded connection's + // long-lived nc.closed() callback can detect it is stale and drop its write + // (codebase "stale-cycle protection" convention). + const connectGenRef = useRef(0) const [connected, setConnected] = useState(false) const [user, setUser] = useState(null) const [error, setError] = useState(null) - const authUrl = AUTH_URL - const natsUrl = NATS_URL + // Resolved per user by the portal lookup at connect time; the JWT-refresh + // loop reads it through the getter so re-mints follow the resolved site. + const authUrlRef = useRef(null) + const getAuthUrl = useCallback(() => authUrlRef.current, []) // Keep the live debug settings in refs so the transport callbacks can read // them at send time without being recreated (and re-rendering consumers) on @@ -45,25 +62,39 @@ export function NatsProvider({ children }) { return h }, []) - const { authenticator, setCredentials, stop } = useJwtRefresh({ authUrl, ncRef }) + const { authenticator, setCredentials, stop } = useJwtRefresh({ getAuthUrl, ncRef }) /** - * Authenticate against auth-service and open the NATS WebSocket - * connection. On success, `user`/`connected` flip true and any - * subsequent server-initiated close updates `error`. + * Resolve the user's home site via the portal lookup, authenticate + * against that site's auth-service, and open the NATS WebSocket + * connection to that site. On success, `user`/`connected` flip true and + * any subsequent server-initiated close updates `error`. * * @param {Object} opts * @param {'dev'|'sso'} opts.mode * @param {string} [opts.account] Dev mode: account name to log in as. * @param {string} [opts.ssoToken] Production mode: OIDC access token. - * @param {string} opts.siteId - * @throws if auth-service rejects or the NATS handshake fails. + * @throws if the portal lookup or auth-service rejects, or the NATS + * handshake fails. */ const connectToNats = useCallback(async (opts) => { + const myGen = ++connectGenRef.current setError(null) - const { mode, account, ssoToken, siteId } = opts || {} + const { mode, account, ssoToken } = opts || {} + // 1) Site discovery: which auth-service, which NATS, which siteId. Discovery + // only — the portal validates no token; the account (derived from the SSO + // token's preferred_username in prod) is the lookup key. auth-service is the + // real gate that re-validates the token before minting the JWT. + const lookupResp = await fetch(`${PORTAL_URL}/api/userInfo?account=${encodeURIComponent(account ?? '')}`) + if (!lookupResp.ok) { + await throwEnvelopeError(lookupResp, 'Portal lookup failed') + } + const portal = await lookupResp.json() + const nextAuthUrl = portal.authServiceUrl + + // 2) Mint the NATS JWT at the resolved site's auth-service. const nkey = createUser() const natsPublicKey = nkey.getPublicKey() @@ -72,51 +103,55 @@ export function NatsProvider({ children }) { ? { ssoToken, natsPublicKey } : { account, natsPublicKey } - const authResp = await fetch(`${authUrl}/auth`, { + const authResp = await fetch(`${nextAuthUrl}/auth`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), }) if (!authResp.ok) { - // auth-service emits the errcode envelope {code, reason?, error, metadata?} - // via errhttp.Write. Older auth deployments may return {error} only — - // err.code is then undefined and consumers fall back to err.message text. - const errBody = await authResp.json().catch(() => ({})) - throw new AsyncJobError( - errBody.error || `Auth failed: ${authResp.status}`, - ASYNC_JOB_ERROR_KINDS.SyncError, - { code: errBody.code, reason: errBody.reason, metadata: errBody.metadata }, - ) + await throwEnvelopeError(authResp, 'Auth failed') } const { natsJwt, user: userInfo } = await authResp.json() - // Populate the credential refs BEFORE connecting so the dynamic - // authenticator's getters return the right values during the handshake. - setCredentials({ - jwt: natsJwt, - seed: nkey.getSeed(), - natsPublicKey, - refreshable: mode === 'sso', - }) + // A failed dial must roll back: stop() disarms the refresh loop the + // staged credentials armed, and the auth URL is committed only on success. + try { + // Populate the credential refs BEFORE connecting so the dynamic + // authenticator's getters return the right values during the handshake. + setCredentials({ + jwt: natsJwt, + seed: nkey.getSeed(), + natsPublicKey, + refreshable: mode === 'sso', + }) - const nc = await natsConnect({ - servers: natsUrl, - authenticator, - }) + // 3) Dial the resolved site's NATS. + const nc = await natsConnect({ + servers: portal.natsUrl, + authenticator, + }) - ncRef.current = nc - setUser({ ...userInfo, siteId }) - setConnected(true) + authUrlRef.current = nextAuthUrl + ncRef.current = nc + setUser({ ...userInfo, siteId: portal.siteId }) + setConnected(true) - nc.closed().then((err) => { - if (err) { - setError(`Disconnected: ${err.message}`) - } - setConnected(false) - }) - }, [authUrl, natsUrl, authenticator, setCredentials]) + nc.closed().then((err) => { + // A newer connect or a disconnect bumped the generation; this old + // link's close must not clobber the live session's state. + if (myGen !== connectGenRef.current) return + if (err) { + setError(`Disconnected: ${err.message}`) + } + setConnected(false) + }) + } catch (err) { + stop() + throw err + } + }, [authenticator, setCredentials, stop]) /** * Send a synchronous NATS request/reply. Use this for handlers that @@ -231,6 +266,9 @@ export function NatsProvider({ children }) { * provider is a no-op. */ const disconnect = useCallback(async () => { + // Invalidate any in-flight connect and the live link's closed() callback so + // a late close can't resurrect error/connected state after we tore down. + connectGenRef.current += 1 stop() if (ncRef.current) { await ncRef.current.drain() diff --git a/chat-frontend/src/context/NatsContext/NatsContext.test.jsx b/chat-frontend/src/context/NatsContext/NatsContext.test.jsx index 617973a6b..d888a0b1d 100644 --- a/chat-frontend/src/context/NatsContext/NatsContext.test.jsx +++ b/chat-frontend/src/context/NatsContext/NatsContext.test.jsx @@ -21,6 +21,7 @@ vi.mock('nkeys.js', () => ({ import { NatsProvider, useNats } from './NatsContext' import { DebugProvider } from '@/context/DebugContext' +import { useJwtRefresh } from './useJwtRefresh' function wrapper({ children }) { return ( @@ -30,23 +31,42 @@ function wrapper({ children }) { ) } +// The provider hands its authUrlRef getter to useJwtRefresh; reading it back +// through the mock observes when the resolved auth URL is committed. +function lastGetAuthUrl() { + return useJwtRefresh.mock.calls.at(-1)[0].getAuthUrl +} + +const PORTAL_RESP = { + account: 'alice', + employeeId: 'E001', + authServiceUrl: 'http://auth.site-a', + natsUrl: 'ws://nats.site-a', + siteId: 'site-a', +} + describe('NatsProvider connect wiring', () => { beforeEach(() => { setCredentials.mockReset() stop.mockReset() natsConnect.mockReset().mockResolvedValue({ closed: () => new Promise(() => {}) }) - global.fetch = vi.fn().mockResolvedValue({ - ok: true, - json: async () => ({ natsJwt: 'JWT123', user: { account: 'alice' } }), + global.fetch = vi.fn(async (url) => { + if (String(url).includes('/api/userInfo')) { + return { ok: true, json: async () => PORTAL_RESP } + } + return { ok: true, json: async () => ({ natsJwt: 'JWT123', user: { account: 'alice' } }) } }) }) afterEach(() => { vi.restoreAllMocks() }) - it('sets credentials before connecting and passes the dynamic authenticator', async () => { + it('resolves the site via portal userInfo, then auths and connects with the resolved URLs', async () => { const { result } = renderHook(() => useNats(), { wrapper }) await act(async () => { - await result.current.connect({ mode: 'sso', ssoToken: 'tok', siteId: 'site-1' }) + await result.current.connect({ mode: 'sso', ssoToken: 'tok', account: 'alice' }) }) + + expect(global.fetch).toHaveBeenNthCalledWith(1, 'http://localhost:8081/api/userInfo?account=alice') + expect(global.fetch).toHaveBeenNthCalledWith(2, 'http://auth.site-a/auth', expect.anything()) expect(setCredentials).toHaveBeenCalledWith({ jwt: 'JWT123', seed: new Uint8Array([7]), @@ -54,18 +74,121 @@ describe('NatsProvider connect wiring', () => { refreshable: true, }) expect(natsConnect).toHaveBeenCalledWith( - expect.objectContaining({ authenticator: fakeAuthenticator })) + expect.objectContaining({ servers: 'ws://nats.site-a', authenticator: fakeAuthenticator })) await waitFor(() => expect(result.current.connected).toBe(true)) + expect(result.current.user.siteId).toBe('site-a') + expect(lastGetAuthUrl()()).toBe('http://auth.site-a') + }) + + it('drops a stale nc.closed() callback from a superseded connection (generation guard)', async () => { + let closeFirst + const firstClosed = new Promise((res) => { closeFirst = res }) + natsConnect + .mockResolvedValueOnce({ closed: () => firstClosed }) + .mockResolvedValueOnce({ closed: () => new Promise(() => {}) }) + + const { result } = renderHook(() => useNats(), { wrapper }) + + await act(async () => { + await result.current.connect({ mode: 'sso', ssoToken: 'tok', account: 'alice' }) + }) + expect(result.current.connected).toBe(true) + + // A newer connect supersedes the first and bumps the connect generation. + await act(async () => { + await result.current.connect({ mode: 'sso', ssoToken: 'tok2', account: 'alice' }) + }) + expect(result.current.connected).toBe(true) + + // The first (now stale) link closes with an error. Its long-lived callback + // must not clobber the live second session's connected/error state. + await act(async () => { + closeFirst(new Error('old link dropped')) + await firstClosed.catch(() => {}) + }) + + expect(result.current.connected).toBe(true) + expect(result.current.error).toBeNull() + }) + + it('rolls back staged credentials when the NATS dial fails', async () => { + natsConnect.mockRejectedValue(new Error('handshake refused')) + const { result } = renderHook(() => useNats(), { wrapper }) + let thrown + await act(async () => { + try { await result.current.connect({ mode: 'sso', ssoToken: 'tok', account: 'alice' }) } catch (err) { thrown = err } + }) + + expect(thrown.message).toBe('handshake refused') + expect(setCredentials).toHaveBeenCalledTimes(1) + expect(stop).toHaveBeenCalledTimes(1) + expect(result.current.connected).toBe(false) + // The refresh loop must not be pointed at the new site by a failed connect. + expect(lastGetAuthUrl()()).toBeNull() }) - it('marks dev-mode connections non-refreshable', async () => { + it('dev mode looks up the account via userInfo and is non-refreshable', async () => { const { result } = renderHook(() => useNats(), { wrapper }) await act(async () => { - await result.current.connect({ mode: 'dev', account: 'alice', siteId: 'site-1' }) + await result.current.connect({ mode: 'dev', account: 'alice' }) }) + expect(global.fetch).toHaveBeenNthCalledWith(1, 'http://localhost:8081/api/userInfo?account=alice') expect(setCredentials).toHaveBeenCalledWith(expect.objectContaining({ refreshable: false })) }) + it('propagates the portal error envelope and never dials auth or NATS', async () => { + global.fetch = vi.fn(async () => ({ + ok: false, + json: async () => ({ code: 'forbidden', reason: 'account_not_ready', error: 'account not ready for chat' }), + })) + const { result } = renderHook(() => useNats(), { wrapper }) + let thrown + await act(async () => { + try { await result.current.connect({ mode: 'sso', ssoToken: 'tok', account: 'alice' }) } catch (err) { thrown = err } + }) + expect(thrown.reason).toBe('account_not_ready') + expect(thrown.code).toBe('forbidden') + expect(global.fetch).toHaveBeenCalledTimes(1) + expect(natsConnect).not.toHaveBeenCalled() + }) + + it('propagates the auth-step error envelope after a successful lookup', async () => { + global.fetch = vi.fn(async (url) => { + if (String(url).includes('/api/userInfo')) { + return { ok: true, json: async () => PORTAL_RESP } + } + return { + ok: false, + json: async () => ({ code: 'unauthenticated', reason: 'sso_token_expired', error: 'SSO token has expired, please re-login' }), + } + }) + const { result } = renderHook(() => useNats(), { wrapper }) + let thrown + await act(async () => { + try { await result.current.connect({ mode: 'sso', ssoToken: 'tok', account: 'alice' }) } catch (err) { thrown = err } + }) + expect(thrown.reason).toBe('sso_token_expired') + expect(thrown.message).toBe('SSO token has expired, please re-login') + expect(global.fetch).toHaveBeenCalledTimes(2) + expect(natsConnect).not.toHaveBeenCalled() + }) + + it('falls back to a status message when the error body is not JSON', async () => { + global.fetch = vi.fn(async () => ({ + ok: false, + status: 502, + json: async () => { throw new Error('not json') }, + })) + const { result } = renderHook(() => useNats(), { wrapper }) + let thrown + await act(async () => { + try { await result.current.connect({ mode: 'sso', ssoToken: 'tok', account: 'alice' }) } catch (err) { thrown = err } + }) + expect(thrown.message).toBe('Portal lookup failed: 502') + expect(thrown.code).toBeUndefined() + expect(natsConnect).not.toHaveBeenCalled() + }) + it('stops the refresh loop on disconnect', async () => { natsConnect.mockResolvedValue({ closed: () => new Promise(() => {}), @@ -73,7 +196,7 @@ describe('NatsProvider connect wiring', () => { }) const { result } = renderHook(() => useNats(), { wrapper }) await act(async () => { - await result.current.connect({ mode: 'sso', ssoToken: 'tok', siteId: 'site-1' }) + await result.current.connect({ mode: 'sso', ssoToken: 'tok', account: 'alice' }) }) await act(async () => { await result.current.disconnect() }) expect(stop).toHaveBeenCalledTimes(1) diff --git a/chat-frontend/src/context/NatsContext/useJwtRefresh.js b/chat-frontend/src/context/NatsContext/useJwtRefresh.js index e7d63f02b..8c4651c3d 100644 --- a/chat-frontend/src/context/NatsContext/useJwtRefresh.js +++ b/chat-frontend/src/context/NatsContext/useJwtRefresh.js @@ -25,8 +25,11 @@ const MAX_REFRESH_ATTEMPTS = RETRY_BACKOFF_MS.length + 1 * each /auth, BEFORE connect(), so the getters are populated for the * handshake. Schedules the next refresh when refreshable. * - stop(): clear the timer and creds (call on disconnect). + * + * getAuthUrl is read at each refresh, so the target can be resolved after + * mount (portal lookup). */ -export function useJwtRefresh({ authUrl, ncRef }) { +export function useJwtRefresh({ getAuthUrl, ncRef }) { const jwtRef = useRef(null) const seedRef = useRef(null) const pubKeyRef = useRef(null) @@ -89,7 +92,7 @@ export function useJwtRefresh({ authUrl, ncRef }) { // 2) Re-mint the NATS JWT. Transport failures are transient (retry with // backoff); a 4xx rejection is terminal. try { - const resp = await fetch(`${authUrl}/auth`, { + const resp = await fetch(`${getAuthUrl()}/auth`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ ssoToken, natsPublicKey: pubKeyRef.current }), @@ -122,7 +125,7 @@ export function useJwtRefresh({ authUrl, ncRef }) { console.warn('NATS JWT re-mint failed; retrying', { attempt, error: err?.message }) armTimer(RETRY_BACKOFF_MS[attempt - 1], attempt + 1) } - }, [authUrl, ncRef, scheduleRefresh, redirect, armTimer]) + }, [getAuthUrl, ncRef, scheduleRefresh, redirect, armTimer]) // Keep refreshRef current so the timer always calls the latest closure — // this breaks the scheduleRefresh <-> refresh dependency cycle. diff --git a/chat-frontend/src/context/NatsContext/useJwtRefresh.test.js b/chat-frontend/src/context/NatsContext/useJwtRefresh.test.js index 38ebdf3a5..cf812d660 100644 --- a/chat-frontend/src/context/NatsContext/useJwtRefresh.test.js +++ b/chat-frontend/src/context/NatsContext/useJwtRefresh.test.js @@ -23,7 +23,7 @@ const okResp = (jwt) => ({ ok: true, json: async () => ({ natsJwt: jwt }) }) const errResp = (status) => ({ ok: false, status, json: async () => ({}) }) function setup({ ncRef = { current: { reconnect: vi.fn() } } } = {}) { - const view = renderHook(() => useJwtRefresh({ authUrl: 'http://auth', ncRef })) + const view = renderHook(() => useJwtRefresh({ getAuthUrl: () => 'http://auth', ncRef })) return { ...view, ncRef } } @@ -192,4 +192,19 @@ describe('useJwtRefresh', () => { await act(async () => { resolveRenew('sso'); await Promise.resolve() }) expect(global.fetch).not.toHaveBeenCalled() }) + + it('re-mints against the URL the getter returns at refresh time', async () => { + renewSsoToken.mockResolvedValue('fresh-sso') + global.fetch.mockResolvedValue(okResp(makeJwt(3600))) + let authUrl = 'http://auth-initial' + const { result } = renderHook(() => + useJwtRefresh({ getAuthUrl: () => authUrl, ncRef: { current: null } })) + + authUrl = 'http://auth.site-a' // resolved later, e.g. by the portal lookup + act(() => { + result.current.setCredentials({ jwt: makeJwt(100), seed: new Uint8Array([9]), natsPublicKey: 'UPUB', refreshable: true }) + }) + await vi.advanceTimersByTimeAsync(95 * 1000) + expect(global.fetch).toHaveBeenCalledWith('http://auth.site-a/auth', expect.anything()) + }) }) diff --git a/chat-frontend/src/lib/runtimeConfig.js b/chat-frontend/src/lib/runtimeConfig.js index ecebe8593..9b4e38a22 100644 --- a/chat-frontend/src/lib/runtimeConfig.js +++ b/chat-frontend/src/lib/runtimeConfig.js @@ -3,14 +3,8 @@ const runtime = (typeof window !== 'undefined' && window.__APP_CONFIG__) || {} -export const AUTH_URL = - runtime.AUTH_URL || import.meta.env.VITE_AUTH_URL || 'http://localhost:8080' - -export const NATS_URL = - runtime.NATS_URL || import.meta.env.VITE_NATS_URL || 'ws://localhost:9222' - -export const DEFAULT_SITE_ID = - runtime.DEFAULT_SITE_ID || import.meta.env.VITE_DEFAULT_SITE_ID || 'site-local' +export const PORTAL_URL = + runtime.PORTAL_URL || import.meta.env.VITE_PORTAL_URL || 'http://localhost:8081' export const DEV_MODE = (runtime.DEV_MODE ?? import.meta.env.VITE_DEV_MODE ?? 'true') === 'true' diff --git a/chat-frontend/src/lib/runtimeConfig.test.js b/chat-frontend/src/lib/runtimeConfig.test.js index 2daa4da5e..21cc04224 100644 --- a/chat-frontend/src/lib/runtimeConfig.test.js +++ b/chat-frontend/src/lib/runtimeConfig.test.js @@ -32,4 +32,22 @@ describe('runtimeConfig', () => { const { OIDC_ISSUER_URL } = await import('./runtimeConfig.js') expect(OIDC_ISSUER_URL).toBe('https://custom-keycloak/realms/myrealm') }) + + it('PORTAL_URL defaults to localhost:8081', async () => { + const { PORTAL_URL } = await import('./runtimeConfig.js') + expect(PORTAL_URL).toBe('http://localhost:8081') + }) + + it('PORTAL_URL reads from window.__APP_CONFIG__', async () => { + window.__APP_CONFIG__ = { PORTAL_URL: 'https://portal.example.com' } + const { PORTAL_URL } = await import('./runtimeConfig.js') + expect(PORTAL_URL).toBe('https://portal.example.com') + }) + + it('no longer exports the retired static connection vars', async () => { + const mod = await import('./runtimeConfig.js') + expect(mod.AUTH_URL).toBeUndefined() + expect(mod.NATS_URL).toBeUndefined() + expect(mod.DEFAULT_SITE_ID).toBeUndefined() + }) }) diff --git a/chat-frontend/src/pages/LoginPage/LoginPage.jsx b/chat-frontend/src/pages/LoginPage/LoginPage.jsx index 0954ebf62..480005569 100644 --- a/chat-frontend/src/pages/LoginPage/LoginPage.jsx +++ b/chat-frontend/src/pages/LoginPage/LoginPage.jsx @@ -1,18 +1,17 @@ -import { useState } from 'react' +import { useEffect, useRef, useState } from 'react' import { useNats } from '@/context/NatsContext' -import { DEFAULT_SITE_ID, DEV_MODE } from '@/lib/runtimeConfig' +import { DEV_MODE } from '@/lib/runtimeConfig' import { getOidcManager, isSSOTokenInvalidError, redirectToReloginOnTokenInvalid } from '@/api/auth/oidcClient' import { formatAsyncJobError } from '@/api' import './style.css' export default function LoginPage() { const { connect, error: natsError } = useNats() - const defaultSiteId = DEFAULT_SITE_ID const [account, setAccount] = useState('') - const [siteId, setSiteId] = useState(defaultSiteId) const [loading, setLoading] = useState(false) const [error, setError] = useState(null) + const didAutoRedirect = useRef(false) const handleDevSubmit = async (e) => { e.preventDefault() @@ -24,7 +23,6 @@ export default function LoginPage() { await connect({ mode: 'dev', account: account.trim(), - siteId: siteId.trim(), }) } catch (err) { if (isSSOTokenInvalidError(err)) { @@ -41,8 +39,6 @@ export default function LoginPage() { setLoading(true) setError(null) try { - // Stash siteId so the /oidc-callback page can read it after redirect. - window.sessionStorage.setItem('oidc.siteId', siteId.trim()) const manager = getOidcManager() await manager.signinRedirect() // Browser navigates away — code below this point is unreachable in prod. @@ -56,6 +52,19 @@ export default function LoginPage() { } } + // Production: a visitor who lands here has no live session, so send them + // straight to Keycloak instead of making them click. After login the browser + // returns via /oidc-callback and connects through the portal. Fire once per + // mount; the ref survives a StrictMode double-invoke. Dev mode keeps the + // account form (no IdP), so it never auto-redirects. + useEffect(() => { + if (DEV_MODE) return + if (didAutoRedirect.current) return + didAutoRedirect.current = true + handleKeycloakLogin() + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []) + if (DEV_MODE) { return (
@@ -74,15 +83,6 @@ export default function LoginPage() { disabled={loading} /> - - setSiteId(e.target.value)} - disabled={loading} - /> - @@ -101,15 +101,6 @@ export default function LoginPage() {

Chat

Sign in with Keycloak

- - setSiteId(e.target.value)} - disabled={loading} - /> - diff --git a/chat-frontend/src/pages/LoginPage/LoginPage.test.jsx b/chat-frontend/src/pages/LoginPage/LoginPage.test.jsx index 3871c0fc6..b889cf401 100644 --- a/chat-frontend/src/pages/LoginPage/LoginPage.test.jsx +++ b/chat-frontend/src/pages/LoginPage/LoginPage.test.jsx @@ -6,7 +6,6 @@ vi.mock('@/context/NatsContext', () => ({ })) vi.mock('@/lib/runtimeConfig', () => ({ - DEFAULT_SITE_ID: 'site-local', DEV_MODE: true, })) @@ -38,30 +37,35 @@ describe('LoginPage in DEV_MODE=true', () => { runtimeConfig.DEV_MODE = true }) - it('renders the dev account form', () => { + it('renders the dev account form without a Site ID field', () => { useNats.mockReturnValue({ connect: vi.fn(), error: null }) render() expect(screen.getByLabelText(/account/i)).toBeInTheDocument() - expect(screen.getByLabelText(/site id/i)).toBeInTheDocument() + expect(screen.queryByLabelText(/site id/i)).not.toBeInTheDocument() expect(screen.queryByRole('button', { name: /keycloak/i })).not.toBeInTheDocument() }) - it('submits with {mode: "dev", account, siteId}', async () => { + it('does NOT auto-redirect to Keycloak in dev mode', () => { + useNats.mockReturnValue({ connect: vi.fn(), error: null }) + const signinRedirect = vi.fn().mockResolvedValue(undefined) + getOidcManager.mockReturnValue({ signinRedirect }) + + render() + + expect(signinRedirect).not.toHaveBeenCalled() + }) + + it('submits with {mode: "dev", account}', async () => { const connect = vi.fn().mockResolvedValue(undefined) useNats.mockReturnValue({ connect, error: null }) render() fireEvent.change(screen.getByLabelText(/account/i), { target: { value: 'alice' } }) - fireEvent.change(screen.getByLabelText(/site id/i), { target: { value: 'site-A' } }) fireEvent.click(screen.getByRole('button', { name: /connect/i })) await waitFor(() => { - expect(connect).toHaveBeenCalledWith({ - mode: 'dev', - account: 'alice', - siteId: 'site-A', - }) + expect(connect).toHaveBeenCalledWith({ mode: 'dev', account: 'alice' }) }) }) @@ -84,38 +88,67 @@ describe('LoginPage in DEV_MODE=false', () => { runtimeConfig.DEV_MODE = false }) - it('renders a Keycloak login button instead of the account form', () => { + it('renders the Keycloak sign-in UI instead of the account form', () => { useNats.mockReturnValue({ connect: vi.fn(), error: null }) + getOidcManager.mockReturnValue({ signinRedirect: vi.fn().mockResolvedValue(undefined) }) render() - expect(screen.getByRole('button', { name: /keycloak/i })).toBeInTheDocument() + // The subtitle is the Keycloak heading; the button itself flips to + // "Redirecting…" because the auto-redirect fires on mount. + expect(screen.getByText('Sign in with Keycloak')).toBeInTheDocument() + expect(screen.getByRole('button')).toHaveTextContent(/redirecting/i) expect(screen.queryByLabelText(/account/i)).not.toBeInTheDocument() + expect(screen.queryByLabelText(/site id/i)).not.toBeInTheDocument() }) - it('stores siteId in sessionStorage and calls signinRedirect on button click', async () => { + it('auto-redirects to Keycloak on mount when unauthenticated (no click needed)', async () => { useNats.mockReturnValue({ connect: vi.fn(), error: null }) const signinRedirect = vi.fn().mockResolvedValue(undefined) getOidcManager.mockReturnValue({ signinRedirect }) render() - fireEvent.change(screen.getByLabelText(/site id/i), { target: { value: 'site-B' } }) - fireEvent.click(screen.getByRole('button', { name: /keycloak/i })) + // The visitor never clicks — landing on the login page in production with no + // session must send them straight to Keycloak. await waitFor(() => { - expect(signinRedirect).toHaveBeenCalled() + expect(signinRedirect).toHaveBeenCalledTimes(1) }) - expect(window.sessionStorage.getItem('oidc.siteId')).toBe('site-B') }) - it('shows an error if signinRedirect throws', async () => { + it('redirects to Keycloak without any Site ID input or stash', async () => { + useNats.mockReturnValue({ connect: vi.fn(), error: null }) + const signinRedirect = vi.fn().mockResolvedValue(undefined) + getOidcManager.mockReturnValue({ signinRedirect }) + + render() + expect(screen.queryByLabelText(/site id/i)).not.toBeInTheDocument() + await waitFor(() => expect(signinRedirect).toHaveBeenCalled()) + expect(window.sessionStorage.getItem('oidc.siteId')).toBeNull() + }) + + it('shows an error if the Keycloak redirect throws', async () => { useNats.mockReturnValue({ connect: vi.fn(), error: null }) const signinRedirect = vi.fn().mockRejectedValue(new Error('idp down')) getOidcManager.mockReturnValue({ signinRedirect }) render() - fireEvent.click(screen.getByRole('button', { name: /keycloak/i })) await waitFor(() => { expect(screen.getByText(/idp down/i)).toBeInTheDocument() }) }) + + it('lets the visitor retry via the button after a failed redirect', async () => { + useNats.mockReturnValue({ connect: vi.fn(), error: null }) + const signinRedirect = vi.fn() + .mockRejectedValueOnce(new Error('idp down')) + .mockResolvedValueOnce(undefined) + getOidcManager.mockReturnValue({ signinRedirect }) + + render() + // Auto-redirect fails first; the button re-enables for a manual retry. + await waitFor(() => expect(screen.getByText(/idp down/i)).toBeInTheDocument()) + + fireEvent.click(screen.getByRole('button', { name: /keycloak/i })) + await waitFor(() => expect(signinRedirect).toHaveBeenCalledTimes(2)) + }) }) diff --git a/chat-frontend/src/pages/OidcCallback/OidcCallback.jsx b/chat-frontend/src/pages/OidcCallback/OidcCallback.jsx index 0557aac07..dfd809635 100644 --- a/chat-frontend/src/pages/OidcCallback/OidcCallback.jsx +++ b/chat-frontend/src/pages/OidcCallback/OidcCallback.jsx @@ -21,12 +21,10 @@ export default function OidcCallback({ onDone }) { const user = await manager.signinRedirectCallback() if (cancelled) return - const siteId = window.sessionStorage.getItem('oidc.siteId') || '' - await connect({ mode: 'sso', ssoToken: user.access_token, - siteId, + account: user.profile?.preferred_username, }) if (cancelled) return diff --git a/chat-frontend/src/pages/OidcCallback/OidcCallback.test.jsx b/chat-frontend/src/pages/OidcCallback/OidcCallback.test.jsx index 6248ddd35..3bef004be 100644 --- a/chat-frontend/src/pages/OidcCallback/OidcCallback.test.jsx +++ b/chat-frontend/src/pages/OidcCallback/OidcCallback.test.jsx @@ -45,13 +45,11 @@ describe('OidcCallback', () => { const onDone = vi.fn() useNats.mockReturnValue({ connect }) - const fakeUser = { access_token: 'access-token-123' } + const fakeUser = { access_token: 'access-token-123', profile: { preferred_username: 'alice' } } getOidcManager.mockReturnValue({ signinRedirectCallback: vi.fn().mockResolvedValue(fakeUser), }) - window.sessionStorage.setItem('oidc.siteId', 'site-A') - const replaceStateSpy = vi.spyOn(window.history, 'replaceState') render() @@ -60,7 +58,7 @@ describe('OidcCallback', () => { expect(connect).toHaveBeenCalledWith({ mode: 'sso', ssoToken: 'access-token-123', - siteId: 'site-A', + account: 'alice', }) }) @@ -95,7 +93,6 @@ describe('OidcCallback', () => { getOidcManager.mockReturnValue({ signinRedirectCallback: vi.fn().mockResolvedValue({ access_token: 'tok' }), }) - window.sessionStorage.setItem('oidc.siteId', 'site-A') render() diff --git a/chat-frontend/vite.config.js b/chat-frontend/vite.config.js index a8586a94a..a142fe410 100644 --- a/chat-frontend/vite.config.js +++ b/chat-frontend/vite.config.js @@ -17,8 +17,5 @@ export default defineConfig({ }, server: { port: 3000, - proxy: { - '/auth': 'http://localhost:8080', - }, }, }) diff --git a/docker-local/compose.services.yaml b/docker-local/compose.services.yaml index 971023fd1..ed3095651 100644 --- a/docker-local/compose.services.yaml +++ b/docker-local/compose.services.yaml @@ -15,6 +15,7 @@ include: - ../message-gatekeeper/deploy/docker-compose.yml - ../message-worker/deploy/docker-compose.yml - ../notification-worker/deploy/docker-compose.yml + - ../portal-service/deploy/docker-compose.yml - ../room-service/deploy/docker-compose.yml - ../room-worker/deploy/docker-compose.yml - ../search-service/deploy/docker-compose.yml diff --git a/docker-local/setup.sh b/docker-local/setup.sh index 98ed6478e..9bee5581a 100755 --- a/docker-local/setup.sh +++ b/docker-local/setup.sh @@ -106,14 +106,14 @@ DEV_MODE=true EOF chmod 600 "$ENV_FILE" -# chat-frontend/.env.local feeds `npm run dev` (Vite). Written only on first -# run so devs can edit it (e.g. point at staging) without losing changes. +# chat-frontend/.env.local feeds `npm run dev` (Vite). Created on first run, +# left editable afterwards (e.g. point at staging) — new vars are appended. if [ ! -f "$FRONTEND_ENV_FILE" ]; then cat > "$FRONTEND_ENV_FILE" <> "$FRONTEND_ENV_FILE" fi cat > "$NATS_CONF" <' or whitespace)" }` — the account becomes a NATS subject token, so separator/wildcard/whitespace characters are refused. | | 401 | `unauthenticated` | `sso_token_expired` | `{ "code": "unauthenticated", "reason": "sso_token_expired", "error": "SSO token has expired, please re-login" }` | | 401 | `unauthenticated` | `invalid_sso_token` | `{ "code": "unauthenticated", "reason": "invalid_sso_token", "error": "invalid SSO token" }` | | 500 | `internal` | — | `{ "code": "internal", "error": "internal error" }` — the real cause is logged server-side and never sent to the client. | @@ -228,9 +230,71 @@ The returned `natsJwt` has a server-configured lifetime (default 2h). Clients sh `None.` +### 2.3 HTTP — GET /api/userInfo (portal-service) + +**Endpoint:** `GET /api/userInfo?account={account}` +**Reply:** synchronous HTTP response + +Site discovery — called once per login, **before** §2.2. Looks the account up in the portal's in-memory directory (loaded from the HR employee feed at startup and refreshed daily), confirms the account is provisioned in the `users` collection (the canonical user record), and returns the home site's connection coordinates. The client then calls `POST {authServiceUrl}/auth` (§2.2) and connects to `natsUrl` (§2.1). JWT renewal does **not** re-query the portal — site assignment is stable within a session. + +**Discovery only — no token is validated here.** The endpoint serves non-secret directory data keyed by `account`. The client supplies the account directly: derived from the SSO token's `preferred_username` claim in production, or the dev login form in dev mode. The authoritative check is auth-service (§2.2), which validates the SSO token before minting a JWT — an account that resolves here still cannot obtain a NATS JWT or connect without a valid token at that step. + +#### Request + +| Field | Source | Type | Required | Notes | +|---|---|---|---|---| +| `account` | query | string | yes | The account to resolve. Becomes the `{account}` in every NATS subject, so separator/wildcard/whitespace characters (`.`, `*`, `>`, whitespace) are refused. | + +```http +GET /api/userInfo?account=alice +``` + +#### Success response + +`HTTP 200` + +| Field | Type | Notes | +|---|---|---| +| `account` | string | The `{account}` used in every NATS subject. | +| `employeeId` | string | From the portal directory; informational. | +| `authServiceUrl` | string | Base URL of the home site's auth-service — call `POST {authServiceUrl}/auth` next. | +| `baseUrl` | string | Base URL of the user's home site itself (site-scoped HTTP origin) — a distinct URL, not the auth-service URL. | +| `natsUrl` | string | WebSocket URL of the home site's NATS. | +| `siteId` | string | The user's home site; scopes site-suffixed NATS subjects. | + +```json +{ + "account": "alice", + "employeeId": "E12345", + "authServiceUrl": "https://auth.site-a.example.com", + "baseUrl": "https://site-a.example.com", + "natsUrl": "wss://nats.site-a.example.com", + "siteId": "site-a" +} +``` + +#### Error response + +See [Error envelope](#6-error-envelope-reference). HTTP statuses: + +| Status | `code` | `reason` | Example body | +|---|---|---|---| +| 400 | `bad_request` | `missing_fields` | `{ "code": "bad_request", "reason": "missing_fields", "error": "account is required" }` | +| 400 | `bad_request` | — | `{ "code": "bad_request", "error": "account must be a single NATS subject token (no '.', '*', '>' or whitespace)" }` — same account rule as §2.2. | +| 403 | `forbidden` | `account_not_ready` | `{ "code": "forbidden", "reason": "account_not_ready", "error": "account not ready for chat" }` — the account is not usable for chat: either absent from the portal's HR directory (fed by a daily sync), or present there but not yet provisioned in the `users` collection. | +| 500 | `internal` | — | `{ "code": "internal", "error": "internal error" }` | + +#### Triggered events — success path + +`None — HTTP-only.` + +#### Triggered events — error path + +`None.` + --- -### 2.3 HTTP — Protected image upload/download +### 2.4 HTTP — Protected image upload/download Two HTTP endpoints on `upload-service` for protected inline images, proxied to/from an internal Drive. Both require the `ssoToken` header (validated via @@ -3676,13 +3740,14 @@ Every error response — NATS reply subjects, JetStream async results, and HTTP | `invalid_sso_token` | unauthenticated | auth-service `POST /auth` | | `invalid_request` | bad_request | auth-service (body parse / required field missing) | | `invalid_nkey` | bad_request | auth-service (natsPublicKey format) | -| `missing_fields` | bad_request | auth-service (ssoToken/account/natsPublicKey missing) | +| `missing_fields` | bad_request | auth-service (ssoToken/account/natsPublicKey missing); portal-service `GET /api/userInfo` (account missing) | +| `account_not_ready` | forbidden | portal-service `GET /api/userInfo` (account absent from the HR directory cache, or not provisioned in the users collection) | ### Where envelopes are sent - **NATS sync replies** — on the reply subject for §3/§4 RPCs. - **JetStream async results** — `model.AsyncJobResult` carries the same `code` + `reason` fields when `status == "error"`, so a failed async job is surfaced the same way as a sync error. -- **HTTP** — auth-service `POST /auth` (§2.2) and upload-service's image endpoints (§2.3) write the envelope as the response body with the matching HTTP status from the table above. +- **HTTP** — auth-service `POST /auth` (§2.2), portal-service `GET /api/userInfo` (§2.3), and upload-service's image endpoints (§2.4) write the envelope as the response body with the matching HTTP status from the table above. ### Client branching guidance diff --git a/docs/superpowers/plans/2026-06-11-portal-service.md b/docs/superpowers/plans/2026-06-11-portal-service.md new file mode 100644 index 000000000..ec0fe0cad --- /dev/null +++ b/docs/superpowers/plans/2026-06-11-portal-service.md @@ -0,0 +1,2351 @@ +# Portal Service (Site Discovery + Provisioning Gate) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +> **⚠️ Partially superseded (2026-06-12).** This plan is the executed working +> record; the portal's data model changed during implementation review, and the +> spec's [Amendments](../specs/2026-06-11-portal-service-design.md#amendments-2026-06-12) +> override it. As shipped: the directory is the HR-owned `hr_employee` +> collection served from an in-memory cache (`ListEmployees`-only store — no +> portal `users`/`sites` collections, no `EnsureIndexes`); `authServiceUrl` is +> derived from `PORTAL_AUTH_URL_TEMPLATE` (plus `PORTAL_CACHE_REFRESH_INTERVAL` +> config); a portal miss returns `account_not_ready` while +> `account_not_provisioned` is emitted only by the auth-service minting gate; +> `GET /readyz` exists alongside `/healthz`; and the dev fallback is synthesized +> (no seeded site record). Identity was also hardened during review: +> `Claims.Account()` trusts **only** `preferred_username` — Task 2's `name` +> fallback snippets are obsolete and must not be implemented. Do not implement +> from this plan without reading the amendments first. + +**Goal:** Build `portal-service` (post-Keycloak site discovery: account → `{account, employeeId, authServiceUrl, natsUrl, siteId}`), add an enforced provisioning gate to auth-service, and rewire chat-frontend so no user ever types a Site ID. + +**Architecture:** Discovery directory — portal validates the SSO token, reads a portal-owned Mongo directory (`users` + `sites`), and returns connection coordinates; the frontend then calls the resolved auth-service directly (refresh included) and dials the resolved NATS. Enforcement lives in auth-service: mint only when `{account, siteId: SITE_ID}` exists in the site's `users` collection. Shared middleware moves to `pkg/ginutil`; account derivation becomes `pkg/oidc Claims.Account()`. + +**Tech Stack:** Go 1.25, Gin, mongo-driver v2, `pkg/errcode` + `errhttp`, mockgen (`go.uber.org/mock`), testify, testcontainers via `pkg/testutil`; React + vitest on the frontend. + +**Spec:** `docs/superpowers/specs/2026-06-11-portal-service-design.md` +**Branch:** `claude/eager-einstein-7u2je6` (already checked out; never push elsewhere) + +**Conventions that apply to every task:** +- Always use `make` targets, never raw `go` commands — except the coverage commands CLAUDE.md itself prescribes and one-package test runs during red/green, where `go test .//...` keeps the loop fast (the Makefile's `test` target is the same command repo-wide). +- TDD: write the failing test, run it, watch it fail, implement, watch it pass, commit. +- If `mockgen` is missing: `go install go.uber.org/mock/mockgen@$(go list -m -f '{{.Version}}' go.uber.org/mock)`. +- Integration tests need Docker. If the execution environment lacks Docker, still write the tests, verify they compile with `go vet -tags integration .//...`, note it in the commit, and rely on CI's integration job. +- Commit messages must NOT mention AI/session/model identifiers. + +--- + +## Implementation Tasks + +### Task 1: `account_not_provisioned` reason in `pkg/errcode` + +**Files:** +- Create: `pkg/errcode/codes_portal.go` +- Modify: `pkg/errcode/codes_test.go:8-20` (the `allReasons` slice) + +- [ ] **Step 1: Write the failing test (compile-red)** + +In `pkg/errcode/codes_test.go`, add `PortalAccountNotProvisioned` to the `allReasons` slice, after the `AuthTokenExpired, ...` line: + +```go + AuthTokenExpired, AuthInvalidToken, AuthInvalidRequest, AuthInvalidNKey, AuthMissingFields, + PortalAccountNotProvisioned, + RequestIDRequired, +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `go test ./pkg/errcode/...` +Expected: FAIL — `undefined: PortalAccountNotProvisioned` + +- [ ] **Step 3: Write the implementation** + +Create `pkg/errcode/codes_portal.go`: + +```go +package errcode + +// Reasons emitted by portal-service and the auth-service minting gate. +const ( + // PortalAccountNotProvisioned: the account authenticated successfully but + // is not provisioned in the chat directory (portal lookup) or not + // provisioned for the target site (auth-service minting gate). + PortalAccountNotProvisioned Reason = "account_not_provisioned" +) +``` + +- [ ] **Step 4: Run to verify it passes** + +Run: `go test ./pkg/errcode/...` +Expected: PASS (snake-case + uniqueness tests cover the new constant) + +- [ ] **Step 5: Commit** + +```bash +git add pkg/errcode/codes_portal.go pkg/errcode/codes_test.go +git commit -m "feat(errcode): add account_not_provisioned reason for portal/auth provisioning" +``` + +--- + +### Task 2: `Claims.Account()` in `pkg/oidc` + +**Files:** +- Modify: `pkg/oidc/oidc.go` (add method after the `Claims` struct, ~line 28) +- Modify: `pkg/oidc/oidc_test.go` (append test) + +- [ ] **Step 1: Write the failing test** + +Append to `pkg/oidc/oidc_test.go`: + +```go +func TestClaims_Account(t *testing.T) { + tests := []struct { + name string + claims Claims + want string + }{ + {"preferred_username wins", Claims{PreferredUsername: "alice", Name: "Alice W"}, "alice"}, + {"falls back to name", Claims{Name: "alice"}, "alice"}, + {"both blank is blank", Claims{}, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.claims.Account(); got != tt.want { + t.Errorf("Account() = %q, want %q", got, tt.want) + } + }) + } +} +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `go test ./pkg/oidc/...` +Expected: FAIL — `tt.claims.Account undefined` + +- [ ] **Step 3: Implement** + +In `pkg/oidc/oidc.go`, directly below the `Claims` struct definition: + +```go +// Account returns the chat account carried by the claims: +// preferred_username, falling back to name. An empty result means the token +// carries no usable account and the caller must reject it. +func (c Claims) Account() string { + if c.PreferredUsername != "" { + return c.PreferredUsername + } + return c.Name +} +``` + +- [ ] **Step 4: Run to verify it passes** + +Run: `go test ./pkg/oidc/...` +Expected: PASS + +- [ ] **Step 5: Use it in auth-service (keep behavior identical)** + +In `auth-service/handler.go:152-155`, replace: + +```go + account := claims.PreferredUsername + if account == "" { + account = claims.Name + } +``` + +with: + +```go + account := claims.Account() +``` + +- [ ] **Step 6: Run auth-service tests** + +Run: `go test ./auth-service/...` +Expected: PASS (behavior unchanged; `TestHandleAuth_ValidToken` et al. still green) + +- [ ] **Step 7: Commit** + +```bash +git add pkg/oidc/oidc.go pkg/oidc/oidc_test.go auth-service/handler.go +git commit -m "feat(oidc): add Claims.Account() and use it in auth-service" +``` + +--- + +### Task 3: extract Gin middleware into `pkg/ginutil` + +**Files:** +- Create: `pkg/ginutil/middleware.go` (content = `auth-service/middleware.go` with exported names) +- Create: `pkg/ginutil/middleware_test.go` (content = `auth-service/middleware_test.go`, renamed calls) +- Modify: `auth-service/main.go:80-82` +- Delete: `auth-service/middleware.go`, `auth-service/middleware_test.go` + +- [ ] **Step 1: Write the failing tests (move the test file first)** + +Create `pkg/ginutil/middleware_test.go`: copy `auth-service/middleware_test.go` verbatim, then change the package line to `package ginutil` and rename every middleware constructor call: `requestIDMiddleware()` → `RequestID()` (4 occurrences), `accessLogMiddleware()` → `AccessLog()` (1), `corsMiddleware()` → `CORS()` (2). Rename the test functions `TestRequestIDMiddleware_*` → `TestRequestID_*`, `TestAccessLogMiddleware_*` → `TestAccessLog_*`, `TestCorsMiddleware_*` → `TestCORS_*` (same bodies). + +- [ ] **Step 2: Run to verify it fails** + +Run: `go test ./pkg/ginutil/...` +Expected: FAIL — `undefined: RequestID` (package has tests but no source) + +- [ ] **Step 3: Implement `pkg/ginutil/middleware.go`** + +Copy `auth-service/middleware.go` and adapt the header + names: + +```go +// Package ginutil holds the Gin middleware shared by the HTTP services +// (auth-service, portal-service): request-ID propagation, JSON access +// logging, and the wildcard-CORS policy for browser-called endpoints. +package ginutil + +import ( + "log/slog" + "net/http" + "time" + + "github.com/gin-gonic/gin" + + "github.com/hmchangw/chat/pkg/idgen" + "github.com/hmchangw/chat/pkg/natsutil" +) + +// RequestID funnels HTTP X-Request-ID through idgen.ResolveRequestID +// (the same primitive the NATS path uses via natsutil.StampRequestID) so the +// mint-vs-pass-through policy has a single owner. Missing → silent mint; +// malformed → mint + Warn preserving the inbound value for traceability. +func RequestID() gin.HandlerFunc { + return func(c *gin.Context) { + inbound := c.GetHeader(natsutil.RequestIDHeader) + id, replaced := idgen.ResolveRequestID(inbound) + c.Set("request_id", id) + c.Request = c.Request.WithContext(natsutil.WithRequestID(c.Request.Context(), id)) + c.Header(natsutil.RequestIDHeader, id) + if replaced { + slog.WarnContext(c.Request.Context(), "minted request_id (inbound invalid)", "inbound", inbound, "path", c.Request.URL.Path) + } + c.Next() + } +} + +// CORS allows browser clients from any origin to call the API and +// short-circuits the preflight OPTIONS request with 204. The wildcard origin +// is incompatible with credentialed requests (cookies / Authorization), but +// these endpoints use neither — they accept a JSON body and return a JWT or +// connection coordinates. +func CORS() gin.HandlerFunc { + return func(c *gin.Context) { + c.Header("Access-Control-Allow-Origin", "*") + c.Header("Access-Control-Allow-Methods", "GET, POST, OPTIONS") + c.Header("Access-Control-Allow-Headers", "Content-Type, Authorization, X-Request-ID") + c.Header("Access-Control-Max-Age", "300") + if c.Request.Method == http.MethodOptions { + c.AbortWithStatus(http.StatusNoContent) + return + } + c.Next() + } +} + +// AccessLog logs method, path, status, and latency for each request. +func AccessLog() gin.HandlerFunc { + return func(c *gin.Context) { + start := time.Now() + c.Next() + + slog.Info("request", + "request_id", c.GetString("request_id"), + "method", c.Request.Method, + "path", c.Request.URL.Path, + "status", c.Writer.Status(), + "latency_ms", time.Since(start).Milliseconds(), + "client_ip", c.ClientIP(), + ) + } +} +``` + +- [ ] **Step 4: Run to verify it passes** + +Run: `go test ./pkg/ginutil/...` +Expected: PASS (all 7 moved tests) + +- [ ] **Step 5: Rewire auth-service and delete the originals** + +In `auth-service/main.go`: add `"github.com/hmchangw/chat/pkg/ginutil"` to imports, replace lines 80-82: + +```go + r.Use(ginutil.RequestID()) + r.Use(ginutil.AccessLog()) + r.Use(ginutil.CORS()) +``` + +Then: `git rm auth-service/middleware.go auth-service/middleware_test.go` + +- [ ] **Step 6: Verify auth-service still builds and passes** + +Run: `go test ./auth-service/... && make build SERVICE=auth-service` +Expected: PASS, clean build + +- [ ] **Step 7: Commit** + +```bash +git add pkg/ginutil auth-service/main.go +git commit -m "refactor: extract shared Gin middleware into pkg/ginutil" +``` + +--- + +### Task 4: auth-service provisioning gate + +**Files:** +- Create: `auth-service/store.go`, `auth-service/store_mongo.go` +- Generate: `auth-service/mock_store_test.go` +- Modify: `auth-service/handler.go` (struct, option, gate in `HandleAuth`) +- Modify: `auth-service/main.go` (config + wiring + shutdown) +- Modify: `auth-service/handler_test.go` (new tests), `auth-service/integration_test.go` (TestMain + store test) +- Modify: `auth-service/deploy/docker-compose.yml` (gate env) + +- [ ] **Step 1: Define the store interface (scaffolding for the mock)** + +Create `auth-service/store.go`: + +```go +package main + +import "context" + +//go:generate mockgen -source=store.go -destination=mock_store_test.go -package=main + +// ProvisionStore answers whether an account is provisioned for this site. +type ProvisionStore interface { + // AccountProvisioned reports whether the account exists in this site's + // users collection homed on siteID. The compound predicate matters: the + // per-site users collection also holds other sites' users (for + // cross-site rooms), so existence alone is not provisioning. + AccountProvisioned(ctx context.Context, account, siteID string) (bool, error) +} +``` + +- [ ] **Step 2: Generate the mock** + +Run: `make generate SERVICE=auth-service` +Expected: `auth-service/mock_store_test.go` created (contains `MockProvisionStore`) + +- [ ] **Step 3: Write the failing handler tests** + +Append to `auth-service/handler_test.go` (also add `"errors"` and `"go.uber.org/mock/gomock"` to its imports): + +```go +func TestHandleAuth_ProvisionGate(t *testing.T) { + signingKP := mustAccountKP(t) + + tests := []struct { + name string + account string + provisioned bool + storeErr error + wantStatus int + wantReason errcode.Reason + }{ + {"provisioned account mints", "alice", true, nil, http.StatusOK, ""}, + {"unprovisioned account refused", "mallory", false, nil, http.StatusForbidden, errcode.PortalAccountNotProvisioned}, + // ivan exists in the users collection but is homed on site-b; this + // auth-service runs site-a — the compound predicate refuses him. + {"wrong-site account refused", "ivan", false, nil, http.StatusForbidden, errcode.PortalAccountNotProvisioned}, + {"store error fails closed", "alice", false, errors.New("mongo down"), http.StatusInternalServerError, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockProvisionStore(ctrl) + store.EXPECT().AccountProvisioned(gomock.Any(), tt.account, "site-a"). + Return(tt.provisioned, tt.storeErr) + + validator := &fakeValidator{account: tt.account, subject: "uuid-" + tt.account} + handler := NewAuthHandler(validator, signingKP, 2*time.Hour, false, + WithProvisionGate(store, "site-a")) + router := setupRouter(t, handler) + + userPub := mustUserNKey(t) + body := `{"ssoToken":"valid-token","natsPublicKey":"` + userPub + `"}` + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/auth", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + + assert.Equal(t, tt.wantStatus, w.Code) + if tt.wantReason != "" { + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeForbidden) + errtest.AssertReason(t, w.Body.Bytes(), tt.wantReason) + } + }) + } +} + +func TestHandleAuth_ProvisionGate_SkippedInDevMode(t *testing.T) { + signingKP := mustAccountKP(t) + ctrl := gomock.NewController(t) + store := NewMockProvisionStore(ctrl) // no EXPECT — any call fails the test + + handler := NewAuthHandler(nil, signingKP, 2*time.Hour, true, + WithProvisionGate(store, "site-a")) + router := setupRouter(t, handler) + + userPub := mustUserNKey(t) + body := `{"account":"anyone","natsPublicKey":"` + userPub + `"}` + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/auth", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) +} +``` + +- [ ] **Step 4: Run to verify they fail** + +Run: `go test ./auth-service/...` +Expected: FAIL — `undefined: WithProvisionGate` + +- [ ] **Step 5: Implement the gate in `auth-service/handler.go`** + +Add fields to `AuthHandler` (after `devMode bool`): + +```go + store ProvisionStore // nil = gate disabled (dev mode or REQUIRE_PROVISIONED=false) + siteID string +``` + +Add the option (after `WithRandFloat`): + +```go +// WithProvisionGate enables the minting gate: an account must exist in this +// site's users collection (account + siteID compound predicate) before a JWT +// is signed. Dev mode never consults the gate. +func WithProvisionGate(store ProvisionStore, siteID string) Option { + return func(h *AuthHandler) { + h.store = store + h.siteID = siteID + } +} +``` + +In `HandleAuth`, after `ctx = errcode.WithLogValues(ctx, "account", account)` and before `natsJWT, err := h.signNATSJWT(...)`: + +```go + if h.store != nil { + provisioned, err := h.store.AccountProvisioned(ctx, account, h.siteID) + if err != nil { + errhttp.Write(ctx, c, fmt.Errorf("check account provisioning: %w", err)) + return + } + if !provisioned { + errhttp.Write(ctx, c, errcode.Forbidden("account not provisioned for this site", + errcode.WithReason(errcode.PortalAccountNotProvisioned))) + return + } + } +``` + +(`handleDevAuth` is untouched — dev mode never reaches the gate.) + +- [ ] **Step 6: Run to verify they pass** + +Run: `go test ./auth-service/...` +Expected: PASS (new + all pre-existing tests) + +- [ ] **Step 7: Implement the Mongo store** + +Create `auth-service/store_mongo.go`: + +```go +package main + +import ( + "context" + "errors" + "fmt" + + "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/mongo" + "go.mongodb.org/mongo-driver/v2/mongo/options" +) + +type mongoProvisionStore struct { + users *mongo.Collection +} + +func newMongoProvisionStore(db *mongo.Database) *mongoProvisionStore { + return &mongoProvisionStore{users: db.Collection("users")} +} + +// AccountProvisioned checks the compound {account, siteId} predicate against +// the site's users collection. Projection keeps the read index-only-ish; a +// missing document is a clean false, anything else is an infra error. +func (s *mongoProvisionStore) AccountProvisioned(ctx context.Context, account, siteID string) (bool, error) { + err := s.users.FindOne(ctx, + bson.M{"account": account, "siteId": siteID}, + options.FindOne().SetProjection(bson.M{"_id": 1}), + ).Err() + if errors.Is(err, mongo.ErrNoDocuments) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("query users for provisioning: %w", err) + } + return true, nil +} +``` + +- [ ] **Step 8: Write the integration test** + +In `auth-service/integration_test.go`, add imports (`context`, `bson`, `testutil`) and append: + +```go +func TestMain(m *testing.M) { testutil.RunTests(m) } + +func TestMongoProvisionStore_AccountProvisioned(t *testing.T) { + db := testutil.MongoDB(t, "authsvc") + store := newMongoProvisionStore(db) + ctx := context.Background() + + _, err := db.Collection("users").InsertMany(ctx, []any{ + bson.M{"_id": "u-alice", "account": "alice", "siteId": "site-a"}, + bson.M{"_id": "u-ivan", "account": "ivan", "siteId": "site-b"}, + }) + require.NoError(t, err) + + tests := []struct { + name string + account string + siteID string + want bool + }{ + {"provisioned on this site", "alice", "site-a", true}, + {"homed on another site", "ivan", "site-a", false}, + {"unknown account", "carol", "site-a", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := store.AccountProvisioned(ctx, tt.account, tt.siteID) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} +``` + +Import block addition for that file: + +```go + "context" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/hmchangw/chat/pkg/testutil" +``` + +- [ ] **Step 9: Run the integration test** + +Run: `make test-integration SERVICE=auth-service` +Expected: PASS (needs Docker; see Conventions if unavailable) + +- [ ] **Step 10: Wire config in `auth-service/main.go`** + +Add to the `config` struct: + +```go + // Provisioning gate — see "Enforced provisioning" in the design spec. + SiteID string `env:"SITE_ID"` + RequireProvisioned bool `env:"REQUIRE_PROVISIONED" envDefault:"true"` + MongoURI string `env:"MONGO_URI"` + MongoDB string `env:"MONGO_DB" envDefault:"chat"` + MongoUsername string `env:"MONGO_USERNAME" envDefault:""` + MongoPassword string `env:"MONGO_PASSWORD" envDefault:""` +``` + +In `run()`, build the options list and gate wiring. Replace the two `NewAuthHandler(...)` calls so both use a shared `opts` slice, and connect Mongo only when the gate is active: + +```go + opts := []Option{WithJitter(cfg.NATSJWTExpiryJitter)} + + var mongoClient *mongo.Client + if !cfg.DevMode && cfg.RequireProvisioned { + if cfg.MongoURI == "" || cfg.SiteID == "" { + return fmt.Errorf("MONGO_URI and SITE_ID are required when REQUIRE_PROVISIONED is true (set REQUIRE_PROVISIONED=false to defer the gate)") + } + mongoClient, err = mongoutil.Connect(ctx, cfg.MongoURI, cfg.MongoUsername, cfg.MongoPassword) + if err != nil { + return fmt.Errorf("connect mongo for provisioning gate: %w", err) + } + opts = append(opts, WithProvisionGate(newMongoProvisionStore(mongoClient.Database(cfg.MongoDB)), cfg.SiteID)) + } +``` + +Dev branch becomes `NewAuthHandler(nil, signingKP, cfg.NATSJWTExpiry, true, opts...)`; prod branch `NewAuthHandler(oidcValidator, signingKP, cfg.NATSJWTExpiry, false, opts...)`. Imports to add: `"go.mongodb.org/mongo-driver/v2/mongo"`, `"github.com/hmchangw/chat/pkg/mongoutil"`. In the `shutdown.Wait` closure, after `srv.Shutdown(ctx)` (HTTP first, then DBs): + +```go + err := srv.Shutdown(ctx) + if mongoClient != nil { + mongoutil.Disconnect(ctx, mongoClient) + } + return err +``` + +- [ ] **Step 11: Update `auth-service/deploy/docker-compose.yml`** + +Add to the `environment:` list (keeps the documented "flip DEV_MODE to false" path working): + +```yaml + # Provisioning gate (active only when DEV_MODE=false). + - SITE_ID=site-local + - REQUIRE_PROVISIONED=${REQUIRE_PROVISIONED:-true} + - MONGO_URI=mongodb://mongodb:27017 + - MONGO_DB=chat +``` + +- [ ] **Step 12: Full verify + commit** + +Run: `go test ./auth-service/... && make build SERVICE=auth-service && make lint` +Expected: PASS, clean + +```bash +git add auth-service +git commit -m "feat(auth-service): enforce account provisioning at the minting gate + +Mint a NATS JWT only when {account, siteId: SITE_ID} exists in the +site's users collection; 403 account_not_provisioned otherwise, 500 +fail-closed on store errors. Gate is skipped in DEV_MODE and can be +deferred with REQUIRE_PROVISIONED=false for staged rollout." +``` + +--- + +### Task 5: portal-service store (TDD via integration tests) + +**Files:** +- Create: `portal-service/store.go`, `portal-service/integration_test.go`, `portal-service/store_mongo.go` +- Generate: `portal-service/mock_store_test.go` + +- [ ] **Step 1: Define the contract** + +Create `portal-service/store.go`: + +```go +package main + +import ( + "context" + "errors" + + "github.com/hmchangw/chat/pkg/model" +) + +var ( + ErrUserNotFound = errors.New("user not found") // FindUserByAccount: no directory entry + ErrSiteNotFound = errors.New("site not found") // FindSiteByID: no sites entry +) + +//go:generate mockgen -source=store.go -destination=mock_store_test.go -package=main + +// site is one row of the ops-owned sites collection: the connection +// coordinates for a single site. +type site struct { + ID string `json:"siteId" bson:"_id"` + AuthServiceURL string `json:"authServiceUrl" bson:"authServiceUrl"` + NATSURL string `json:"natsUrl" bson:"natsUrl"` +} + +// DirectoryStore reads the global account→site directory and the per-site +// connection coordinates. +type DirectoryStore interface { + FindUserByAccount(ctx context.Context, account string) (*model.User, error) + FindSiteByID(ctx context.Context, siteID string) (*site, error) + EnsureIndexes(ctx context.Context) error +} +``` + +- [ ] **Step 2: Write the failing integration tests** + +Create `portal-service/integration_test.go`: + +```go +//go:build integration + +package main + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/mongo" + + "github.com/hmchangw/chat/pkg/testutil" +) + +func TestMain(m *testing.M) { testutil.RunTests(m) } + +func seedDirectory(t *testing.T) *mongoDirectoryStore { + t.Helper() + db := testutil.MongoDB(t, "portal") + store := newMongoDirectoryStore(db) + ctx := context.Background() + require.NoError(t, store.EnsureIndexes(ctx)) + + _, err := db.Collection("users").InsertOne(ctx, + bson.M{"_id": "u-alice", "account": "alice", "siteId": "site-a", "employeeId": "E001"}) + require.NoError(t, err) + _, err = db.Collection("sites").InsertOne(ctx, + bson.M{"_id": "site-a", "authServiceUrl": "https://auth.site-a.example.com", "natsUrl": "wss://nats.site-a.example.com"}) + require.NoError(t, err) + return store +} + +func TestMongoDirectoryStore_FindUserByAccount(t *testing.T) { + store := seedDirectory(t) + ctx := context.Background() + + u, err := store.FindUserByAccount(ctx, "alice") + require.NoError(t, err) + assert.Equal(t, "alice", u.Account) + assert.Equal(t, "site-a", u.SiteID) + assert.Equal(t, "E001", u.EmployeeID) + + _, err = store.FindUserByAccount(ctx, "nobody") + assert.ErrorIs(t, err, ErrUserNotFound) +} + +func TestMongoDirectoryStore_FindSiteByID(t *testing.T) { + store := seedDirectory(t) + ctx := context.Background() + + s, err := store.FindSiteByID(ctx, "site-a") + require.NoError(t, err) + assert.Equal(t, "site-a", s.ID) + assert.Equal(t, "https://auth.site-a.example.com", s.AuthServiceURL) + assert.Equal(t, "wss://nats.site-a.example.com", s.NATSURL) + + _, err = store.FindSiteByID(ctx, "site-z") + assert.ErrorIs(t, err, ErrSiteNotFound) +} + +func TestMongoDirectoryStore_EnsureIndexes(t *testing.T) { + store := seedDirectory(t) + ctx := context.Background() + + // Idempotent: a second call must not error. + require.NoError(t, store.EnsureIndexes(ctx)) + + // Uniqueness: a second document with the same account must be rejected. + _, err := store.users.InsertOne(ctx, + bson.M{"_id": "u-alice-dup", "account": "alice", "siteId": "site-b"}) + require.Error(t, err) + assert.True(t, mongo.IsDuplicateKeyError(err)) +} +``` + +- [ ] **Step 3: Run to verify they fail** + +Run: `go vet -tags integration ./portal-service/...` +Expected: FAIL — `undefined: newMongoDirectoryStore` + +- [ ] **Step 4: Implement `portal-service/store_mongo.go`** + +```go +package main + +import ( + "context" + "errors" + "fmt" + + "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/mongo" + "go.mongodb.org/mongo-driver/v2/mongo/options" + + "github.com/hmchangw/chat/pkg/model" +) + +type mongoDirectoryStore struct { + users *mongo.Collection + sites *mongo.Collection +} + +func newMongoDirectoryStore(db *mongo.Database) *mongoDirectoryStore { + return &mongoDirectoryStore{ + users: db.Collection("users"), + sites: db.Collection("sites"), + } +} + +func (s *mongoDirectoryStore) FindUserByAccount(ctx context.Context, account string) (*model.User, error) { + var u model.User + err := s.users.FindOne(ctx, + bson.M{"account": account}, + options.FindOne().SetProjection(bson.M{"_id": 1, "account": 1, "siteId": 1, "employeeId": 1}), + ).Decode(&u) + if errors.Is(err, mongo.ErrNoDocuments) { + return nil, fmt.Errorf("find user %q: %w", account, ErrUserNotFound) + } + if err != nil { + return nil, fmt.Errorf("find user %q: %w", account, err) + } + return &u, nil +} + +func (s *mongoDirectoryStore) FindSiteByID(ctx context.Context, siteID string) (*site, error) { + var st site + err := s.sites.FindOne(ctx, bson.M{"_id": siteID}).Decode(&st) + if errors.Is(err, mongo.ErrNoDocuments) { + return nil, fmt.Errorf("find site %q: %w", siteID, ErrSiteNotFound) + } + if err != nil { + return nil, fmt.Errorf("find site %q: %w", siteID, err) + } + return &st, nil +} + +// EnsureIndexes creates the unique account index the lookup path depends on. +// Mongo treats index creation as idempotent when the key spec matches. +func (s *mongoDirectoryStore) EnsureIndexes(ctx context.Context) error { + _, err := s.users.Indexes().CreateOne(ctx, mongo.IndexModel{ + Keys: bson.D{{Key: "account", Value: 1}}, + Options: options.Index().SetUnique(true), + }) + if err != nil { + return fmt.Errorf("ensure users (account) unique index: %w", err) + } + return nil +} +``` + +- [ ] **Step 5: Run to verify they pass** + +Run: `make test-integration SERVICE=portal-service` +Expected: PASS (3 tests) + +- [ ] **Step 6: Generate the mock for the handler task** + +Run: `make generate SERVICE=portal-service` +Expected: `portal-service/mock_store_test.go` created (`MockDirectoryStore`) + +- [ ] **Step 7: Commit** + +```bash +git add portal-service +git commit -m "feat(portal-service): Mongo directory store (users + sites) with integration tests" +``` + +--- + +### Task 6: portal-service handler + routes + +**Files:** +- Create: `portal-service/handler_test.go` (first), `portal-service/handler.go`, `portal-service/routes.go` + +- [ ] **Step 1: Write the failing handler tests** + +Create `portal-service/handler_test.go`: + +```go +package main + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/hmchangw/chat/pkg/errcode" + "github.com/hmchangw/chat/pkg/errcode/errtest" + "github.com/hmchangw/chat/pkg/model" + pkgoidc "github.com/hmchangw/chat/pkg/oidc" +) + +// fakeValidator implements TokenValidator for testing. +type fakeValidator struct { + account string + name string + expired bool + invalid bool +} + +func (f *fakeValidator) Validate(_ context.Context, _ string) (pkgoidc.Claims, error) { + if f.expired { + return pkgoidc.Claims{}, pkgoidc.ErrTokenExpired + } + if f.invalid { + return pkgoidc.Claims{}, fmt.Errorf("oidc token verification failed: invalid signature") + } + return pkgoidc.Claims{PreferredUsername: f.account, Name: f.name}, nil +} + +func setupRouter(t *testing.T, h *PortalHandler) *gin.Engine { + t.Helper() + gin.SetMode(gin.TestMode) + r := gin.New() + registerRoutes(r, h) + return r +} + +func postLookup(t *testing.T, r *gin.Engine, body string) *httptest.ResponseRecorder { + t.Helper() + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/lookup", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + return w +} + +var testSite = &site{ + ID: "site-a", + AuthServiceURL: "https://auth.site-a.example.com", + NATSURL: "wss://nats.site-a.example.com", +} + +func TestHandleLookup_HappyPath(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + store.EXPECT().FindUserByAccount(gomock.Any(), "alice"). + Return(&model.User{ID: "u-alice", Account: "alice", SiteID: "site-a", EmployeeID: "E001"}, nil) + store.EXPECT().FindSiteByID(gomock.Any(), "site-a").Return(testSite, nil) + + h := NewPortalHandler(&fakeValidator{account: "alice"}, store, false, "site-local") + w := postLookup(t, setupRouter(t, h), `{"ssoToken":"valid-token"}`) + + require.Equal(t, http.StatusOK, w.Code) + var resp lookupResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(t, lookupResponse{ + Account: "alice", + EmployeeID: "E001", + AuthServiceURL: "https://auth.site-a.example.com", + NATSURL: "wss://nats.site-a.example.com", + SiteID: "site-a", + }, resp) +} + +func TestHandleLookup_AccountFallsBackToNameClaim(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + store.EXPECT().FindUserByAccount(gomock.Any(), "alice"). + Return(&model.User{Account: "alice", SiteID: "site-a"}, nil) + store.EXPECT().FindSiteByID(gomock.Any(), "site-a").Return(testSite, nil) + + h := NewPortalHandler(&fakeValidator{name: "alice"}, store, false, "site-local") + w := postLookup(t, setupRouter(t, h), `{"ssoToken":"valid-token"}`) + assert.Equal(t, http.StatusOK, w.Code) +} + +func TestHandleLookup_TokenErrors(t *testing.T) { + tests := []struct { + name string + validator *fakeValidator + wantReason errcode.Reason + }{ + {"expired token", &fakeValidator{expired: true}, errcode.AuthTokenExpired}, + {"invalid token", &fakeValidator{invalid: true}, errcode.AuthInvalidToken}, + {"blank account claim", &fakeValidator{}, errcode.AuthInvalidToken}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) // no EXPECT — must not be touched + + h := NewPortalHandler(tt.validator, store, false, "site-local") + w := postLookup(t, setupRouter(t, h), `{"ssoToken":"tok"}`) + + assert.Equal(t, http.StatusUnauthorized, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeUnauthenticated) + errtest.AssertReason(t, w.Body.Bytes(), tt.wantReason) + }) + } +} + +func TestHandleLookup_MissingBody(t *testing.T) { + ctrl := gomock.NewController(t) + h := NewPortalHandler(&fakeValidator{account: "alice"}, NewMockDirectoryStore(ctrl), false, "site-local") + router := setupRouter(t, h) + + for _, body := range []string{`{}`, ``, `{"account":"alice"}`} { + w := postLookup(t, router, body) + assert.Equal(t, http.StatusBadRequest, w.Code, "body: %s", body) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeBadRequest) + errtest.AssertReason(t, w.Body.Bytes(), errcode.AuthMissingFields) + } +} + +func TestHandleLookup_AccountNotProvisioned(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + store.EXPECT().FindUserByAccount(gomock.Any(), "mallory"). + Return(nil, fmt.Errorf("find user: %w", ErrUserNotFound)) + + h := NewPortalHandler(&fakeValidator{account: "mallory"}, store, false, "site-local") + w := postLookup(t, setupRouter(t, h), `{"ssoToken":"tok"}`) + + assert.Equal(t, http.StatusForbidden, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeForbidden) + errtest.AssertReason(t, w.Body.Bytes(), errcode.PortalAccountNotProvisioned) +} + +func TestHandleLookup_StoreErrors(t *testing.T) { + tests := []struct { + name string + setup func(store *MockDirectoryStore) + }{ + {"user query fails", func(store *MockDirectoryStore) { + store.EXPECT().FindUserByAccount(gomock.Any(), "alice"). + Return(nil, errors.New("mongo down")) + }}, + {"site missing for known user", func(store *MockDirectoryStore) { + store.EXPECT().FindUserByAccount(gomock.Any(), "alice"). + Return(&model.User{Account: "alice", SiteID: "site-gone"}, nil) + store.EXPECT().FindSiteByID(gomock.Any(), "site-gone"). + Return(nil, fmt.Errorf("find site: %w", ErrSiteNotFound)) + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + tt.setup(store) + + h := NewPortalHandler(&fakeValidator{account: "alice"}, store, false, "site-local") + w := postLookup(t, setupRouter(t, h), `{"ssoToken":"tok"}`) + + assert.Equal(t, http.StatusInternalServerError, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeInternal) + assert.NotContains(t, w.Body.String(), "mongo down", "raw cause must not leak") + }) + } +} + +func TestHandleLookup_DevMode(t *testing.T) { + t.Run("known account resolves normally", func(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + store.EXPECT().FindUserByAccount(gomock.Any(), "alice"). + Return(&model.User{Account: "alice", SiteID: "site-a", EmployeeID: "E001"}, nil) + store.EXPECT().FindSiteByID(gomock.Any(), "site-a").Return(testSite, nil) + + h := NewPortalHandler(nil, store, true, "site-local") + w := postLookup(t, setupRouter(t, h), `{"account":"alice"}`) + require.Equal(t, http.StatusOK, w.Code) + var resp lookupResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(t, "site-a", resp.SiteID) + }) + + t.Run("unknown account falls back to the dev site", func(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + store.EXPECT().FindUserByAccount(gomock.Any(), "newdev"). + Return(nil, fmt.Errorf("find user: %w", ErrUserNotFound)) + store.EXPECT().FindSiteByID(gomock.Any(), "site-local").Return(&site{ + ID: "site-local", AuthServiceURL: "http://localhost:8080", NATSURL: "ws://localhost:9222", + }, nil) + + h := NewPortalHandler(nil, store, true, "site-local") + w := postLookup(t, setupRouter(t, h), `{"account":"newdev"}`) + require.Equal(t, http.StatusOK, w.Code) + var resp lookupResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(t, lookupResponse{ + Account: "newdev", EmployeeID: "", + AuthServiceURL: "http://localhost:8080", NATSURL: "ws://localhost:9222", + SiteID: "site-local", + }, resp) + }) + + t.Run("fallback site unseeded is internal", func(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + store.EXPECT().FindUserByAccount(gomock.Any(), "newdev"). + Return(nil, fmt.Errorf("find user: %w", ErrUserNotFound)) + store.EXPECT().FindSiteByID(gomock.Any(), "site-local"). + Return(nil, fmt.Errorf("find site: %w", ErrSiteNotFound)) + + h := NewPortalHandler(nil, store, true, "site-local") + w := postLookup(t, setupRouter(t, h), `{"account":"newdev"}`) + assert.Equal(t, http.StatusInternalServerError, w.Code) + }) + + t.Run("missing account is bad request", func(t *testing.T) { + ctrl := gomock.NewController(t) + h := NewPortalHandler(nil, NewMockDirectoryStore(ctrl), true, "site-local") + w := postLookup(t, setupRouter(t, h), `{}`) + assert.Equal(t, http.StatusBadRequest, w.Code) + errtest.AssertReason(t, w.Body.Bytes(), errcode.AuthMissingFields) + }) +} + +func TestHandleHealth(t *testing.T) { + ctrl := gomock.NewController(t) + h := NewPortalHandler(&fakeValidator{}, NewMockDirectoryStore(ctrl), false, "site-local") + router := setupRouter(t, h) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/healthz", nil) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), "ok") +} +``` + +- [ ] **Step 2: Run to verify they fail** + +Run: `go test ./portal-service/...` +Expected: FAIL — `undefined: PortalHandler` / `registerRoutes` + +- [ ] **Step 3: Implement `portal-service/handler.go`** + +```go +package main + +import ( + "context" + "errors" + "fmt" + "net/http" + + "github.com/gin-gonic/gin" + + "github.com/hmchangw/chat/pkg/errcode" + "github.com/hmchangw/chat/pkg/errcode/errhttp" + "github.com/hmchangw/chat/pkg/model" + pkgoidc "github.com/hmchangw/chat/pkg/oidc" +) + +// TokenValidator validates an SSO token and returns OIDC claims. +type TokenValidator interface { + Validate(ctx context.Context, rawToken string) (pkgoidc.Claims, error) +} + +type lookupRequest struct { + SSOToken string `json:"ssoToken" binding:"required"` +} + +type devLookupRequest struct { + Account string `json:"account" binding:"required"` +} + +type lookupResponse struct { + Account string `json:"account"` + EmployeeID string `json:"employeeId"` + AuthServiceURL string `json:"authServiceUrl"` + NATSURL string `json:"natsUrl"` + SiteID string `json:"siteId"` +} + +// PortalHandler resolves a logged-in user's home-site connection coordinates +// from the portal directory. It is a discovery endpoint — the authoritative +// provisioning gate lives in auth-service. +type PortalHandler struct { + validator TokenValidator + store DirectoryStore + devMode bool + devFallbackSiteID string +} + +// NewPortalHandler creates a PortalHandler. In devMode the SSO token is not +// required and unknown accounts fall back to devFallbackSiteID. +func NewPortalHandler(validator TokenValidator, store DirectoryStore, devMode bool, devFallbackSiteID string) *PortalHandler { + return &PortalHandler{ + validator: validator, + store: store, + devMode: devMode, + devFallbackSiteID: devFallbackSiteID, + } +} + +// HandleLookup validates the SSO token, derives the account from its claims, +// and resolves the account's home-site connection coordinates. +func (h *PortalHandler) HandleLookup(c *gin.Context) { + if h.devMode { + h.handleDevLookup(c) + return + } + + ctx := errcode.WithLogValues(c.Request.Context(), "request_id", c.GetString("request_id")) + + var req lookupRequest + if err := c.ShouldBindJSON(&req); err != nil { + errhttp.Write(ctx, c, errcode.BadRequest("ssoToken is required", + errcode.WithReason(errcode.AuthMissingFields))) + return + } + + claims, err := h.validator.Validate(ctx, req.SSOToken) + if err != nil { + if errors.Is(err, pkgoidc.ErrTokenExpired) { + errhttp.Write(ctx, c, errcode.Unauthenticated("SSO token has expired, please re-login", + errcode.WithReason(errcode.AuthTokenExpired))) + return + } + errhttp.Write(ctx, c, errcode.Unauthenticated("invalid SSO token", + errcode.WithReason(errcode.AuthInvalidToken), + errcode.WithCause(err))) + return + } + + account := claims.Account() + if account == "" { + errhttp.Write(ctx, c, errcode.Unauthenticated("token missing account claim", + errcode.WithReason(errcode.AuthInvalidToken))) + return + } + ctx = errcode.WithLogValues(ctx, "account", account) + + h.resolve(ctx, c, account, false) +} + +// handleDevLookup accepts a raw account without OIDC, for local development. +func (h *PortalHandler) handleDevLookup(c *gin.Context) { + ctx := errcode.WithLogValues(c.Request.Context(), "request_id", c.GetString("request_id")) + + var req devLookupRequest + if err := c.ShouldBindJSON(&req); err != nil { + errhttp.Write(ctx, c, errcode.BadRequest("account is required", + errcode.WithReason(errcode.AuthMissingFields))) + return + } + ctx = errcode.WithLogValues(ctx, "account", req.Account) + + h.resolve(ctx, c, req.Account, true) +} + +// resolve maps account → directory user → site coordinates and writes the +// lookup response. devFallback substitutes the dev site for unknown accounts +// so local logins need no per-account seeding. +func (h *PortalHandler) resolve(ctx context.Context, c *gin.Context, account string, devFallback bool) { + user, err := h.store.FindUserByAccount(ctx, account) + switch { + case err == nil: + case errors.Is(err, ErrUserNotFound) && devFallback: + user = &model.User{Account: account, SiteID: h.devFallbackSiteID} + case errors.Is(err, ErrUserNotFound): + errhttp.Write(ctx, c, errcode.Forbidden("account not provisioned for chat", + errcode.WithReason(errcode.PortalAccountNotProvisioned))) + return + default: + errhttp.Write(ctx, c, fmt.Errorf("find user by account: %w", err)) + return + } + + st, err := h.store.FindSiteByID(ctx, user.SiteID) + if err != nil { + // Includes ErrSiteNotFound: a user pointing at an unconfigured site + // is an ops data bug, not a client error. + errhttp.Write(ctx, c, fmt.Errorf("find site %q: %w", user.SiteID, err)) + return + } + + c.JSON(http.StatusOK, lookupResponse{ + Account: user.Account, + EmployeeID: user.EmployeeID, + AuthServiceURL: st.AuthServiceURL, + NATSURL: st.NATSURL, + SiteID: st.ID, + }) +} + +func (h *PortalHandler) HandleHealth(c *gin.Context) { + c.JSON(http.StatusOK, gin.H{"status": "ok"}) +} +``` + +Create `portal-service/routes.go`: + +```go +package main + +import "github.com/gin-gonic/gin" + +func registerRoutes(r *gin.Engine, h *PortalHandler) { + r.POST("/lookup", h.HandleLookup) + r.GET("/healthz", h.HandleHealth) +} +``` + +- [ ] **Step 4: Run to verify they pass** + +Run: `go test ./portal-service/...` +Expected: PASS (all handler tests) + +- [ ] **Step 5: Commit** + +```bash +git add portal-service +git commit -m "feat(portal-service): POST /lookup handler — token-validated site discovery" +``` + +--- + +### Task 7: portal-service main, deploy files, docker-local wiring + +**Files:** +- Create: `portal-service/main.go`, `portal-service/deploy/Dockerfile`, `portal-service/deploy/docker-compose.yml`, `portal-service/deploy/azure-pipelines.yml` +- Modify: `docker-local/compose.services.yaml`, `docker-local/setup.sh:111-117` + +- [ ] **Step 1: Write `portal-service/main.go`** + +```go +package main + +import ( + "context" + "fmt" + "log/slog" + "net/http" + "os" + "time" + + "github.com/caarlos0/env/v11" + "github.com/gin-gonic/gin" + + "github.com/hmchangw/chat/pkg/ginutil" + "github.com/hmchangw/chat/pkg/mongoutil" + pkgoidc "github.com/hmchangw/chat/pkg/oidc" + "github.com/hmchangw/chat/pkg/shutdown" +) + +type config struct { + Port string `env:"PORT" envDefault:"8081"` + DevMode bool `env:"DEV_MODE" envDefault:"false"` + DevFallbackSiteID string `env:"PORTAL_DEV_FALLBACK_SITE_ID" envDefault:"site-local"` + + // OIDC settings — required when DEV_MODE is false. + OIDCIssuerURL string `env:"OIDC_ISSUER_URL"` + OIDCAudiences []string `env:"OIDC_AUDIENCES" envSeparator:","` + TLSSkipVerify bool `env:"TLS_SKIP_VERIFY" envDefault:"false"` + + MongoURI string `env:"MONGO_URI,required"` + MongoDB string `env:"MONGO_DB" envDefault:"portal"` + MongoUsername string `env:"MONGO_USERNAME" envDefault:""` + MongoPassword string `env:"MONGO_PASSWORD" envDefault:""` +} + +func main() { + slog.SetDefault(slog.New(slog.NewJSONHandler(os.Stdout, nil))) + + if err := run(); err != nil { + slog.Error("fatal error", "error", err) + os.Exit(1) + } +} + +func run() error { + cfg, err := env.ParseAs[config]() + if err != nil { + return fmt.Errorf("parse config: %w", err) + } + + ctx := context.Background() + + mongoClient, err := mongoutil.Connect(ctx, cfg.MongoURI, cfg.MongoUsername, cfg.MongoPassword) + if err != nil { + return fmt.Errorf("connect mongo: %w", err) + } + + store := newMongoDirectoryStore(mongoClient.Database(cfg.MongoDB)) + idxCtx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + if err := store.EnsureIndexes(idxCtx); err != nil { + return fmt.Errorf("ensure directory indexes: %w", err) + } + + var handler *PortalHandler + if cfg.DevMode { + slog.Info("dev mode enabled — OIDC validation disabled") + handler = NewPortalHandler(nil, store, true, cfg.DevFallbackSiteID) + } else { + if cfg.OIDCIssuerURL == "" || len(cfg.OIDCAudiences) == 0 { + return fmt.Errorf("OIDC_ISSUER_URL and OIDC_AUDIENCES are required when DEV_MODE is false") + } + oidcValidator, err := pkgoidc.NewValidator(ctx, pkgoidc.Config{ + IssuerURL: cfg.OIDCIssuerURL, + Audiences: cfg.OIDCAudiences, + TLSSkipVerify: cfg.TLSSkipVerify, + }) + if err != nil { + return fmt.Errorf("create oidc validator: %w", err) + } + slog.Info("oidc validator initialized", "issuer", cfg.OIDCIssuerURL) + handler = NewPortalHandler(oidcValidator, store, false, cfg.DevFallbackSiteID) + } + + gin.SetMode(gin.ReleaseMode) + r := gin.New() + r.Use(gin.Recovery()) + r.Use(ginutil.RequestID()) + r.Use(ginutil.AccessLog()) + r.Use(ginutil.CORS()) + registerRoutes(r, handler) + + addr := fmt.Sprintf(":%s", cfg.Port) + srv := &http.Server{ + Addr: addr, + Handler: r, + ReadTimeout: 10 * time.Second, + WriteTimeout: 10 * time.Second, + } + + srvErr := make(chan error, 1) + go func() { + slog.Info("portal service starting", "addr", addr) + srvErr <- srv.ListenAndServe() + }() + + shutdownDone := make(chan struct{}) + go func() { + defer close(shutdownDone) + shutdown.Wait(ctx, 25*time.Second, func(ctx context.Context) error { + slog.Info("shutting down portal service") + err := srv.Shutdown(ctx) + mongoutil.Disconnect(ctx, mongoClient) + return err + }) + }() + + err = <-srvErr + if err != nil && err != http.ErrServerClosed { + return fmt.Errorf("listen portal server: %w", err) + } + <-shutdownDone + + return nil +} +``` + +- [ ] **Step 2: Verify build + tests** + +Run: `make build SERVICE=portal-service && go test ./portal-service/...` +Expected: clean build, tests PASS + +- [ ] **Step 3: Write `portal-service/deploy/Dockerfile`** + +```dockerfile +FROM golang:1.25.11-alpine AS builder +WORKDIR /app +COPY go.mod go.sum ./ +RUN go mod download +COPY pkg/ pkg/ +COPY portal-service/ portal-service/ +RUN CGO_ENABLED=0 go build -o /portal-service ./portal-service/ + +FROM alpine:3.21 +RUN apk add --no-cache ca-certificates && adduser -D -u 10001 app +COPY --from=builder /portal-service /portal-service +USER app +ENTRYPOINT ["/portal-service"] +``` + +- [ ] **Step 4: Write `portal-service/deploy/docker-compose.yml`** + +```yaml +name: portal-service + +services: + # One-shot directory seed: the local site's connection coordinates plus the + # demo users that tools/seed-sample-data also creates in the site DB. + portal-seed: + image: mongo:8.2.9 + entrypoint: + - mongosh + - --host + - mongodb + - --quiet + - --eval + - | + const p = db.getSiblingDB('portal'); + p.sites.replaceOne( + { _id: 'site-local' }, + { authServiceUrl: 'http://localhost:8080', natsUrl: 'ws://localhost:9222' }, + { upsert: true }, + ); + [['alice', 'E001'], ['bob', 'E002']].forEach(([account, employeeId]) => + p.users.replaceOne( + { _id: 'portal-demo-' + account }, + { account: account, siteId: 'site-local', employeeId: employeeId }, + { upsert: true }, + ), + ); + print('portal seed: sites=' + p.sites.countDocuments() + ' users=' + p.users.countDocuments()); + networks: + - chat-local + + portal-service: + build: + context: ../.. + dockerfile: portal-service/deploy/Dockerfile + pull_policy: build + depends_on: + portal-seed: + condition: service_completed_successfully + ports: + - "8081:8081" + env_file: + - path: ../../docker-local/.env + required: false + environment: + - PORT=8081 + # Bypass OIDC; accept any account name. Flip to false to test OIDC. + - DEV_MODE=${DEV_MODE:-true} + - PORTAL_DEV_FALLBACK_SITE_ID=site-local + - MONGO_URI=mongodb://mongodb:27017 + - MONGO_DB=portal + - OIDC_ISSUER_URL=http://keycloak:8080/realms/chatapp + - OIDC_AUDIENCES=nats-chat + - TLS_SKIP_VERIFY=false + networks: + - chat-local + +networks: + chat-local: + external: true +``` + +- [ ] **Step 5: Write `portal-service/deploy/azure-pipelines.yml`** + +Copy `auth-service/deploy/azure-pipelines.yml` verbatim, replacing every `auth-service` with `portal-service` (trigger/pr path includes, `SERVICE_NAME`). + +- [ ] **Step 6: Wire docker-local** + +In `docker-local/compose.services.yaml`, add to the `include:` list (alphabetical, after `notification-worker`): + +```yaml + - ../portal-service/deploy/docker-compose.yml +``` + +In `docker-local/setup.sh`, replace the `.env.local` heredoc body (lines 112-116): + +```bash +if [ ! -f "$FRONTEND_ENV_FILE" ]; then + cat > "$FRONTEND_ENV_FILE" <" } +``` + +#### Success response + +`HTTP 200` + +| Field | Type | Notes | +|---|---|---| +| `account` | string | The `{account}` used in every NATS subject. | +| `employeeId` | string | From the portal directory; informational. | +| `authServiceUrl` | string | Base URL of the home site's auth-service — call `POST {authServiceUrl}/auth` next. | +| `natsUrl` | string | WebSocket URL of the home site's NATS. | +| `siteId` | string | The user's home site; scopes site-suffixed NATS subjects. | + +```json +{ + "account": "alice", + "employeeId": "E12345", + "authServiceUrl": "https://auth.site-a.example.com", + "natsUrl": "wss://nats.site-a.example.com", + "siteId": "site-a" +} +``` + +#### Error response + +See [Error envelope](#6-error-envelope-reference). HTTP statuses: + +| Status | `code` | `reason` | Example body | +|---|---|---|---| +| 400 | `bad_request` | `missing_fields` | `{ "code": "bad_request", "reason": "missing_fields", "error": "ssoToken is required" }` | +| 401 | `unauthenticated` | `sso_token_expired` | `{ "code": "unauthenticated", "reason": "sso_token_expired", "error": "SSO token has expired, please re-login" }` | +| 401 | `unauthenticated` | `invalid_sso_token` | `{ "code": "unauthenticated", "reason": "invalid_sso_token", "error": "invalid SSO token" }` | +| 403 | `forbidden` | `account_not_provisioned` | `{ "code": "forbidden", "reason": "account_not_provisioned", "error": "account not provisioned for chat" }` | +| 500 | `internal` | — | `{ "code": "internal", "error": "internal error" }` | + +#### Triggered events — success path + +`None — HTTP-only.` + +#### Triggered events — error path + +`None.` +```` + +- [ ] **Step 4: §6 catalog row** + +After the `missing_fields` row (~line 3509): + +```markdown +| `account_not_provisioned` | forbidden | portal-service `POST /lookup`; auth-service `POST /auth` minting gate | +``` + +- [ ] **Step 5: frontend reason catalog** + +In `chat-frontend/CLAUDE.md`, in the "Reasons emitted today" list, after the auth-service line: + +```markdown +- `account_not_provisioned` — portal-service lookup / auth-service minting gate (account passed Keycloak but is not provisioned for chat; show "contact your administrator" copy) +``` + +- [ ] **Step 6: Commit** + +```bash +git add docs/client-api.md chat-frontend/CLAUDE.md +git commit -m "docs(client-api): document POST /lookup and the account_not_provisioned reason" +``` + +--- + +### Task 9: frontend runtimeConfig + vite proxy + +**Files:** +- Modify: `chat-frontend/src/lib/runtimeConfig.js`, `chat-frontend/src/lib/runtimeConfig.test.js`, `chat-frontend/vite.config.js` + +- [ ] **Step 1: Write the failing tests** + +Append to `runtimeConfig.test.js` inside the existing `describe`: + +```js + it('PORTAL_URL defaults to localhost:8081', async () => { + const { PORTAL_URL } = await import('./runtimeConfig.js') + expect(PORTAL_URL).toBe('http://localhost:8081') + }) + + it('PORTAL_URL reads from window.__APP_CONFIG__', async () => { + window.__APP_CONFIG__ = { PORTAL_URL: 'https://portal.example.com' } + const { PORTAL_URL } = await import('./runtimeConfig.js') + expect(PORTAL_URL).toBe('https://portal.example.com') + }) + + it('no longer exports the retired static connection vars', async () => { + const mod = await import('./runtimeConfig.js') + expect(mod.AUTH_URL).toBeUndefined() + expect(mod.NATS_URL).toBeUndefined() + expect(mod.DEFAULT_SITE_ID).toBeUndefined() + }) +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `cd chat-frontend && npx vitest run src/lib/runtimeConfig.test.js` +Expected: FAIL (PORTAL_URL undefined; retired vars still exported) + +- [ ] **Step 3: Implement** + +Replace lines 6-13 of `runtimeConfig.js` (the `AUTH_URL`, `NATS_URL`, `DEFAULT_SITE_ID` exports) with: + +```js +export const PORTAL_URL = + runtime.PORTAL_URL || import.meta.env.VITE_PORTAL_URL || 'http://localhost:8081' +``` + +(`DEV_MODE`, `OIDC_ISSUER_URL`, `OIDC_CLIENT_ID` stay.) + +In `vite.config.js`, delete the dead proxy from the `server` block, leaving: + +```js + server: { + port: 3000, + }, +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `cd chat-frontend && npx vitest run src/lib/runtimeConfig.test.js` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add chat-frontend/src/lib/runtimeConfig.js chat-frontend/src/lib/runtimeConfig.test.js chat-frontend/vite.config.js +git commit -m "feat(frontend): replace static AUTH_URL/NATS_URL/DEFAULT_SITE_ID with PORTAL_URL" +``` + +> Note: `NatsContext.jsx` and `LoginPage.jsx` still import the removed exports after this commit, so the **suite-wide** test run is red until Tasks 10-12 land. The per-file runs above stay green; run the full suite only at Task 12 Step 6. If your pre-commit hook runs the full frontend suite, commit Tasks 9-12 together at Task 12 instead. + +--- + +### Task 10: `useJwtRefresh` takes a dynamic auth URL + +**Files:** +- Modify: `chat-frontend/src/context/NatsContext/useJwtRefresh.js`, `chat-frontend/src/context/NatsContext/useJwtRefresh.test.js` + +- [ ] **Step 1: Update the tests (red)** + +In `useJwtRefresh.test.js`, change the `setup` helper to pass a getter: + +```js +function setup({ ncRef = { current: { reconnect: vi.fn() } } } = {}) { + const view = renderHook(() => useJwtRefresh({ getAuthUrl: () => 'http://auth', ncRef })) + return { ...view, ncRef } +} +``` + +Add one new test at the end of the `describe`: + +```js + it('re-mints against the URL the getter returns at refresh time', async () => { + renewSsoToken.mockResolvedValue('fresh-sso') + global.fetch.mockResolvedValue(okResp(makeJwt(3600))) + let authUrl = 'http://auth-initial' + const { result } = renderHook(() => + useJwtRefresh({ getAuthUrl: () => authUrl, ncRef: { current: null } })) + + authUrl = 'http://auth.site-a' // resolved later, e.g. by the portal lookup + act(() => { + result.current.setCredentials({ jwt: makeJwt(100), seed: new Uint8Array([9]), natsPublicKey: 'UPUB', refreshable: true }) + }) + await vi.advanceTimersByTimeAsync(95 * 1000) + expect(global.fetch).toHaveBeenCalledWith('http://auth.site-a/auth', expect.anything()) + }) +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `cd chat-frontend && npx vitest run src/context/NatsContext/useJwtRefresh.test.js` +Expected: FAIL (hook still destructures `authUrl`; fetch hits `undefined/auth`) + +- [ ] **Step 3: Implement** + +In `useJwtRefresh.js`: +- Signature: `export function useJwtRefresh({ getAuthUrl, ncRef }) {` +- The doc comment gains one line under "Returns:": `getAuthUrl is read at each refresh, so the target can be resolved after mount (portal lookup).` +- The re-mint fetch becomes: + +```js + const resp = await fetch(`${getAuthUrl()}/auth`, { +``` + +- The `refresh` callback's dependency array: `[getAuthUrl, ncRef, scheduleRefresh, redirect, armTimer]` + +- [ ] **Step 4: Run to verify pass** + +Run: `cd chat-frontend && npx vitest run src/context/NatsContext/useJwtRefresh.test.js` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add chat-frontend/src/context/NatsContext/useJwtRefresh.js chat-frontend/src/context/NatsContext/useJwtRefresh.test.js +git commit -m "feat(frontend): useJwtRefresh resolves the auth URL via getter at refresh time" +``` + +--- + +### Task 11: NatsContext — portal lookup drives connect + +**Files:** +- Modify: `chat-frontend/src/context/NatsContext/NatsContext.jsx`, `chat-frontend/src/context/NatsContext/NatsContext.test.jsx` + +- [ ] **Step 1: Rewrite the connect-wiring tests (red)** + +Replace the `beforeEach` fetch mock and the connect tests in `NatsContext.test.jsx` with: + +```js +const PORTAL_RESP = { + account: 'alice', + employeeId: 'E001', + authServiceUrl: 'http://auth.site-a', + natsUrl: 'ws://nats.site-a', + siteId: 'site-a', +} + +describe('NatsProvider connect wiring', () => { + beforeEach(() => { + setCredentials.mockReset() + stop.mockReset() + natsConnect.mockReset().mockResolvedValue({ closed: () => new Promise(() => {}) }) + global.fetch = vi.fn(async (url) => { + if (String(url).endsWith('/lookup')) { + return { ok: true, json: async () => PORTAL_RESP } + } + return { ok: true, json: async () => ({ natsJwt: 'JWT123', user: { account: 'alice' } }) } + }) + }) + afterEach(() => { vi.restoreAllMocks() }) + + it('resolves the site via portal, then auths and connects with the resolved URLs', async () => { + const { result } = renderHook(() => useNats(), { wrapper }) + await act(async () => { + await result.current.connect({ mode: 'sso', ssoToken: 'tok' }) + }) + + expect(global.fetch).toHaveBeenNthCalledWith(1, 'http://localhost:8081/lookup', + expect.objectContaining({ body: JSON.stringify({ ssoToken: 'tok' }) })) + expect(global.fetch).toHaveBeenNthCalledWith(2, 'http://auth.site-a/auth', expect.anything()) + expect(setCredentials).toHaveBeenCalledWith({ + jwt: 'JWT123', + seed: new Uint8Array([7]), + natsPublicKey: 'UPUBKEY', + refreshable: true, + }) + expect(natsConnect).toHaveBeenCalledWith( + expect.objectContaining({ servers: 'ws://nats.site-a', authenticator: fakeAuthenticator })) + await waitFor(() => expect(result.current.connected).toBe(true)) + expect(result.current.user.siteId).toBe('site-a') + }) + + it('dev mode sends the account to the portal and is non-refreshable', async () => { + const { result } = renderHook(() => useNats(), { wrapper }) + await act(async () => { + await result.current.connect({ mode: 'dev', account: 'alice' }) + }) + expect(global.fetch).toHaveBeenNthCalledWith(1, 'http://localhost:8081/lookup', + expect.objectContaining({ body: JSON.stringify({ account: 'alice' }) })) + expect(setCredentials).toHaveBeenCalledWith(expect.objectContaining({ refreshable: false })) + }) + + it('propagates the portal error envelope and never dials auth or NATS', async () => { + global.fetch = vi.fn(async () => ({ + ok: false, + json: async () => ({ code: 'forbidden', reason: 'account_not_provisioned', error: 'account not provisioned for chat' }), + })) + const { result } = renderHook(() => useNats(), { wrapper }) + let thrown + await act(async () => { + try { await result.current.connect({ mode: 'sso', ssoToken: 'tok' }) } catch (err) { thrown = err } + }) + expect(thrown.reason).toBe('account_not_provisioned') + expect(thrown.code).toBe('forbidden') + expect(global.fetch).toHaveBeenCalledTimes(1) + expect(natsConnect).not.toHaveBeenCalled() + }) + + it('stops the refresh loop on disconnect', async () => { + natsConnect.mockResolvedValue({ + closed: () => new Promise(() => {}), + drain: vi.fn().mockResolvedValue(), + }) + const { result } = renderHook(() => useNats(), { wrapper }) + await act(async () => { + await result.current.connect({ mode: 'sso', ssoToken: 'tok' }) + }) + await act(async () => { await result.current.disconnect() }) + expect(stop).toHaveBeenCalledTimes(1) + }) +}) +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `cd chat-frontend && npx vitest run src/context/NatsContext/NatsContext.test.jsx` +Expected: FAIL (still imports removed `AUTH_URL`/`NATS_URL`; no portal call) + +- [ ] **Step 3: Implement in `NatsContext.jsx`** + +Import change (line 4): `import { PORTAL_URL } from '@/lib/runtimeConfig'`. + +Replace lines 22-26 (`const authUrl = AUTH_URL` … `useJwtRefresh({ authUrl, ncRef })`) with: + +```jsx + // Resolved per user by the portal lookup at connect time; the JWT-refresh + // loop reads it through the getter so re-mints follow the resolved site. + const authUrlRef = useRef(null) + const getAuthUrl = useCallback(() => authUrlRef.current, []) + + const { authenticator, setCredentials, stop } = useJwtRefresh({ getAuthUrl, ncRef }) +``` + +Replace `connectToNats` (keep the JSDoc, updating `@param`s: drop `siteId`, note the portal step) with: + +```jsx + const connectToNats = useCallback(async (opts) => { + setError(null) + + const { mode, account, ssoToken } = opts || {} + + // 1) Site discovery: which auth-service, which NATS, which siteId. + const lookupResp = await fetch(`${PORTAL_URL}/lookup`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(mode === 'sso' ? { ssoToken } : { account }), + }) + if (!lookupResp.ok) { + const errBody = await lookupResp.json().catch(() => ({})) + throw new AsyncJobError( + errBody.error || `Portal lookup failed: ${lookupResp.status}`, + ASYNC_JOB_ERROR_KINDS.SyncError, + { code: errBody.code, reason: errBody.reason, metadata: errBody.metadata }, + ) + } + const portal = await lookupResp.json() + authUrlRef.current = portal.authServiceUrl + + // 2) Mint the NATS JWT at the resolved site's auth-service. + const nkey = createUser() + const natsPublicKey = nkey.getPublicKey() + + const body = + mode === 'sso' + ? { ssoToken, natsPublicKey } + : { account, natsPublicKey } + + const authResp = await fetch(`${portal.authServiceUrl}/auth`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }) + + if (!authResp.ok) { + const errBody = await authResp.json().catch(() => ({})) + throw new AsyncJobError( + errBody.error || `Auth failed: ${authResp.status}`, + ASYNC_JOB_ERROR_KINDS.SyncError, + { code: errBody.code, reason: errBody.reason, metadata: errBody.metadata }, + ) + } + + const { natsJwt, user: userInfo } = await authResp.json() + + // Populate the credential refs BEFORE connecting so the dynamic + // authenticator's getters return the right values during the handshake. + setCredentials({ + jwt: natsJwt, + seed: nkey.getSeed(), + natsPublicKey, + refreshable: mode === 'sso', + }) + + // 3) Dial the resolved site's NATS. + const nc = await natsConnect({ + servers: portal.natsUrl, + authenticator, + }) + + ncRef.current = nc + setUser({ ...userInfo, siteId: portal.siteId }) + setConnected(true) + + nc.closed().then((err) => { + if (err) { + setError(`Disconnected: ${err.message}`) + } + setConnected(false) + }) + }, [authenticator, setCredentials]) +``` + +(Also add `useRef` is already imported; keep the existing error-envelope comments.) + +- [ ] **Step 4: Run to verify pass** + +Run: `cd chat-frontend && npx vitest run src/context/NatsContext/NatsContext.test.jsx` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add chat-frontend/src/context/NatsContext +git commit -m "feat(frontend): NatsContext resolves site via portal lookup before auth + connect" +``` + +--- + +### Task 12: LoginPage, OidcCallback, oidcClient, REASON_COPY + +**Files:** +- Modify: `chat-frontend/src/pages/LoginPage/LoginPage.jsx` + `LoginPage.test.jsx` +- Modify: `chat-frontend/src/pages/OidcCallback/OidcCallback.jsx` + `OidcCallback.test.jsx` +- Modify: `chat-frontend/src/api/auth/oidcClient.js` +- Modify: `chat-frontend/src/api/_transport/asyncJob.ts` + +- [ ] **Step 1: Update the page tests (red)** + +`LoginPage.test.jsx` — remove `DEFAULT_SITE_ID: 'site-local',` from the runtimeConfig mock; in the DEV_MODE=true block replace the two affected tests: + +```js + it('renders the dev account form without a Site ID field', () => { + useNats.mockReturnValue({ connect: vi.fn(), error: null }) + render() + expect(screen.getByLabelText(/account/i)).toBeInTheDocument() + expect(screen.queryByLabelText(/site id/i)).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: /keycloak/i })).not.toBeInTheDocument() + }) + + it('submits with {mode: "dev", account}', async () => { + const connect = vi.fn().mockResolvedValue(undefined) + useNats.mockReturnValue({ connect, error: null }) + + render() + + fireEvent.change(screen.getByLabelText(/account/i), { target: { value: 'alice' } }) + fireEvent.click(screen.getByRole('button', { name: /connect/i })) + + await waitFor(() => { + expect(connect).toHaveBeenCalledWith({ mode: 'dev', account: 'alice' }) + }) + }) +``` + +In the DEV_MODE=false block, replace the sessionStorage test: + +```js + it('redirects to Keycloak without any Site ID input or stash', async () => { + useNats.mockReturnValue({ connect: vi.fn(), error: null }) + const signinRedirect = vi.fn().mockResolvedValue(undefined) + getOidcManager.mockReturnValue({ signinRedirect }) + + render() + expect(screen.queryByLabelText(/site id/i)).not.toBeInTheDocument() + fireEvent.click(screen.getByRole('button', { name: /keycloak/i })) + + await waitFor(() => { + expect(signinRedirect).toHaveBeenCalled() + }) + expect(window.sessionStorage.getItem('oidc.siteId')).toBeNull() + }) +``` + +`OidcCallback.test.jsx` — in the success test, delete the `window.sessionStorage.setItem('oidc.siteId', 'site-A')` line and change the assertion to: + +```js + expect(connect).toHaveBeenCalledWith({ + mode: 'sso', + ssoToken: 'access-token-123', + }) +``` + +Also delete the `window.sessionStorage.setItem('oidc.siteId', 'site-A')` line in the "connect() fails" test. + +- [ ] **Step 2: Run to verify failure** + +Run: `cd chat-frontend && npx vitest run src/pages` +Expected: FAIL (components still render Site ID / pass siteId) + +- [ ] **Step 3: Implement the components** + +`LoginPage.jsx` — full replacement: + +```jsx +import { useState } from 'react' +import { useNats } from '@/context/NatsContext' +import { DEV_MODE } from '@/lib/runtimeConfig' +import { getOidcManager, isSSOTokenInvalidError, redirectToReloginOnTokenInvalid } from '@/api/auth/oidcClient' +import { formatAsyncJobError } from '@/api' +import './style.css' + +export default function LoginPage() { + const { connect, error: natsError } = useNats() + + const [account, setAccount] = useState('') + const [loading, setLoading] = useState(false) + const [error, setError] = useState(null) + + const handleDevSubmit = async (e) => { + e.preventDefault() + if (!account.trim()) return + + setLoading(true) + setError(null) + try { + await connect({ + mode: 'dev', + account: account.trim(), + }) + } catch (err) { + if (isSSOTokenInvalidError(err)) { + await redirectToReloginOnTokenInvalid() + return + } + setError(formatAsyncJobError(err)) + } finally { + setLoading(false) + } + } + + const handleKeycloakLogin = async () => { + setLoading(true) + setError(null) + try { + const manager = getOidcManager() + await manager.signinRedirect() + // Browser navigates away — code below this point is unreachable in prod. + } catch (err) { + if (isSSOTokenInvalidError(err)) { + await redirectToReloginOnTokenInvalid() + return + } + setError(formatAsyncJobError(err)) + setLoading(false) + } + } + + if (DEV_MODE) { + return ( +
+
+

Chat

+

Dev Mode Login

+ + + setAccount(e.target.value)} + placeholder="e.g. alice" + autoFocus + disabled={loading} + /> + + + + {(error || natsError) && ( +
{error || natsError}
+ )} +
+
+ ) + } + + return ( +
+
+

Chat

+

Sign in with Keycloak

+ + + + {(error || natsError) && ( +
{error || natsError}
+ )} +
+
+ ) +} +``` + +`OidcCallback.jsx` — in `run()`, delete the `const siteId = window.sessionStorage.getItem('oidc.siteId') || ''` line and change the connect call to: + +```jsx + await connect({ + mode: 'sso', + ssoToken: user.access_token, + }) +``` + +`oidcClient.js` — in `redirectToReloginOnTokenInvalid`, delete the line `window.sessionStorage.removeItem('oidc.siteId')` and change the preceding comment to `// Clear oidc-client-ts's stashed user state.` + +`asyncJob.ts` — add to `REASON_COPY` (after the `pin_room_too_large` line): + +```ts + account_not_provisioned: "Your account isn't set up for chat yet — contact your administrator.", +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `cd chat-frontend && npx vitest run src/pages src/api` +Expected: PASS + +- [ ] **Step 5: Run the whole frontend suite + typecheck** + +Run: `cd chat-frontend && npm test && npm run typecheck` +Expected: PASS — this is the point where the Task 9 breakage window must be fully closed. Fix any straggler imports of the retired config exports. + +- [ ] **Step 6: Commit** + +```bash +git add chat-frontend/src +git commit -m "feat(frontend): remove typed Site ID — portal resolves the site in both login modes" +``` + +--- + +### Task 13: frontend deploy config + +**Files:** +- Modify: `chat-frontend/deploy/config.js.template`, `chat-frontend/deploy/30-render-config.sh`, `chat-frontend/deploy/docker-compose.yml` + +- [ ] **Step 1: `config.js.template`** + +```js +// Generated at container start — edit template, not output. +window.__APP_CONFIG__ = { + PORTAL_URL: "${PORTAL_URL}", + DEV_MODE: "${DEV_MODE}", + OIDC_ISSUER_URL: "${OIDC_ISSUER_URL}", + OIDC_CLIENT_ID: "${OIDC_CLIENT_ID}" +}; +``` + +- [ ] **Step 2: `30-render-config.sh`** + +```sh +#!/bin/sh +# Renders /config.js from env vars at container start. +set -eu + +: "${PORTAL_URL:=http://localhost:8081}" +: "${DEV_MODE:=false}" +: "${OIDC_ISSUER_URL:=}" +: "${OIDC_CLIENT_ID:=nats-chat}" +export PORTAL_URL DEV_MODE OIDC_ISSUER_URL OIDC_CLIENT_ID + +envsubst '${PORTAL_URL} ${DEV_MODE} ${OIDC_ISSUER_URL} ${OIDC_CLIENT_ID}' \ + < /etc/config.js.template \ + > /usr/share/nginx/html/config.js + +echo "rendered /config.js PORTAL_URL=$PORTAL_URL DEV_MODE=$DEV_MODE OIDC_ISSUER_URL=$OIDC_ISSUER_URL OIDC_CLIENT_ID=$OIDC_CLIENT_ID" +``` + +- [ ] **Step 3: `deploy/docker-compose.yml` environment block** + +```yaml + environment: + PORTAL_URL: ${PORTAL_URL:-http://localhost:8081} + # OIDC values are only consumed when DEV_MODE=false. The Keycloak + # realm at OIDC_ISSUER_URL must serve OIDC_CLIENT_ID with a redirect + # URI matching this frontend's /oidc-callback. + DEV_MODE: ${DEV_MODE:-false} + OIDC_ISSUER_URL: ${OIDC_ISSUER_URL:-http://localhost:8180/realms/chatapp} + OIDC_CLIENT_ID: ${OIDC_CLIENT_ID:-nats-chat} +``` + +- [ ] **Step 4: Commit** + +```bash +git add chat-frontend/deploy +git commit -m "feat(frontend): deploy config serves PORTAL_URL instead of static auth/NATS URLs" +``` + +--- + +### Task 14: full verification sweep + push + +- [ ] **Step 1: Format + lint** + +Run: `make fmt && make lint` +Expected: no diffs, no findings. Fix anything reported. + +- [ ] **Step 2: All unit tests** + +Run: `make test` +Expected: PASS across the repo. + +- [ ] **Step 3: Integration tests for the touched services** + +Run: `make test-integration SERVICE=portal-service && make test-integration SERVICE=auth-service` +Expected: PASS (Docker required). + +- [ ] **Step 4: Coverage floor (CLAUDE.md: ≥80%)** + +```bash +go test -race -tags integration -coverprofile=coverage.out ./portal-service/... ./auth-service/... ./pkg/ginutil/... ./pkg/oidc/... ./pkg/errcode/... +go tool cover -func=coverage.out | tail -5 +``` + +Expected: every listed package ≥80% total. If portal-service falls short, the uncovered lines are almost certainly in `main.go` — do NOT pad with fake tests; check the handler/store branches first. + +- [ ] **Step 5: Frontend suite, typecheck, production build** + +Run: `cd chat-frontend && npm test && npm run typecheck && npm run build` +Expected: all green, clean build. + +- [ ] **Step 6: SAST (blocking CI gate)** + +Run: `make sast` +Expected: PASS. If tools are missing run `make tools` first; if the environment can't install them, note it and rely on the CI `sast` job. + +- [ ] **Step 7: Push** + +```bash +git push -u origin claude/eager-einstein-7u2je6 +``` + +(Retry per repo policy on network failure: 2s/4s/8s/16s backoff.) + +--- + +## Spec-coverage checklist (self-review) + +- POST /lookup prod + dev shapes, response object — Tasks 5, 6 +- Error semantics incl. `account_not_provisioned` 403 — Tasks 1, 6 +- Dev fallback site — Task 6 (`resolve` devFallback branch) +- Mongo users + sites, unique account index, sites ownership (docs) — Tasks 5, 8 +- Enforced provisioning gate with siteId match, fail-closed, REQUIRE_PROVISIONED, dev skip — Task 4 +- `pkg/ginutil`, `Claims.Account()` — Tasks 2, 3 +- Frontend: PORTAL_URL, connect via portal, dynamic refresh URL, no Site ID anywhere, REASON_COPY, vite proxy removal, deploy templates — Tasks 9-13 +- docker-local: compose include, seed, setup.sh env — Task 7 +- client-api.md §2.1/§2.2/§2.3/§6 + frontend CLAUDE.md catalog — Task 8 +- Testing pyramid per spec (unit, integration, frontend) — Tasks 4-6, 9-12; coverage gate — Task 14 diff --git a/docs/superpowers/specs/2026-06-11-portal-service-design.md b/docs/superpowers/specs/2026-06-11-portal-service-design.md new file mode 100644 index 000000000..d8dca88c3 --- /dev/null +++ b/docs/superpowers/specs/2026-06-11-portal-service-design.md @@ -0,0 +1,377 @@ +# Portal Service — Site Discovery for Login — Design + +**Date:** 2026-06-11 +**Status:** Approved (amended 2026-06-11: enforced provisioning at auth-service, shared `pkg/ginutil` middleware; amended 2026-06-12: in-memory directory cache — see below) +**Branch:** `claude/eager-einstein-7u2je6` + +## Amendments (2026-06-12) + +The portal's data model changed during implementation review. **These +amendments supersede the body below**, which is retained unchanged as the +original approved design. As shipped: + +- **No portal `users`/`sites` collections.** The directory is the HR-owned + `hr_employee` collection (`account`, `employeeId`, `siteId`, `natsUrl`), + rewritten by the daily HR cron. +- **In-memory cache, zero per-request Mongo.** A startup goroutine loads + `hr_employee` into a map; the whole map is swapped on a periodic refresh + (`PORTAL_CACHE_REFRESH_INTERVAL`, default 24h, matching the cron; 30s retry + after a failed load). Entries have no TTL. `/lookup` is a single cache hit. +- **`authServiceUrl` is derived, not stored**: `PORTAL_AUTH_URL_TEMPLATE` + (required) substitutes `{siteId}` into a placeholder URL; a value without + the placeholder is used verbatim (single-site local dev). +- **Liveness/readiness split**: `/healthz` is liveness-only; new `GET /readyz` + returns 503 until the cache holds directory data. +- **Portal miss reason is `account_not_ready`** (403), reflecting the daily + sync cadence; `account_not_provisioned` is emitted only by the auth-service + minting gate. The `DirectoryStore` interface is `ListEmployees` only; the + Mongo store additionally ensures a unique `hr_employee(account)` index at + startup, so duplicate HR rows fail at write time (the cache also rejects a + duplicate-bearing snapshot defensively, e.g. if the cron drops and recreates + the collection, losing the index until the next portal restart). +- **Dev fallback** synthesizes an entry from `PORTAL_DEV_FALLBACK_SITE_ID` + + `PORTAL_DEV_FALLBACK_NATS_URL` and the same auth-URL template — no seeded + fallback site record required. +- **Endpoint is `GET /api/userInfo?account={account}`, discovery-only.** The + portal validates no SSO token and has no OIDC dependency at all; the account + is the query-param lookup key (the frontend derives it from the SSO token's + `preferred_username` in prod, or the dev login form in dev). It returns + non-secret directory data; the authoritative gate is auth-service, which + validates the token and enforces provisioning before minting a JWT. `devMode` + now governs only the dev-site fallback, not token handling. Body references to + `POST /lookup` and portal-side OIDC validation are obsolete. + +## Problem + +The system is federated: each site runs its own NATS, MongoDB, and +auth-service. But the frontend's connection targets are **static, single-site +runtime config** (`AUTH_URL`, `NATS_URL` in +`chat-frontend/src/lib/runtimeConfig.js`), and the user must **manually type +their Site ID** on the LoginPage before signing in (stashed in +`sessionStorage('oidc.siteId')` across the Keycloak redirect). A user has no +way to discover which site they belong to — they must be told out-of-band, and +a multi-site deployment cannot serve all users from one frontend config. + +## Goal + +After a successful Keycloak login, the frontend resolves the user's home site +automatically: which auth-service to mint the NATS JWT from, which NATS server +to connect to, and which siteId scopes their data. No typed Site ID anywhere — +SSO **and** dev-mode login. One frontend deployment serves users of all sites. + +Provisioning is also **enforced**, not advisory: an account absent from the +directory cannot mint NATS credentials at all. The gate lives in auth-service +(the minting authority), so it holds at login *and* at every background JWT +refresh — see "Enforced provisioning" below. + +## Approach + +A new **`portal-service`**: a small Gin HTTP service sitting *in front of* +auth-service in the login sequence. It is a **discovery directory, not an auth +proxy** — it validates the SSO token, looks the account up in a portal-owned +directory, and returns connection coordinates. The frontend then talks to the +resolved auth-service directly (including the background NATS-JWT refresh +loop) and opens the NATS WebSocket itself. + +**Rejected alternative — auth facade (credential broker):** portal could +proxy the `/auth` call and return `natsJwt` + `natsUrl` in one round trip, +with auth-service locked to internal-only ingress (the approach on branch +`claude/dreamy-bell-cqdibt`). Rejected because every JWT re-mint for every +user of every site would then flow through the portal (a new choke point and +outage domain on the credential hot path), it duplicates auth-service's +contract, and it needs network-level lockdown plus reload choreography for an +in-memory directory. As a pure directory, a portal outage only blocks new +logins; connected users keep refreshing against their site's auth-service. +The broker's two genuinely architecture-independent ideas — an *enforced* +provisioning gate and shared Gin middleware — are adopted below, with the +gate relocated to the minting authority where it cannot be bypassed. + +### Login flow (after) + +```text +Browser portal-service site auth-service site NATS + │ Keycloak login (PKCE) │ │ │ + │──────────────────────────▶ │ │ │ + │ POST /lookup {ssoToken} │ │ │ + │──────────────────────────▶ │ validate OIDC, │ │ + │ │ users[account] → siteId │ │ + │ ◀──────────────────────────│ sites[siteId] → URLs │ │ + │ {account, employeeId, │ │ │ + │ authServiceUrl, natsUrl, │ │ │ + │ siteId} │ │ │ + │ POST {authServiceUrl}/auth {ssoToken, natsPublicKey} │ │ + │────────────────────────────────────────────────────────▶│ minting gate: │ + │ │ │ users[account] │ + │ │ │ + siteId == SITE_ID │ + │ ◀────────────────────────────────────────────{natsJwt, user} │ + │ WebSocket connect {natsUrl} + natsJwt │ + │──────────────────────────────────────────────────────────────────────────────▶│ +``` + +Dev mode is the same shape with `{account}` replacing `{ssoToken}` at both +HTTP hops (portal and auth-service both run `DEV_MODE=true` locally). + +## portal-service (backend) + +Flat service directory at repo root, `package main`, mirroring auth-service's +layout and middleware (request-ID, access log, CORS — portal is called +directly from the browser). + +### Endpoints + +- `POST /lookup` + - Prod (`DEV_MODE=false`): body `{ "ssoToken": "..." }`. Validate via + `pkg/oidc` (same `Validator` + `TokenValidator` interface shape as + auth-service). Account via the shared `Claims.Account()` helper + (`preferred_username` → `name`); blank → reject (same rule as + auth-service). + - Dev (`DEV_MODE=true`): body `{ "account": "alice" }`, no OIDC. + - Lookup: `users` by account → user's `siteId`; `sites` by that siteId → + URLs. Success response (200): + + ```json + { + "account": "alice", + "employeeId": "E12345", + "authServiceUrl": "https://auth.site-a.example.com", + "natsUrl": "wss://nats.site-a.example.com", + "siteId": "site-a" + } + ``` +- `GET /healthz` → `{"status":"ok"}`. + +### Errors (standard `pkg/errcode` envelope via `errhttp.Write`) + +| HTTP | code | reason | when | +|------|------|--------|------| +| 400 | `bad_request` | `missing_fields` | body missing `ssoToken` (prod) / `account` (dev) | +| 401 | `unauthenticated` | `sso_token_expired` | OIDC says expired — reuses auth-service's reason so the frontend's existing redirect-to-relogin handles it unchanged | +| 401 | `unauthenticated` | `invalid_sso_token` | any other OIDC validation failure (cause attached server-side) / blank account claim | +| 403 | `forbidden` | `account_not_provisioned` | account authenticated but not in the portal directory (prod; see dev fallback). Same reason auth-service emits from its minting gate. | +| 500 | `internal` | — | store errors; **also** a `sites` record missing for a user's `siteId` — that is an ops data bug, not a client error (raw wrapped error collapses at the boundary) | + +Token/validation reasons reuse the existing constants +(`errcode.AuthMissingFields`, `AuthTokenExpired`, `AuthInvalidToken`). The +provisioning case gets one new constant in `pkg/errcode/codes_portal.go`: +`PortalAccountNotProvisioned Reason = "account_not_provisioned"`, emitted by +both portal-service (lookup) and auth-service (minting gate). + +### Dev fallback (account not in directory, DEV_MODE only) + +Local devs log in as arbitrary accounts ("alice", "bob") without seeding each +one. In `DEV_MODE=true`, when the account is not in `users`, portal falls back +to `siteId = PORTAL_DEV_FALLBACK_SITE_ID` (envDefault `site-local`) with +`employeeId: ""` and the account echoed back. The fallback site's `sites` +record must still exist (seeded by docker-local); if it doesn't, the lookup +fails as internal. In prod the same miss is a 403 `account_not_provisioned`. + +### Data model (portal-owned MongoDB, `MONGO_DB` default `portal`) + +- `users` — the global account → site directory. Documents decode into + `pkg/model.User` (projection: `_id`, `account`, `siteId`, `employeeId`). + Unique index on `account` (created by `EnsureIndexes` at startup, per repo + convention). Populated by ops/HR sync — out of scope here beyond local + seeding. +- `sites` — one document per site: + + ```json + { "_id": "site-a", "authServiceUrl": "https://auth.site-a.example.com", "natsUrl": "wss://nats.site-a.example.com" } + ``` + + Local `site` struct in portal-service (json+bson tags, `bson:"_id"` ↔ `ID`); + not shared in `pkg/model` until a second consumer exists. + + **Ownership:** `sites` is infra configuration, owned by ops/IaC (same model + as JetStream stream configs) — maintained by hand or pipeline, never + written by services or the HR cron. The portal only reads it. + +### Store interface (`store.go`, consumer-defined, mockgen-generated mock) + +```go +type DirectoryStore interface { + FindUserByAccount(ctx, account) (*model.User, error) // ErrUserNotFound sentinel + FindSiteByID(ctx, siteID) (*site, error) // ErrSiteNotFound sentinel + EnsureIndexes(ctx) error +} +``` + +### Config (`caarlos0/env`) + +| Env | Default | Notes | +|-----|---------|-------| +| `PORT` | `8081` | 8080 is auth-service locally | +| `DEV_MODE` | `false` | | +| `OIDC_ISSUER_URL`, `OIDC_AUDIENCES` | — | required when `DEV_MODE=false`, exactly like auth-service | +| `TLS_SKIP_VERIFY` | `false` | optional, dev/staging only — same as auth-service | +| `MONGO_URI` | required | | +| `MONGO_DB` | `portal` | | +| `MONGO_USERNAME`, `MONGO_PASSWORD` | `""` | | +| `PORTAL_DEV_FALLBACK_SITE_ID` | `site-local` | dev-mode only | + +### Files + +`main.go`, `handler.go`, `routes.go`, `store.go`, `store_mongo.go`, +`handler_test.go`, `integration_test.go`, `mock_store_test.go` (generated), +`deploy/Dockerfile`, `deploy/docker-compose.yml` (with one-shot mongosh seed +of the local site + demo users), `deploy/azure-pipelines.yml`. Gin middleware +comes from the shared `pkg/ginutil` (below) — no per-service copy. No +JetStream, no streams, no OTEL (parity with auth-service). + +## Enforced provisioning — the auth-service minting gate + +The portal's directory check alone would be advisory: auth-service is public +(the frontend calls it directly to mint and to refresh), so a valid-SSO but +unprovisioned user could skip the portal and still obtain a NATS JWT. The +broker design closes this by hiding auth-service behind the portal; we close +it at the minting authority instead. The portal check remains as fast, +friendly feedback at login; auth-service is the enforcement point. + +- **Check:** after deriving the account from the validated token (and before + signing), auth-service queries its site's `users` collection — the same + per-site collection `pkg/userstore` consumers already read, maintained by + the daily ops sync — for `{account: , siteId: }`. Match → + mint. No match → 403 `forbidden` with reason `account_not_provisioned`. + The explicit `siteId` predicate matters: the per-site `users` collection + contains users of **other** sites too (e.g. the sample seeder writes + `site-remote` users into the local DB for cross-site rooms), so a bare + existence check would not refuse minting on the wrong site's auth-service — + the compound predicate refuses both unprovisioned and wrong-site accounts. +- **Refresh inherits the gate:** the background JWT re-mint calls the same + `POST /auth`, so deprovisioning locks a user out within one JWT lifetime — + no portal involvement and no network lockdown required. +- **Store:** auth-service gains `store.go` (`ProvisionStore` interface: + `AccountProvisioned(ctx context.Context, account, siteID string) (bool, + error)` + mockgen directive) and `store_mongo.go` (compound existence query + against `users`), per the standard service layout. `NewAuthHandler` takes + the store. +- **Config:** auth-service gains `SITE_ID` (required when the gate is active; + same convention as room-service/room-worker) and `REQUIRE_PROVISIONED` + (envDefault `true`) for staged rollout; `MONGO_URI` / `MONGO_DB` (default + `chat`) / `MONGO_USERNAME` / `MONGO_PASSWORD` are validated and connected + only when the gate is active. In `DEV_MODE=true` the gate is skipped + entirely (no Mongo needed locally), preserving log-in-as-anyone. +- **Fail closed:** a store error returns 500 `internal` (raw wrapped error) — + a Mongo outage must not mint credentials for unverifiable accounts. +- **Rollout note:** this is a breaking deployment change for auth-service in + prod — upgrading requires either supplying `MONGO_URI` + `SITE_ID` (gate + on) or setting `REQUIRE_PROVISIONED=false` (gate deferred until the cron + data is verified). The local `deploy/docker-compose.yml` gains the Mongo + + `SITE_ID` env lines so the documented "flip `DEV_MODE` to false to test + OIDC" path keeps working with the gate on. + +## Shared pieces adopted from the broker branch + +- **`pkg/ginutil`** — auth-service's request-ID / access-log / CORS Gin + middleware moves to a shared `pkg/ginutil` (exported `RequestID()`, + `AccessLog()`, `CORS()`), consumed by both auth-service and portal-service + instead of a copy-paste. The middleware tests move with it. +- **`pkg/oidc` `Claims.Account()`** — the account-derivation fallback chain + (`preferred_username` → `name`, blank = reject) becomes a method on + `pkg/oidc.Claims`, used by both services so they can never disagree about + which account a token belongs to. + +## chat-frontend changes + +- **`runtimeConfig.js`**: add `PORTAL_URL` (`VITE_PORTAL_URL`, default + `http://localhost:8081`). `AUTH_URL`, `NATS_URL`, `DEFAULT_SITE_ID` are no + longer read by the connect path and are removed along with their config + plumbing (`config.js.template`, `30-render-config.sh`, deploy compose, + `setup.sh`'s `.env.local`) — portal is the single source of connection + coordinates. +- **`NatsContext.connect(opts)`**: drops `siteId` from its signature. New + sequence: portal lookup (`POST {PORTAL_URL}/lookup`, body `{ssoToken}` or + `{account}` by mode) → `POST {resolved authServiceUrl}/auth` → + `natsConnect({servers: resolved natsUrl})` → `setUser({...userInfo, siteId: + resolved siteId})`. Portal errors throw the same `AsyncJobError` shape the + auth fetch already throws (envelope-aware), so `sso_token_expired` / + `invalid_sso_token` trigger the existing relogin redirect for free. +- **`useJwtRefresh`**: takes a `getAuthUrl()` getter (ref populated at + connect time) instead of a static `authUrl`, so background re-mints hit the + resolved site's auth-service. Refresh does **not** re-query portal — site + assignment is stable within a session. +- **`LoginPage`**: Site ID input removed from both branches. Dev branch: + account only. SSO branch: just the "Sign in with Keycloak" button. The + `oidc.siteId` sessionStorage stash is deleted everywhere (LoginPage, + OidcCallback, oidcClient cleanup). +- **`OidcCallback`**: `connect({ mode: 'sso', ssoToken })` — no siteId. +- **`REASON_COPY`** (`api/_transport/asyncJob.ts`) gains + `account_not_provisioned: "Your account isn't set up for chat yet — contact + your administrator."` so both the portal lookup refusal and an auth-service + minting refusal render friendly copy via `formatAsyncJobError`. +- **`vite.config.js`**: drop the now-dead `/auth → localhost:8080` dev-server + proxy — every credential call uses absolute URLs (portal, then the resolved + `authServiceUrl`). + +## docker-local / local dev + +- Portal service joins `docker-local/compose.services.yaml` via its + `deploy/docker-compose.yml`, using the shared `mongodb` container with DB + `portal`. +- Seed (one-shot mongosh container in portal's deploy compose): + `sites`: `{_id: "site-local", authServiceUrl: "http://localhost:8080", + natsUrl: "ws://localhost:9222"}`; demo `users` reusing the + `tools/seed-sample-data` fixture accounts (`alice`/E001, `bob`/E002 → + `site-local`) so the portal directory agrees with the site data `make seed` + creates. Unlisted accounts still work via the dev fallback. +- `setup.sh` writes `VITE_PORTAL_URL=http://localhost:8081` into + `chat-frontend/.env.local` (and stops writing the retired vars for fresh + setups). + +## Documentation + +`docs/client-api.md`: new §2 subsection "Site discovery — POST /lookup" +(request/response field tables, JSON example, error table per current doc +style); the §2 connection narrative gains the portal step; the §2.2 +`POST /auth` error table gains the 403 `account_not_provisioned` row; the §6 +reason catalog gains `account_not_provisioned` (emitted by portal-service and +auth-service). `chat-frontend/CLAUDE.md`'s reason catalog gains the same +entry. + +## Testing (TDD, ≥80% coverage) + +- **Backend unit (`handler_test.go`)** — table-driven, mocked store + fake + `TokenValidator`: happy path (prod + dev); missing fields; invalid token; + expired token; blank account claim; unprovisioned account (prod 403 + `account_not_provisioned`); dev fallback (unknown account → fallback site); + dev fallback site missing → internal; site missing for known user → + internal; store error → internal. +- **auth-service gate (`auth-service/handler_test.go`)** — provisioned + account mints; unprovisioned → 403 `account_not_provisioned`; account + provisioned on a **different** site → 403 (wrong-site refusal); store error + → 500 (fail closed); gate skipped in dev mode and when + `REQUIRE_PROVISIONED=false`. auth-service `integration_test.go` gains a + Mongo-backed `ProvisionStore` test via `testutil.MongoDB` (including the + compound account+siteId predicate). +- **`pkg/ginutil`** (moved middleware tests) and **`pkg/oidc` + `Claims.Account()`** get their own unit tests. +- **Backend integration (`integration_test.go`)** — `testutil.MongoDB` + + `TestMain(testutil.RunTests)`: find user by account (hit/miss), find site + (hit/miss), `EnsureIndexes` idempotent + uniqueness enforced. +- **Frontend (vitest)** — NatsContext connect: portal lookup feeds auth fetch + URL + NATS servers + user.siteId (both modes); portal error propagation + (envelope → AsyncJobError, relogin reasons); LoginPage renders without Site + ID field, dev submit passes account only; OidcCallback passes only the + token; useJwtRefresh uses the resolved auth URL. + +## Confirmed context + +- Multiple NATS sites run in production; connection coordinates differ per + site. +- A daily ops cron maintains directory data — the global account → site + mapping and the per-site `users` collections. This spec relies on that sync + but does not build it. + +## Out of scope + +- Populating the production portal directory and per-site `users` collections + (the confirmed daily cron owns both; this PR only seeds docker-local). +- Multi-site docker-local topology (single `site-local` remains). +- Cross-site frontend redirects (`redirectTo`) — moot while one global + frontend deployment serves all sites; revisit if per-site frontends arrive. +- Immediate revocation of an already-issued NATS JWT — bounded by JWT + lifetime; deprovisioning locks out at the next refresh via the minting + gate. +- Repointing the frontend smoke scripts (`smoke-test.mjs`, + `scripts/*.smoke.mjs`) at the portal — they call auth-service directly with + dev-mode bodies, and that contract is unchanged; a portal hop in + `smoke:livestack` is a worthwhile follow-up. diff --git a/pkg/errcode/codes_portal.go b/pkg/errcode/codes_portal.go new file mode 100644 index 000000000..5736f217e --- /dev/null +++ b/pkg/errcode/codes_portal.go @@ -0,0 +1,7 @@ +package errcode + +// Reasons emitted by portal-service. +const ( + // PortalAccountNotReady: account absent from the portal's in-memory employee directory cache (portal lookup). + PortalAccountNotReady Reason = "account_not_ready" +) diff --git a/pkg/errcode/codes_test.go b/pkg/errcode/codes_test.go index f78912e82..00250ebca 100644 --- a/pkg/errcode/codes_test.go +++ b/pkg/errcode/codes_test.go @@ -15,6 +15,7 @@ var allReasons = []Reason{ MessageLargeRoomPostRestricted, MessageNotSubscribed, MessageOutsideAccessWindow, PinDisabled, PinLimitReached, PinRoomTooLarge, AuthTokenExpired, AuthInvalidToken, AuthInvalidRequest, AuthInvalidNKey, AuthMissingFields, + PortalAccountNotReady, RequestIDRequired, } diff --git a/auth-service/middleware.go b/pkg/ginutil/middleware.go similarity index 61% rename from auth-service/middleware.go rename to pkg/ginutil/middleware.go index 8da55c38d..815d47d38 100644 --- a/auth-service/middleware.go +++ b/pkg/ginutil/middleware.go @@ -1,4 +1,5 @@ -package main +// Package ginutil holds the Gin middleware shared by the HTTP services: request-ID, access log, CORS. +package ginutil import ( "log/slog" @@ -11,11 +12,9 @@ import ( "github.com/hmchangw/chat/pkg/natsutil" ) -// requestIDMiddleware funnels HTTP X-Request-ID through idgen.ResolveRequestID -// (the same primitive the NATS path uses via natsutil.StampRequestID) so the -// mint-vs-pass-through policy has a single owner. Missing → silent mint; -// malformed → mint + Warn preserving the inbound value for traceability. -func requestIDMiddleware() gin.HandlerFunc { +// RequestID funnels X-Request-ID through idgen.ResolveRequestID, the single +// owner of mint-vs-pass-through policy. Missing → mint; malformed → mint + Warn. +func RequestID() gin.HandlerFunc { return func(c *gin.Context) { inbound := c.GetHeader(natsutil.RequestIDHeader) id, replaced := idgen.ResolveRequestID(inbound) @@ -29,11 +28,9 @@ func requestIDMiddleware() gin.HandlerFunc { } } -// corsMiddleware allows browser clients from any origin to call the API and -// short-circuits the preflight OPTIONS request with 204. The wildcard origin -// is incompatible with credentialed requests (cookies / Authorization), but -// /auth uses neither — it accepts a JSON body and returns a JWT. -func corsMiddleware() gin.HandlerFunc { +// CORS allows any origin and answers preflight OPTIONS with 204. Wildcard is +// safe here: these endpoints take a JSON body, no cookies or credentials. +func CORS() gin.HandlerFunc { return func(c *gin.Context) { c.Header("Access-Control-Allow-Origin", "*") c.Header("Access-Control-Allow-Methods", "GET, POST, OPTIONS") @@ -47,8 +44,8 @@ func corsMiddleware() gin.HandlerFunc { } } -// accessLogMiddleware logs method, path, status, and latency for each request. -func accessLogMiddleware() gin.HandlerFunc { +// AccessLog logs method, path, status, and latency for each request. +func AccessLog() gin.HandlerFunc { return func(c *gin.Context) { start := time.Now() c.Next() diff --git a/auth-service/middleware_test.go b/pkg/ginutil/middleware_test.go similarity index 83% rename from auth-service/middleware_test.go rename to pkg/ginutil/middleware_test.go index d2484efed..212ca62c9 100644 --- a/auth-service/middleware_test.go +++ b/pkg/ginutil/middleware_test.go @@ -1,4 +1,4 @@ -package main +package ginutil import ( "net/http" @@ -12,10 +12,10 @@ import ( "github.com/hmchangw/chat/pkg/natsutil" ) -func TestRequestIDMiddleware_AttachesIDToRequestContext(t *testing.T) { +func TestRequestID_AttachesIDToRequestContext(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() - r.Use(requestIDMiddleware()) + r.Use(RequestID()) var fromCtx string var fromGin string @@ -36,10 +36,10 @@ func TestRequestIDMiddleware_AttachesIDToRequestContext(t *testing.T) { assert.Equal(t, testID, w.Header().Get(natsutil.RequestIDHeader), "echoed in response header") } -func TestRequestIDMiddleware_GeneratesAndAttachesWhenHeaderAbsent(t *testing.T) { +func TestRequestID_GeneratesAndAttachesWhenHeaderAbsent(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() - r.Use(requestIDMiddleware()) + r.Use(RequestID()) var fromCtx string var fromGin string @@ -59,10 +59,10 @@ func TestRequestIDMiddleware_GeneratesAndAttachesWhenHeaderAbsent(t *testing.T) "the same minted ID must be echoed in the response header") } -func TestRequestIDMiddleware_RegeneratesOnMalformedHeader(t *testing.T) { +func TestRequestID_RegeneratesOnMalformedHeader(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() - r.Use(requestIDMiddleware()) + r.Use(RequestID()) var fromCtx string r.GET("/test", func(c *gin.Context) { @@ -81,12 +81,12 @@ func TestRequestIDMiddleware_RegeneratesOnMalformedHeader(t *testing.T) { "echoed response header must match the regenerated ID, not the malformed input") } -func TestAccessLogMiddleware_LogsAndPassesThrough(t *testing.T) { +func TestAccessLog_LogsAndPassesThrough(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() - // requestIDMiddleware first, so the access log gets a non-empty request_id field. - r.Use(requestIDMiddleware()) - r.Use(accessLogMiddleware()) + // RequestID first, so the access log gets a non-empty request_id field. + r.Use(RequestID()) + r.Use(AccessLog()) handlerCalled := false r.GET("/test", func(c *gin.Context) { @@ -98,14 +98,14 @@ func TestAccessLogMiddleware_LogsAndPassesThrough(t *testing.T) { w := httptest.NewRecorder() r.ServeHTTP(w, req) - assert.True(t, handlerCalled, "downstream handler must run after accessLogMiddleware") + assert.True(t, handlerCalled, "downstream handler must run after AccessLog") assert.Equal(t, http.StatusOK, w.Code, "status passes through unchanged") } -func TestCorsMiddleware_AnyOrigin_GetsWildcardHeaders(t *testing.T) { +func TestCORS_AnyOrigin_GetsWildcardHeaders(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() - r.Use(corsMiddleware()) + r.Use(CORS()) r.POST("/auth", func(c *gin.Context) { c.Status(http.StatusOK) }) w := httptest.NewRecorder() @@ -118,10 +118,10 @@ func TestCorsMiddleware_AnyOrigin_GetsWildcardHeaders(t *testing.T) { assert.Contains(t, w.Header().Get("Access-Control-Allow-Methods"), "POST") } -func TestCorsMiddleware_PreflightOptions_Returns204WithoutHandler(t *testing.T) { +func TestCORS_PreflightOptions_Returns204WithoutHandler(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() - r.Use(corsMiddleware()) + r.Use(CORS()) r.POST("/auth", func(c *gin.Context) { t.Fatal("downstream handler must NOT run on preflight") }) diff --git a/pkg/oidc/oidc.go b/pkg/oidc/oidc.go index 18dbf8373..0979e6b75 100644 --- a/pkg/oidc/oidc.go +++ b/pkg/oidc/oidc.go @@ -27,6 +27,12 @@ type Claims struct { Extra map[string]interface{} } +// Account returns preferred_username — the only claim trusted as a principal; +// name is user-editable display data. Empty means callers must reject the token. +func (c *Claims) Account() string { + return c.PreferredUsername +} + var ( ErrTokenExpired = errors.New("oidc: token has expired") ErrNoAudiences = errors.New("oidc: at least one allowed audience is required") diff --git a/pkg/oidc/oidc_test.go b/pkg/oidc/oidc_test.go index a9ede0ee2..fe774aa94 100644 --- a/pkg/oidc/oidc_test.go +++ b/pkg/oidc/oidc_test.go @@ -35,3 +35,20 @@ func TestNewValidator_RejectsEmptyAudiences(t *testing.T) { }) assert.ErrorIs(t, err, ErrNoAudiences) } + +func TestClaims_Account(t *testing.T) { + tests := []struct { + name string + claims Claims + want string + }{ + {"preferred_username wins", Claims{PreferredUsername: "alice", Name: "Alice W"}, "alice"}, + {"name alone is not an account", Claims{Name: "Alice W"}, ""}, + {"both blank is blank", Claims{}, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, tt.claims.Account()) + }) + } +} diff --git a/pkg/subject/subject.go b/pkg/subject/subject.go index 59fc6cfa0..9e8a13c6b 100644 --- a/pkg/subject/subject.go +++ b/pkg/subject/subject.go @@ -3,8 +3,23 @@ package subject import ( "fmt" "strings" + "unicode" ) +// IsValidAccountToken reports whether s can serve as the {account} token of a +// NATS subject: non-empty, no '.'/'*'/'>' runes, no whitespace or control runes. +func IsValidAccountToken(s string) bool { + if s == "" { + return false + } + for _, r := range s { + if r == '.' || r == '*' || r == '>' || unicode.IsSpace(r) || unicode.IsControl(r) { + return false + } + } + return true +} + // ParseUserRoomSubject extracts the user account and roomID from subjects // matching the pattern "chat.user.{account}.*.room.{roomID}.…". // Returns the user account, roomID, and ok=true on success. @@ -744,11 +759,10 @@ func RoomAppCmdMenuPattern(siteID string) string { return fmt.Sprintf("chat.user.{account}.request.room.{roomID}.%s.app.cmd-menu", siteID) } -// isValidAccountToken rejects empty tokens and tokens containing NATS wildcard -// characters ('*' or '>'). Subject parsers use it as the boundary guard for the -// account token so wildcard semantics never leak into identity parsing. +// isValidAccountToken is the parsers' boundary guard for the account token so +// wildcard semantics never leak into identity parsing. func isValidAccountToken(token string) bool { - return token != "" && !strings.ContainsAny(token, "*>") + return IsValidAccountToken(token) } // ParseRoomCreateSubject extracts the account from chat.user.{account}.request.room.{siteID}.create. diff --git a/pkg/subject/subject_test.go b/pkg/subject/subject_test.go index 64b79747b..0d7ab14de 100644 --- a/pkg/subject/subject_test.go +++ b/pkg/subject/subject_test.go @@ -666,6 +666,31 @@ func TestUserServiceWildcards(t *testing.T) { } } +func TestIsValidAccountToken(t *testing.T) { + tests := []struct { + name string + account string + want bool + }{ + {"plain account", "alice", true}, + {"dash underscore digits", "user-01_X", true}, + {"at sign is routable", "alice@corp", true}, + {"unicode is routable", "júlio", true}, + {"empty", "", false}, + {"dot splits subject tokens", "john.doe", false}, + {"single-token wildcard", "mal*ory", false}, + {"multi-token wildcard", "mal>ory", false}, + {"space", "mal ory", false}, + {"tab", "mal\tory", false}, + {"control rune", "mal\x00ory", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, subject.IsValidAccountToken(tt.account)) + }) + } +} + func TestUserServiceBuildersRejectWildcardAccounts(t *testing.T) { builders := []struct { name string diff --git a/portal-service/cache.go b/portal-service/cache.go new file mode 100644 index 000000000..992bc34a6 --- /dev/null +++ b/portal-service/cache.go @@ -0,0 +1,106 @@ +package main + +import ( + "context" + "fmt" + "log/slog" + "sync/atomic" + "time" +) + +// cacheLoadTimeout bounds a single full scan of the hr_employee collection. +const cacheLoadTimeout = time.Minute + +// directoryCache is the in-memory account → employee directory. The backing +// hr_employee collection is rewritten wholesale by a daily HR cron, so the +// whole map is swapped wholesale by Load (at startup and on the periodic +// refresh) — entries have no TTL. +// +// The snapshot lives in an atomic.Pointer: reads (Get/Ready) happen all day and +// are a single lock-free load of an immutable map, while the once-a-day refresh +// swaps in a freshly built map with one atomic store. The map is never mutated +// in place, so readers never need a lock. +type directoryCache struct { + entries atomic.Pointer[map[string]employee] +} + +func newDirectoryCache() *directoryCache { + return &directoryCache{} +} + +// Get returns the directory entry for account. +func (c *directoryCache) Get(account string) (employee, bool) { + m := c.entries.Load() + if m == nil { + return employee{}, false + } + e, ok := (*m)[account] + return e, ok +} + +// Ready reports whether the cache holds directory data — the /readyz signal. +// An empty directory is not ready: the portal cannot resolve any account. +func (c *directoryCache) Ready() bool { + m := c.entries.Load() + return m != nil && len(*m) > 0 +} + +// Load reads the full employee directory and swaps it in. An empty snapshot +// after a prior successful load (the HR cron may be mid-rewrite) is rejected; +// on any error the previous entries keep serving and the refresh loop retries. +func (c *directoryCache) Load(ctx context.Context, store DirectoryStore) error { + ctx, cancel := context.WithTimeout(ctx, cacheLoadTimeout) + defer cancel() + emps, err := store.ListEmployees(ctx) + if err != nil { + return fmt.Errorf("list employee directory: %w", err) + } + if c.Ready() && len(emps) == 0 { + return fmt.Errorf("refresh employee directory: empty snapshot after a successful load") + } + n := c.replace(emps) + slog.Info("directory cache loaded", "rows", len(emps), "entries", n) + return nil +} + +// replace builds a snapshot keyed by account and swaps it in atomically. A +// duplicate account is skipped with a warning rather than rejecting the whole +// snapshot, so one malformed HR row cannot stall the directory; the first +// occurrence wins (deterministic, independent of Mongo cursor order). Returns +// the number of accounts published. +func (c *directoryCache) replace(emps []employee) int { + entries := make(map[string]employee, len(emps)) + for _, e := range emps { + if _, dup := entries[e.Account]; dup { + slog.Warn("duplicate account in directory, skipping", "account", e.Account) + continue + } + entries[e.Account] = e + } + c.entries.Store(&entries) + return len(entries) +} + +// RefreshLoop populates the cache immediately, then refreshes it every +// refreshEvery. A failed attempt is retried after retryEvery instead, so a +// transient Mongo failure does not leave the portal stale until the next +// scheduled refresh. Returns when ctx is cancelled. +func (c *directoryCache) RefreshLoop(ctx context.Context, store DirectoryStore, refreshEvery, retryEvery time.Duration) { + for { + wait := refreshEvery + if err := c.Load(ctx, store); err != nil { + if ctx.Err() != nil { + return + } + slog.Error("refresh directory cache", "error", err) + wait = retryEvery + } + timer := time.NewTimer(wait) + select { + case <-ctx.Done(): + timer.Stop() + return + case <-timer.C: + } + } +} diff --git a/portal-service/cache_test.go b/portal-service/cache_test.go new file mode 100644 index 000000000..c9409b724 --- /dev/null +++ b/portal-service/cache_test.go @@ -0,0 +1,220 @@ +package main + +import ( + "context" + "errors" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" +) + +var ( + aliceEmployee = employee{ + Account: "alice", + EmployeeID: "E001", + SiteID: "site-a", + NATSURL: "wss://nats-3.site-a.example.com", + } + bobEmployee = employee{ + Account: "bob", + EmployeeID: "E002", + SiteID: "site-b", + NATSURL: "wss://nats.site-b.example.com", + } +) + +func TestDirectoryCache_EmptyUntilLoaded(t *testing.T) { + cache := newDirectoryCache() + + assert.False(t, cache.Ready()) + _, ok := cache.Get("alice") + assert.False(t, ok) +} + +func TestDirectoryCache_Load(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + store.EXPECT().ListEmployees(gomock.Any()).Return([]employee{aliceEmployee, bobEmployee}, nil) + + cache := newDirectoryCache() + require.NoError(t, cache.Load(context.Background(), store)) + + assert.True(t, cache.Ready()) + got, ok := cache.Get("alice") + require.True(t, ok) + assert.Equal(t, aliceEmployee, got) + got, ok = cache.Get("bob") + require.True(t, ok) + assert.Equal(t, bobEmployee, got) + _, ok = cache.Get("mallory") + assert.False(t, ok) +} + +func TestDirectoryCache_LoadError(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + store.EXPECT().ListEmployees(gomock.Any()).Return(nil, errors.New("mongo down")) + + cache := newDirectoryCache() + require.Error(t, cache.Load(context.Background(), store)) + assert.False(t, cache.Ready()) +} + +func TestDirectoryCache_LoadErrorKeepsPreviousEntries(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + gomock.InOrder( + store.EXPECT().ListEmployees(gomock.Any()).Return([]employee{aliceEmployee}, nil), + store.EXPECT().ListEmployees(gomock.Any()).Return(nil, errors.New("mongo down")), + ) + + cache := newDirectoryCache() + require.NoError(t, cache.Load(context.Background(), store)) + require.Error(t, cache.Load(context.Background(), store)) + + assert.True(t, cache.Ready(), "a failed refresh must keep serving the previous data") + _, ok := cache.Get("alice") + assert.True(t, ok) +} + +func TestDirectoryCache_EmptyLoadIsNotReady(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + store.EXPECT().ListEmployees(gomock.Any()).Return([]employee{}, nil) + + cache := newDirectoryCache() + require.NoError(t, cache.Load(context.Background(), store)) + + assert.False(t, cache.Ready(), "an empty directory must not report ready") +} + +func TestDirectoryCache_EmptyRefreshAfterReadyKeepsPrevious(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + gomock.InOrder( + store.EXPECT().ListEmployees(gomock.Any()).Return([]employee{aliceEmployee}, nil), + // The daily HR cron rewrites hr_employee; a refresh racing it can see zero rows. + store.EXPECT().ListEmployees(gomock.Any()).Return([]employee{}, nil), + ) + + cache := newDirectoryCache() + require.NoError(t, cache.Load(context.Background(), store)) + require.Error(t, cache.Load(context.Background(), store), + "an empty snapshot after a successful load is a failed refresh") + + assert.True(t, cache.Ready(), "the previous snapshot must keep serving") + _, ok := cache.Get("alice") + assert.True(t, ok) +} + +func TestDirectoryCache_DuplicateAccountSkippedKeepsRest(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + dupAlice := aliceEmployee + dupAlice.SiteID = "site-b" + // The first occurrence wins; the later duplicate row is skipped (with a + // warning) rather than rejecting the whole snapshot. + store.EXPECT().ListEmployees(gomock.Any()).Return([]employee{aliceEmployee, bobEmployee, dupAlice}, nil) + + cache := newDirectoryCache() + require.NoError(t, cache.Load(context.Background(), store), + "a duplicate row must be skipped, not reject the whole snapshot") + + assert.True(t, cache.Ready()) + got, ok := cache.Get("alice") + require.True(t, ok) + assert.Equal(t, aliceEmployee, got, "the first occurrence wins") + _, ok = cache.Get("bob") + assert.True(t, ok, "non-duplicate rows are still published") +} + +func TestDirectoryCache_DuplicateAccountAtStartupReady(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + store.EXPECT().ListEmployees(gomock.Any()).Return([]employee{aliceEmployee, aliceEmployee}, nil) + + cache := newDirectoryCache() + require.NoError(t, cache.Load(context.Background(), store)) + + assert.True(t, cache.Ready(), "a duplicate at startup is skipped, not fatal") + got, ok := cache.Get("alice") + require.True(t, ok) + assert.Equal(t, aliceEmployee, got) +} + +// TestDirectoryCache_ConcurrentReadDuringLoad guards the lock-free read path: +// many readers hit Get/Ready while the writer swaps the snapshot. The race +// detector fails this if the swap is not atomic against concurrent reads. +func TestDirectoryCache_ConcurrentReadDuringLoad(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + store.EXPECT().ListEmployees(gomock.Any()).Return([]employee{aliceEmployee, bobEmployee}, nil).AnyTimes() + + cache := newDirectoryCache() + require.NoError(t, cache.Load(context.Background(), store)) + + var wg sync.WaitGroup + stop := make(chan struct{}) + for range 8 { + wg.Add(1) + go func() { + defer wg.Done() + for { + select { + case <-stop: + return + default: + cache.Get("alice") + cache.Ready() + } + } + }() + } + for range 50 { + require.NoError(t, cache.Load(context.Background(), store)) + } + close(stop) + wg.Wait() +} + +func TestDirectoryCache_RefreshLoop(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockDirectoryStore(ctrl) + loads := make(chan struct{}, 2) + gomock.InOrder( + // First attempt fails — the loop must retry on the short interval. + store.EXPECT().ListEmployees(gomock.Any()).DoAndReturn(func(context.Context) ([]employee, error) { + loads <- struct{}{} + return nil, errors.New("mongo down") + }), + store.EXPECT().ListEmployees(gomock.Any()).DoAndReturn(func(context.Context) ([]employee, error) { + loads <- struct{}{} + return []employee{aliceEmployee}, nil + }), + ) + + cache := newDirectoryCache() + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + defer close(done) + cache.RefreshLoop(ctx, store, time.Hour, time.Millisecond) + }() + + <-loads + <-loads + require.Eventually(t, cache.Ready, time.Second, time.Millisecond) + _, ok := cache.Get("alice") + assert.True(t, ok) + + cancel() + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("RefreshLoop did not stop on context cancel") + } +} diff --git a/portal-service/deploy/Dockerfile b/portal-service/deploy/Dockerfile new file mode 100644 index 000000000..f82336716 --- /dev/null +++ b/portal-service/deploy/Dockerfile @@ -0,0 +1,13 @@ +FROM golang:1.25.11-alpine AS builder +WORKDIR /app +COPY go.mod go.sum ./ +RUN go mod download +COPY pkg/ pkg/ +COPY portal-service/ portal-service/ +RUN CGO_ENABLED=0 go build -o /portal-service ./portal-service/ + +FROM alpine:3.21 +RUN apk add --no-cache ca-certificates && adduser -D -u 10001 app +COPY --from=builder /portal-service /portal-service +USER app +ENTRYPOINT ["/portal-service"] diff --git a/portal-service/deploy/azure-pipelines.yml b/portal-service/deploy/azure-pipelines.yml new file mode 100644 index 000000000..57b5e5a03 --- /dev/null +++ b/portal-service/deploy/azure-pipelines.yml @@ -0,0 +1,79 @@ +trigger: + branches: + include: + - main + - develop + paths: + include: + - portal-service/ + - pkg/ + +pr: + branches: + include: + - main + paths: + include: + - portal-service/ + - pkg/ + +variables: + GO_VERSION: '1.25.11' + SERVICE_NAME: portal-service + REGISTRY: '$(containerRegistry)' + # Repo-wide 80% minimum, gated like search-service's pipeline. main.go is + # excluded: it's the startup harness, covered by integration tests only. + MIN_COVERAGE_SERVICE: '80' + +stages: + - stage: Validate + displayName: 'Lint & Test' + jobs: + - job: LintAndTest + pool: + vmImage: 'ubuntu-latest' + steps: + - task: GoTool@0 + inputs: + version: '$(GO_VERSION)' + displayName: 'Install Go $(GO_VERSION)' + + - script: go vet ./$(SERVICE_NAME)/... ./pkg/... + displayName: 'Go Vet' + + - script: go test ./pkg/... -v -race -coverprofile=coverage-pkg.out + displayName: 'Test shared packages' + + - script: | + set -euo pipefail + go test ./$(SERVICE_NAME)/... -v -race -coverprofile=coverage-$(SERVICE_NAME)-raw.out + # The first line of a cover profile is `mode: ...` — keep it verbatim. + awk 'NR==1 || $0 !~ "/main\\.go:"' coverage-$(SERVICE_NAME)-raw.out > coverage-$(SERVICE_NAME).out + cov="$(go tool cover -func=coverage-$(SERVICE_NAME).out | awk '/^total:/ {gsub("%","",$3); print $3}')" + echo "$(SERVICE_NAME) coverage (excl. main.go): ${cov}% (threshold ${MIN_COVERAGE_SERVICE}%)" + awk -v c="$cov" -v t="$MIN_COVERAGE_SERVICE" 'BEGIN { exit (c >= t ? 0 : 1) }' + displayName: 'Test $(SERVICE_NAME) (with coverage gate)' + + - script: go build -o /dev/null ./$(SERVICE_NAME)/ + displayName: 'Build $(SERVICE_NAME)' + + - stage: Build + displayName: 'Build & Push Image' + dependsOn: Validate + condition: and(succeeded(), eq(variables['Build.SourceBranch'], 'refs/heads/main')) + jobs: + - job: BuildImage + pool: + vmImage: 'ubuntu-latest' + steps: + - task: Docker@2 + inputs: + containerRegistry: '$(containerRegistry)' + repository: 'chat/$(SERVICE_NAME)' + command: 'buildAndPush' + Dockerfile: '$(SERVICE_NAME)/deploy/Dockerfile' + buildContext: '.' + tags: | + $(Build.BuildId) + latest + displayName: 'Build & push $(SERVICE_NAME)' diff --git a/portal-service/deploy/docker-compose.yml b/portal-service/deploy/docker-compose.yml new file mode 100644 index 000000000..b560dcdb2 --- /dev/null +++ b/portal-service/deploy/docker-compose.yml @@ -0,0 +1,62 @@ +name: portal-service + +services: + # One-shot directory seed: hr_employee + a users stand-in in the portal DB so + # the ListEmployees $lookup (hr_employee × users on {account, siteId}) resolves + # the demo accounts. They match the ones tools/seed-sample-data creates. + portal-seed: + image: mongo:8.2.9 + entrypoint: + - mongosh + - --host + - mongodb + - --quiet + - --eval + - | + const p = db.getSiblingDB('portal'); + [['alice', 'E001'], ['bob', 'E002']].forEach(([account, employeeId]) => { + p.hr_employee.replaceOne( + { account: account }, + { account: account, employeeId: employeeId, siteId: 'site-local', natsUrl: 'ws://localhost:9222' }, + { upsert: true }, + ); + p.users.replaceOne( + { account: account, siteId: 'site-local' }, + { _id: 'u-' + account, account: account, siteId: 'site-local' }, + { upsert: true }, + ); + }); + print('portal seed: hr_employee=' + p.hr_employee.countDocuments() + ' users=' + p.users.countDocuments()); + networks: + - chat-local + + portal-service: + build: + context: ../.. + dockerfile: portal-service/deploy/Dockerfile + pull_policy: build + depends_on: + portal-seed: + condition: service_completed_successfully + ports: + - "8081:8081" + env_file: + - path: ../../docker-local/.env + required: false + environment: + - PORT=8081 + # Dev fallback: accounts absent from the directory resolve to the local site. + - DEV_MODE=${DEV_MODE:-true} + - PORTAL_DEV_FALLBACK_SITE_ID=site-local + - PORTAL_DEV_FALLBACK_NATS_URL=ws://localhost:9222 + # Per-site URL registry (JSON): siteId -> {authServiceUrl, baseUrl}. + # baseUrl is the site's own origin, distinct from the auth-service URL. + - 'PORTAL_SITE_URLS={"site-local":{"authServiceUrl":"http://localhost:8080","baseUrl":"http://localhost:3000"}}' + - MONGO_URI=mongodb://mongodb:27017 + - MONGO_DB=portal + networks: + - chat-local + +networks: + chat-local: + external: true diff --git a/portal-service/handler.go b/portal-service/handler.go new file mode 100644 index 000000000..7926355b2 --- /dev/null +++ b/portal-service/handler.go @@ -0,0 +1,148 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + + "github.com/gin-gonic/gin" + + "github.com/hmchangw/chat/pkg/errcode" + "github.com/hmchangw/chat/pkg/errcode/errhttp" + "github.com/hmchangw/chat/pkg/subject" +) + +// siteURL holds a site's externally reachable HTTP URLs, looked up by siteId +// from the PORTAL_SITE_URLS registry. AuthServiceURL is where the client mints +// its JWT (POST /auth); BaseURL is the site's own origin, a distinct URL — not +// derived from AuthServiceURL. A single template can't express sites on +// different domains (siteA.xx.com vs siteB.yy.com), so each site is explicit. +type siteURL struct { + AuthServiceURL string `json:"authServiceUrl"` + BaseURL string `json:"baseUrl"` +} + +// parseSiteURLs decodes the PORTAL_SITE_URLS registry — a JSON object mapping +// siteId to that site's URLs — and requires every site to carry both URLs, so a +// misconfigured registry fails at startup rather than at a user's login. +func parseSiteURLs(raw string) (map[string]siteURL, error) { + var sites map[string]siteURL + if err := json.Unmarshal([]byte(raw), &sites); err != nil { + return nil, fmt.Errorf("decode site URL registry: %w", err) + } + if len(sites) == 0 { + return nil, fmt.Errorf("site URL registry is empty") + } + for id, s := range sites { + if s.AuthServiceURL == "" || s.BaseURL == "" { + return nil, fmt.Errorf("site %q: both authServiceUrl and baseUrl are required", id) + } + } + return sites, nil +} + +type userInfoResponse struct { + Account string `json:"account"` + EmployeeID string `json:"employeeId"` + AuthServiceURL string `json:"authServiceUrl"` + BaseURL string `json:"baseUrl"` + NATSURL string `json:"natsUrl"` + SiteID string `json:"siteId"` +} + +// PortalHandler resolves a user's home-site coordinates from the in-memory +// directory cache. The cache holds only accounts present in both hr_employee +// and the users collection (intersected at load time), so a cache hit already +// means the account is a provisioned user. Discovery only: it serves non-secret +// directory data keyed by account and validates no token. The authoritative +// gate is auth-service, which validates the SSO token before minting a JWT. +type PortalHandler struct { + cache *directoryCache + devMode bool + devFallbackSiteID string + devFallbackNatsURL string + sites map[string]siteURL +} + +// NewPortalHandler creates a PortalHandler. devMode synthesizes a dev-site +// entry for accounts absent from the directory so local logins need no seeding. +// sites is the siteId → URL registry used to resolve each account's home-site +// auth-service and base URLs. +func NewPortalHandler(cache *directoryCache, devMode bool, devFallbackSiteID, devFallbackNatsURL string, sites map[string]siteURL) *PortalHandler { + return &PortalHandler{ + cache: cache, + devMode: devMode, + devFallbackSiteID: devFallbackSiteID, + devFallbackNatsURL: devFallbackNatsURL, + sites: sites, + } +} + +// HandleUserInfo resolves the home-site coordinates for the `account` query +// parameter. The frontend supplies the account directly — derived from the SSO +// token's preferred_username claim in production, or the dev login form in dev. +// No token is validated here; this endpoint is discovery only. +func (h *PortalHandler) HandleUserInfo(c *gin.Context) { + ctx := errcode.WithLogValues(c.Request.Context(), "request_id", c.GetString("request_id")) + + account := c.Query("account") + if account == "" { + errhttp.Write(ctx, c, errcode.BadRequest("account is required", + errcode.WithReason(errcode.AuthMissingFields))) + return + } + h.resolve(ctx, c, account) +} + +// resolve answers from the directory cache — a single in-memory lookup, no +// per-request datastore round trips. In devMode an account absent from the +// directory is synthesized onto the dev site. +func (h *PortalHandler) resolve(ctx context.Context, c *gin.Context, account string) { + if !subject.IsValidAccountToken(account) { + errhttp.Write(ctx, c, errcode.BadRequest("account must be a single NATS subject token (no '.', '*', '>' or whitespace)")) + return + } + ctx = errcode.WithLogValues(ctx, "account", account) + + e, ok := h.cache.Get(account) + if !ok { + if !h.devMode { + errhttp.Write(ctx, c, errcode.Forbidden("account not ready for chat", + errcode.WithReason(errcode.PortalAccountNotReady))) + return + } + e = employee{Account: account, SiteID: h.devFallbackSiteID, NATSURL: h.devFallbackNatsURL} + } + + site, ok := h.sites[e.SiteID] + if !ok { + // A directory entry homed on a site missing from the registry is an ops + // misconfiguration, not a client error — surface it as internal. + errhttp.Write(ctx, c, fmt.Errorf("no URLs configured for siteId %q", e.SiteID)) + return + } + + c.JSON(http.StatusOK, userInfoResponse{ + Account: e.Account, + EmployeeID: e.EmployeeID, + AuthServiceURL: site.AuthServiceURL, + BaseURL: site.BaseURL, + NATSURL: e.NATSURL, + SiteID: e.SiteID, + }) +} + +// HandleHealth is the liveness probe: the process is up and serving HTTP. +func (h *PortalHandler) HandleHealth(c *gin.Context) { + c.JSON(http.StatusOK, gin.H{"status": "ok"}) +} + +// HandleReady is the readiness probe: fails until the directory cache holds data. +func (h *PortalHandler) HandleReady(c *gin.Context) { + if !h.cache.Ready() { + c.JSON(http.StatusServiceUnavailable, gin.H{"status": "unavailable"}) + return + } + c.JSON(http.StatusOK, gin.H{"status": "ok"}) +} diff --git a/portal-service/handler_test.go b/portal-service/handler_test.go new file mode 100644 index 000000000..309922c11 --- /dev/null +++ b/portal-service/handler_test.go @@ -0,0 +1,248 @@ +package main + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/hmchangw/chat/pkg/errcode" + "github.com/hmchangw/chat/pkg/errcode/errtest" +) + +func setupRouter(t *testing.T, h *PortalHandler) *gin.Engine { + t.Helper() + gin.SetMode(gin.TestMode) + r := gin.New() + registerRoutes(r, h) + return r +} + +// getUserInfo issues GET /api/userInfo with account as a query parameter. An +// empty account is sent with no query parameter at all (the missing-param case). +func getUserInfo(t *testing.T, r *gin.Engine, account string) *httptest.ResponseRecorder { + t.Helper() + target := "/api/userInfo" + if account != "" { + target += "?account=" + url.QueryEscape(account) + } + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, target, nil)) + return w +} + +func getPath(t *testing.T, r *gin.Engine, path string) *httptest.ResponseRecorder { + t.Helper() + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil)) + return w +} + +// cacheWith returns a directory cache populated with the given entries. +func cacheWith(emps ...employee) *directoryCache { + c := newDirectoryCache() + c.replace(emps) + return c +} + +// testSites is the per-site URL registry used by the handler tests, with a +// distinct domain per site to prove URLs are looked up, not templated. +var testSites = map[string]siteURL{ + "site-a": {AuthServiceURL: "https://auth.site-a.example.com", BaseURL: "https://site-a.example.com"}, + "site-b": {AuthServiceURL: "https://auth.site-b.example.com", BaseURL: "https://site-b.example.com"}, + "site-local": {AuthServiceURL: "https://auth.site-local.example.com", BaseURL: "http://localhost:3000"}, +} + +// newTestHandler builds a PortalHandler with the test site registry and the +// local dev-fallback coordinates. +func newTestHandler(cache *directoryCache, devMode bool) *PortalHandler { + return NewPortalHandler(cache, devMode, "site-local", "ws://localhost:9222", testSites) +} + +func TestHandleUserInfo_HappyPath(t *testing.T) { + h := newTestHandler(cacheWith(aliceEmployee), false) + w := getUserInfo(t, setupRouter(t, h), "alice") + + require.Equal(t, http.StatusOK, w.Code) + var resp userInfoResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(t, userInfoResponse{ + Account: "alice", + EmployeeID: "E001", + AuthServiceURL: "https://auth.site-a.example.com", + BaseURL: "https://site-a.example.com", + NATSURL: "wss://nats-3.site-a.example.com", + SiteID: "site-a", + }, resp) +} + +func TestHandleUserInfo_PerSiteURLs(t *testing.T) { + t.Run("each site resolves its own auth and base URL from the registry", func(t *testing.T) { + h := newTestHandler(cacheWith(aliceEmployee, bobEmployee), false) + r := setupRouter(t, h) + + var alice userInfoResponse + require.NoError(t, json.Unmarshal(getUserInfo(t, r, "alice").Body.Bytes(), &alice)) + assert.Equal(t, "https://auth.site-a.example.com", alice.AuthServiceURL) + assert.Equal(t, "https://site-a.example.com", alice.BaseURL) + + var bob userInfoResponse + require.NoError(t, json.Unmarshal(getUserInfo(t, r, "bob").Body.Bytes(), &bob)) + assert.Equal(t, "https://auth.site-b.example.com", bob.AuthServiceURL) + assert.Equal(t, "https://site-b.example.com", bob.BaseURL) + }) + + t.Run("a site missing from the registry is a server error, not a client error", func(t *testing.T) { + orphan := employee{Account: "carol", EmployeeID: "E003", SiteID: "site-unknown", NATSURL: "wss://nats.example.com"} + h := newTestHandler(cacheWith(orphan), false) + w := getUserInfo(t, setupRouter(t, h), "carol") + assert.Equal(t, http.StatusInternalServerError, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeInternal) + }) +} + +func TestHandleUserInfo_InvalidAccountFormat(t *testing.T) { + // Same token invariant auth-service enforces at minting — refusing here + // keeps the portal from blessing an account the next step must reject. + for _, tt := range []struct{ name, account string }{ + {"dotted account refused", "john.doe"}, + {"wildcard account refused", "mal*ory"}, + } { + t.Run(tt.name, func(t *testing.T) { + h := newTestHandler(cacheWith(aliceEmployee), false) + w := getUserInfo(t, setupRouter(t, h), tt.account) + assert.Equal(t, http.StatusBadRequest, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeBadRequest) + }) + } +} + +func TestHandleUserInfo_MissingAccount(t *testing.T) { + h := newTestHandler(cacheWith(aliceEmployee), false) + w := getUserInfo(t, setupRouter(t, h), "") + + assert.Equal(t, http.StatusBadRequest, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeBadRequest) + errtest.AssertReason(t, w.Body.Bytes(), errcode.AuthMissingFields) +} + +func TestHandleUserInfo_AccountNotReady(t *testing.T) { + t.Run("account absent from a populated cache", func(t *testing.T) { + h := newTestHandler(cacheWith(aliceEmployee), false) + w := getUserInfo(t, setupRouter(t, h), "mallory") + + assert.Equal(t, http.StatusForbidden, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeForbidden) + errtest.AssertReason(t, w.Body.Bytes(), errcode.PortalAccountNotReady) + }) + + t.Run("cache not yet loaded", func(t *testing.T) { + h := newTestHandler(newDirectoryCache(), false) + w := getUserInfo(t, setupRouter(t, h), "alice") + + assert.Equal(t, http.StatusForbidden, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeForbidden) + errtest.AssertReason(t, w.Body.Bytes(), errcode.PortalAccountNotReady) + }) +} + +func TestHandleUserInfo_DevMode(t *testing.T) { + t.Run("known account resolves normally", func(t *testing.T) { + h := newTestHandler(cacheWith(aliceEmployee), true) + w := getUserInfo(t, setupRouter(t, h), "alice") + require.Equal(t, http.StatusOK, w.Code) + var resp userInfoResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(t, "site-a", resp.SiteID) + }) + + t.Run("unknown account falls back to the dev site", func(t *testing.T) { + h := newTestHandler(cacheWith(aliceEmployee), true) + w := getUserInfo(t, setupRouter(t, h), "newdev") + require.Equal(t, http.StatusOK, w.Code) + var resp userInfoResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(t, userInfoResponse{ + Account: "newdev", EmployeeID: "", + AuthServiceURL: "https://auth.site-local.example.com", BaseURL: "http://localhost:3000", + NATSURL: "ws://localhost:9222", SiteID: "site-local", + }, resp) + }) + + t.Run("fallback works before the cache is loaded", func(t *testing.T) { + h := newTestHandler(newDirectoryCache(), true) + w := getUserInfo(t, setupRouter(t, h), "newdev") + require.Equal(t, http.StatusOK, w.Code) + var resp userInfoResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(t, "site-local", resp.SiteID) + }) + + t.Run("bad account format is refused even in dev mode", func(t *testing.T) { + h := newTestHandler(cacheWith(aliceEmployee), true) + w := getUserInfo(t, setupRouter(t, h), "mal*ory") + assert.Equal(t, http.StatusBadRequest, w.Code) + errtest.AssertCode(t, w.Body.Bytes(), errcode.CodeBadRequest) + }) + + t.Run("missing account is bad request", func(t *testing.T) { + h := newTestHandler(cacheWith(aliceEmployee), true) + w := getUserInfo(t, setupRouter(t, h), "") + assert.Equal(t, http.StatusBadRequest, w.Code) + errtest.AssertReason(t, w.Body.Bytes(), errcode.AuthMissingFields) + }) +} + +func TestHandleHealth_AlwaysOK(t *testing.T) { + // Liveness must not depend on directory data being loaded. + h := newTestHandler(newDirectoryCache(), false) + w := getPath(t, setupRouter(t, h), "/healthz") + + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), "ok") +} + +func TestHandleReady(t *testing.T) { + tests := []struct { + name string + cache *directoryCache + wantCode int + }{ + {"cache never loaded", newDirectoryCache(), http.StatusServiceUnavailable}, + {"cache loaded but empty", cacheWith(), http.StatusServiceUnavailable}, + {"cache holds directory data", cacheWith(aliceEmployee), http.StatusOK}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := newTestHandler(tt.cache, false) + w := getPath(t, setupRouter(t, h), "/readyz") + assert.Equal(t, tt.wantCode, w.Code) + }) + } +} + +func TestParseSiteURLs(t *testing.T) { + t.Run("valid registry decodes per-site URLs", func(t *testing.T) { + sites, err := parseSiteURLs(`{"site-a":{"authServiceUrl":"https://auth.a.com","baseUrl":"https://a.com"},"site-b":{"authServiceUrl":"https://auth.b.com","baseUrl":"https://b.com"}}`) + require.NoError(t, err) + assert.Equal(t, siteURL{AuthServiceURL: "https://auth.a.com", BaseURL: "https://a.com"}, sites["site-a"]) + assert.Equal(t, siteURL{AuthServiceURL: "https://auth.b.com", BaseURL: "https://b.com"}, sites["site-b"]) + }) + + for _, tt := range []struct{ name, raw string }{ + {"not JSON", "site-a=https://auth.a.com"}, + {"empty object", "{}"}, + {"missing baseUrl", `{"site-a":{"authServiceUrl":"https://auth.a.com"}}`}, + {"missing authServiceUrl", `{"site-a":{"baseUrl":"https://a.com"}}`}, + } { + t.Run(tt.name+" is rejected", func(t *testing.T) { + _, err := parseSiteURLs(tt.raw) + assert.Error(t, err) + }) + } +} diff --git a/portal-service/integration_test.go b/portal-service/integration_test.go new file mode 100644 index 000000000..5bd31d336 --- /dev/null +++ b/portal-service/integration_test.go @@ -0,0 +1,91 @@ +//go:build integration + +package main + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/mongo" + + "github.com/hmchangw/chat/pkg/testutil" +) + +func TestMain(m *testing.M) { testutil.RunTests(m) } + +func TestMongoDirectoryStore_ListEmployees(t *testing.T) { + db := testutil.MongoDB(t, "portal") + store := newMongoDirectoryStore(db) + ctx := context.Background() + + for _, doc := range []bson.M{ + {"account": "alice", "employeeId": "E001", "siteId": "site-a", "natsUrl": "wss://nats-3.site-a.example.com"}, + {"account": "bob", "employeeId": "E002", "siteId": "site-b", "natsUrl": "wss://nats.site-b.example.com"}, + {"account": "carol", "employeeId": "E003", "siteId": "site-a", "natsUrl": "wss://nats.site-a.example.com"}, + {"account": "eve", "employeeId": "E004", "siteId": "site-a", "natsUrl": "wss://nats.site-a.example.com"}, + } { + _, err := db.Collection("hr_employee").InsertOne(ctx, doc) + require.NoError(t, err) + } + // alice/bob are provisioned on their home site; carol has no users row; eve's + // users row is on a different site; dave is only in users. + _, err := db.Collection("users").InsertMany(ctx, []any{ + bson.M{"_id": "u-alice", "account": "alice", "siteId": "site-a"}, + bson.M{"_id": "u-bob", "account": "bob", "siteId": "site-b"}, + bson.M{"_id": "u-eve", "account": "eve", "siteId": "site-b"}, + bson.M{"_id": "u-dave", "account": "dave", "siteId": "site-a"}, + }) + require.NoError(t, err) + + emps, err := store.ListEmployees(ctx) + require.NoError(t, err) + + byAccount := make(map[string]employee, len(emps)) + for _, e := range emps { + byAccount[e.Account] = e + } + // Only accounts present in BOTH collections for the same site survive the + // $lookup, with users._id projected as UserID. + assert.Equal(t, employee{ + Account: "alice", EmployeeID: "E001", SiteID: "site-a", + NATSURL: "wss://nats-3.site-a.example.com", UserID: "u-alice", + }, byAccount["alice"]) + assert.Equal(t, employee{ + Account: "bob", EmployeeID: "E002", SiteID: "site-b", + NATSURL: "wss://nats.site-b.example.com", UserID: "u-bob", + }, byAccount["bob"]) + require.Len(t, emps, 2, "carol (no users row), eve (site mismatch), and dave (users-only) must be excluded") +} + +func TestMongoDirectoryStore_ListEmployees_Empty(t *testing.T) { + db := testutil.MongoDB(t, "portal") + store := newMongoDirectoryStore(db) + + emps, err := store.ListEmployees(context.Background()) + require.NoError(t, err) + assert.Empty(t, emps) +} + +func TestMongoDirectoryStore_EnsureIndexes_UniqueAccount(t *testing.T) { + db := testutil.MongoDB(t, "portal") + store := newMongoDirectoryStore(db) + ctx := context.Background() + + require.NoError(t, store.EnsureIndexes(ctx)) + require.NoError(t, store.EnsureIndexes(ctx), "EnsureIndexes must be idempotent") + + coll := db.Collection("hr_employee") + _, err := coll.InsertOne(ctx, bson.M{"account": "alice", "employeeId": "E001", "siteId": "site-a", "natsUrl": "wss://nats.site-a.example.com"}) + require.NoError(t, err) + + _, err = coll.InsertOne(ctx, bson.M{"account": "alice", "employeeId": "E099", "siteId": "site-b", "natsUrl": "wss://nats.site-b.example.com"}) + require.Error(t, err, "a second row for the same account must be rejected at write time") + assert.True(t, mongo.IsDuplicateKeyError(err)) + + // A distinct account is unaffected by the unique index. + _, err = coll.InsertOne(ctx, bson.M{"account": "bob", "employeeId": "E002", "siteId": "site-b", "natsUrl": "wss://nats.site-b.example.com"}) + require.NoError(t, err) +} diff --git a/portal-service/main.go b/portal-service/main.go new file mode 100644 index 000000000..fc89dbf88 --- /dev/null +++ b/portal-service/main.go @@ -0,0 +1,139 @@ +package main + +import ( + "context" + "fmt" + "log/slog" + "net/http" + "os" + "sync" + "time" + + "github.com/caarlos0/env/v11" + "github.com/gin-gonic/gin" + + "github.com/hmchangw/chat/pkg/ginutil" + "github.com/hmchangw/chat/pkg/mongoutil" + "github.com/hmchangw/chat/pkg/shutdown" +) + +// cacheRetryInterval paces reload attempts after a failed directory load, so +// a Mongo blip at startup does not leave the portal unready for a full +// refresh interval. +const cacheRetryInterval = 30 * time.Second + +type config struct { + Port string `env:"PORT" envDefault:"8081"` + DevMode bool `env:"DEV_MODE" envDefault:"false"` + DevFallbackSiteID string `env:"PORTAL_DEV_FALLBACK_SITE_ID" envDefault:"site-local"` + DevFallbackNatsURL string `env:"PORTAL_DEV_FALLBACK_NATS_URL" envDefault:"ws://localhost:9222"` + + // SiteURLs is the per-site URL registry: a JSON object mapping siteId to + // {authServiceUrl, baseUrl}. A single template can't express sites on + // different domains, so each site's URLs are listed explicitly. + SiteURLs string `env:"PORTAL_SITE_URLS,required"` + + // CacheRefreshInterval drives how often the directory is reloaded (the + // hr_employee × users intersection via $lookup). Shorter than the daily HR + // cron so a newly provisioned user appears within a couple of hours. + CacheRefreshInterval time.Duration `env:"PORTAL_CACHE_REFRESH_INTERVAL" envDefault:"2h"` + + MongoURI string `env:"MONGO_URI,required"` + MongoDB string `env:"MONGO_DB" envDefault:"portal"` + MongoUsername string `env:"MONGO_USERNAME" envDefault:""` + MongoPassword string `env:"MONGO_PASSWORD" envDefault:""` +} + +func main() { + slog.SetDefault(slog.New(slog.NewJSONHandler(os.Stdout, nil))) + + if err := run(); err != nil { + slog.Error("fatal error", "error", err) + os.Exit(1) + } +} + +func run() error { + cfg, err := env.ParseAs[config]() + if err != nil { + return fmt.Errorf("parse config: %w", err) + } + + sites, err := parseSiteURLs(cfg.SiteURLs) + if err != nil { + return fmt.Errorf("parse site URL registry: %w", err) + } + + ctx := context.Background() + + mongoClient, err := mongoutil.Connect(ctx, cfg.MongoURI, cfg.MongoUsername, cfg.MongoPassword) + if err != nil { + return fmt.Errorf("connect mongo: %w", err) + } + + store := newMongoDirectoryStore(mongoClient.Database(cfg.MongoDB)) + if err := store.EnsureIndexes(ctx); err != nil { + return fmt.Errorf("ensure directory indexes: %w", err) + } + + // Populate the directory cache in the background; /readyz stays + // unavailable until the first successful load. + cache := newDirectoryCache() + refreshCtx, refreshCancel := context.WithCancel(ctx) + defer refreshCancel() + var refreshWG sync.WaitGroup + refreshWG.Go(func() { + cache.RefreshLoop(refreshCtx, store, cfg.CacheRefreshInterval, cacheRetryInterval) + }) + + slog.Info("directory config", "sites", len(sites), "refreshInterval", cfg.CacheRefreshInterval.String()) + + handler := NewPortalHandler(cache, cfg.DevMode, + cfg.DevFallbackSiteID, cfg.DevFallbackNatsURL, sites) + if cfg.DevMode { + slog.Info("dev mode enabled — unknown accounts fall back to the dev site") + } + + gin.SetMode(gin.ReleaseMode) + r := gin.New() + r.Use(gin.Recovery()) + r.Use(ginutil.RequestID()) + r.Use(ginutil.AccessLog()) + r.Use(ginutil.CORS()) + registerRoutes(r, handler) + + addr := fmt.Sprintf(":%s", cfg.Port) + srv := &http.Server{ + Addr: addr, + Handler: r, + ReadTimeout: 10 * time.Second, + WriteTimeout: 10 * time.Second, + } + + srvErr := make(chan error, 1) + go func() { + slog.Info("portal service starting", "addr", addr) + srvErr <- srv.ListenAndServe() + }() + + shutdownDone := make(chan struct{}) + go func() { + defer close(shutdownDone) + shutdown.Wait(ctx, 25*time.Second, func(ctx context.Context) error { + slog.Info("shutting down portal service") + err := srv.Shutdown(ctx) + refreshCancel() + refreshWG.Wait() + mongoutil.Disconnect(ctx, mongoClient) + return err + }) + }() + + err = <-srvErr + if err != nil && err != http.ErrServerClosed { + return fmt.Errorf("listen portal server: %w", err) + } + <-shutdownDone + + return nil +} diff --git a/portal-service/mock_store_test.go b/portal-service/mock_store_test.go new file mode 100644 index 000000000..6b29247bf --- /dev/null +++ b/portal-service/mock_store_test.go @@ -0,0 +1,56 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: store.go +// +// Generated by this command: +// +// mockgen -source=store.go -destination=mock_store_test.go -package=main +// + +// Package main is a generated GoMock package. +package main + +import ( + context "context" + reflect "reflect" + + gomock "go.uber.org/mock/gomock" +) + +// MockDirectoryStore is a mock of DirectoryStore interface. +type MockDirectoryStore struct { + ctrl *gomock.Controller + recorder *MockDirectoryStoreMockRecorder + isgomock struct{} +} + +// MockDirectoryStoreMockRecorder is the mock recorder for MockDirectoryStore. +type MockDirectoryStoreMockRecorder struct { + mock *MockDirectoryStore +} + +// NewMockDirectoryStore creates a new mock instance. +func NewMockDirectoryStore(ctrl *gomock.Controller) *MockDirectoryStore { + mock := &MockDirectoryStore{ctrl: ctrl} + mock.recorder = &MockDirectoryStoreMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockDirectoryStore) EXPECT() *MockDirectoryStoreMockRecorder { + return m.recorder +} + +// ListEmployees mocks base method. +func (m *MockDirectoryStore) ListEmployees(ctx context.Context) ([]employee, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListEmployees", ctx) + ret0, _ := ret[0].([]employee) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListEmployees indicates an expected call of ListEmployees. +func (mr *MockDirectoryStoreMockRecorder) ListEmployees(ctx any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListEmployees", reflect.TypeOf((*MockDirectoryStore)(nil).ListEmployees), ctx) +} diff --git a/portal-service/routes.go b/portal-service/routes.go new file mode 100644 index 000000000..b7ff3d137 --- /dev/null +++ b/portal-service/routes.go @@ -0,0 +1,9 @@ +package main + +import "github.com/gin-gonic/gin" + +func registerRoutes(r *gin.Engine, h *PortalHandler) { + r.GET("/api/userInfo", h.HandleUserInfo) + r.GET("/healthz", h.HandleHealth) + r.GET("/readyz", h.HandleReady) +} diff --git a/portal-service/store.go b/portal-service/store.go new file mode 100644 index 000000000..d853dac87 --- /dev/null +++ b/portal-service/store.go @@ -0,0 +1,26 @@ +package main + +import "context" + +//go:generate mockgen -source=store.go -destination=mock_store_test.go -package=main + +// employee is one row of the load-time intersection of the HR-owned hr_employee +// collection and the users collection: an account's home site, NATS +// coordinates, and canonical userId. hr_employee is rewritten by a daily HR +// sync cron; the portal reads it, joins it to users, and enforces a unique +// account index at startup. +type employee struct { + Account string `json:"account" bson:"account"` + EmployeeID string `json:"employeeId" bson:"employeeId"` + SiteID string `json:"siteId" bson:"siteId"` + NATSURL string `json:"natsUrl" bson:"natsUrl"` + // UserID is users._id, projected by the directory $lookup. Held in memory so + // the portal needs no per-request users query; not returned to the client. + UserID string `json:"userId" bson:"userId"` +} + +// DirectoryStore reads the HR employee directory (joined with users) that backs +// the in-memory cache. +type DirectoryStore interface { + ListEmployees(ctx context.Context) ([]employee, error) +} diff --git a/portal-service/store_mongo.go b/portal-service/store_mongo.go new file mode 100644 index 000000000..1137bd027 --- /dev/null +++ b/portal-service/store_mongo.go @@ -0,0 +1,76 @@ +package main + +import ( + "context" + "fmt" + + "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/mongo" + "go.mongodb.org/mongo-driver/v2/mongo/options" +) + +type mongoDirectoryStore struct { + employees *mongo.Collection +} + +func newMongoDirectoryStore(db *mongo.Database) *mongoDirectoryStore { + return &mongoDirectoryStore{employees: db.Collection("hr_employee")} +} + +// EnsureIndexes enforces account uniqueness on hr_employee so a buggy HR cron +// write fails at insert time instead of publishing two home sites for one account. +func (s *mongoDirectoryStore) EnsureIndexes(ctx context.Context) error { + if _, err := s.employees.Indexes().CreateOne(ctx, mongo.IndexModel{ + Keys: bson.D{{Key: "account", Value: 1}}, + Options: options.Index().SetUnique(true), + }); err != nil { + return fmt.Errorf("ensure hr_employee (account) unique index: %w", err) + } + return nil +} + +// ListEmployees returns the intersection of hr_employee and the users +// collection: every account in hr_employee that is also provisioned in users +// for the same site, with the canonical users._id projected as userId. The +// $lookup keyed on {account, siteId} performs the join in Mongo, so the +// in-memory cache already means "employee AND provisioned" and the portal does +// no per-request users query. users must live in the same database as +// hr_employee — $lookup cannot cross databases. +func (s *mongoDirectoryStore) ListEmployees(ctx context.Context) ([]employee, error) { + pipeline := mongo.Pipeline{ + bson.D{{Key: "$lookup", Value: bson.D{ + {Key: "from", Value: "users"}, + {Key: "let", Value: bson.D{ + {Key: "acct", Value: "$account"}, + {Key: "site", Value: "$siteId"}, + }}, + {Key: "pipeline", Value: mongo.Pipeline{ + bson.D{{Key: "$match", Value: bson.D{{Key: "$expr", Value: bson.D{{Key: "$and", Value: bson.A{ + bson.D{{Key: "$eq", Value: bson.A{"$account", "$$acct"}}}, + bson.D{{Key: "$eq", Value: bson.A{"$siteId", "$$site"}}}, + }}}}}}}, + bson.D{{Key: "$project", Value: bson.D{{Key: "_id", Value: 1}}}}, + bson.D{{Key: "$limit", Value: 1}}, + }}, + {Key: "as", Value: "provisioned"}, + }}}, + bson.D{{Key: "$match", Value: bson.D{{Key: "provisioned.0", Value: bson.D{{Key: "$exists", Value: true}}}}}}, + bson.D{{Key: "$project", Value: bson.D{ + {Key: "_id", Value: 0}, + {Key: "account", Value: 1}, + {Key: "employeeId", Value: 1}, + {Key: "siteId", Value: 1}, + {Key: "natsUrl", Value: 1}, + {Key: "userId", Value: bson.D{{Key: "$arrayElemAt", Value: bson.A{"$provisioned._id", 0}}}}, + }}}, + } + cur, err := s.employees.Aggregate(ctx, pipeline) + if err != nil { + return nil, fmt.Errorf("aggregate hr_employee with users lookup: %w", err) + } + var emps []employee + if err := cur.All(ctx, &emps); err != nil { + return nil, fmt.Errorf("decode employees: %w", err) + } + return emps, nil +}