History

Normalize Slack OAuth scopes (#1652)

Rhys Sullivan committed 2 months ago

Commit 12a1bb21 parent 22a03832 files · +93 −4Browse files

Unified · Split

2 files
  1. packages/core/sdk/src/oauth-helpers.test.ts +66 −0
  2. packages/core/sdk/src/oauth-helpers.ts +27 −4
packages/core/sdk/src/oauth-helpers.test.ts+66−0
@@ -99,6 +99,14 @@ const tokenResponse = · 98 unmodified lines
9999 () =>
100100 json(200, body);
101101
102+const tokenResponseFetch =
103+ (body: unknown): typeof globalThis.fetch =>
104+ async () =>
105+ new Response(JSON.stringify(body), {
106+ status: 200,
107+ headers: { "content-type": "application/json" },
108+ });
109+
102110 // ---------------------------------------------------------------------------
103111 // PKCE
104112 // ---------------------------------------------------------------------------
@@ -575,6 +583,46 @@ describe("exchangeAuthorizationCode", () => { · 470 unmodified lines
575583 ),
576584 );
577585
586+ it.effect("normalizes Slack's comma-delimited top-level scopes", () =>
587+ Effect.gen(function* () {
588+ const result = yield* exchangeAuthorizationCode({
589+ tokenUrl: "https://slack.com/api/oauth.v2.user.access",
590+ clientId: "cid",
591+ clientSecret: "csecret",
592+ redirectUrl: "https://app.example.com/cb",
593+ codeVerifier: "verifier",
594+ code: "abc",
595+ fetch: tokenResponseFetch({
596+ access_token: "xoxp-user-token",
597+ token_type: "Bearer",
598+ scope: "channels:read,chat:write,reactions:read",
599+ }),
600+ });
601+
602+ expect(result.scope).toBe("channels:read chat:write reactions:read");
603+ }),
604+ );
605+
606+ it.effect("preserves commas in scope tokens from non-Slack providers", () =>
607+ Effect.gen(function* () {
608+ const result = yield* exchangeAuthorizationCode({
609+ tokenUrl: "https://oauth.example.com/token",
610+ clientId: "cid",
611+ clientSecret: "csecret",
612+ redirectUrl: "https://app.example.com/cb",
613+ codeVerifier: "verifier",
614+ code: "abc",
615+ fetch: tokenResponseFetch({
616+ access_token: "provider-token",
617+ token_type: "Bearer",
618+ scope: "scope,with-comma other.scope",
619+ }),
620+ });
621+
622+ expect(result.scope).toBe("scope,with-comma other.scope");
623+ }),
624+ );
625+
578626 it.effect("keeps a standard top-level scope ahead of nested provider metadata", () =>
579627 withTokenEndpoint(
580628 tokenResponse({
@@ -856,6 +904,24 @@ describe("exchangeClientCredentials", () => { · 275 unmodified lines
856904 });
857905
858906 describe("refreshAccessToken", () => {
907+ it.effect("normalizes Slack's comma-delimited scopes on refresh", () =>
908+ Effect.gen(function* () {
909+ const result = yield* refreshAccessToken({
910+ tokenUrl: "https://slack.com/api/oauth.v2.user.access",
911+ clientId: "cid",
912+ clientSecret: "csecret",
913+ refreshToken: "refresh-token",
914+ fetch: tokenResponseFetch({
915+ access_token: "xoxp-refreshed-token",
916+ token_type: "Bearer",
917+ scope: "channels:read,chat:write,reactions:read",
918+ }),
919+ });
920+
921+ expect(result.scope).toBe("channels:read chat:write reactions:read");
922+ }),
923+ );
924+
859925 it.effect("posts grant_type=refresh_token with the refresh token", () =>
860926 withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) =>
861927 Effect.gen(function* () {
packages/core/sdk/src/oauth-helpers.ts+27−4
@@ -552,12 +552,34 @@ const pickClientAuth = ( · 551 unmodified lines
552552 : oauth.ClientSecretPost(clientSecret);
553553 };
554554
555−const tokenResponseFrom = (r: oauth.TokenEndpointResponse): OAuth2TokenResponse => ({
555+const normalizedTokenScope = (
556+ as: oauth.AuthorizationServer,
557+ scope: string | undefined,
558+): string | undefined => {
559+ if (scope === undefined || scope.trim().length === 0) return undefined;
560+ const tokenEndpoint = typeof as.token_endpoint === "string" ? URL.parse(as.token_endpoint) : null;
561+ const isSlackTokenEndpoint =
562+ tokenEndpoint?.hostname.toLowerCase() === "slack.com" &&
563+ (tokenEndpoint.pathname === "/api/oauth.v2.access" ||
564+ tokenEndpoint.pathname === "/api/oauth.v2.user.access");
565+ if (!isSlackTokenEndpoint) return scope;
566+
567+ const normalized = scope
568+ .split(/[\s,]+/)
569+ .filter(Boolean)
570+ .join(" ");
571+ return normalized.length > 0 ? normalized : undefined;
572+};
573+
574+const tokenResponseFrom = (
575+ as: oauth.AuthorizationServer,
576+ r: oauth.TokenEndpointResponse,
577+): OAuth2TokenResponse => ({
556578 access_token: r.access_token,
557579 token_type: r.token_type,
558580 refresh_token: r.refresh_token,
559581 expires_in: typeof r.expires_in === "number" ? r.expires_in : undefined,
560− scope: typeof r.scope === "string" && r.scope.trim().length > 0 ? r.scope : undefined,
582+ scope: normalizedTokenScope(as, typeof r.scope === "string" ? r.scope : undefined),
561583 });
562584
563585 const JwtClaims = Schema.Record(Schema.String, Schema.Unknown);
@@ -694,6 +716,7 @@ const processTokenEndpointResponse = async ( · 130 unmodified lines
694716 const stripped = await stripIdToken(response);
695717 const providerUserGrant = await nestedAuthedUserGrant(stripped.response);
696718 const parsed = tokenResponseFrom(
719+ as,
697720 await oauth.processGenericTokenEndpointResponse(as, client, stripped.response),
698721 );
699722 const token =
@@ -839,7 +862,7 @@ export const exchangeClientCredentials = ( · 139 unmodified lines
839862 ),
840863 );
841864 const result = await oauth.processClientCredentialsResponse(as, client, response);
842− return tokenResponseFrom(result);
865+ return tokenResponseFrom(as, result);
843866 },
844867 catch: (cause) => cause,
845868 }).pipe(
@@ -918,7 +941,7 @@ export const refreshAccessToken = ( · 72 unmodified lines
918941 client,
919942 (await stripIdToken(response)).response,
920943 );
921− return tokenResponseFrom(result);
944+ return tokenResponseFrom(as, result);
922945 },
923946 catch: (cause) => cause,
924947 }).pipe(