Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
58 changes: 13 additions & 45 deletions packages/oauth-provider/src/scopes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,8 @@

import type { Nsid as AtcuteNsid } from "@atcute/lexicons/syntax";
import {
AccountPermission,
BlobPermission,
IdentityPermission,
IncludeScope,
RepoPermission,
RpcPermission,
isAtprotoOauthScope,
ScopeMissingError,
ScopePermissionsTransition,
ScopesSet,
Expand Down Expand Up @@ -64,17 +60,6 @@ export class ScopeParseError extends Error {
}
}

const STRUCTURAL_PARSERS: Record<
(typeof GRANULAR_RESOURCES)[number],
(s: string) => unknown
> = {
repo: (s) => RepoPermission.fromString(s),
rpc: (s) => RpcPermission.fromString(s),
blob: (s) => BlobPermission.fromString(s),
account: (s) => AccountPermission.fromString(s),
identity: (s) => IdentityPermission.fromString(s),
};

export interface ParseScopeOptions {
/**
* When true, `include:` scopes are accepted (and structurally validated)
Expand All @@ -97,7 +82,13 @@ export function parseScope(
input: string | undefined | null,
{ allowIncludes = false }: ParseScopeOptions = {},
): ScopesSet {
const set = ScopesSet.fromString(input ?? "");
const filtered =
(input ?? "")
.split(" ")
.filter(Boolean)
.filter(isAtprotoOauthScope)
.join(" ") || undefined;
const set = ScopesSet.fromString(filtered);

if (!set.has(ATPROTO_SCOPE)) {
throw new ScopeParseError(
Expand All @@ -109,34 +100,11 @@ export function parseScope(
for (const scope of set) {
if (scope === ATPROTO_SCOPE) continue;
if ((TRANSITION_SCOPES as readonly string[]).includes(scope)) continue;

if (scope.startsWith("include:")) {
if (!IncludeScope.fromString(scope)) {
throw new ScopeParseError(`Malformed include scope: ${scope}`, scope);
}
if (!allowIncludes) {
throw new ScopeParseError(
`Permission sets cannot be requested in this context: ${scope}`,
scope,
);
}
continue;
}

const colon = scope.indexOf(":");
const question = scope.indexOf("?");
const end =
colon === -1 ? question : question === -1 ? colon : Math.min(colon, question);
const resource = end === -1 ? scope : scope.slice(0, end);
const parser =
STRUCTURAL_PARSERS[
resource as (typeof GRANULAR_RESOURCES)[number]
];
if (!parser) {
throw new ScopeParseError(`Unknown scope resource: ${scope}`, scope);
}
if (!parser(scope)) {
throw new ScopeParseError(`Malformed scope: ${scope}`, scope);
if (scope.startsWith("include:") && !allowIncludes) {
throw new ScopeParseError(
`Permission sets cannot be requested in this context: ${scope}`,
scope,
);
}
}

Expand Down
6 changes: 2 additions & 4 deletions packages/oauth-provider/test/oauth-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ describe("OAuth Flow", () => {
);
});

it("PAR rejects malformed granular scope", async () => {
it("PAR silently drops unsupported scopes", async () => {
const verifier = generateCodeVerifier();
const challenge = await generateCodeChallenge(verifier);
const parBody = new URLSearchParams({
Expand All @@ -971,9 +971,7 @@ describe("OAuth Flow", () => {
body: parBody.toString(),
}),
);
expect(response.status).toBe(400);
const json = (await response.json()) as { error: string };
expect(json.error).toBe("invalid_scope");
expect(response.status).toBe(201);
});
});
});
26 changes: 18 additions & 8 deletions packages/oauth-provider/test/scopes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,27 @@ describe("parseScope", () => {
expect(set.size).toBe(4);
});

it("rejects malformed granular scopes", () => {
expect(() => parseScope("atproto repo:not a real nsid")).toThrow(
ScopeParseError,
it("silently drops malformed scope tokens", () => {
const set = parseScope("atproto repo:not a real nsid");
expect(set.has("atproto")).toBe(true);
expect(set.size).toBe(1);
});

it("silently drops scopes with unsupported actions", () => {
const set = parseScope(
"atproto repo:com.example.post?action=read repo:com.example.post?action=create",
);
expect(() =>
parseScope("atproto rpc:app.bsky.feed.getTimeline"), // missing aud
).toThrow(ScopeParseError);
expect(set.has("atproto")).toBe(true);
expect(set.has("repo:com.example.post?action=read")).toBe(false);
expect(set.has("repo:com.example.post?action=create")).toBe(true);
expect(set.has("repo:com.example.post")).toBe(false);
expect(set.size).toBe(2);
});

it("rejects unknown resources", () => {
expect(() => parseScope("atproto madeup:thing")).toThrow(ScopeParseError);
it("silently drops unknown scope resources", () => {
const set = parseScope("atproto madeup:thing");
expect(set.has("atproto")).toBe(true);
expect(set.size).toBe(1);
});

it("accepts repo scopes in query-only form (no positional)", () => {
Expand Down