feat: add structured authentication event logging (#1104)#1289
Merged
feat: add structured authentication event logging (#1104)#1289
Conversation
Add auth-logger module emitting structured JSON lines to stdout for Docker/self-hosted log forwarding with secret redaction. Events logged: - auth.login.success — valid session on /auth/check - auth.login.failure — invalid/missing session on /auth/check - auth.logout — POST /auth/logout with valid session - auth.session.invalid — auth gate rejects anonymous request - auth.denied — authorization middleware blocks non-admin write Each event includes timestamp, event type, request method/path, and optionally the subject (email) and denial reason. Sensitive data (tokens, cookies, session IDs) is never included in log output. - New api/src/auth-logger.ts with emitAuthEvent(), logAuthEvent(), redactHeaders(), and extractRequestContext() - Integrated into auth gate (security.ts), authorization middleware, logout handler, and auth check handler (app.ts) - 11 new tests covering event emission, redaction, and integration Fixes #1104 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughAdds a new auth-event logger that emits redacted, optionally pseudonymized JSON-line auth events to stdout and wires logging into auth/authorization/security flows (login success/failure, logout, session invalid, denied) with configurable HMAC hash key handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant App as App / Middleware
participant AuthLogger as Auth Logger (auth-logger.ts)
participant Stdout as Stdout
Client->>App: HTTP request (may include session/cookie)
alt missing or invalid session
App->>AuthLogger: logAuthEvent("auth.session.invalid", request, { reason })
else authenticated
App->>AuthLogger: logAuthEvent("auth.login.success" / "auth.login.failure" / "auth.logout" / "auth.denied", request, { subject?, reason? })
end
AuthLogger->>Stdout: emitAuthEvent -> write redacted, pseudonymized JSON line
App->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
bot
added
the
size:L
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
Sequence DiagramShows how the app emits structured JSON auth events for key authentication and authorization outcomes (login success/failure, logout, session invalidation, and authorization denial) — the core change in this PR. sequenceDiagram
participant Client
participant API
participant AuthLogger
Client->>API: GET /auth/check
API->>API: Resolve session from cookie
alt Session valid
API->>AuthLogger: logAuthEvent("auth.login.success", req, {subject})
AuthLogger-->>API: (JSON line to stdout)
API-->>Client: 200 OK
else Session missing/invalid
API->>AuthLogger: logAuthEvent("auth.login.failure", req, {reason})
AuthLogger-->>API: (JSON line to stdout)
API-->>Client: 401 Unauthorized
end
Client->>API: POST /auth/logout (with session cookie)
API->>API: Invalidate session
API->>AuthLogger: logAuthEvent("auth.logout", req, {subject})
AuthLogger-->>API: (JSON line to stdout)
API-->>Client: 204 No Content
Client->>API: PUT /layouts/* (state-changing request)
API->>API: Authorization middleware checks role
alt Not admin
API->>AuthLogger: logAuthEvent("auth.denied", req, {subject, reason})
AuthLogger-->>API: (JSON line to stdout)
API-->>Client: 403 Forbidden
end
Client->>API: Any request without valid session (auth gate)
API->>AuthLogger: logAuthEvent("auth.session.invalid", req, {reason})
AuthLogger-->>API: (JSON line to stdout)
API-->>Client: 401 Unauthorized
Generated by CodeAnt AI |
Contributor
Nitpicks 🔍
|
Contributor
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/src/auth-logger.test.ts`:
- Around line 1-8: The test imports unused symbols `mock` and `logAuthEvent`
which triggers the `@typescript-eslint/no-unused-vars` error; update the import
list in auth-logger.test.ts to remove `mock` and/or `logAuthEvent` (or
alternatively use them explicitly in the test if the intent was to exercise them
directly) so only actually used symbols remain (keep `beforeEach`, `describe`,
`expect`, `it`, `spyOn` and remove `mock` and `logAuthEvent` unless you add a
direct call to `logAuthEvent` or use `mock` in a test).
- Around line 104-314: Install and restore the stdout spy in
beforeEach/afterEach instead of per-test to avoid leakage: in both describe
blocks ("emitAuthEvent" and "auth event integration") declare a let writeSpy; in
beforeEach set writeSpy = spyOn(process.stdout, "write").mockImplementation(()
=> true); and in afterEach call writeSpy.mockRestore(); then remove per-test
mockRestore() calls and any duplicate spy setup inside individual tests (leave
only the emits and assertions that read writeSpy.mock.calls). This keeps the
unique symbol spyOn(process.stdout, "write") managed safely across tests.
In `@api/src/auth-logger.ts`:
- Around line 15-23: AuthEvent currently logs raw personal identifiers (subject
from claims.sub and ip from x-real-ip); replace those plain values with
pseudonymized hashes at emit time by adding a helper (e.g.,
pseudonymizeIdentifier(value: string): string) that computes a stable SHA-256
(or HMAC-SHA256 with a config-held key/salt) and use it when populating
AuthEvent.subject and AuthEvent.ip so logs contain non-reversible identifiers
while remaining linkable for audit; update any code paths that create AuthEvent
(references: AuthEvent, subject, ip, claims.sub, x-real-ip) to call that helper
before logging and ensure the hashing key/salt is configurable and protected.
Follow up on CodeRabbit review for #1104 auth event logging:\n- pseudonymize subject/ip identifiers with keyed, namespaced hashes\n- wire configurable auth log hash key from security config\n- derive log hash key from session secret context or generate ephemeral fallback\n- tighten hash key validation and add config diagnostics\n- refactor stdout spy lifecycle and auth logger tests\n\nFixes #1104
Contributor
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
bot
added
size:XL
Contributor
|
CodeAnt AI Incremental review completed. |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/src/app.ts (1)
119-134:⚠️ Potential issue | 🟡 MinorAuth check logging is well-placed with correct event semantics.
auth.login.failureon missing session andauth.login.successon valid session are logged at the right points. The rawclaims.subis correctly passed tologAuthEvent, with pseudonymization handled byemitAuthEventdownstream.Note: the same reliability concern raised on
security.tsline 1120 applies here — an exception fromlogAuthEventwould prevent the 401 response or the 204+cookie refresh from being sent. Consider wrapping in try/catch for the same reason.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/app.ts` around lines 119 - 134, The logAuthEvent calls around the authentication check (after resolveAuthenticatedSessionClaims) can throw and block sending the 401 or success response; wrap both invocations of logAuthEvent (the failure branch and the success branch where you pass claims.sub) in try/catch blocks so any logging error is caught and does not prevent returning the JSON 401 or proceeding with the 204+cookie refresh; ensure the catch either swallows or records the logging error safely (e.g., fallback to console.error or a non-throwing logger) so response flow from resolveAuthenticatedSessionClaims remains unaffected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/src/auth-logger.test.ts`:
- Around line 185-188: The test reads from authEvents using .some() then calls
authEvents.find(...) and accesses properties on the result (event) without null
checking; change each pattern (e.g., the block with authEvents.some(...) and
const event = authEvents.find(...)) to either assert the find result is defined
before accessing properties (e.g., const event = authEvents.find(...);
expect(event).toBeDefined(); /* then access event!.reason */) or collapse to a
single assertion that directly expects the found object (e.g., const event =
authEvents.find(...); expect(event).toMatchObject({ reason: "missing or invalid
session cookie", path: "/api/layouts" })); apply the same guard/non-null
assertion fix to the other occurrences noted (the blocks around lines referenced
for authEvents).
- Around line 170-189: Create a small shared helper (e.g., extractAuthEvents)
that accepts the test spy (writeSpy) and returns parsed auth-related event
objects by mapping writeSpy.mock.calls, JSON.parse-ing each call, and filtering
where e?.event?.startsWith("auth."); replace the repeated map→parse→filter
blocks in the tests (references: the test that asserts "auth.session.invalid"
and the other duplicated blocks) with calls to extractAuthEvents(writeSpy) and
use the returned array in assertions (e.g., authEvents.some(...) and
authEvents.find(...)); keep helper local to the test file and ensure it returns
typed/nullable-safe results so existing expects (including .reason and .path)
work unchanged.
In `@api/src/auth-logger.ts`:
- Around line 139-148: The extractRequestContext function currently reads only
the x-real-ip header; add a concise JSDoc above extractRequestContext explaining
that this is intentional because x-real-ip is expected to be set by a trusted
reverse proxy and that x-forwarded-for (and similar headers like
cf-connecting-ip or forwarded) are intentionally not used due to spoofing
concerns and because x-forwarded-for is already handled/redacted via
REDACTED_FIELDS—do not change logic, just document the security rationale and
reference the REDACTED_FIELDS policy so future contributors don’t replace
x-real-ip with x-forwarded-for.
- Around line 52-74: The info-level log in configureAuthLogHashKey is too noisy;
replace the console.info call that reports a configured authLogHashKey with a
debug-level or conditional log so it doesn't fire on every startup or test.
Locate configureAuthLogHashKey and change the console.info(`[auth-logger] Auth
log hash key configured...`) to either console.debug or wrap it behind a runtime
check (e.g., only log when NODE_ENV !== 'test' or when a DEBUG/VERBOSE flag is
set) so only operators/devs requesting verbose logs see it; leave the warning
paths and hasWarnedDefaultHashKey behavior unchanged.
In `@api/src/security.ts`:
- Around line 92-94: MIN_AUTH_LOG_HASH_KEY_LENGTH is defined in two places
causing drift; remove the duplicate by exporting the canonical constant from one
module (e.g., export const MIN_AUTH_LOG_HASH_KEY_LENGTH = 16 from security.ts)
and import that single constant where needed in auth-logger.ts. Update
references in resolveAuthLogHashKey and configureAuthLogHashKey to use the
imported constant, remove the local duplicate, and run TypeScript/TSLint to
ensure no leftover shadowed definitions remain.
- Around line 1120-1122: The unguarded call to logAuthEvent in the auth gate
(the middleware that checks sessions) can throw and crash requests; wrap the
logAuthEvent("auth.session.invalid", c.req.raw, {...}) invocation in a try/catch
so any errors from logging/HMAC/IO are swallowed or routed to a non-throwing
fallback (e.g., processLogger.error) and do not propagate, ensuring the
middleware still returns the intended 401/redirect response; apply the same
defensive wrapping pattern to other high-traffic callers of logAuthEvent such as
the handlers referenced in app.ts and authorization.ts.
---
Outside diff comments:
In `@api/src/app.ts`:
- Around line 119-134: The logAuthEvent calls around the authentication check
(after resolveAuthenticatedSessionClaims) can throw and block sending the 401 or
success response; wrap both invocations of logAuthEvent (the failure branch and
the success branch where you pass claims.sub) in try/catch blocks so any logging
error is caught and does not prevent returning the JSON 401 or proceeding with
the 204+cookie refresh; ensure the catch either swallows or records the logging
error safely (e.g., fallback to console.error or a non-throwing logger) so
response flow from resolveAuthenticatedSessionClaims remains unaffected.
- Deduplicate MIN_AUTH_LOG_HASH_KEY_LENGTH (export from auth-logger, import in security) - Add JSDoc to extractRequestContext explaining x-real-ip choice - Reduce startup log noise (console.info → console.debug) - Extract safeLogAuthEvent helper for fault-tolerant logging at all 5 call sites - Extract extractAuthEvents test helper to deduplicate parse/filter pattern - Add null guards on .find() results in integration tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
bot
added
size:XL
Contributor
Sequence DiagramShows how auth-related HTTP requests trigger high-level auth events that are pseudonymized/redacted and emitted as JSON lines to stdout. This captures the primary success path and key denial/invalid-session cases introduced in this PR. sequenceDiagram
participant Client
participant API
participant Security
participant AuthLogger
participant Stdout
Client->>API: HTTP request (/auth/check, /auth/logout, or API write)
API->>Security: Validate session / check authorization
alt /auth/check with valid session
Security-->>AuthLogger: auth.login.success (subject)
else /auth/check missing/invalid
Security-->>AuthLogger: auth.login.failure (reason)
end
alt POST /auth/logout with valid session
Security-->>AuthLogger: auth.logout (subject)
end
alt Protected write by non-admin
Security-->>AuthLogger: auth.denied (subject, reason)
end
alt Anonymous request rejected by auth gate
Security-->>AuthLogger: auth.session.invalid (reason, path)
end
AuthLogger->>Stdout: Write redacted, pseudonymized JSON line (timestamp, event, method, path, subject?, ip?)
Stdout-->>API: Log forwarded to host/syslog/SIEM (out of band)
Generated by CodeAnt AI |
Contributor
|
CodeAnt AI Incremental review completed. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
api/src/app.ts (1)
20-20:⚠️ Potential issue | 🟠 MajorRemove unused
safeLogAuthEventimport to satisfy lint.
safeLogAuthEventis imported but never used, which triggers@typescript-eslint/no-unused-varsand can fail CI.♻️ Minimal fix
-import { configureAuthLogHashKey, safeLogAuthEvent } from "./auth-logger"; +import { configureAuthLogHashKey } from "./auth-logger";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/app.ts` at line 20, Remove the unused import safeLogAuthEvent from the import statement that currently reads "import { configureAuthLogHashKey, safeLogAuthEvent } from \"./auth-logger\""; keep configureAuthHashKey (or configureAuthLogHashKey) as needed and only import symbols actually used to resolve the `@typescript-eslint/no-unused-vars` lint error and restore CI; update the import to only include configureAuthLogHashKey.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/src/auth-logger.ts`:
- Around line 26-27: Replace the fixed DEFAULT_AUTH_LOG_HASH_KEY constant with a
module-scoped lazy fallback: introduce a let fallbackAuthLogHashKey variable and
a getter function (e.g., getAuthLogHashKey()) that returns the configured key
when present and otherwise generates and caches a per-process random key (using
crypto.randomBytes) that meets MIN_AUTH_LOG_HASH_KEY_LENGTH; update places
currently using DEFAULT_AUTH_LOG_HASH_KEY to call getAuthLogHashKey() so the
random fallback is consistent per process and only generated once.
---
Duplicate comments:
In `@api/src/app.ts`:
- Line 20: Remove the unused import safeLogAuthEvent from the import statement
that currently reads "import { configureAuthLogHashKey, safeLogAuthEvent } from
\"./auth-logger\""; keep configureAuthHashKey (or configureAuthLogHashKey) as
needed and only import symbols actually used to resolve the
`@typescript-eslint/no-unused-vars` lint error and restore CI; update the import
to only include configureAuthLogHashKey.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
api/src/app.tsapi/src/auth-logger.test.tsapi/src/auth-logger.tsapi/src/authorization.tsapi/src/security.ts
Comment on lines
+26
to
+27
| const DEFAULT_AUTH_LOG_HASH_KEY = "rackula:auth-log:v1:default"; | ||
| export const MIN_AUTH_LOG_HASH_KEY_LENGTH = 16; |
There was a problem hiding this comment.
Replace fixed default hash key with per-process random fallback.
When configuration is absent, the current constant fallback generates predictable pseudonyms across instances. A lazily generated per-process random key is safer and matches the intended fallback model.
🔐 Suggested fix
-import { createHmac } from "node:crypto";
+import { createHmac, randomBytes } from "node:crypto";
-const DEFAULT_AUTH_LOG_HASH_KEY = "rackula:auth-log:v1:default";
export const MIN_AUTH_LOG_HASH_KEY_LENGTH = 16;
+const FALLBACK_AUTH_LOG_HASH_KEY_BYTES = 32;
type AuthIdentifierType = "subject" | "ip";
let authLogHashKey: string | undefined;
let hasWarnedDefaultHashKey = false;
+let fallbackAuthLogHashKey: string | undefined;
export function resetAuthLogHashConfigForTests(): void {
authLogHashKey = undefined;
hasWarnedDefaultHashKey = false;
+ fallbackAuthLogHashKey = undefined;
}
function getAuthLogHashKey(): string {
if (authLogHashKey) {
return authLogHashKey;
}
+ if (!fallbackAuthLogHashKey) {
+ fallbackAuthLogHashKey = randomBytes(FALLBACK_AUTH_LOG_HASH_KEY_BYTES).toString("hex");
+ }
if (!hasWarnedDefaultHashKey) {
console.warn(
- "[auth-logger] No auth log hash key configured. Falling back to default auth log hash key; configure RACKULA_AUTH_LOG_HASH_KEY to avoid predictable cross-instance pseudonyms.",
+ "[auth-logger] No auth log hash key configured. Falling back to an ephemeral per-process auth log hash key; configure RACKULA_AUTH_LOG_HASH_KEY for stable pseudonymization across restarts.",
);
hasWarnedDefaultHashKey = true;
}
- return DEFAULT_AUTH_LOG_HASH_KEY;
+ return fallbackAuthLogHashKey;
}Also applies to: 76-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/src/auth-logger.ts` around lines 26 - 27, Replace the fixed
DEFAULT_AUTH_LOG_HASH_KEY constant with a module-scoped lazy fallback: introduce
a let fallbackAuthLogHashKey variable and a getter function (e.g.,
getAuthLogHashKey()) that returns the configured key when present and otherwise
generates and caches a per-process random key (using crypto.randomBytes) that
meets MIN_AUTH_LOG_HASH_KEY_LENGTH; update places currently using
DEFAULT_AUTH_LOG_HASH_KEY to call getAuthLogHashKey() so the random fallback is
consistent per process and only generated once.
8 tasks
This was referenced Feb 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
api/src/auth-logger.tsmodule for structured JSON auth event loggingFiles Changed
api/src/auth-logger.ts— new structured logger moduleapi/src/auth-logger.test.ts— 11 tests for events, redaction, integrationapi/src/security.ts— auth gate logs session.invalid on rejectionapi/src/authorization.ts— logs auth.denied on non-admin writeapi/src/app.ts— logs login success/failure on auth check, logoutTest plan
Closes #1104
🤖 Generated with Claude Code
CodeAnt-AI Description
Emit structured, redacted authentication event logs (JSON) to stdout
What Changed
Impact
✅ Clearer audit trails for authentication events✅ Fewer leaked secrets in logs✅ Stable pseudonymized identifiers when hash key configured💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.