Skip to content
Merged
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
81 changes: 46 additions & 35 deletions AGENTS.md

Large diffs are not rendered by default.

13 changes: 8 additions & 5 deletions src/commands/auth/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ import {
} from "../../lib/formatters/human.js";
import { CommandOutput } from "../../lib/formatters/output.js";
import type { LoginResult } from "../../lib/interactive-login.js";
import { runInteractiveLogin } from "../../lib/interactive-login.js";
import {
runInteractiveLogin,
toLoginUser,
} from "../../lib/interactive-login.js";
import { logger } from "../../lib/logger.js";
import { clearResponseCache } from "../../lib/response-cache.js";

Expand Down Expand Up @@ -173,11 +176,11 @@ export const loginCommand = buildCommand({
const user = await getCurrentUser();
setUserInfo({
userId: user.id,
email: user.email,
username: user.username,
name: user.name,
email: user.email ?? undefined,
username: user.username ?? undefined,
name: user.name ?? undefined,
});
result.user = user;
result.user = toLoginUser(user);
} catch {
// Non-fatal: user info is supplementary. Token remains stored and valid.
}
Expand Down
6 changes: 3 additions & 3 deletions src/commands/auth/whoami.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ export const whoamiCommand = buildCommand({
try {
setUserInfo({
userId: user.id,
email: user.email,
username: user.username,
name: user.name,
email: user.email ?? undefined,
username: user.username ?? undefined,
name: user.name ?? undefined,
});
} catch {
// Cache update failure is non-essential — user identity was already fetched.
Expand Down
23 changes: 20 additions & 3 deletions src/lib/interactive-login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,23 @@ export type LoginResult = {
expiresIn?: number;
};

/**
* Build a clean user object for {@link LoginResult}, omitting falsy
* fields (null, undefined, empty string) so they don't leak into output.
*
* Empty-string names/emails are semantically absent for display purposes.
*/
export function toLoginUser(user: {
id: string;
name?: string | null;
email?: string | null;
username?: string | null;
}): NonNullable<LoginResult["user"]> {
return Object.fromEntries(
Object.entries(user).filter(([k, v]) => k === "id" || Boolean(v))
) as NonNullable<LoginResult["user"]>;
}

/** Options for the interactive login flow */
export type InteractiveLoginOptions = {
/** Timeout for OAuth flow in milliseconds (default: 900000 = 15 minutes) */
Expand Down Expand Up @@ -163,8 +180,8 @@ export async function runInteractiveLogin(
try {
setUserInfo({
userId: user.id,
email: user.email,
name: user.name,
email: user.email ?? undefined,
name: user.name ?? undefined,
});
} catch (error) {
// Report to Sentry but don't block auth - user info is not critical
Expand All @@ -178,7 +195,7 @@ export async function runInteractiveLogin(
expiresIn: tokenResponse.expires_in,
};
if (user) {
result.user = user;
result.user = toLoginUser(user);
}
return result;
} catch (err) {
Expand Down
4 changes: 2 additions & 2 deletions src/types/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ export const TokenResponseSchema = z
user: z
.object({
id: z.string(),
name: z.string(),
email: z.string(),
name: z.string().nullable(),
email: z.string().nullable(),
})
.passthrough()
.optional(),
Expand Down
14 changes: 11 additions & 3 deletions src/types/sentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,20 @@ export type UserRegionsResponse = z.infer<typeof UserRegionsResponseSchema>;

// User

/**
* Minimal user schema for the `/auth/` endpoint response.
*
* All optional fields use `.nullish()` (accepts both `null` and `undefined`)
* because the Sentry API can return `null` for any of these.
* Note: `@sentry/api` doesn't export types for the `/auth/` endpoint —
* it's undocumented, so we define this schema manually.
*/
export const SentryUserSchema = z
.object({
id: z.string(),
email: z.string().optional(),
username: z.string().optional(),
name: z.string().optional(),
email: z.string().nullish(),
username: z.string().nullish(),
name: z.string().nullish(),
})
.passthrough();

Expand Down
31 changes: 31 additions & 0 deletions test/commands/auth/login.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,37 @@ describe("loginCommand.func --token path", () => {
expect(out).toContain("Jane Doe");
});

test("--token: null user.name is converted to undefined in setUserInfo", async () => {
isAuthenticatedSpy.mockResolvedValue(false);
setAuthTokenSpy.mockResolvedValue(undefined);
getUserRegionsSpy.mockResolvedValue([]);
getCurrentUserSpy.mockResolvedValue({
id: "5",
name: null,
email: "x@y.com",
username: "xuser",
});
setUserInfoSpy.mockReturnValue(undefined);

const { context, getStdout } = createContext();
await func.call(context, {
token: "valid-token",
force: false,
timeout: 900,
});

expect(setUserInfoSpy).toHaveBeenCalledWith({
userId: "5",
email: "x@y.com",
username: "xuser",
name: undefined,
});
const out = getStdout();
expect(out).toContain("Authenticated");
// With null name, formatUserIdentity falls back to email
expect(out).toContain("x@y.com");
});

test("--token: invalid token clears auth and throws AuthError", async () => {
isAuthenticatedSpy.mockResolvedValue(false);
setAuthTokenSpy.mockResolvedValue(undefined);
Expand Down
168 changes: 164 additions & 4 deletions test/lib/interactive-login.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,40 @@
/**
* Tests for buildDeviceFlowDisplay — the extracted display logic from the
* interactive login flow's onUserCode callback.
* Tests for interactive login flow.
*
* - buildDeviceFlowDisplay: extracted display logic (pure function)
* - runInteractiveLogin: full OAuth device flow with mocked dependencies
*
* Uses SENTRY_PLAIN_OUTPUT=1 to get predictable raw markdown output
* (no ANSI codes) for string assertions.
*/

import { afterAll, beforeAll, describe, expect, test } from "bun:test";
import { buildDeviceFlowDisplay } from "../../src/lib/interactive-login.js";
import {
afterAll,
afterEach,
beforeAll,
beforeEach,
describe,
expect,
spyOn,
test,
} from "bun:test";
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
import * as browser from "../../src/lib/browser.js";
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
import * as clipboard from "../../src/lib/clipboard.js";
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
import * as dbInstance from "../../src/lib/db/index.js";
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
import * as dbUser from "../../src/lib/db/user.js";
import {
buildDeviceFlowDisplay,
runInteractiveLogin,
} from "../../src/lib/interactive-login.js";
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
import * as oauth from "../../src/lib/oauth.js";
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
import * as qrcode from "../../src/lib/qrcode.js";
import type { TokenResponse } from "../../src/types/index.js";

// Force plain output for predictable string matching
let origPlain: string | undefined;
Expand Down Expand Up @@ -84,3 +111,136 @@ describe("buildDeviceFlowDisplay", () => {
expect(withoutBrowser.length).toBeGreaterThan(withBrowser.length);
});
});

describe("runInteractiveLogin", () => {
let performDeviceFlowSpy: ReturnType<typeof spyOn>;
let completeOAuthFlowSpy: ReturnType<typeof spyOn>;
let openBrowserSpy: ReturnType<typeof spyOn>;
let generateQRCodeSpy: ReturnType<typeof spyOn>;
let setupCopyKeyListenerSpy: ReturnType<typeof spyOn>;
let setUserInfoSpy: ReturnType<typeof spyOn>;
let getDbPathSpy: ReturnType<typeof spyOn>;

/** Helper to build a mock TokenResponse with a user whose fields may be null. */
function makeTokenResponse(user?: {
id: string;
name: string | null;
email: string | null;
}): TokenResponse {
return {
access_token: "sntrys_test_token",
token_type: "Bearer",
expires_in: 3600,
...(user ? { user } : {}),
};
}

beforeEach(() => {
completeOAuthFlowSpy = spyOn(oauth, "completeOAuthFlow").mockResolvedValue(
undefined
);
openBrowserSpy = spyOn(browser, "openBrowser").mockResolvedValue(false);
generateQRCodeSpy = spyOn(qrcode, "generateQRCode").mockResolvedValue(
"[QR]"
);
setupCopyKeyListenerSpy = spyOn(
clipboard,
"setupCopyKeyListener"
).mockReturnValue(() => {
// no-op cleanup
});
setUserInfoSpy = spyOn(dbUser, "setUserInfo").mockReturnValue(undefined);
getDbPathSpy = spyOn(dbInstance, "getDbPath").mockReturnValue("/tmp/db");
});

afterEach(() => {
performDeviceFlowSpy.mockRestore();
completeOAuthFlowSpy.mockRestore();
openBrowserSpy.mockRestore();
generateQRCodeSpy.mockRestore();
setupCopyKeyListenerSpy.mockRestore();
setUserInfoSpy.mockRestore();
getDbPathSpy.mockRestore();
});

test("null user.name is omitted from result and stored as undefined in setUserInfo", async () => {
performDeviceFlowSpy = spyOn(oauth, "performDeviceFlow").mockImplementation(
async (callbacks) => {
await callbacks.onUserCode(
"ABCD",
"https://sentry.io/auth/device/",
"https://sentry.io/auth/device/?user_code=ABCD"
);
return makeTokenResponse({
id: "48168",
name: null,
email: "user@example.com",
});
}
);

const result = await runInteractiveLogin({ timeout: 1000 });

expect(result).not.toBeNull();
expect(result!.user).toBeDefined();
// name must be absent from the result object (not present as undefined)
expect("name" in result!.user!).toBe(false);
expect(result!.user!.email).toBe("user@example.com");
expect(result!.user!.id).toBe("48168");

expect(setUserInfoSpy).toHaveBeenCalledWith({
userId: "48168",
email: "user@example.com",
name: undefined,
});
});

test("null user.email is omitted from result", async () => {
performDeviceFlowSpy = spyOn(oauth, "performDeviceFlow").mockImplementation(
async (callbacks) => {
await callbacks.onUserCode(
"EFGH",
"https://sentry.io/auth/device/",
"https://sentry.io/auth/device/?user_code=EFGH"
);
return makeTokenResponse({
id: "123",
name: "Jane Doe",
email: null,
});
}
);

const result = await runInteractiveLogin({ timeout: 1000 });

expect(result).not.toBeNull();
expect(result!.user!.name).toBe("Jane Doe");
// email must be absent from the result object
expect("email" in result!.user!).toBe(false);

expect(setUserInfoSpy).toHaveBeenCalledWith({
userId: "123",
email: undefined,
name: "Jane Doe",
});
});

test("no user in token response: result.user is undefined, setUserInfo not called", async () => {
performDeviceFlowSpy = spyOn(oauth, "performDeviceFlow").mockImplementation(
async (callbacks) => {
await callbacks.onUserCode(
"WXYZ",
"https://sentry.io/auth/device/",
"https://sentry.io/auth/device/?user_code=WXYZ"
);
return makeTokenResponse(); // no user
}
);

const result = await runInteractiveLogin({ timeout: 1000 });

expect(result).not.toBeNull();
expect(result!.user).toBeUndefined();
expect(setUserInfoSpy).not.toHaveBeenCalled();
});
});
Loading
Loading