Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions auth-service/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
97 changes: 97 additions & 0 deletions auth-service/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
4 changes: 4 additions & 0 deletions auth-service/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) }
15 changes: 9 additions & 6 deletions auth-service/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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")
Expand All @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion chat-frontend/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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_<service>.go` rather than substring-matching the english text.

Expand Down
13 changes: 6 additions & 7 deletions chat-frontend/deploy/30-render-config.sh
Original file line number Diff line number Diff line change
@@ -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"
4 changes: 1 addition & 3 deletions chat-frontend/deploy/config.js.template
Original file line number Diff line number Diff line change
@@ -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}"
Expand Down
4 changes: 1 addition & 3 deletions chat-frontend/deploy/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions chat-frontend/src/api/_transport/asyncJob.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions chat-frontend/src/api/_transport/asyncJob.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ const REASON_COPY: Record<string, string> = {
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.",
}

/**
Expand Down
3 changes: 1 addition & 2 deletions chat-frontend/src/api/auth/oidcClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading
Loading