History

Canonicalize Microsoft scopes and decode JWT claims as UTF-8

Rhys Sullivan committed 4 months ago

Commit 67ed78a1 parent d5eca194 files · +82 −10Browse files

Unified · Split

4 files
  1. packages/core/sdk/src/executor.ts +9 −5
  2. packages/core/sdk/src/oauth-flow.test.ts +43 −0
  3. packages/core/sdk/src/oauth-helpers.ts +9 −2
  4. packages/core/sdk/src/oauth-service.ts +21 −3
packages/core/sdk/src/executor.ts+9−5
@@ -2709,9 +2709,9 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 2708 unmodified lines
27092709 );
27102710
27112711 const healthFromCredentialResolutionFailure = (
2712− failure: StorageFailure | CredentialResolutionError,
2712+ failure: CredentialResolutionError,
27132713 ): HealthCheckResult =>
2714− Predicate.isTagged("CredentialResolutionError")(failure) && failure.reauthRequired === true
2714+ failure.reauthRequired === true
27152715 ? {
27162716 status: "expired",
27172717 checkedAt: Date.now(),
@@ -2721,20 +2721,24 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 3 unmodified lines
27212721 : {
27222722 status: "degraded",
27232723 checkedAt: Date.now(),
2724− // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: failures here carry typed safe messages
2724+ // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: CredentialResolutionError carries a typed `message` field
27252725 detail: failure.message,
27262726 };
27272727
2728+ // Genuine storage failures propagate: an infra blip must fail the request,
2729+ // not persist as a "degraded" verdict on the connection.
27282730 const oauthCredentialHealthWithoutProbe = (
27292731 row: ConnectionRow,
2730− ): Effect.Effect<HealthCheckResult, never> =>
2732+ ): Effect.Effect<HealthCheckResult, StorageFailure> =>
27312733 resolveConnectionValues(row).pipe(
27322734 Effect.as({
27332735 status: "healthy" as const,
27342736 checkedAt: Date.now(),
27352737 detail: "Credential resolved (no probe configured).",
27362738 }),
2737− Effect.catch((failure) => Effect.succeed(healthFromCredentialResolutionFailure(failure))),
2739+ Effect.catchTag("CredentialResolutionError", (failure) =>
2740+ Effect.succeed(healthFromCredentialResolutionFailure(failure)),
2741+ ),
27382742 );
27392743
27402744 // Resolve an in-flight credential's value map (key-first validation) without
packages/core/sdk/src/oauth-flow.test.ts+43−0
@@ -12,6 +12,7 @@ import { · 11 unmodified lines
1212 } from "./ids";
1313 import { decodeOAuthCallbackState } from "./oauth";
1414 import { OAuthStartError } from "./oauth-client";
15+import { missingGrantedOAuthScopes } from "./oauth-service";
1516 import { definePlugin } from "./plugin";
1617 import { makeTestWorkspaceHarness, memoryCredentialsPlugin } from "./test-config";
1718 import { serveOAuthTestServer } from "./testing/oauth-test-server";
@@ -1177,3 +1178,45 @@ describe("oauth.complete regional token-endpoint rebind (Datadog multi-site)", ( · 1159 unmodified lines
11771178 ),
11781179 );
11791180 });
1181+
1182+describe("missingGrantedOAuthScopes canonicalization", () => {
1183+ it("treats Microsoft fully-qualified granted scopes as covering short-form requests", () => {
1184+ const missing = missingGrantedOAuthScopes(
1185+ [
1186+ "offline_access",
1187+ "User.Read",
1188+ "https://graph.microsoft.com/.default",
1189+ "Mail.ReadWrite",
1190+ "Mail.Send",
1191+ "MailboxSettings.ReadWrite",
1192+ ],
1193+ [
1194+ "https://graph.microsoft.com/Mail.ReadWrite",
1195+ "https://graph.microsoft.com/Mail.Send",
1196+ "https://graph.microsoft.com/MailboxSettings.ReadWrite",
1197+ "https://graph.microsoft.com/User.Read",
1198+ ].join(" "),
1199+ );
1200+ expect(missing).toEqual([]);
1201+ });
1202+
1203+ it("still reports genuinely ungranted scopes under the qualified Microsoft shape", () => {
1204+ const missing = missingGrantedOAuthScopes(
1205+ ["User.Read", "Mail.ReadWrite", "Mail.Send"],
1206+ "https://graph.microsoft.com/User.Read https://graph.microsoft.com/Mail.ReadWrite",
1207+ );
1208+ expect(missing).toEqual(["Mail.Send"]);
1209+ });
1210+
1211+ it("maps Google userinfo aliases and ignores identity meta-scopes", () => {
1212+ const missing = missingGrantedOAuthScopes(
1213+ ["openid", "email", "profile", "https://www.googleapis.com/auth/calendar"],
1214+ [
1215+ "https://www.googleapis.com/auth/userinfo.email",
1216+ "https://www.googleapis.com/auth/userinfo.profile",
1217+ "https://www.googleapis.com/auth/calendar",
1218+ ].join(" "),
1219+ );
1220+ expect(missing).toEqual([]);
1221+ });
1222+});
packages/core/sdk/src/oauth-helpers.ts+9−2
@@ -470,7 +470,9 @@ const stringClaim = ( · 469 unmodified lines
470470 key: string,
471471 ): string | undefined => {
472472 const value = claims[key];
473− return typeof value === "string" && value.trim().length > 0 ? value : undefined;
473+ if (typeof value !== "string") return undefined;
474+ const trimmed = value.trim();
475+ return trimmed.length > 0 ? trimmed : undefined;
474476 };
475477
476478 const decodeJwtPayload = (token: string): Readonly<Record<string, unknown>> | null => {
@@ -479,7 +481,12 @@ const decodeJwtPayload = (token: string): Readonly<Record<string, unknown>> | nu · 2 unmodified lines
479481 if (!/^[A-Za-z0-9_-]+$/.test(payload) || payload.length % 4 === 1) return null;
480482 const base64 = payload.replaceAll("-", "+").replaceAll("_", "/");
481483 const padded = base64.padEnd(base64.length + ((4 - (base64.length % 4)) % 4), "=");
482− const decoded = decodeJwtClaims(globalThis.atob(padded));
484+ // atob yields latin1 code units; JWT payloads are UTF-8 bytes, so re-decode
485+ // them properly or non-ASCII claim values (accented emails, names) garble.
486+ const utf8 = new TextDecoder().decode(
487+ Uint8Array.from(globalThis.atob(padded), (char) => char.charCodeAt(0)),
488+ );
489+ const decoded = decodeJwtClaims(utf8);
483490 return Option.isSome(decoded) ? decoded.value : null;
484491 };
485492
packages/core/sdk/src/oauth-service.ts+21−3
@@ -211,12 +211,28 @@ const OAUTH_SCOPE_ALIASES: Readonly<Record<string, string>> = { · 210 unmodified lines
211211
212212 const informationalOAuthScopes = new Set(["openid", "email", "profile", "offline_access"]);
213213
214−const canonicalOAuthScope = (scope: string): string => OAUTH_SCOPE_ALIASES[scope] ?? scope;
214+/** Canonicalize a scope for granted-vs-requested comparison. Microsoft's token
215+ * endpoint returns Graph scopes fully qualified
216+ * (`https://graph.microsoft.com/Mail.ReadWrite`) even when the request used
217+ * the short form, so resource-URI prefixes are stripped down to the scope's
218+ * final path segment before comparing. */
219+const canonicalOAuthScope = (scope: string): string => {
220+ const aliased = OAUTH_SCOPE_ALIASES[scope];
221+ if (aliased) return aliased;
222+ if (/^https?:\/\/graph\.microsoft\.(com|us|de)\//i.test(scope)) {
223+ return scope.slice(scope.lastIndexOf("/") + 1);
224+ }
225+ return scope;
226+};
227+
228+/** `.default` is a request-time meta-scope (Microsoft expands it server-side
229+ * and never echoes it in the granted scope), so it can never be "missing". */
230+const isMetaOAuthScope = (scope: string): boolean => scope.toLowerCase().endsWith("/.default");
215231
216232 const normalizedOAuthScopeSet = (scopes: readonly string[]): ReadonlySet<string> =>
217233 new Set(scopes.map((scope) => canonicalOAuthScope(scope.trim())).filter(Boolean));
218234
219−const missingGrantedOAuthScopes = (
235+export const missingGrantedOAuthScopes = (
220236 requestedScopes: readonly string[],
221237 recordedScope: string | null,
222238 ): readonly string[] => {
@@ -224,7 +240,9 @@ const missingGrantedOAuthScopes = ( · 1 unmodified line
224240 const seen = new Set<string>();
225241 const out: string[] = [];
226242 for (const raw of requestedScopes) {
227− const scope = canonicalOAuthScope(raw.trim());
243+ const trimmed = raw.trim();
244+ if (isMetaOAuthScope(trimmed)) continue;
245+ const scope = canonicalOAuthScope(trimmed);
228246 if (scope.length === 0 || informationalOAuthScopes.has(scope) || seen.has(scope)) continue;
229247 seen.add(scope);
230248 if (!granted.has(scope)) out.push(scope);