Skip to content

Commit 257346c

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 257346c

File tree

4 files changed

+214
-87
lines changed

4 files changed

+214
-87
lines changed

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/lib/db/auth.property.test.ts

Lines changed: 47 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -74,28 +74,34 @@ describe("property: env var priority", () => {
7474
);
7575
});
7676

77-
test("env var always wins over stored token", () => {
77+
test("stored OAuth wins over env var (default behavior)", () => {
7878
fcAssert(
7979
property(tokenArb, tokenArb, (envToken, storedToken) => {
8080
setAuthToken(storedToken);
8181
process.env.SENTRY_AUTH_TOKEN = envToken;
8282

83-
expect(getAuthToken()).toBe(envToken.trim());
83+
// Stored OAuth takes priority — env token is for build tooling
84+
expect(getAuthToken()).toBe(storedToken);
85+
expect(getAuthConfig()?.source).toBe("oauth" satisfies AuthSource);
8486
}),
8587
{ numRuns: DEFAULT_NUM_RUNS }
8688
);
8789
});
8890

89-
test("SENTRY_TOKEN wins over stored token when SENTRY_AUTH_TOKEN is absent", () => {
91+
test("SENTRY_FORCE_ENV_TOKEN overrides stored OAuth", () => {
9092
fcAssert(
9193
property(tokenArb, tokenArb, (envToken, storedToken) => {
9294
setAuthToken(storedToken);
93-
process.env.SENTRY_TOKEN = envToken;
94-
95-
expect(getAuthToken()).toBe(envToken.trim());
96-
expect(getAuthConfig()?.source).toBe(
97-
"env:SENTRY_TOKEN" satisfies AuthSource
98-
);
95+
process.env.SENTRY_AUTH_TOKEN = envToken;
96+
try {
97+
process.env.SENTRY_FORCE_ENV_TOKEN = "1";
98+
expect(getAuthToken()).toBe(envToken.trim());
99+
expect(getAuthConfig()?.source).toBe(
100+
"env:SENTRY_AUTH_TOKEN" satisfies AuthSource
101+
);
102+
} finally {
103+
delete process.env.SENTRY_FORCE_ENV_TOKEN;
104+
}
99105
}),
100106
{ numRuns: DEFAULT_NUM_RUNS }
101107
);
@@ -145,37 +151,40 @@ describe("property: env tokens never trigger refresh", () => {
145151
});
146152

147153
describe("property: isEnvTokenActive consistency", () => {
148-
test("isEnvTokenActive matches whether getAuthConfig returns env source", () => {
154+
test("when no env token, getAuthConfig never returns env source", () => {
149155
fcAssert(
150-
property(
151-
option(tokenArb),
152-
option(tokenArb),
153-
option(tokenArb),
154-
(authTokenOpt, sentryTokenOpt, storedTokenOpt) => {
155-
// Clean slate
156-
delete process.env.SENTRY_AUTH_TOKEN;
157-
delete process.env.SENTRY_TOKEN;
158-
159-
if (authTokenOpt !== null) {
160-
process.env.SENTRY_AUTH_TOKEN = authTokenOpt;
161-
}
162-
if (sentryTokenOpt !== null) {
163-
process.env.SENTRY_TOKEN = sentryTokenOpt;
164-
}
165-
if (storedTokenOpt !== null) {
166-
setAuthToken(storedTokenOpt);
167-
}
168-
169-
const config = getAuthConfig();
170-
const envActive = isEnvTokenActive();
171-
172-
if (envActive) {
173-
expect(config?.source).toMatch(/^env:/);
174-
} else if (config) {
175-
expect(config.source).toBe("oauth");
176-
}
156+
property(option(tokenArb), (storedTokenOpt) => {
157+
// Clean slate — no env tokens
158+
delete process.env.SENTRY_AUTH_TOKEN;
159+
delete process.env.SENTRY_TOKEN;
160+
161+
if (storedTokenOpt !== null) {
162+
setAuthToken(storedTokenOpt);
163+
}
164+
165+
const config = getAuthConfig();
166+
const envActive = isEnvTokenActive();
167+
168+
expect(envActive).toBe(false);
169+
if (config) {
170+
expect(config.source).toBe("oauth");
177171
}
178-
),
172+
}),
173+
{ numRuns: DEFAULT_NUM_RUNS }
174+
);
175+
});
176+
177+
test("stored OAuth takes priority: getAuthConfig returns oauth even when env token is set", () => {
178+
fcAssert(
179+
property(tokenArb, tokenArb, (envToken, storedToken) => {
180+
process.env.SENTRY_AUTH_TOKEN = envToken;
181+
setAuthToken(storedToken);
182+
183+
const config = getAuthConfig();
184+
expect(config?.source).toBe("oauth");
185+
// But isEnvTokenActive is still true (env token exists)
186+
expect(isEnvTokenActive()).toBe(true);
187+
}),
179188
{ numRuns: DEFAULT_NUM_RUNS }
180189
);
181190
});

test/lib/db/auth.test.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,14 +114,34 @@ describe("env var auth: getActiveEnvVarName", () => {
114114
});
115115

116116
describe("env var auth: refreshToken edge cases", () => {
117-
test("env token skips stored token entirely", async () => {
118-
setAuthToken("stored_token", -1, "refresh_token");
117+
test("env token used when no stored OAuth exists", async () => {
119118
process.env.SENTRY_AUTH_TOKEN = "env_token";
120119
const result = await refreshToken();
121120
expect(result.token).toBe("env_token");
122121
expect(result.refreshed).toBe(false);
123122
});
124123

124+
test("stored OAuth preferred over env token", async () => {
125+
setAuthToken("stored_token", 3600);
126+
process.env.SENTRY_AUTH_TOKEN = "env_token";
127+
const result = await refreshToken();
128+
expect(result.token).toBe("stored_token");
129+
expect(result.refreshed).toBe(false);
130+
});
131+
132+
test("SENTRY_FORCE_ENV_TOKEN overrides stored OAuth in refreshToken", async () => {
133+
setAuthToken("stored_token", 3600);
134+
process.env.SENTRY_AUTH_TOKEN = "env_token";
135+
try {
136+
process.env.SENTRY_FORCE_ENV_TOKEN = "1";
137+
const result = await refreshToken();
138+
expect(result.token).toBe("env_token");
139+
expect(result.refreshed).toBe(false);
140+
} finally {
141+
delete process.env.SENTRY_FORCE_ENV_TOKEN;
142+
}
143+
});
144+
125145
test("has no expiresAt or expiresIn for env tokens", async () => {
126146
process.env.SENTRY_AUTH_TOKEN = "env_token";
127147
const result = await refreshToken();

0 commit comments

Comments
 (0)