Skip to content

feat(auth): add AuthProvider Redux state integration#2

Open
gs-layer wants to merge 1 commit intofeat/auth-providerfrom
feat/redux-auth-provider
Open

feat(auth): add AuthProvider Redux state integration#2
gs-layer wants to merge 1 commit intofeat/auth-providerfrom
feat/redux-auth-provider

Conversation

@gs-layer
Copy link
Copy Markdown
Owner

Summary

  • Add two Redux slices (auth/session, auth/permissions) to the auth plugin, projecting AuthProvider state into the store via event-driven effects
  • Introduce AuthSessionMeta type for token-safe state — no sensitive tokens in Redux by default, configurable via redux.includeTokens
  • Add useAuth() React hook in @cyberfabric/react with safe degradation when auth plugin is not registered
  • Cover integration with 40 unit tests (slices, effects, actions, error paths, cleanup, provider without subscribe)

Key design decisions

  • Two slices (SRP): auth/session for session lifecycle, auth/permissions for RBAC — independent update cycles
  • Token safety: AuthSessionMeta ({ kind, expiresAt }) instead of sentinel values — compile-time protection against accidental token leakage
  • Redux = projection: Provider remains source of truth; useAuth() for React, app.auth.getSession() for imperative code
  • Module augmentation: HAI3Actions extended from auth plugin file — auth actions visible in types only when plugin is imported
  • Plugin dependencies: dependencies: ['effects'] for guaranteed init order

Test plan

  • toSessionOrMeta — bearer/cookie/custom × includeTokens true/false (6 tests)
  • Slice initial states and reducers (2 tests)
  • Action event emission (5 tests)
  • Sync flow — authenticated, unauthenticated, fallback getSession, auto-fetch permissions (6 tests)
  • Subscribe bridge — state, error, includeTokens (3 tests)
  • Login/logout/refresh flows including no-op when method absent (6 tests)
  • Permissions fetch — success, no-op, error (3 tests)
  • Error serialization — Error, non-Error, per operation (5 tests)
  • Cleanup — EventBus unsub + provider unsub (2 tests)
  • Provider without subscribe — imperative-only mode (2 tests)
  • Type check: npx tsc --noEmit — 0 new errors

Integrate AuthProvider contract into Redux store via two event-driven
slices (auth/session, auth/permissions), enabling reactive auth state
consumption in UI components through useAppSelector and useAuth() hook.

- Add auth/session slice (status, session, error, lastSyncAt, capabilities)
- Add auth/permissions slice (permissions, loading, error) for independent RBAC
- Add AuthSessionMeta type for token-safe Redux state (no sensitive tokens by default)
- Add auth effects: subscribe() bridge, sync, login, logout, refresh, permissions fetch
- Add auth actions: syncAuth, loginAction, logoutAction, refreshAuth, fetchPermissions
- Add useAuth() React hook in @cyberfabric/react with safe degradation
- Extend auth plugin config: redux.includeTokens option (default: false)
- Add HAI3Actions module augmentation from auth plugin
- Add 40 unit tests covering slices, effects, actions, error paths, cleanup
- Update AI target docs, package CLAUDE.md files, architecture exploration
@tscbmstubp
Copy link
Copy Markdown

Correctness ❌

  1. packages/framework/src/effects/authEffects.ts:118-137 updates only auth/session when the provider emits subscribe() state changes. If the provider pushes unauthenticated or error after a prior successful login, auth/permissions is left untouched, so the store can still expose the previous user's roles/permissions after logout or session expiry. The imperative sync and logout paths clear permissions, but the reactive path does not, which makes behavior inconsistent and leaks stale authorization state into the UI.

  2. packages/framework/src/authTypes.ts:34-38 exports isFullAuthSession() as a type guard, but for any non-bearer session it returns true unconditionally. That is incorrect when redux.includeTokens is false, because toSessionOrMeta() intentionally redacts cookie/custom sessions to { kind, expiresAt? }. Consumers narrowing with this guard will believe they have a full AuthSession and may read missing fields such as csrfToken or custom properties.

Test Coverage ⚠️

The new unit tests are substantial, but they miss the two edge cases above:

  • no test covers a provider.subscribe() transition from authenticated state to unauthenticated or error and asserts that permissions are cleared
  • no test covers isFullAuthSession() for redacted cookie/custom session snapshots

Security ❌

The stale-permissions bug in authEffects.ts is also a security issue. RBAC decisions in React code can continue using old permissions after the provider reports logout/session failure, which can keep privileged UI paths visible until a manual sync happens.

