Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/org-switcher-same-origin-links.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@agent-native/core": patch
---

Fix the org app-switcher escaping to the official *.agent-native.com site on
custom deployments. `defaultOrgAppLinks` hardcoded each template's prod URL, so a
path-prefixed deployment (served at `<origin>/<app>/`, which bakes
`VITE_APP_BASE_PATH`) linked sibling apps to the first-party hosted site instead
of the current deployment. It now builds `<origin>/<app>/` links when the app is
path-prefixed, and keeps the prod URLs for the first-party subdomain layout.
13 changes: 13 additions & 0 deletions .changeset/shared-oauth-client-for-auth-users.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
"@agent-native/core": patch
---

Allow a shared deployment OAuth client for authenticated users. OAuth *client*
credentials (`GOOGLE_CLIENT_ID/SECRET`, `GOOGLE_SIGN_IN_CLIENT_*`,
`GITHUB_CLIENT_*`) are deployment-wide identity, not per-tenant secrets — every
user authorizes the same app for their own account and gets their own tokens.
`resolveSecret` now lets these keys fall back to the deploy env even for
signed-in users in a hosted runtime (the same treatment Builder credential keys
get), so one deployment can offer a single "Sign in with Google" instead of
forcing every user to register their own Google Cloud OAuth app. A per-tenant
client (BYO upload) still wins — user/org/workspace scopes are resolved first.
12 changes: 12 additions & 0 deletions .changeset/signin-redirect-loop-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"@agent-native/core": patch
---

Fix an infinite sign-in redirect loop under base-path deploys. When the
authenticated app shell (wrapped in `RequireSession`) was served at the sign-in
path — e.g. `/<app>/_agent-native/sign-in` on a path-prefixed deploy — the gate
redirected to the sign-in page from the sign-in page, nesting and re-encoding the
current URL as a fresh `?return=` on every hop (`…sign-in?return=%252F…sign-in%253Freturn…`).
`RequireSession` now refuses to redirect when already on the sign-in entry point
(new exported `isOnSignInPage` helper), and `safeReturnPath` collapses any
`return` that resolves back to `…/_agent-native/sign-in` to `/`.
32 changes: 32 additions & 0 deletions packages/core/src/client/org/workspace-app-links.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,38 @@ describe("org switcher app links", () => {
expect(apps.map((app) => app.id)).not.toContain("videos");
});

it("uses this origin under /<app>/ for a path-prefixed deployment", () => {
const prevWindow = (globalThis as { window?: unknown }).window;
(globalThis as { window?: unknown }).window = {
location: { origin: "https://apps.evinced.tech" },
};
try {
// VITE_APP_BASE_PATH is baked by a path-prefixed deploy — links must stay
// on this origin, not escape to *.agent-native.com.
const apps = defaultOrgAppLinks({ VITE_APP_BASE_PATH: "/mail" });
expect(apps.find((a) => a.id === "mail")?.href).toBe(
"https://apps.evinced.tech/mail",
);
expect(apps.find((a) => a.id === "dispatch")?.href).toBe(
"https://apps.evinced.tech/dispatch/overview",
);
expect(apps.every((a) => !a.href.includes("agent-native.com"))).toBe(
true,
);
} finally {
(globalThis as { window?: unknown }).window = prevWindow;
}
});

it("keeps the first-party *.agent-native.com URLs when not path-prefixed", () => {
// No VITE_APP_BASE_PATH → the app is served at its own root (subdomain
// layout); keep the prod URLs.
const apps = defaultOrgAppLinks({});
expect(apps.find((a) => a.id === "mail")?.href).toBe(
"https://mail.agent-native.com",
);
});

it("normalizes workspace app manifests against the workspace gateway", () => {
const apps = parseWorkspaceAppLinks(
{
Expand Down
42 changes: 28 additions & 14 deletions packages/core/src/client/org/workspace-app-links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,22 +206,36 @@ export function parseWorkspaceAppLinksJson(
}
}

export function defaultOrgAppLinks(): OrgSwitcherAppLink[] {
export function defaultOrgAppLinks(
env: RuntimeEnv = runtimeEnv(),
): OrgSwitcherAppLink[] {
// A path-prefixed deployment (e.g. apps.example.com/<app>/) bakes
// VITE_APP_BASE_PATH=/<app> into the client at build time. There, the sibling
// apps live on THIS origin under /<app>/ — using the hardcoded
// *.agent-native.com prod URLs would send users off to the official hosted
// site instead of the current deployment. Fall back to prodUrl only when the
// app is served at its own root (the first-party agent-native.com layout).
const pathPrefixed = Boolean(envString(env, "VITE_APP_BASE_PATH"));
const origin =
pathPrefixed && typeof window !== "undefined"
? window.location.origin
: null;
return sortAppLinks(
coreTemplates()
.filter((template) => !template.hidden && template.prodUrl)
.map((template) => ({
id: template.name,
name: template.label,
href:
template.name === DISPATCH_ID
? appendPath(template.prodUrl!, "overview")
: template.prodUrl!,
description: template.hint,
icon: template.icon,
isDispatch: template.name === DISPATCH_ID,
status: "ready" as const,
})),
.map((template) => {
const base = origin ? `${origin}/${template.name}` : template.prodUrl!;
return {
id: template.name,
name: template.label,
href:
template.name === DISPATCH_ID ? appendPath(base, "overview") : base,
description: template.hint,
icon: template.icon,
isDispatch: template.name === DISPATCH_ID,
status: "ready" as const,
};
}),
);
}

Expand Down Expand Up @@ -318,7 +332,7 @@ export function useOrgSwitcherAppLinks(
const env = useMemo(() => runtimeEnv(), []);
const isWorkspace = useMemo(() => isWorkspaceAppEnvironment(env), [env]);
const [apps, setApps] = useState<OrgSwitcherAppLink[]>(() =>
isWorkspace ? initialWorkspaceLinks(env) : defaultOrgAppLinks(),
isWorkspace ? initialWorkspaceLinks(env) : defaultOrgAppLinks(env),
);
const [isLoading, setIsLoading] = useState(false);

Expand Down
26 changes: 26 additions & 0 deletions packages/core/src/client/require-session.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,32 @@ describe("RequireSession", () => {
expect(href).toContain(encodeURIComponent("/inbox?label=important"));
});

it("never redirects when already on the sign-in page (no infinite loop)", () => {
// Simulates the base-path deploy case where the app shell is served at the
// sign-in path. Redirecting here would nest the sign-in URL as a fresh
// `?return=` and loop forever — the gate must not redirect to itself.
Object.defineProperty(window, "location", {
configurable: true,
value: {
pathname: "/_agent-native/sign-in",
search: "?return=%2Finbox",
hash: "",
origin: "https://mail.example.com",
href: "https://mail.example.com/_agent-native/sign-in?return=%2Finbox",
replace: replaceMock,
assign: vi.fn(),
},
});
useSessionMock.mockReturnValue({ session: null, isLoading: false });
render(
<RequireSession>
<Child />
</RequireSession>,
);
expect(replaceMock).not.toHaveBeenCalled();
expect(container.querySelector('[data-testid="protected"]')).toBeNull();
});

it("does not redirect twice across re-renders", () => {
useSessionMock.mockReturnValue({ session: null, isLoading: false });
render(
Expand Down
21 changes: 20 additions & 1 deletion packages/core/src/client/require-session.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,24 @@ export function buildSignInReturnHref(): string {
return `${base}?return=${encodeURIComponent(ret)}`;
}

/**
* True when the browser is already sitting on the framework sign-in entry
* point. Redirecting to sign-in from here would append the current sign-in
* URL as a fresh `?return=`, re-encode it, and navigate again — an infinite
* loop of ever-growing `…sign-in?return=%252F…sign-in%253Freturn%253D…` URLs.
*
* This happens when the authenticated app shell (wrapped in `RequireSession`)
* is served at the sign-in path instead of the framework login HTML — e.g.
* under a base-path deploy where the SPA shell answers `/<app>/_agent-native/
* sign-in`. The sign-in page is the framework's job, not the gate's: never
* redirect to ourselves. Compared against `agentNativePath(...)` so it matches
* whether or not the app is mounted under a base path.
*/
export function isOnSignInPage(): boolean {
if (typeof window === "undefined") return false;
return window.location.pathname === agentNativePath("/_agent-native/sign-in");
}

export function RequireSession({
children,
fallback,
Expand All @@ -83,7 +101,8 @@ export function RequireSession({
// flight is harmless but noisy.
const redirectedRef = useRef(false);

const mustRedirect = !bypass && !isLoading && !session && redirect;
const mustRedirect =
!bypass && !isLoading && !session && redirect && !isOnSignInPage();

useEffect(() => {
if (!mustRedirect) return;
Expand Down
21 changes: 21 additions & 0 deletions packages/core/src/server/auth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2855,6 +2855,27 @@ describe("server/auth", () => {
// the parsed segments.)
expect(safeReturnPath("/foo?bar=1#baz")).toBe("/foo?bar=1#baz");
});

it("collapses a return that points back at the sign-in page (loop guard)", async () => {
const safeReturnPath = await load();
// A `return` resolving to the sign-in entry point would re-enter the
// redirect loop — collapse to "/". Covers root and base-path mounts,
// and a nested already-encoded loop URL.
expect(safeReturnPath("/_agent-native/sign-in")).toBe("/");
expect(safeReturnPath("/_agent-native/sign-in?return=%2Finbox")).toBe(
"/",
);
expect(safeReturnPath("/mail/_agent-native/sign-in")).toBe("/");
expect(
safeReturnPath(
"/mail/_agent-native/sign-in?return=%252Fmail%252F_agent-native%252Fsign-in",
),
).toBe("/");
// A normal app path that merely contains the words is unaffected.
expect(safeReturnPath("/mail/inbox?label=important")).toBe(
"/mail/inbox?label=important",
);
});
});

describe("OAuth return URLs", () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/server/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,12 @@ export function safeReturnPath(raw: string | null | undefined): string {
try {
const parsed = new URL(raw, "http://safe-base.invalid");
if (parsed.origin !== "http://safe-base.invalid") return "/";
// Never return to the sign-in entry point itself. A `return` that resolves
// back to `…/_agent-native/sign-in` re-enters the redirect loop — each hop
// nests the prior sign-in URL as a fresh `?return=`. Collapse it to "/" so
// the post-sign-in 302 lands on the app, not another sign-in page. Matches
// with or without an app base-path prefix (`/<app>/_agent-native/sign-in`).
if (parsed.pathname.endsWith("/_agent-native/sign-in")) return "/";
Comment on lines +545 to +550

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Loop guard drops base-path deployments onto site root

When safeReturnPath() collapses a looping sign-in return to /, the /_agent-native/sign-in handler later redirects to that raw path instead of remapping it to getAppBasePath() || "/". On base-path deploys, a successful sign-in can therefore land on the host root (or 404) instead of the mounted app.

Additional Info
Found by 2/4 review passes; validated against auth.ts lines 1798-1808 (`Location: safeReturn`) and 1819-1827 (existing base-path-aware `/login`/`/signup` handling).

Fix in Builder

return parsed.pathname + parsed.search + parsed.hash;
} catch {
return "/";
Expand Down
42 changes: 42 additions & 0 deletions packages/core/src/server/credential-provider.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -865,4 +865,46 @@ describe("resolveSecret (generic)", () => {
expect(await resolveSecret("SOME_KEY")).toBe("v");
delete process.env.SOME_KEY;
});

it("uses the deploy env for OAuth client keys in an authenticated production request (shared deployment client)", async () => {
process.env.NODE_ENV = "production";
process.env.GOOGLE_CLIENT_ID = "deploy-google-client-id";
process.env.GOOGLE_CLIENT_SECRET = "deploy-google-client-secret";
process.env.OPENAI_API_KEY = "deploy-openai-key";
mockIsLocalDatabase.mockReturnValue(false);
mockGetRequestUserEmail.mockReturnValue("a@b.com");
mockGetRequestOrgId.mockReturnValue(undefined);
mockReadAppSecret.mockResolvedValue(null);
// A shared OAuth client is deployment-wide identity, so the deploy env is a
// safe fallback for signed-in users (each still does their own OAuth).
expect(await resolveSecret("GOOGLE_CLIENT_ID")).toBe(
"deploy-google-client-id",
);
expect(await resolveSecret("GOOGLE_CLIENT_SECRET")).toBe(
"deploy-google-client-secret",
);
// A per-tenant API key stays blocked from the deploy env.
expect(await resolveSecret("OPENAI_API_KEY")).toBeNull();
delete process.env.GOOGLE_CLIENT_ID;
delete process.env.GOOGLE_CLIENT_SECRET;
});

it("lets a per-tenant OAuth client override the deploy env (BYO wins)", async () => {
process.env.NODE_ENV = "production";
process.env.GOOGLE_CLIENT_ID = "deploy-google-client-id";
mockIsLocalDatabase.mockReturnValue(false);
mockGetRequestUserEmail.mockReturnValue("a@b.com");
mockGetRequestOrgId.mockReturnValue("org_1");
mockReadAppSecret
.mockResolvedValueOnce(null) // user scope miss
.mockResolvedValueOnce({
value: "tenant-google-client-id",
last4: "t-id",
updatedAt: 1,
}); // org scope hit — returned before the env fallback
expect(await resolveSecret("GOOGLE_CLIENT_ID")).toBe(
"tenant-google-client-id",
);
delete process.env.GOOGLE_CLIENT_ID;
});
});
43 changes: 36 additions & 7 deletions packages/core/src/server/credential-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,33 @@ function isBuilderCredentialKey(key: string): boolean {
return (BUILDER_CREDENTIAL_KEYS as readonly string[]).includes(key);
}

/**
* OAuth *client* credentials — the client_id / client_secret that identify the
* deployment's own OAuth app (Google / GitHub sign-in, Gmail / Calendar / Docs
* connect). Unlike a per-tenant API key, a shared OAuth client is deployment-
* wide identity, not a secret that belongs to one tenant: every user authorizes
* the SAME app for their OWN account and receives their OWN tokens (stored
* per-user in `oauth_tokens`), so a deploy-level value can never impersonate
* another tenant. A tenant that supplies its own client (the BYO upload path)
* still wins — user/org/workspace scopes are read first, above this env
* fallback. So these keys are allowed to fall back to the deploy env even for
* authenticated hosted users (the same treatment Builder credential keys get),
* which lets one deployment offer a single shared "Sign in with Google" instead
* of forcing every user to register their own Google Cloud OAuth app.
*/
const DEPLOYMENT_WIDE_OAUTH_CLIENT_KEYS = [
"GOOGLE_CLIENT_ID",
"GOOGLE_CLIENT_SECRET",
"GOOGLE_SIGN_IN_CLIENT_ID",
"GOOGLE_SIGN_IN_CLIENT_SECRET",
"GITHUB_CLIENT_ID",
"GITHUB_CLIENT_SECRET",
] as const;

function isDeploymentWideOAuthClientKey(key: string): boolean {
return (DEPLOYMENT_WIDE_OAUTH_CLIENT_KEYS as readonly string[]).includes(key);
}

function isHostedWorkspaceRuntime(): boolean {
const hasFusionPreview = Boolean(
process.env.FUSION_ENVIRONMENT ||
Expand Down Expand Up @@ -966,13 +993,15 @@ export async function resolveSecret(key: string): Promise<string | null> {
// The deploy-level value would silently impersonate the actual key
// owner across every tenant. Local/single-tenant deployments keep the
// original env fallback for BYO-server workflows.
const envFallback = (
isBuilderCredentialKey(key)
? canUseBuilderDeployCredentialFallbackForRequest()
: canUseDeployCredentialFallbackForRequest()
)
? process.env[key] || null
: null;
const allowDeployFallback = isBuilderCredentialKey(key)
? canUseBuilderDeployCredentialFallbackForRequest()
: isDeploymentWideOAuthClientKey(key)
? // A shared OAuth client is deployment-wide identity, not a per-tenant
// secret — safe to use for authenticated users (BYO per-tenant client
// is still resolved first, above). See DEPLOYMENT_WIDE_OAUTH_CLIENT_KEYS.
true
: canUseDeployCredentialFallbackForRequest();
const envFallback = allowDeployFallback ? process.env[key] || null : null;
Comment on lines +996 to +1004

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 OAuth client env fallback can silently mix tenant and deploy credentials

resolveSecret() now allows deploy-level fallback for Google OAuth client keys independently, so a tenant-scoped GOOGLE_CLIENT_ID can be paired with a deploy-scoped GOOGLE_CLIENT_SECRET (or vice versa). Mail and Calendar resolve those keys separately, which turns partial overrides into broken invalid_client/unauthorized_client OAuth flows instead of a clean missing-credentials state.

Additional Info
Found by 1/3 review passes; validated against templates/mail/server/lib/google-auth.ts:59-62 and templates/calendar/server/handlers/google-auth.ts:48-53, both of which resolve the pair via separate resolveSecret calls.

Fix in Builder

if (traceLookup) {
console.log(
`[resolve-secret] key=${key} email=${email} orgId=${getRequestOrgId() ?? "(none)"} scope=${envFallback ? "env-fallback" : "none"} hit=${!!envFallback}`,
Expand Down
14 changes: 12 additions & 2 deletions templates/mail/app/components/GoogleConnectBanner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@ interface EnvKeyStatus {
configured: boolean;
}

// Only the Google OAuth client keys decide whether Google is set up. The
// env-status endpoint returns every registered key (Slack, Resend, all the LLM
// providers, …); requiring all of them to be configured meant Google was never
// recognized as ready, so the setup wizard showed even with valid creds.
const GOOGLE_CRED_KEYS = ["GOOGLE_CLIENT_ID", "GOOGLE_CLIENT_SECRET"];

function googleCredsConfigured(keys: EnvKeyStatus[]): boolean {
const googleKeys = keys.filter((k) => GOOGLE_CRED_KEYS.includes(k.key));
return googleKeys.length > 0 && googleKeys.every((k) => k.configured);
}

const STEPS = [
{
titleKey: "mail.googleConnect.enableGmailApi",
Expand Down Expand Up @@ -185,8 +196,7 @@ export function GoogleConnectBanner({
if (res.ok) {
const data: EnvKeyStatus[] = await res.json();
setEnvStatus(data);
const allConfigured = data.every((k) => k.configured);
if (allConfigured && data.length > 0) {
if (googleCredsConfigured(data)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Google setup UI still depends on unrelated env keys

fetchStatus() now treats only the Google client keys as the readiness signal, but the rest of the component still branches on envStatus.every((k) => k.configured). That means missing optional keys like Slack/Anthropic/A2A can still leave the banner in the “not configured” state even when Google is ready.

Additional Info
Validated manually against GoogleConnectBanner.tsx: `googleCredsConfigured(data)` is used at line 199, while render state still uses `allConfigured = envStatus.length > 0 && envStatus.every(...)` for banner copy, CTA, and wizard visibility.

Fix in Builder

setSaved(true);
setCurrentStep(STEPS.length - 1);
}
Expand Down
Loading