Skip to content

Commit 6e5dc3d

Browse files
feat(cli): request org:integrations by default and elevate before integration delete
Add org:integrations to the default OAuth scopes (no admin scopes). integration delete now checks the stored grant before confirming and, for older logins, explains it needs elevated permissions and re-runs authorization itself instead of telling the user which command to run. A 403 after that explains the org role requirement. Co-Authored-By: Burak Yigit Kaya <burak.kaya@sentry.io>
1 parent 8bcddfa commit 6e5dc3d

9 files changed

Lines changed: 254 additions & 28 deletions

File tree

‎apps/cli-docs/src/content/docs/self-hosted.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ If your instance is on an older version or you prefer not to create an OAuth app
5858
1. Go to **Settings → Developer Settings → Personal Tokens** in your Sentry instance (or visit `https://sentry.example.com/settings/account/api/auth-tokens/new-token/`)
5959
2. Create a new token with the following scopes:
6060
<!-- GENERATED:START oauth-scopes -->
61-
`project:read`, `project:write`, `project:admin`, `org:read`, `event:read`, `event:write`, `member:read`, `team:read`, `team:write`, `team:admin`, `alerts:read`, `alerts:write`
61+
`project:read`, `project:write`, `project:admin`, `org:read`, `org:integrations`, `event:read`, `event:write`, `member:read`, `team:read`, `team:write`, `team:admin`, `alerts:read`, `alerts:write`
6262
<!-- GENERATED:END oauth-scopes -->
6363
3. Pass it to the CLI:
6464

‎apps/cli-docs/src/fragments/commands/integration.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,8 @@ sentry integration delete my-org/12345 --dry-run
1616
sentry integration delete my-org/12345 --yes
1717
```
1818

19-
Removing an integration requires the `org:integrations` or `org:admin` scope.
20-
The default `sentry auth login` scopes do not include it; re-authenticate with
21-
`sentry auth refresh --scope ...` including `org:integrations`, or use an auth
22-
token that has it.
19+
Removing an integration needs the `org:integrations` permission, which Sentry
20+
grants to org owners and managers. If your CLI login predates that scope,
21+
`sentry integration delete` asks you to re-authorize before removing anything.
2322

2423
Installing new integrations from the CLI is not supported yet.

‎packages/cli/DEVELOPMENT.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ When creating your Sentry OAuth application:
6666
- **Scopes**: The CLI requests these scopes:
6767
<!-- GENERATED:START oauth-scopes -->
6868
- `project:read`, `project:write`, `project:admin`
69-
- `org:read`
69+
- `org:read`, `org:integrations`
7070
- `event:read`, `event:write`
7171
- `member:read`
7272
- `team:read`, `team:write`, `team:admin`

‎packages/cli/src/commands/integration/delete.ts‎

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,21 @@ import { deleteIntegration, getIntegration } from "../../lib/api-client.js";
1212
import { ApiError, ContextError, ValidationError } from "../../lib/errors.js";
1313
import { renderMarkdown, safeCodeSpan } from "../../lib/formatters/markdown.js";
1414
import { CommandOutput } from "../../lib/formatters/output.js";
15+
import { runInteractiveLogin } from "../../lib/interactive-login.js";
1516
import {
1617
buildDeleteCommand,
1718
confirmByTyping,
1819
isConfirmationBypassed,
1920
} from "../../lib/mutate-command.js";
20-
import { OAUTH_SCOPES } from "../../lib/oauth.js";
2121
import { resolveOrg } from "../../lib/resolve-target.js";
22+
import { ensureScopesForAction } from "../../lib/scope-recovery.js";
2223

2324
const USAGE_HINT = "sentry integration delete [<org>/]<integration-id>";
2425

25-
/** Scope Sentry requires (besides org:admin) to uninstall an integration. */
26+
/**
27+
* Scope Sentry requires (besides org:admin) to uninstall an integration.
28+
* Part of the CLI's default scopes, but logins from before it was added lack it.
29+
*/
2630
const DELETE_SCOPE = "org:integrations";
2731

2832
const INTEGRATION_ID_RE = /^\d+$/;
@@ -86,22 +90,28 @@ export function parseIntegrationArg(arg: string | undefined): {
8690
}
8791

8892
/**
89-
* Turn a 403 into actionable guidance: the CLI's default OAuth login does not
90-
* request `org:integrations`, and Sentry's 403 body doesn't name the scope.
93+
* Explain a 403 from the delete call. Sentry's 403 body names neither the
94+
* scope nor the role, and the generic hint ("re-authenticate") doesn't help
95+
* when the user's org role is what's missing.
96+
*
97+
* @param scopeMissing - The stored login lacks the scope and couldn't be
98+
* elevated here (no interactive terminal)
9199
*/
92-
function enrichDeleteError(error: unknown): unknown {
93-
if (error instanceof ApiError && error.status === 403) {
94-
const scopes = [...OAUTH_SCOPES, DELETE_SCOPE].join(",");
95-
return new ApiError(
96-
"Not allowed to remove this integration",
97-
403,
98-
`Removing integrations requires the '${DELETE_SCOPE}' or 'org:admin' scope and an owner or manager role.\n ` +
99-
`Re-authenticate with: sentry auth refresh --scope ${scopes}\n ` +
100-
"Or use an auth token that has the 'org:integrations' scope.",
101-
error.endpoint,
102-
);
100+
function enrichDeleteError(error: unknown, scopeMissing: boolean): unknown {
101+
if (!(error instanceof ApiError && error.status === 403)) {
102+
return error;
103103
}
104-
return error;
104+
const detail = scopeMissing
105+
? `Your saved login predates the '${DELETE_SCOPE}' scope, and authorization can't be updated without an interactive terminal.\n ` +
106+
`Run this command in a terminal to re-authorize, or use an auth token with the '${DELETE_SCOPE}' scope.`
107+
: `Removing integrations requires the '${DELETE_SCOPE}' permission, which Sentry grants to org owners and managers.\n ` +
108+
`If you use an auth token, make sure it has the '${DELETE_SCOPE}' scope.`;
109+
return new ApiError(
110+
"Not allowed to remove this integration",
111+
403,
112+
detail,
113+
error.endpoint,
114+
);
105115
}
106116

