diff --git a/src/slack/app.ts b/src/slack/app.ts index 3e732e2..0c13c7b 100644 --- a/src/slack/app.ts +++ b/src/slack/app.ts @@ -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( + ` + + ${APP_NAME} +
+ ${ICON_SVG} +

${ + expired ? "That link expired." : "Authorization failed." + }

+

${APP_NAME}

+

${ + 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 ?? "unknown"}. The server log has ` + + "the detail." + }

+
` + ); }, }, }, diff --git a/src/slack/installStore.ts b/src/slack/installStore.ts index a5ebc5b..027a769 100644 --- a/src/slack/installStore.ts +++ b/src/slack/installStore.ts @@ -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: * @@ -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,