Skip to content

Commit d871054

Browse files
committed
fix(auth): prefer stored OAuth over env token by default (#646)
When SENTRY_AUTH_TOKEN is set by build tooling (e.g. the Sentry wizard) but lacks the scopes needed for interactive CLI commands, the CLI now prefers stored OAuth credentials by default. OAuth-preferred default: when the user is logged in via OAuth, env tokens are automatically skipped. This prevents wizard-generated build tokens from silently overriding interactive credentials. Set SENTRY_FORCE_ENV_TOKEN=1 to restore the old env-token-first priority. Key changes: - getEnvToken() returns undefined when stored OAuth credentials exist, cascading through all consumers (isEnvTokenActive, refreshToken, etc.) - SENTRY_FORCE_ENV_TOKEN env var to force env token priority - sentry auth login no longer blocks when an env token is present - sentry auth status shows env token info (active vs bypassed) - Enhanced 401/403 error messages when env token is the only auth source
1 parent ca14e7c commit d871054

File tree

7 files changed

+265
-105
lines changed

7 files changed

+265
-105
lines changed

src/commands/auth/login.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { buildCommand, numberParser } from "../../lib/command.js";
99
import {
1010
clearAuth,
1111
getActiveEnvVarName,
12+
hasStoredAuthCredentials,
1213
isAuthenticated,
1314
isEnvTokenActive,
1415
setAuthToken,
@@ -71,11 +72,15 @@ type LoginFlags = {
7172
async function handleExistingAuth(force: boolean): Promise<boolean> {
7273
if (isEnvTokenActive()) {
7374
const envVar = getActiveEnvVarName();
74-
log.info(
75-
`Authentication is provided via ${envVar} environment variable. ` +
76-
`Unset ${envVar} to use OAuth-based login instead.`
75+
log.warn(
76+
`${envVar} is set in your environment (likely from build tooling).\n` +
77+
" OAuth credentials will be stored separately and used for CLI commands."
7778
);
78-
return false;
79+
// If no stored OAuth token exists, proceed directly to login
80+
if (!hasStoredAuthCredentials()) {
81+
return true;
82+
}
83+
// Fall through to the re-auth confirmation logic below
7984
}
8085

8186
if (!force) {

src/lib/api/infrastructure.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import * as Sentry from "@sentry/node-core/light";
1111
import type { z } from "zod";
1212

13+
import { getRawEnvToken } from "../db/auth.js";
1314
import { getEnv } from "../env.js";
1415
import { ApiError, AuthError, stringifyUnknown } from "../errors.js";
1516
import { resolveOrgRegion } from "../region.js";
@@ -57,6 +58,29 @@ export function throwApiError(
5758
error && typeof error === "object" && "detail" in error
5859
? stringifyUnknown((error as { detail: unknown }).detail)
5960
: stringifyUnknown(error);
61+
62+
// When an env token is set and we get 401, the HTTP-layer fallback to
63+
// stored OAuth already failed (no stored credentials). Convert to AuthError
64+
// so the auto-login middleware in cli.ts can trigger interactive login.
65+
if (status === 401 && getRawEnvToken()) {
66+
throw new AuthError(
67+
"not_authenticated",
68+
`${context}: ${status} ${response.statusText ?? "Unknown"}.\n` +
69+
" SENTRY_AUTH_TOKEN is set but lacks permissions for this endpoint.\n" +
70+
" Run 'sentry auth login' to authenticate with OAuth."
71+
);
72+
}
73+
74+
// For 403 with env token, keep as ApiError but add guidance
75+
if (status === 403 && getRawEnvToken()) {
76+
throw new ApiError(
77+
`${context}: ${status} ${response.statusText ?? "Unknown"}`,
78+
status,
79+
`${detail}\n\n SENTRY_AUTH_TOKEN may lack permissions for this endpoint.\n` +
80+
" Run 'sentry auth login' to authenticate with OAuth."
81+
);
82+
}
83+
6084
throw new ApiError(
6185
`${context}: ${status} ${response.statusText ?? "Unknown"}`,
6286
status,

src/lib/db/auth.ts

Lines changed: 117 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,34 @@ export type AuthConfig = {
3636
source: AuthSource;
3737
};
3838

39+
/**
40+
* Read the raw token string from environment variables, ignoring all filters.
41+
*
42+
* Unlike {@link getEnvToken}, this always returns the env token if set, even
43+
* when stored OAuth credentials would normally take priority. Used by the HTTP
44+
* layer to check "was an env token provided?" independent of whether it's being
45+
* used, and by the per-endpoint permission cache.
46+
*/
47+
export function getRawEnvToken(): string | undefined {
48+
const authToken = getEnv().SENTRY_AUTH_TOKEN?.trim();
49+
if (authToken) {
50+
return authToken;
51+
}
52+
const sentryToken = getEnv().SENTRY_TOKEN?.trim();
53+
if (sentryToken) {
54+
return sentryToken;
55+
}
56+
return;
57+
}
58+
3959
/**
4060
* Read token from environment variables.
4161
* `SENTRY_AUTH_TOKEN` takes priority over `SENTRY_TOKEN` (matches legacy sentry-cli).
4262
* Empty or whitespace-only values are treated as unset.
63+
*
64+
* This function is intentionally pure (no DB access). The "prefer stored OAuth
65+
* over env token" logic lives in {@link getAuthToken} and {@link getAuthConfig}
66+
* which check the DB first when `SENTRY_FORCE_ENV_TOKEN` is not set.
4367
*/
4468
function getEnvToken(): { token: string; source: AuthSource } | undefined {
4569
const authToken = getEnv().SENTRY_AUTH_TOKEN?.trim();
@@ -62,28 +86,39 @@ export function isEnvTokenActive(): boolean {
6286
}
6387

6488
/**
65-
* Get the name of the active env var providing authentication.
89+
* Get the name of the env var providing a token, for error messages.
6690
* Returns the specific variable name (e.g. "SENTRY_AUTH_TOKEN" or "SENTRY_TOKEN").
6791
*
68-
* **Important**: Call only after checking {@link isEnvTokenActive} returns true.
69-
* Falls back to "SENTRY_AUTH_TOKEN" if no env source is active, which is a safe
70-
* default for error messages but may be misleading if used unconditionally.
92+
* Uses {@link getRawEnvToken} (not {@link getEnvToken}) so the result is
93+
* independent of whether stored OAuth takes priority.
94+
* Falls back to "SENTRY_AUTH_TOKEN" if no env var is set.
7195
*/
7296
export function getActiveEnvVarName(): string {
73-
const env = getEnvToken();
74-
if (env) {
75-
return env.source.slice(ENV_SOURCE_PREFIX.length);
97+
const authToken = getEnv().SENTRY_AUTH_TOKEN?.trim();
98+
if (authToken) {
99+
return "SENTRY_AUTH_TOKEN";
100+
}
101+
const sentryToken = getEnv().SENTRY_TOKEN?.trim();
102+
if (sentryToken) {
103+
return "SENTRY_TOKEN";
76104
}
77105
return "SENTRY_AUTH_TOKEN";
78106
}
79107

80108
export function getAuthConfig(): AuthConfig | undefined {
81-
const envToken = getEnvToken();
82-
if (envToken) {
83-
return { token: envToken.token, source: envToken.source };
109+
// When SENTRY_FORCE_ENV_TOKEN is set, check env first (old behavior).
110+
// Otherwise, check the DB first — stored OAuth takes priority over env tokens.
111+
// This is the core fix for #646: wizard-generated build tokens no longer
112+
// silently override the user's interactive login.
113+
const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim();
114+
if (forceEnv) {
115+
const envToken = getEnvToken();
116+
if (envToken) {
117+
return { token: envToken.token, source: envToken.source };
118+
}
84119
}
85120

86-
return withDbSpan("getAuthConfig", () => {
121+
const dbConfig = withDbSpan("getAuthConfig", () => {
87122
const db = getDatabase();
88123
const row = db.query("SELECT * FROM auth WHERE id = 1").get() as
89124
| AuthRow
@@ -101,16 +136,34 @@ export function getAuthConfig(): AuthConfig | undefined {
101136
source: "oauth" as const,
102137
};
103138
});
104-
}
139+
if (dbConfig) {
140+
return dbConfig;
141+
}
105142

106-
/** Get the active auth token. Checks env vars first, then falls back to SQLite. */
107-
export function getAuthToken(): string | undefined {
143+
// No stored OAuth — fall back to env token
108144
const envToken = getEnvToken();
109145
if (envToken) {
110-
return envToken.token;
146+
return { token: envToken.token, source: envToken.source };
147+
}
148+
return;
149+
}
150+
151+
/**
152+
* Get the active auth token.
153+
*
154+
* Default: checks the DB first (stored OAuth wins), then falls back to env vars.
155+
* With `SENTRY_FORCE_ENV_TOKEN=1`: checks env vars first (old behavior).
156+
*/
157+
export function getAuthToken(): string | undefined {
158+
const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim();
159+
if (forceEnv) {
160+
const envToken = getEnvToken();
161+
if (envToken) {
162+
return envToken.token;
163+
}
111164
}
112165

113-
return withDbSpan("getAuthToken", () => {
166+
const dbToken = withDbSpan("getAuthToken", () => {
114167
const db = getDatabase();
115168
const row = db.query("SELECT * FROM auth WHERE id = 1").get() as
116169
| AuthRow
@@ -126,6 +179,16 @@ export function getAuthToken(): string | undefined {
126179

127180
return row.token;
128181
});
182+
if (dbToken) {
183+
return dbToken;
184+
}
185+
186+
// No stored OAuth — fall back to env token
187+
const envToken = getEnvToken();
188+
if (envToken) {
189+
return envToken.token;
190+
}
191+
return;
129192
}
130193

131194
export function setAuthToken(
@@ -179,6 +242,32 @@ export function isAuthenticated(): boolean {
179242
return !!token;
180243
}
181244

245+
/**
246+
* Check if usable OAuth credentials are stored in the database.
247+
*
248+
* Returns true when the `auth` table has either:
249+
* - A non-expired token, or
250+
* - An expired token with a refresh token (will be refreshed on next use)
251+
*
252+
* Used by the login command to decide whether to prompt for re-authentication
253+
* when an env token is present.
254+
*/
255+
export function hasStoredAuthCredentials(): boolean {
256+
const db = getDatabase();
257+
const row = db.query("SELECT * FROM auth WHERE id = 1").get() as
258+
| AuthRow
259+
| undefined;
260+
if (!row?.token) {
261+
return false;
262+
}
263+
// Non-expired token
264+
if (!row.expires_at || Date.now() <= row.expires_at) {
265+
return true;
266+
}
267+
// Expired but has refresh token — will be refreshed on next use
268+
return !!row.refresh_token;
269+
}
270+
182271
export type RefreshTokenOptions = {
183272
/** Bypass threshold check and always refresh */
184273
force?: boolean;
@@ -229,10 +318,13 @@ async function performTokenRefresh(
229318
export async function refreshToken(
230319
options: RefreshTokenOptions = {}
231320
): Promise<RefreshTokenResult> {
232-
// Env var tokens are assumed valid — no refresh, no expiry check
233-
const envToken = getEnvToken();
234-
if (envToken) {
235-
return { token: envToken.token, refreshed: false };
321+
// With SENTRY_FORCE_ENV_TOKEN, env token takes priority (no refresh needed).
322+
const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim();
323+
if (forceEnv) {
324+
const envToken = getEnvToken();
325+
if (envToken) {
326+
return { token: envToken.token, refreshed: false };
327+
}
236328
}
237329

238330
const { force = false } = options;
@@ -244,6 +336,11 @@ export async function refreshToken(
244336
| undefined;
245337

246338
if (!row?.token) {
339+
// No stored token — try env token as fallback
340+
const envToken = getEnvToken();
341+
if (envToken) {
342+
return { token: envToken.token, refreshed: false };
343+
}
247344
throw new AuthError("not_authenticated");
248345
}
249346

test/commands/auth/login.test.ts

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ describe("loginCommand.func --token path", () => {
8686
let getCurrentUserSpy: ReturnType<typeof spyOn>;
8787
let setUserInfoSpy: ReturnType<typeof spyOn>;
8888
let runInteractiveLoginSpy: ReturnType<typeof spyOn>;
89+
let hasStoredAuthCredentialsSpy: ReturnType<typeof spyOn>;
8990
let func: LoginFunc;
9091

9192
beforeEach(async () => {
@@ -97,7 +98,9 @@ describe("loginCommand.func --token path", () => {
9798
getCurrentUserSpy = spyOn(apiClient, "getCurrentUser");
9899
setUserInfoSpy = spyOn(dbUser, "setUserInfo");
99100
runInteractiveLoginSpy = spyOn(interactiveLogin, "runInteractiveLogin");
101+
hasStoredAuthCredentialsSpy = spyOn(dbAuth, "hasStoredAuthCredentials");
100102
isEnvTokenActiveSpy.mockReturnValue(false);
103+
hasStoredAuthCredentialsSpy.mockReturnValue(false);
101104
func = (await loginCommand.loader()) as unknown as LoginFunc;
102105
});
103106

@@ -110,6 +113,7 @@ describe("loginCommand.func --token path", () => {
110113
getCurrentUserSpy.mockRestore();
111114
setUserInfoSpy.mockRestore();
112115
runInteractiveLoginSpy.mockRestore();
116+
hasStoredAuthCredentialsSpy.mockRestore();
113117
});
114118

115119
test("already authenticated (non-TTY, no --force): prints re-auth message with --force hint", async () => {
@@ -122,33 +126,31 @@ describe("loginCommand.func --token path", () => {
122126
expect(getCurrentUserSpy).not.toHaveBeenCalled();
123127
});
124128

125-
test("already authenticated (env token SENTRY_AUTH_TOKEN): tells user to unset specific var", async () => {
129+
test("already authenticated (env token SENTRY_AUTH_TOKEN): warns and proceeds to OAuth login", async () => {
126130
isAuthenticatedSpy.mockReturnValue(true);
127131
isEnvTokenActiveSpy.mockReturnValue(true);
128-
// Need to also spy on getAuthConfig for the specific env var name
129-
const getAuthConfigSpy = spyOn(dbAuth, "getAuthConfig");
130-
getAuthConfigSpy.mockReturnValue({
131-
token: "sntrys_env_123",
132-
source: "env:SENTRY_AUTH_TOKEN",
133-
});
132+
hasStoredAuthCredentialsSpy.mockReturnValue(false);
133+
runInteractiveLoginSpy.mockResolvedValue(null);
134134

135135
const { context } = createContext();
136136
await func.call(context, { force: false, timeout: 900 });
137137

138-
expect(setAuthTokenSpy).not.toHaveBeenCalled();
139-
getAuthConfigSpy.mockRestore();
138+
// With no stored OAuth, login proceeds directly (no clearAuth needed)
139+
expect(runInteractiveLoginSpy).toHaveBeenCalled();
140140
});
141141

142-
test("already authenticated (env token SENTRY_TOKEN): shows specific var name", async () => {
142+
test("already authenticated (env token SENTRY_TOKEN): warns and proceeds to OAuth login", async () => {
143143
isAuthenticatedSpy.mockReturnValue(true);
144144
isEnvTokenActiveSpy.mockReturnValue(true);
145+
hasStoredAuthCredentialsSpy.mockReturnValue(false);
145146
// Set env var directly — getActiveEnvVarName() reads env vars via getEnvToken()
146147
process.env.SENTRY_TOKEN = "sntrys_token_456";
148+
runInteractiveLoginSpy.mockResolvedValue(null);
147149

148150
const { context } = createContext();
149151
await func.call(context, { force: false, timeout: 900 });
150152

151-
expect(setAuthTokenSpy).not.toHaveBeenCalled();
153+
expect(runInteractiveLoginSpy).toHaveBeenCalled();
152154
delete process.env.SENTRY_TOKEN;
153155
});
154156

@@ -315,14 +317,16 @@ describe("loginCommand.func --token path", () => {
315317
expect(getStdout()).toContain("Authenticated");
316318
});
317319

318-
test("--force with env token: still blocks (env var case unchanged)", async () => {
320+
test("--force with env token: proceeds to OAuth login (no longer blocks)", async () => {
319321
isAuthenticatedSpy.mockReturnValue(true);
320322
isEnvTokenActiveSpy.mockReturnValue(true);
323+
hasStoredAuthCredentialsSpy.mockReturnValue(false);
324+
runInteractiveLoginSpy.mockResolvedValue(null);
321325

322326
const { context } = createContext();
323327
await func.call(context, { force: true, timeout: 900 });
324328

325-
expect(clearAuthSpy).not.toHaveBeenCalled();
326-
expect(runInteractiveLoginSpy).not.toHaveBeenCalled();
329+
// Env token no longer blocks — login proceeds
330+
expect(runInteractiveLoginSpy).toHaveBeenCalled();
327331
});
328332
});

0 commit comments

Comments
 (0)