107117
export const deleteCommand = buildDeleteCommand({
@@ -111,7 +121,8 @@ export const deleteCommand = buildDeleteCommand({
111121
"Uninstall an integration from an organization. Sentry also removes the " +
112122
"integration's linked issues and stops syncing with the provider.\n\n" +
113123
"Find the integration ID with `sentry integration list`.\n\n" +
114-
"Requires the org:integrations or org:admin scope.\n\n" +
124+
"Needs the org:integrations permission (org owners and managers). If your " +
125+
"login predates that scope, the CLI asks you to re-authorize first.\n\n" +
115126
"Examples:\n" +
116127
" sentry integration delete 12345\n" +
117128
" sentry integration delete my-org/12345\n" +
@@ -156,6 +167,18 @@ export const deleteCommand = buildDeleteCommand({
156167
return;
157168
}
158169

170+
// Elevate before confirming so an older login authorizes once instead of
171+
// failing with 403 and re-running the prompt via scope recovery.
172+
const elevation = await ensureScopesForAction(
173+
[DELETE_SCOPE],
174+
"Removing an integration",
175+
runInteractiveLogin,
176+
);
177+
if (elevation === "declined") {
178+
yield new CommandOutput(result);
179+
return { hint: "Authorization was not completed. Nothing was removed." };
180+
}
181+
159182
// Confirmation gate — non-interactive guard is handled by buildDeleteCommand
160183
if (!isConfirmationBypassed(flags)) {
161184
const confirmed = await confirmByTyping(
@@ -171,7 +194,7 @@ export const deleteCommand = buildDeleteCommand({
171194
try {
172195
await deleteIntegration(org, id);
173196
} catch (error) {
174-
throw enrichDeleteError(error);
197+
throw enrichDeleteError(error, elevation === "unavailable");
175198
}
176199
yield new CommandOutput({ ...result, deleted: true });
177200
},

‎packages/cli/src/lib/oauth.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ export const OAUTH_SCOPES: readonly string[] = [
9090
"project:write",
9191
"project:admin",
9292
"org:read",
93+
"org:integrations",
9394
"event:read",
9495
"event:write",
9596
"member:read",
@@ -108,8 +109,9 @@ const SCOPES = OAUTH_SCOPES.join(" ");
108109
*
109110
* Derived by keeping only scopes whose action is `:read`. This assumes every
110111
* read-granting scope the CLI requests carries the `:read` suffix — true for
111-
* the current list. If a future read-ish scope without that suffix is added to
112-
* `OAUTH_SCOPES` (e.g. `org:integrations`), update this filter explicitly.
112+
* the current list. `org:integrations` grants install/uninstall, so excluding
113+
* it here is intended. If a future read-ish scope without that suffix is
114+
* added to `OAUTH_SCOPES`, update this filter explicitly.
113115
*/
114116
const OAUTH_SCOPES_READ_ONLY: readonly string[] = OAUTH_SCOPES.filter((scope) =>
115117
scope.endsWith(":read"),

‎packages/cli/src/lib/scope-recovery.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,65 @@ export async function ensureCurrentOAuthScopes(
148148
return await refreshOAuthScopes(state, runInteractiveLogin, runtime);
149149
}
150150

151+
/**
152+
* Outcome of {@link ensureScopesForAction}.
153+
*
154+
* - `ready`: the token already has the scopes, or it isn't a stored OAuth
155+
* grant the CLI can elevate (env/auth tokens), or the check itself failed.
156+
* Callers proceed and let the API decide.
157+
* - `elevated`: the user re-authorized and the new grant is stored.
158+
* - `unavailable`: elevation is needed but prompts aren't possible here.
159+
* - `declined`: the authorization flow failed or was cancelled.
160+
*/
161+
export type ScopeElevationResult =
162+
| "ready"
163+
| "elevated"
164+
| "unavailable"
165+
| "declined";
166+
167+
/**
168+
* Make sure the stored OAuth grant has `required` before running an action.
169+
*
170+
* Commands that need a default scope older logins may lack call this before
171+
* any confirmation prompt, so the user authorizes once instead of hitting a
172+
* 403 and re-running the command (and its prompts) through
173+
* {@link runWithScopeRecovery}. The device flow requests the CLI's default
174+
* scopes, so `required` must be a subset of {@link OAUTH_SCOPES}.
175+
*
176+
* @param required - Scopes the action needs
177+
* @param action - Human description for the prompt, e.g. "Removing an integration"
178+
*/
179+
export async function ensureScopesForAction(
180+
required: readonly string[],
181+
action: string,
182+
runInteractiveLogin: InteractiveLogin,
183+
runtime: ScopeRecoveryRuntime = defaultRuntime,
184+
): Promise<ScopeElevationResult> {
185+
let missing: string[];
186+
try {
187+
if (runtime.getAuthSource() !== "oauth") {
188+
return "ready";
189+
}
190+
const granted = new Set((await runtime.getAuthScopes()) ?? []);
191+
missing = required.filter((scope) => !granted.has(scope));
192+
} catch (error) {
193+
logger.debug("Could not inspect OAuth scopes before action", error);
194+
return "ready";
195+
}
196+
if (missing.length === 0) {
197+
return "ready";
198+
}
199+
if (!(runtime.inputIsTty() && runtime.promptsAllowed())) {
200+
return "unavailable";
201+
}
202+
203+
runtime.assertTrustedHost();
204+
runtime.write(
205+
`${action} needs elevated permissions (${missing.join(", ")}). Starting authorization...\n\n`,
206+
);
207+
return (await runInteractiveLogin()) ? "elevated" : "declined";
208+
}
209+
151210
/** Check an OAuth token after a 401/403, re-authorize if needed, and retry once. */
152211
export async function runWithScopeRecovery(
153212
proceed: (commandArgs: string[]) => Promise<void>,

‎packages/cli/test/commands/integration/delete.test.ts‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,15 @@ vi.mock("../../../src/lib/resolve-target.js", async (importOriginal) => {
4141
// oxlint-disable-next-line sentry-cli/no-namespace-import -- needed for spyOn mocking
4242
import * as resolveTarget from "../../../src/lib/resolve-target.js";
4343

44+
vi.mock("../../../src/lib/scope-recovery.js", async (importOriginal) => {
45+
const actual =
46+
await importOriginal<typeof import("../../../src/lib/scope-recovery.js")>();
47+
return { ...actual, ensureScopesForAction: vi.fn() };
48+
});
49+
50+
// oxlint-disable-next-line sentry-cli/no-namespace-import -- needed for vi.mocked access
51+
import * as scopeRecovery from "../../../src/lib/scope-recovery.js";
52+
4453
const sampleIntegration = {
4554
id: "42",
4655
name: "getsentry",
@@ -87,8 +96,10 @@ describe("integration delete", () => {
8796
let getSpy: ReturnType<typeof vi.spyOn>;
8897
let deleteSpy: ReturnType<typeof vi.spyOn>;
8998
let resolveOrgSpy: ReturnType<typeof vi.spyOn>;
99+
const ensureScopesSpy = vi.mocked(scopeRecovery.ensureScopesForAction);
90100

91101
beforeEach(() => {
102+
ensureScopesSpy.mockResolvedValue("ready");
92103
getSpy = vi.spyOn(apiClient, "getIntegration");
93104
deleteSpy = vi.spyOn(apiClient, "deleteIntegration");
94105
resolveOrgSpy = vi.spyOn(resolveTarget, "resolveOrg");
@@ -101,6 +112,7 @@ describe("integration delete", () => {
101112
getSpy.mockRestore();
102113
deleteSpy.mockRestore();
103114
resolveOrgSpy.mockRestore();
115+
ensureScopesSpy.mockReset();
104116
});
105117

106118
test("removes the integration with --yes", async () => {
@@ -114,12 +126,37 @@ describe("integration delete", () => {
114126
expect(output(stdoutWrite)).toContain("Removed GitHub integration");
115127
});
116128

129+
test("elevates to org:integrations before deleting", async () => {
130+
ensureScopesSpy.mockResolvedValue("elevated");
131+
const { context } = createMockContext();
132+
const func = await deleteCommand.loader();
133+
await func.call(context, yesFlags, "my-org/42");
134+
135+
expect(ensureScopesSpy).toHaveBeenCalledWith(
136+
["org:integrations"],
137+
"Removing an integration",
138+
expect.any(Function),
139+
);
140+
expect(deleteSpy).toHaveBeenCalledWith("my-org", "42");
141+
});
142+
143+
test("does not delete when authorization is declined", async () => {
144+
ensureScopesSpy.mockResolvedValue("declined");
145+
const { context, stdoutWrite } = createMockContext();
146+
const func = await deleteCommand.loader();
147+
await func.call(context, { ...yesFlags, json: true }, "my-org/42");
148+
149+
expect(deleteSpy).not.toHaveBeenCalled();
150+
expect(JSON.parse(output(stdoutWrite))).toMatchObject({ deleted: false });
151+
});
152+
117153
test("dry-run does not delete", async () => {
118154
const { context, stdoutWrite } = createMockContext();
119155
const func = await deleteCommand.loader();
120156
await func.call(context, { ...yesFlags, "dry-run": true }, "my-org/42");
121157

122158
expect(deleteSpy).not.toHaveBeenCalled();
159+
expect(ensureScopesSpy).not.toHaveBeenCalled();
123160
expect(output(stdoutWrite)).toContain("Would remove");
124161
});
125162

@@ -147,7 +184,7 @@ describe("integration delete", () => {
147184
expect(deleteSpy).not.toHaveBeenCalled();
148185
});
149186

150-
test("explains the missing org:integrations scope on 403", async () => {
187+
test("explains the role requirement on a 403 that survives elevation", async () => {
151188
deleteSpy.mockRejectedValue(new ApiError("Forbidden", 403));
152189
const { context } = createMockContext();
153190
const func = await deleteCommand.loader();
@@ -157,7 +194,21 @@ describe("integration delete", () => {
157194
.catch((e: unknown) => e);
158195
expect(error).toBeInstanceOf(ApiError);
159196
expect((error as ApiError).detail).toContain("org:integrations");
160-
expect((error as ApiError).detail).toContain("sentry auth refresh --scope");
197+
expect((error as ApiError).detail).toContain("owners and managers");
198+
});
199+
200+
test("explains the missing scope on a 403 when elevation was unavailable", async () => {
201+
ensureScopesSpy.mockResolvedValue("unavailable");
202+
deleteSpy.mockRejectedValue(new ApiError("Forbidden", 403));
203+
const { context } = createMockContext();
204+
const func = await deleteCommand.loader();
205+
206+
const error = await func
207+
.call(context, yesFlags, "my-org/42")
208+
.catch((e: unknown) => e);
209+
expect(error).toBeInstanceOf(ApiError);
210+
expect((error as ApiError).detail).toContain("predates");
211+
expect((error as ApiError).detail).toContain("interactive terminal");
161212
});
162213

163214
test("throws ContextError when no org can be resolved", async () => {

‎packages/cli/test/lib/oauth.test.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ describe("resolveOAuthScopeString", () => {
119119
expect(OAUTH_SCOPES).toContain("team:admin");
120120
});
121121

122+
test("default scopes include org:integrations but not org:admin", () => {
123+
expect(OAUTH_SCOPES).toContain("org:integrations");
124+
expect(OAUTH_SCOPES).not.toContain("org:admin");
125+
});
126+
122127
test("default (no selection) returns the full OAUTH_SCOPES set", () => {
123128
expect(resolveOAuthScopeString()).toBe(OAUTH_SCOPES.join(" "));
124129
expect(resolveOAuthScopeString({})).toBe(OAUTH_SCOPES.join(" "));

0 commit comments

Comments
 (0)