fix(auth): carry app_id in redirectToLogin from foreign origins

[davidsu]

Problem

redirectToLogin builds ${appBaseUrl}/login?from_url=... with no app identity. In production (appBaseUrl === '') the relative /login resolves the app by Host, so it works. In local dev, where the vite-plugin injects an absolute appBaseUrl (e.g. https://base44.app for standalone npm run dev), the redirect lands on the apex host, which cannot resolve the app → 404 "App not found".

Fix

When appBaseUrl is a non-empty absolute origin, build the login URL with the existing getLoginUrl util so it carries app_id:

${appBaseUrl}/login?from_url=<encoded absolute URL>&app_id=<appId>

When appBaseUrl === '' (production/published/custom domains), the emitted URL stays byte-identical to today's (/login?from_url=...) — no behavior change on working paths.

Cross-repo context

This is the SDK half of a two-PR feature (option A of the login-redirect decision):

• apper PR (counterpart, in flight): apex /login?app_id= resolution + UserAppMain app_id fallback + AppLogin from_url localhost carve-out. The round-trip to a local dev origin only completes once both halves land.
• Under base44 dev, the CLI dev-server /login carve-out forwards the full query string verbatim, so the same URL serves both dev modes.
• OAuth state shape converges on what loginWithProvider already emits (verified round-tripping from localhost).

Tests

• Updated the three absolute-appBaseUrl expectations in tests/unit/auth.test.js and tests/unit/client.test.js to assert the appended app_id.
• The relative-path test (appBaseUrl absent) is untouched and asserts the exact legacy string — the byte-identical guard.
• Full suite: 187/187 green, lint clean.

🤖 Generated with Claude Code

[claude]

Claude finished @davidsu's task —— View job

---

PR Review: carry app_id in redirectToLogin from foreign origins

• Read changed files (auth.ts, tests)
• Review getLoginUrl util and surrounding context
• Assess code quality, bugs, security, performance, tests
• Post review feedback

---

Overall this is a clean, well-scoped fix. Reusing the existing getLoginUrl util keeps the logic DRY, and the byte-identical guard for the appBaseUrl === '' path is a sensible way to avoid regressions in production. Below is my detailed feedback.

Code quality — good ✅

• Delegating to getLoginUrl (src/utils/auth-utils.ts:209) instead of hand-rolling a second URL builder is the right call — it also picks up getLoginUrl's serverUrl && appId validation for free.
• The inline comment at src/modules/auth.ts:120-121 explains why (Host-based resolution can't work from a foreign origin) rather than what — exactly the kind of comment that earns its place.
• The byte-identical claim checks out: the else-branch string `/login?from_url=${encodeURIComponent(redirectUrl)}` matches the legacy output, and the relative-path test at tests/unit/auth.test.js:240-242 still asserts it verbatim.

Potential issues / things to confirm

1. Param ordering asymmetry — the foreign-origin branch emits ...&from_url=...&app_id=... (via getLoginUrl) whereas loginWithProvider at src/modules/auth.ts:135 emits app_id=...&from_url=.... Functionally irrelevant to the server, but the PR body says "OAuth state shape converges on what loginWithProvider already emits" — worth a quick confirmation that the consuming apper /login handler is order-agnostic (it should be, reading query params by key).

2. getLoginUrl uses options.appBaseUrl as serverUrl — passing appBaseUrl into the serverUrl slot is correct here, but the naming is slightly confusing at the call site ({ serverUrl: options.appBaseUrl, appId }). Not a bug; just flag for future readers. No change needed.

3. Trailing-slash edge case — if appBaseUrl ever arrives with a trailing slash (e.g. https://base44.app/), you'd get https://base44.app//login. The PR notes the vite-plugin injects a bare origin so this shouldn't happen in practice, and the old relative branch had the same implicit assumption. Optional: a normalization at client-config time would harden both this and logout/loginWithProvider, which share the concatenation pattern. Out of scope for this PR.

Security — no concerns 🔒

• from_url is encodeURIComponent-encoded, consistent with the rest of the module.
• No token/secret handling touched. The known open-redirect surface (an attacker-controlled nextUrl) is pre-existing and unchanged by this PR — the from_url validation belongs on the server side (the apper from_url carve-out mentioned in the PR body), so nothing to add here.

Performance — n/a

Single synchronous string build on a user-initiated redirect. No impact.

Test coverage — solid ✅

• Absolute-appBaseUrl expectations updated in three places (auth.test.js:168,188,215, client.test.js:101) to assert the appended app_id.
• Relative-path guard retained (auth.test.js:240, client.test.js:125).

One suggested addition (non-blocking): none of the existing cases exercise a nextUrl that already contains a query string (e.g. https://example.com/dashboard?tab=x). Since from_url is encoded, this should round-trip fine, but an explicit test would lock in that behavior against future refactors.

Verdict: LGTM. The one thing I'd actively verify before merge is item (1) — that the counterpart apper handler reads app_id regardless of query-param order, since this PR emits it last. The trailing-slash and query-in-nextUrl notes are optional hardening.
• fix/redirect-to-login-app-id

[github-actions]

🚀 Package Preview Available!

---

Install this PR's preview build with npm:

npm i @base44-preview/sdk@0.8.40-pr.242.b4656c4

Prefer not to change any import paths? Install using npm alias so your code still imports @base44/sdk:

npm i "@base44/sdk@npm:@base44-preview/sdk@0.8.40-pr.242.b4656c4"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/sdk@0.8.40-pr.242.b4656c4"
  }
}

• 📦 Preview Package: @base44-preview/sdk@0.8.40-pr.242.b4656c4
• 🔗 View this commit on GitHub

---

_{Preview published to npm registry — try new features instantly!}

[davidsu]

Integration-verified end-to-end (2026-07-28, composed 4-system run: this PR's preview build + apper#17843 + cli integration branch #578+#579 + vite-plugin#100 preview):

• redirectToLogin emitted the exact contract URL from the composed npm run dev app ✅
• apex shell served, UserAppMain app_id fallback fired, patched AppLogin rendered app-scoped ✅
• from_url=localhost survived the OAuth provider hop (the original round-trip bug) ✅ — verified in both the live-branch frontend and the as-built velino bundle
• Google consent → callback → landed back on localhost:5173 with a working session (User/me 200, token stored) ✅

One test-harness artifact documented along the way (host-only cookie split between 127.0.0.1/localhost in the lab setup — not a product defect, root-caused by curl reproduction). Prod apex stays expected-404 until apper#17843 merges, by design.

[davidsu]

Re: review feedback —

1. Param ordering: verified with the apper counterpart during the contract huddle — the /login handler (AppLogin) reads query params by key, order-agnostic (explicit confirmation on record from the apper implementation agent). Also exercised live in the integration run: this PR's emitted order (from_url first) round-tripped end-to-end.
2. Query-string-in-nextUrl test: added in 93d6e3f — locks in the encodeURIComponent round-trip behavior.
3. Trailing-slash normalization: agreed out of scope — logout/loginWithProvider share the concatenation pattern, so a client-config-time normalization would be a separate small PR if wanted.

[claude]

Claude finished @davidsu's task —— View job

---

PR Review: carry app_id in redirectToLogin from foreign origins

• Read changed files (auth.ts, tests)
• Review getLoginUrl util + the new query-string test (93d6e3f)
• Assess code quality, bugs, security, performance, tests
• Post review feedback

---

Re-reviewed after the follow-up commits. All three items from my previous pass are now resolved — this is ready to go. Summary below.

Follow-ups from prior review — all addressed ✅

1. Param ordering — confirmed on record that the apper /login handler reads query params by key (order-agnostic), and the emitted order (from_url first) round-tripped end-to-end in the composed integration run. No change needed.
2. Query-string-in-nextUrl — new test added in 93d6e3f (auth.test.js:222-237) exercises ?tab=x&page=2 and asserts the encodeURIComponent round-trip. Nicely locks in that behavior.
3. Trailing-slash normalization — agreed out of scope; shared with logout/loginWithProvider, better as a separate hardening PR.

Code quality — good ✅

• src/modules/auth.ts:122-124 delegates to getLoginUrl (src/utils/auth-utils.ts:209), which also inherits the serverUrl && appId validation for free.
• The why comment at auth.ts:120-121 earns its place.
• Byte-identical guard holds: the else-branch string matches legacy output and the relative-path test at auth.test.js:257-259 still asserts it verbatim.

Test coverage — solid ✅

• Absolute-appBaseUrl expectations updated (auth.test.js:168, 188, 215, client.test.js:101) to assert the appended app_id.
• Query-string round-trip locked in (auth.test.js:222).
• Legacy relative-path guard retained (auth.test.js:257).

Security — no concerns 🔒

from_url is encodeURIComponent-encoded throughout. No token/secret handling touched. The from_url open-redirect surface is a pre-existing, server-side validation concern (the apper carve-out) — unchanged by this PR.

Performance — n/a

Single synchronous string build on a user-initiated redirect.

Minor observation (non-blocking)

getLoginUrl uses serverUrl as the base but is called here with appBaseUrl in that slot ({ serverUrl: options.appBaseUrl, appId }). Correct, but the naming reads slightly off at the call site. Purely cosmetic — no change requested.

Verdict: LGTM. Prior blockers cleared, integration-verified end-to-end, suite green. Ship it (pending the apper counterpart, as noted).

View job
• fix/redirect-to-login-app-id