Skip to content

fix(security): internal authorize hop + structured logging - #366

Merged
izzywdev merged 2 commits into
masterfrom
feature/security-authorize-internal-logging
Jul 23, 2026
Merged

fix(security): internal authorize hop + structured logging#366
izzywdev merged 2 commits into
masterfrom
feature/security-authorize-internal-logging

Conversation

@izzywdev

Copy link
Copy Markdown
Owner

Summary

  • Task 1: completeOidcWithSession (authentikPassword.ts) now rewrites the brokered OIDC authorize URL onto the internal AUTHENTIK_BASE_URL (protocol+host only; redirect_uri/state/PKCE unchanged) instead of hairpinning out through Cloudflare/ingress. Prod timing showed this hop alone at ~6.5s; expected ~0.2s after the fix. Remaining ~5.4s (Authentik's internal flow executor, 6 hops) is out of scope — flagged for follow-up.
  • Task 2: adds a shared pino logger (backend/security/src/lib/logger.ts) — LOG_LEVEL-controlled (default info, debug for per-hop detail without redeploy), JSON/ISO timestamps, mandatory redaction of password/token/code/cookie/authorization-shaped fields. Instruments the previously-silent auth-critical files (authentikPassword.ts, oidc.ts, googleOidc.ts, brokerCodes.ts, api-token.ts, organizationProvisioning.ts, routes/authz.ts) with boundary start/end + elapsed-ms + decision-branch + error-context logging, and converts PR fix(security): hard per-fetch timeout + per-step timing on password login #362's temporary console.log timing lines to leveled logger calls.

Test plan

  • tsc --noEmit against edited files (local workspace npm install hit the documented Windows npm-cache ENOTEMPTY corruption; verified with a clean isolated typescript install instead)
  • Orchestrator to re-measure live prod login latency post-merge (expect ~14s -> ~8s)

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com

…uctured logging

Task 1: completeOidcWithSession's authorize fetch hairpinned out through
Cloudflare/ingress to the EXTERNAL authorize URL (~6.5s per prod timing logs)
instead of the internal Authentik service DNS oidc.ts already uses for
token/userinfo/jwks. toInternalAuthorizeUrl() rewrites protocol+host only
(redirect_uri/state/PKCE unchanged) so token validation is unaffected.
Expected ~6.5s -> ~0.2s on that hop.

Task 2: add a shared pino logger (src/lib/logger.ts) with LOG_LEVEL control
and mandatory credential/token/cookie redaction, and instrument the
previously-silent auth-critical paths (authentikPassword, oidc, googleOidc,
brokerCodes, api-token, organizationProvisioning, authz.ts) with boundary
start/end + elapsed-ms + decision-branch + error-context logging. Converts
PR #362's temporary console.log timing lines to leveled logger calls.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session-Id: f636c22e-1cd7-401e-8843-97e3e3a4ba01
@izzywdev izzywdev added the auto-merge Enable squash auto-merge once CI passes label Jul 23, 2026
@github-actions
github-actions Bot enabled auto-merge (squash) July 23, 2026 16:27
pino@^9.5.0 was added to backend/security/package.json but the root
package-lock.json was not updated, causing npm ci to fail with a
lock-file/package.json mismatch. Ran npm install --package-lock-only
to resolve pino@9.14.0 into backend/node_modules/pino and record it
in the workspace dependencies section.

Fixes failing CI on PR #366.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

CI Fix: package-lock.json out of sync with pino addition

Branch with fix: claude-auto-fix-ci-feature/security-authorize-internal-logging-30025227097 (commit fb6033d)

Root cause

This PR added pino@^9.5.0 to backend/security/package.json (the new src/lib/logger.ts) but did not update the root package-lock.json. In an npm workspace monorepo, npm ci reads all workspace dependencies from the single root lock file. It found pino: ^9.5.0 in package.json but not recorded in the lock file's backend/security workspace entry, causing both Lint & Test (18.x) and Identity UI + Security (unit) to fail with exit code 1 on npm ci.

The tail of the error (thread-stream@3.2.0 from lock file) is pino's own dependency — evidence that pino's resolution is incomplete in the lock file.

Fix applied

Ran npm install --package-lock-only --ignore-scripts at repo root. npm resolved pino@9.14.0 into backend/node_modules/pino and recorded it in the backend/security workspace dependencies section. Only package-lock.json changed (87 lines added).

To land this fix

A project maintainer needs to open a PR from claude-auto-fix-ci-feature/security-authorize-internal-logging-30025227097 targeting feature/security-authorize-internal-logging (GitHub Actions cannot create PRs on this repo per repo policy).

@github-actions

Copy link
Copy Markdown
Contributor

CI fix — gate-frontend-build lock-file mismatch

Root cause: This PR added pino@^9.5.0 to backend/security/package.json for structured logging, but the root package-lock.json was never regenerated. Because backend/security is a workspace, npm ci at root fails with a lock-file mismatch — the dependency that triggers the error is thread-stream@3.2.0, which pino@9 requires but was absent from the lockfile.

Fix (already on branch): I've regenerated package-lock.json on branch claude-auto-fix-ci-feature/security-authorize-internal-logging-30025387812 by running npm install at the root. The updated lockfile captures:

  • pino@9.x scoped under backend/node_modules/pino
  • thread-stream@3.2.0 scoped under backend/node_modules/thread-stream

The existing pino@8.11.0 / thread-stream@2.7.0 used by other workspace members is preserved — npm correctly hoists both versions side-by-side.

To apply: Merge or cherry-pick from claude-auto-fix-ci-feature/security-authorize-internal-logging-30025387812, or simply run npm install locally and commit the resulting package-lock.json to this branch.

@github-actions

Copy link
Copy Markdown
Contributor

CI Fix — branch pushed, PR creation blocked

Root cause: PR #366 added pino@^9.5.0 to backend/security/package.json but the root package-lock.json was never updated. npm ci requires the lock file to be in sync with all workspace package.json files, so all three CI jobs failed at "Install dependencies (workspace root)":

npm error Missing: pino@9.14.0 from lock file
npm error Missing: @pinojs/redact@0.4.0 from lock file
npm error Missing: pino-abstract-transport@2.0.0 from lock file
npm error Missing: pino-std-serializers@7.1.0 from lock file
npm error Missing: process-warning@5.0.0 from lock file
npm error Missing: safe-stable-stringify@2.5.0 from lock file
npm error Missing: sonic-boom@4.2.1 from lock file
npm error Missing: thread-stream@3.2.0 from lock file

This cascaded: node_modules was never installed, so every subsequent test step got jest: not found (exit 127).

Fix: Ran npm install at workspace root. npm resolved pino@9.14.0 under backend/node_modules/pino (nested because root already has pino 8.11.0 from another dep), along with its peer packages at the correct versions. Only package-lock.json changed — 87 lines added, no app code touched.

Fix branch: claude-auto-fix-ci-feature/security-authorize-internal-logging-30025250147

A PR from that branch → feature/security-authorize-internal-logging could not be opened because createPullRequest is disabled for bot identities on this repo (per CLAUDE.md). Please open the PR manually or merge the branch directly into feature/security-authorize-internal-logging.

@izzywdev
izzywdev merged commit b7478f1 into master Jul 23, 2026
50 checks passed
@izzywdev
izzywdev deleted the feature/security-authorize-internal-logging branch July 23, 2026 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge Enable squash auto-merge once CI passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant