Skip to content

fix: align nginx auth contract with API auth endpoints (#1332)#1339

Merged
ggfevans merged 4 commits intomainfrom
fix/1332-nginx-auth-contract
Feb 27, 2026
Merged

fix: align nginx auth contract with API auth endpoints (#1332)#1339
ggfevans merged 4 commits intomainfrom
fix/1332-nginx-auth-contract

Conversation

@ggfevans
Copy link
Collaborator

@ggfevans ggfevans commented Feb 26, 2026

User description

Summary

  • align nginx auth bootstrap and auth-check upstream calls to /api/auth/* compatibility endpoints
  • preserve path+query in login redirects via next=$uri$is_args$args
  • update invalid API-path hints to include /api/auth/*
  • update deployment docs to document the reverse-proxy auth contract and include /api/auth/* in explicit API allowlist examples
  • add request throttling on protected frontend location in the self-hosting hardening example

Test Plan

  • npm run lint
  • npm run test:run
  • npm run build
  • codeant static-analysis --uncommitted --fail-on INFO
  • codeant security-analysis --uncommitted --fail-on INFO
  • codeant secrets --uncommitted --fail-on all

Notes

Closes #1332


CodeAnt-AI Description

Align Nginx reverse-proxy auth contract, preserve deep links, and fix OIDC availability detection

What Changed

  • Nginx templates now forward public auth requests to the API compatibility routes (/api/auth/*), so reverse-proxy auth checks and the API use the same endpoints
  • Login redirects preserve both path and query (next=?), so deep links return users to the exact page and query string after login
  • Internal auth probe now treats 204 as authorized and proxies to /api/auth/check; unauthorized probes return 401
  • Self-hosting docs and examples updated to document the /api/auth/* contract, RACKULA_AUTH_MODE behavior (none vs oidc|local), and recommend rate limiting for protected frontend locations
  • Added explicit Nginx allowlist and request throttling for /api/auth/* and for the protected frontend location so proxy-protected API endpoints are handled correctly
  • Fixed OIDC detection so social sign-in becomes available when OIDC is configured (detects configured plugins correctly)

Impact

✅ Fewer auth misconfigurations behind Nginx reverse-proxies
✅ Preserve deep links after login
✅ Clearer unauthorized responses from protected routes

💡 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:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

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:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

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.

@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 26, 2026

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 ·
Reddit ·
LinkedIn

@coderabbitai
Copy link

coderabbitai bot commented Feb 26, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Aligns reverse-proxy auth routing to API endpoints by switching nginx auth paths to /api/auth/*, simplifies auth plugin detection in api/src/app.ts, and updates deployment and self-hosting docs to document the new reverse-proxy auth contract and rate-limiting/allowlist behavior.

Changes

Cohort / File(s) Summary
API Auth Logic
api/src/app.ts
Removed optional chaining when retrieving auth plugins and simplified the OIDC availability check to require any auth plugin entry plus existing function validations (signInWithOAuth2, oAuth2Callback).
NGINX Configuration
deploy/nginx.conf.template
Repointed proxy_pass targets and auth_request paths from /auth/* to /api/auth/*, updated internal auth probe to /api/auth/check (204/401 semantics), expanded comments on redirects and preserved next handling, and adjusted upstream proxy routes.
Deployment Docs
docs/deployment/AUTHENTICATION.md
Renamed/clarified auth mode semantics (Optional auth mode), added a Reverse Proxy Auth Contract section, documented /api/auth/* compatibility routes and auth-check response codes (204/401).
Self-Hosting Guide
docs/guides/SELF-HOSTING.md
Switched nginx rate limit zone name to per_ip, applied per-IP limits to frontend and API locations, and added an explicit allowlist and proxy handling for /api/auth(/.*)?.

Sequence Diagram(s)

sequenceDiagram
  participant Browser as Browser
  participant Nginx as NGINX (reverse-proxy)
  participant API as Rackula API (/api/auth/*)
  participant IdP as Identity Provider

  Browser->>Nginx: Request protected resource
  Nginx->>API: auth_request /api/auth/check
  API-->>Nginx: 204 (authorized) / 401 (unauthorized)
  alt authorized
    Nginx->>Browser: proxy_pass resource
  else unauthorized
    Nginx->>Browser: redirect -> /api/auth/login?next=...
    Browser->>API: GET /api/auth/login
    API->>IdP: redirect to IdP (OAuth2)
    IdP->>Browser: callback -> /api/auth/callback
    Browser->>API: GET/POST /api/auth/callback
    API-->>Nginx: set session / subsequent auth_check 204
    Nginx->>Browser: redirect to original resource
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped from old /auth to /api/auth lanes,
NGINX and API now speak with matching refrains,
Probes say 204, redirects keep the track,
Sessions set, callbacks return me back,
A carrot for alignment — golly, what gains! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: align nginx auth contract with API auth endpoints (#1332)' clearly and specifically describes the main change—aligning Nginx configuration with API auth endpoints.
Description check ✅ Passed The pull request description provides detailed information about aligning Nginx auth contract, preserving deep links, fixing OIDC detection, and updating deployment documentation, all of which relate to the changeset.
Linked Issues check ✅ Passed The PR addresses all major objectives from #1332: aligning Nginx auth endpoints to /api/auth/*, documenting the reverse-proxy auth contract, preserving path/query in redirects, fixing OIDC detection, and updating deployment docs.
Out of Scope Changes check ✅ Passed All changes directly support the objectives: Nginx template updates align endpoints, code simplifies OIDC detection, and docs clarify auth contract and self-hosting configuration.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/1332-nginx-auth-contract

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codeant-ai codeant-ai bot added the size:L This PR changes 100-499 lines, ignoring generated files label Feb 26, 2026
@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 26, 2026

CodeAnt AI finished reviewing your PR.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/app.ts`:
- Around line 109-130: The oidc detection currently uses plugin.id ===
"generic-oauth" which always fails because built-in genericOAuth plugins don't
expose id; update the check in the authPlugins.some(...) used by
oidcApiAvailable to detect OIDC by inspecting the plugin's actual shape (for
example check (plugin as any).providerId === "oidc" or use a type
guard/constructor name like plugin.constructor?.name === "GenericOAuth"),
ensuring you still verify authApi.signInSocial and authApi.callbackOAuth; update
the condition that defines oidcApiAvailable accordingly so it correctly reflects
when OIDC is configured.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 068bbea and d723885.

📒 Files selected for processing (5)
  • api/src/app.ts
  • api/src/auth-logger.test.ts
  • deploy/nginx.conf.template
  • docs/deployment/AUTHENTICATION.md
  • docs/guides/SELF-HOSTING.md

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/app.ts`:
- Around line 109-118: The OIDC detection is fine, but the redirect targets are
wrong: where the code currently redirects to
`/api/auth/sign-in/social?provider=oidc` and `/api/auth/callback/oidc`, change
those redirect targets to match Better Auth generic-oauth routes; use `POST
/sign-in/oauth2` for sign-in and `GET /oauth2/callback/:providerId` (replace
:providerId with the provider identifier used, e.g. oidc) for the callback.
Locate the logic that uses securityConfig.authEnabled, auth
(authApi/authPlugins/oidcApiAvailable) and update the sign-in and callback
redirect URLs accordingly so they point to `/sign-in/oauth2` and
`/oauth2/callback/oidc` (or the configured providerId).

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d723885 and ebb5f7d.

📒 Files selected for processing (1)
  • api/src/app.ts

api/src/app.ts Outdated
Comment on lines 109 to 118
if (securityConfig.authEnabled) {
const authApi = (auth?.api ?? {}) as Record<string, unknown>;
const authPlugins = Array.isArray(auth?.options?.plugins)
? auth.options.plugins
: [];
const oidcApiAvailable =
Boolean(auth) &&
authPlugins.length > 0 &&
typeof authApi.signInSocial === "function" &&
typeof authApi.callbackOAuth === "function";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cd api && head -250 src/app.ts | tail -150

Repository: RackulaLives/Rackula

Length of output: 5176


🌐 Web query:

Better Auth genericOAuth plugin routes endpoints sign-in callback

💡 Result:

For Better Auth’s Generic OAuth plugin, the server mounts these routes:

  • Start sign-in: POST /sign-in/oauth2 (client: authClient.signIn.oauth2({ providerId, ... })) [1][2]
  • OAuth callback: GET /oauth2/callback/:providerId (so the full callback URL is typically ${baseURL}/api/auth/oauth2/callback/:providerId) [1][2]
  • Link an OAuth account: POST /oauth2/link (client: authClient.oauth2.link({ providerId, callbackURL })) [1][2]

Callback URL to register with your OAuth provider: ${baseURL}/api/auth/oauth2/callback/:providerId (unless you’ve customized Better Auth’s base path/routing in your framework). [1]

Sources: [1] [2]


Fix endpoint paths: Better Auth's generic-oauth plugin uses different route paths than currently implemented.

The OIDC detection logic fix is correct—replacing plugin.id === "generic-oauth" with authPlugins.length > 0 appropriately leverages the fact that plugins are only populated when OIDC environment variables are configured. However, the redirect paths are incorrect.

Better Auth's generic-oauth plugin uses these routes:

  • Sign-in: POST /sign-in/oauth2
  • Callback: GET /oauth2/callback/:providerId

The current code redirects to /api/auth/sign-in/social?provider=oidc and /api/auth/callback/oidc, which do not match Better Auth's routes and will cause authentication failures. Update the redirect targets to align with Better Auth's actual generic-oauth plugin endpoints.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/src/app.ts` around lines 109 - 118, The OIDC detection is fine, but the
redirect targets are wrong: where the code currently redirects to
`/api/auth/sign-in/social?provider=oidc` and `/api/auth/callback/oidc`, change
those redirect targets to match Better Auth generic-oauth routes; use `POST
/sign-in/oauth2` for sign-in and `GET /oauth2/callback/:providerId` (replace
:providerId with the provider identifier used, e.g. oidc) for the callback.
Locate the logic that uses securityConfig.authEnabled, auth
(authApi/authPlugins/oidcApiAvailable) and update the sign-in and callback
redirect URLs accordingly so they point to `/sign-in/oauth2` and
`/oauth2/callback/oidc` (or the configured providerId).

ggfevans and others added 2 commits February 26, 2026 22:28
Better Auth's genericOAuth plugin doesn't expose an id property,
so plugin.id === "generic-oauth" always evaluated to false. Use
authPlugins.length > 0 instead — the plugins array is only populated
when OIDC env vars are configured in createAuth(). API method checks
(signInSocial/callbackOAuth) are kept as a secondary guard.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ggfevans ggfevans force-pushed the fix/1332-nginx-auth-contract branch from ebb5f7d to b5803b8 Compare February 27, 2026 06:29
@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 27, 2026

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 ·
Reddit ·
LinkedIn

@codeant-ai codeant-ai bot added size:M This PR changes 30-99 lines, ignoring generated files and removed size:L This PR changes 100-499 lines, ignoring generated files labels Feb 27, 2026
@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 27, 2026

Sequence Diagram

Shows how Nginx forwards auth checks and public auth routes to the API compatibility endpoints (/api/auth/*), and how it preserves path+query in login redirects so deep links return after authentication.

sequenceDiagram
    participant Browser
    participant Nginx
    participant API

    Browser->>Nginx: Request protected page
    Nginx->>API: Internal auth probe -> GET /api/auth/check (session cookie)
    alt Valid session (authorized)
        API-->>Nginx: 204 (authorized)
        Nginx-->>Browser: Serve protected page
    else No session (unauthorized)
        API-->>Nginx: 401 (unauthenticated)
        Nginx-->>Browser: 302 Redirect /auth/login?next=<path>?<query>
        Browser->>Nginx: GET /auth/login?next=... 
        Nginx->>API: Proxy to /api/auth/login?next=... (preserve path+query)
        API-->>Browser: Continue login flow (IdP redirect / callback)
Loading

Generated by CodeAnt AI

@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 27, 2026

CodeAnt AI Incremental review completed.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
docs/guides/SELF-HOSTING.md (1)

285-299: 🧹 Nitpick | 🔵 Trivial

Rate limiting location / is reasonable, but consider clarifying scope (and/or zone naming).

limit_req zone=api_per_ip ... on the main frontend location will throttle all UI requests, not just API calls. That may be intended for a stop-gap hardening guide, but I’d either:

  • rename the zone to something generic (e.g., per_ip), or
  • add a one-liner note explaining this also rate-limits UI navigation/assets.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/guides/SELF-HOSTING.md` around lines 285 - 299, The nginx config in the
frontend location (the block starting with "location /") uses limit_req
zone=api_per_ip which will rate-limit all UI requests; either rename the zone to
a generic identifier (e.g., change zone name from api_per_ip to per_ip in the
limit_req directive and its zone definition) or add a one-line comment in the
SELF-HOSTING.md near the location / block clarifying that limit_req on location
/ affects UI navigation and static assets as well as API calls so readers know
the scope of the rate limit.
docs/deployment/AUTHENTICATION.md (1)

9-82: ⚠️ Potential issue | 🟠 Major

Docs update is directionally correct, but the next=<path-and-query> claim depends on fixing the nginx redirect encoding/contract.

The “Optional auth mode” and “Reverse Proxy Auth Contract (Nginx)” sections align well with the /api/auth/* compatibility endpoints and the 204/401 auth-check behavior.

Two follow-ups to keep the contract precise:

  • Consider explicitly documenting HTTP methods for compatibility routes too (e.g., POST /api/auth/logout), since you already do for /auth/logout.
  • The line “/auth/login?next=<normalized-path-and-query>” is only true if the proxy preserves query inside next (see nginx template comment about next=$uri$is_args$args parsing).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/deployment/AUTHENTICATION.md` around lines 9 - 82, Update the docs to
(1) explicitly list HTTP methods for the compatibility endpoints (e.g.,
/api/auth/login, /api/auth/callback, /api/auth/check, /api/auth/logout) to match
the /auth/* entries and make it clear which are GET vs POST, and (2) qualify the
“/auth/login?next=<normalized-path-and-query>” claim by stating it only works if
the reverse proxy preserves the full path+query when building next (reference
the nginx template approach like using next=$uri$is_args$args or an equivalent
that preserves/encodes the query); add a short sentence advising proxies must
percent‑encode/escape the next value or use the shown template to ensure the
callback receives the original path+query.
deploy/nginx.conf.template (1)

165-201: ⚠️ Potential issue | 🔴 Critical

Critical: next=$uri$is_args$args will not preserve full query strings (embedded & will be parsed as separate parameters).

When the original request contains multiple query parameters like /rack/view?tab=2&sort=asc, the unencoded redirect /auth/login?next=/rack/view?tab=2&sort=asc will be parsed by the server as next="/rack/view?tab=2" with sort="asc" as a separate parameter—losing the query params after the first &.

The API-side code (api/src/security.ts) correctly uses encodeURIComponent() to handle this, but the nginx config bypasses that with raw variable expansion. Either URL-encode the value in nginx (which requires external modules), use $request_uri with careful handling to avoid double-encoding, or refactor to pass path and query separately and reconstruct server-side.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@deploy/nginx.conf.template` around lines 165 - 201, The redirect in location
`@auth_login_redirect` uses raw expansion next=$uri$is_args$args which breaks
multi-parameter querystrings (e.g. /rack/view?tab=2&sort=asc) because the
embedded & is treated as a separate parameter; change the implementation so the
full original path+query is passed safely: either use $request_uri (which
already contains the encoded query) or pass path and query separately and
reassemble/URL-encode on the server side (see api/src/security.ts which uses
encodeURIComponent()); update the `@auth_login_redirect` location to stop emitting
unencoded next=$uri$is_args$args and instead forward a single, safe next value
(or remove next and reconstruct server-side) so deep links keep their entire
query string.
🤖 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/app.ts`:
- Around line 321-330: Extract auth plugins into a safely-narrowed variable
before the guard: check if auth is truthy then set authPlugins =
Array.isArray(auth.options?.plugins) ? auth.options.plugins : []; use that
authPlugins in the oidcApiAvailable expression and add an inline comment on the
authPlugins.length > 0 check noting "Confirms OIDC provider was initialized with
credentials"; keep the duck-typing checks for authApi.signInWithOAuth2 and
authApi.oAuth2Callback as-is. Ensure you reference the existing symbols auth,
auth?.options?.plugins, authPlugins, oidcApiAvailable, and
authApi.signInWithOAuth2/authApi.oAuth2Callback when making the change.

In `@docs/deployment/AUTHENTICATION.md`:
- Around line 66-76: The API compatibility routes section lists endpoints but
omits HTTP methods; update the `/api/auth/*` entries to mirror the
browser-facing annotations (e.g., `GET /api/auth/login`, `GET
/api/auth/callback`, `GET /api/auth/check`, `POST /api/auth/logout`) so each
`/api/auth/*` route has the same method annotations as `/auth/login`,
`/auth/callback`, `/auth/check`, and `/auth/logout`.

---

Outside diff comments:
In `@deploy/nginx.conf.template`:
- Around line 165-201: The redirect in location `@auth_login_redirect` uses raw
expansion next=$uri$is_args$args which breaks multi-parameter querystrings (e.g.
/rack/view?tab=2&sort=asc) because the embedded & is treated as a separate
parameter; change the implementation so the full original path+query is passed
safely: either use $request_uri (which already contains the encoded query) or
pass path and query separately and reassemble/URL-encode on the server side (see
api/src/security.ts which uses encodeURIComponent()); update the
`@auth_login_redirect` location to stop emitting unencoded next=$uri$is_args$args
and instead forward a single, safe next value (or remove next and reconstruct
server-side) so deep links keep their entire query string.

In `@docs/deployment/AUTHENTICATION.md`:
- Around line 9-82: Update the docs to (1) explicitly list HTTP methods for the
compatibility endpoints (e.g., /api/auth/login, /api/auth/callback,
/api/auth/check, /api/auth/logout) to match the /auth/* entries and make it
clear which are GET vs POST, and (2) qualify the
“/auth/login?next=<normalized-path-and-query>” claim by stating it only works if
the reverse proxy preserves the full path+query when building next (reference
the nginx template approach like using next=$uri$is_args$args or an equivalent
that preserves/encodes the query); add a short sentence advising proxies must
percent‑encode/escape the next value or use the shown template to ensure the
callback receives the original path+query.

In `@docs/guides/SELF-HOSTING.md`:
- Around line 285-299: The nginx config in the frontend location (the block
starting with "location /") uses limit_req zone=api_per_ip which will rate-limit
all UI requests; either rename the zone to a generic identifier (e.g., change
zone name from api_per_ip to per_ip in the limit_req directive and its zone
definition) or add a one-line comment in the SELF-HOSTING.md near the location /
block clarifying that limit_req on location / affects UI navigation and static
assets as well as API calls so readers know the scope of the rate limit.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ebb5f7d and b5803b8.

📒 Files selected for processing (4)
  • api/src/app.ts
  • deploy/nginx.conf.template
  • docs/deployment/AUTHENTICATION.md
  • docs/guides/SELF-HOSTING.md

Comment on lines 321 to 330
if (securityConfig.authEnabled) {
const authPlugins = Array.isArray(auth?.options?.plugins)
? auth?.options?.plugins
? auth.options.plugins
: [];
const oidcApiAvailable =
Boolean(auth) &&
securityConfig.authMode === "oidc" &&
authPlugins.some(
(plugin) => (plugin as { id?: unknown }).id === "generic-oauth",
) &&
authPlugins.length > 0 &&
typeof authApi.signInWithOAuth2 === "function" &&
typeof authApi.oAuth2Callback === "function";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify createAuth() plugin population + OIDC configuration contract
rg -n --type ts "createAuth\(" api/src -C 5

Repository: RackulaLives/Rackula

Length of output: 1425


🏁 Script executed:

#!/bin/bash
# Find all usages of auth.options.plugins
rg -n --type ts "auth\.options\.plugins" api/src -C 3

Repository: RackulaLives/Rackula

Length of output: 388


🏁 Script executed:

#!/bin/bash
# Find OIDC-related configuration
rg -n --type ts "authMode|OIDC|oidc" api/src -C 3

Repository: RackulaLives/Rackula

Length of output: 22483


🏁 Script executed:

#!/bin/bash
# Check the specific lines mentioned in the review (321-330)
sed -n '315,335p' api/src/app.ts

Repository: RackulaLives/Rackula

Length of output: 785


Clarify authPlugins extraction for better type narrowing; validate OIDC guard assumptions.

The current ternary mixes optional chaining in the condition with direct access in the true branch. While runtime-safe, splitting the condition improves readability and helps TS analysis. Additionally, using authPlugins.length > 0 as an OIDC configured predicate is valid—createAuth() only populates the genericOAuth plugin when OIDC credentials are present, and throws if authMode is "oidc" but credentials are missing. However, the redundancy could be clarified with a comment.

Suggested clarification (same behavior, better narrowing)
-    const authPlugins = Array.isArray(auth?.options?.plugins)
-      ? auth.options.plugins
-      : [];
+    const configuredPlugins = auth?.options?.plugins;
+    const authPlugins = Array.isArray(configuredPlugins) ? configuredPlugins : [];

Optionally, add a comment on line 328:

authPlugins.length > 0 && // Confirms OIDC provider was initialized with credentials

The duck-typing checks for authApi methods (lines 329–330) already follow the learning guidance.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/src/app.ts` around lines 321 - 330, Extract auth plugins into a
safely-narrowed variable before the guard: check if auth is truthy then set
authPlugins = Array.isArray(auth.options?.plugins) ? auth.options.plugins : [];
use that authPlugins in the oidcApiAvailable expression and add an inline
comment on the authPlugins.length > 0 check noting "Confirms OIDC provider was
initialized with credentials"; keep the duck-typing checks for
authApi.signInWithOAuth2 and authApi.oAuth2Callback as-is. Ensure you reference
the existing symbols auth, auth?.options?.plugins, authPlugins,
oidcApiAvailable, and authApi.signInWithOAuth2/authApi.oAuth2Callback when
making the change.

- Fix nginx auth redirect to use next=$uri (path only) to prevent query
  string params from splitting across the redirect URL
- Add HTTP method annotations to API compatibility routes in auth docs
- Clarify that next= preserves path only, not query strings
- Rename misleading api_per_ip rate limit zone to per_ip

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@deploy/nginx.conf.template`:
- Around line 199-202: The redirect currently uses return 302
/auth/login?next=$uri; which drops query params; change the redirect to include
the original query string by using $uri$is_args$args (e.g. return 302
/auth/login?next=$uri$is_args$args;) so the full path plus its query parameters
are preserved across the auth redirect.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b5803b8 and 4f575b9.

📒 Files selected for processing (3)
  • deploy/nginx.conf.template
  • docs/deployment/AUTHENTICATION.md
  • docs/guides/SELF-HOSTING.md

The login redirect was dropping query strings, so users returning from
auth would lose their original URL parameters. Now uses $is_args$args
to carry them through. Multi-param query strings with & will still
split at the nginx level (proper fix needs OpenResty), but this is
strictly better than losing everything.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
deploy/nginx.conf.template (1)

199-203: ⚠️ Potential issue | 🟠 Major

next= redirect is still lossy for multi-parameter URLs.

Line 203 still drops part of deep-link state when the original URL contains multiple & query params; only the first segment remains in next. This misses the “exact page + query” objective for a common case.

A robust fix needs an opaque state/cookie flow or backend-side reconstruction from raw query bytes (not parsed query params).

#!/bin/bash
set -euo pipefail

# Verify how login/callback handlers parse and reconstruct "next" today.
# Expected: evidence whether code reads parsed query only (lossy) or raw query (can preserve full args).

fd 'app.ts'
rg -nP --type=ts -C3 'auth/login|auth/callback|next_path|next_query|searchParams\.get\("next"\)|URLSearchParams|ctx\.req\.query' api
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@deploy/nginx.conf.template` around lines 199 - 203, The nginx redirect that
returns 302 /auth/login?next=$uri$is_args$args is lossy for multi-parameter
URLs; change to an opaque-state flow instead: have nginx set a short-lived
cookie or include a state token (not a raw next= query) that represents the full
original request URI (including the raw query string), then update your
authentication handlers (auth/login and auth/callback) to read that opaque token
(or cookie) to reconstruct the exact post-login redirect; implement
generation/storage/validation of the state token server-side (or sign/encrypt
it) and remove reliance on parsing next from URL query parameters so deep-link
state is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@deploy/nginx.conf.template`:
- Around line 199-203: The nginx redirect that returns 302
/auth/login?next=$uri$is_args$args is lossy for multi-parameter URLs; change to
an opaque-state flow instead: have nginx set a short-lived cookie or include a
state token (not a raw next= query) that represents the full original request
URI (including the raw query string), then update your authentication handlers
(auth/login and auth/callback) to read that opaque token (or cookie) to
reconstruct the exact post-login redirect; implement
generation/storage/validation of the state token server-side (or sign/encrypt
it) and remove reliance on parsing next from URL query parameters so deep-link
state is preserved.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f575b9 and 5944e83.

📒 Files selected for processing (2)
  • deploy/nginx.conf.template
  • docs/deployment/AUTHENTICATION.md

@ggfevans ggfevans merged commit c9023de into main Feb 27, 2026
10 checks passed
@ggfevans ggfevans deleted the fix/1332-nginx-auth-contract branch February 27, 2026 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: align nginx auth-mode routing contract with API auth endpoints

1 participant