Skip to content

Fix: group-editing authorization dead-ends on Approve #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tytremblay merged 1 commit into from
Aug 21, 2026
Merged

Fix: group-editing authorization dead-ends on Approve #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 38 additions & 3 deletions src/slack/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,44 @@ export function createApp(): App {
);
},
failure: (error, _options, _req, res) => {
log.error("oauth failure", { error: String(error) });
res.writeHead(500, { "content-type": "text/plain; charset=utf-8" });
res.end("Authorization failed. Tell a coach.");
// An authorization link is valid for ten minutes. Expiring is the
// most common way this fails and the only one the person in front of
// it can fix, so it gets its own answer rather than being lumped in
// with "tell a coach" — which is what to do about the rest.
const code = (error as { code?: string } | undefined)?.code;
const expired =
code === "slack_oauth_invalid_state" ||
code === "slack_oauth_missing_state";

log.error("oauth failure", {
code: code ?? "none",
error: String(error),
});

res.writeHead(expired ? 400 : 500, {
"content-type": "text/html; charset=utf-8",
});
res.end(
`<!doctype html><meta charset="utf-8">
<meta name="viewport" content="width=device-width,initial-scale=1">
<title>${APP_NAME}</title>
<div style="font:16px/1.6 system-ui,sans-serif;color:#1f2023;max-width:34rem;margin:4rem auto;padding:0 1.25rem">
${ICON_SVG}
<h1 style="font-size:1.6rem;margin:1.25rem 0 .25rem">${
expired ? "That link expired." : "Authorization failed."
}</h1>
<p style="margin:0 0 1.5rem;color:${BRAND.red};font-weight:600;letter-spacing:.04em;text-transform:uppercase;font-size:.8rem">${APP_NAME}</p>
<p>${
expired
? "Authorization links are good for ten minutes. Run the " +
"command again and follow the new link — nothing was " +
"changed."
: "Nothing was changed. Tell a coach, and give them this: " +
`<code>${code ?? "unknown"}</code>. The server log has ` +
"the detail."
}</p>
</div>`
);
},
},
},
Expand Down
25 changes: 24 additions & 1 deletion src/slack/installStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,29 @@ function teamKey(
/** Marks an authorization that came from the group-editing flow. */
export const GROUP_ADMIN_METADATA = "group-admin";

/** Present on an enrolment token, absent on a group-editing one. */
const DM_SCOPE = "im:history";
const GROUP_WRITE_SCOPE = "usergroups:write";

/**
* Which of the two per-person grants came back.
*
* Decided from the token's *scopes* rather than from the metadata marker
* alone. The marker is a hint that has to survive a round trip through Slack,
* and when it did not, this fell through to the enrolment branch — where the
* scope guard correctly refused to overwrite a mentor's DM token, and the whole
* authorization died with "Authorization failed. Tell a coach."
*
* What the token can actually do is not a hint. A token carrying
* `usergroups:write` and no DM history scope cannot be an enrolment, whatever
* the state parameter says it was.
*/
function isGroupAdminGrant(installation: Installation): boolean {
if (installation.metadata === GROUP_ADMIN_METADATA) return true;
const scopes = installation.user?.scopes ?? [];
return scopes.includes(GROUP_WRITE_SCOPE) && !scopes.includes(DM_SCOPE);
}

/**
* Three kinds of row live here:
*
Expand Down Expand Up @@ -73,7 +96,7 @@ export const installationStore: InstallationStore = {
// The group-editing grant is a different consent for a different purpose
// and gets its own row. Critically it must not fall through to the
// enrolment branch below, which would overwrite a mentor's DM token.
if (installation.metadata === GROUP_ADMIN_METADATA) {
if (isGroupAdminGrant(installation as Installation)) {
saveInstallation({
teamId,
enterpriseId,
Expand Down
Loading