Mistakes & Potential Misbehaviors ❌

Because useAuth() exposes both status and permissions reactively, the stale reactive permissions state is not theoretical. Any component keyed off permissions without separately guarding on status can render as authorized after the auth provider has already transitioned away from an authenticated state.


Summary

Area Rating
Correctness ❌ reactive auth state handling is incomplete
Conformance ✅ aligns with the intended event-driven architecture
Style ✅ generally clear and consistent
Performance ✅ no meaningful concerns found
Tests ⚠️ good baseline coverage, but key reactive edge cases are untested
Security ❌ stale RBAC state can survive provider-driven logout/error
Reviewer concerns ⚠️ unavailable in this environment
Risk 🟡 Medium

Recommendation

Minimum (blocking) — clear auth/permissions whenever the provider emits a non-authenticated terminal state (unauthenticated, and likely error) through the subscribe bridge, and add tests for that transition.

The problem is that the PR implements two different paths for auth state updates, and only one of them clears permissions.

In the imperative path, permissions are cleared correctly:

  • syncAuth() handles checkAuth().authenticated === false by dispatching clearPermissions() in packages/framework/src/
    effects/authEffects.ts:147
    But in the reactive path, the provider can push state changes through subscribe(), and that handler only updates the
    session slice:

  • the provider event is bridged at packages/framework/src/effects/authEffects.ts:118

  • that event is consumed at packages/framework/src/effects/authEffects.ts:130

  • inside that handler, it dispatches only setAuthSessionState(...) and never clearPermissions()

So this sequence can happen:

  1. User logs in.
  2. auth/permissions gets populated, for example, with admin roles.
  3. Later, the auth provider emits a reactive state change like unauthenticated because the token expired, a session was
    revoked, or a logout happened elsewhere.
  4. The code updates auth/session.status to unauthenticated, but leaves auth/permissions untouched.

That means Redux can end up in an inconsistent state:

  • auth/session.status = 'unauthenticated'
  • auth/permissions.permissions = previous user's permissions

Why that matters:

  • useAuth() exposes both values independently in packages/react/src/hooks/useAuth.ts:82
  • any component that checks permissions directly, or checks them before checking status, can still render privileged UI
    based on stale permissions

The fix is straightforward: in the AuthEvents.StateChanged handler, when the pushed state is no longer authenticated, also clear permissions. At minimum, that should happen for unauthenticated; depending on intended semantics, probably also for error.


Minimum (blocking) — fix isFullAuthSession() so it distinguishes full vs redacted cookie/custom sessions correctly, or remove the guard if that distinction cannot be made safely from the projected state.

The second issue is a bad type guard. It tells TypeScript something is a full AuthSession when, at runtime, that is not
necessarily true.

The guard is here: packages/framework/src/authTypes.ts:34

export function isFullAuthSession(
session: AuthSession | AuthSessionMeta,
): session is AuthSession {
return session.kind === 'bearer' ? 'token' in session: true;
}

What it claims:

  • bearer session is “full” only if it has a token
  • every cookie or custom session is always “full”

That last part is wrong because this PR also introduces redacted session snapshots via toSessionOrMeta() in packages/
framework/src/effects/authEffects.ts:90. When redux.includeTokens is false, it deliberately strips session data down to:

  • bearer: { kind, expiresAt }
  • cookie: { kind, expiresAt }
  • custom: { kind, expiresAt: undefined }

So for cookie/custom sessions, Redux may hold only metadata, not the full provider session.

Example:

  • provider returns cookie session { kind: 'cookie', csrfToken: 'abc', expiresAt: 123 }
  • toSessionOrMeta(..., false) stores { kind: 'cookie', expiresAt: 123 }
  • isFullAuthSession() returns true anyway, because it treats all non-bearer sessions as full

After that, consumer code can do something like:

if (isFullAuthSession(session)) {
useCsrf(session.csrfToken);
}

TypeScript will allow it, but csrfToken is actually missing at runtime.

Why this matters:

  • it defeats the purpose of the safe redacted Redux projection
  • it creates runtime undefined-field bugs
  • it can mislead consumers into assuming sensitive/session-specific fields are present when they were intentionally
    removed

The fix is one of these:

  1. Remove isFullAuthSession() entirely if the projected state cannot safely distinguish full vs redacted cookie/custom
    sessions.
  2. Add an explicit discriminator to the stored shape, such as redacted: true/false.
  3. Make the guard check for required runtime fields per session kind instead of returning true for all non-bearer kinds.

Right now, the guard is unsound because the type narrowing is stronger than the runtime guarantee.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants