diff --git a/package.json b/package.json index 908573ec..137cc25f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "impro", - "version": "0.18.10", + "version": "0.18.11", "type": "module", "scripts": { "start": "rm -rf \"${BUILD_DIR:-build}\" && NODE_ENV=development eleventy --serve", diff --git a/src/_includes/head.html b/src/_includes/head.html index e4ea1c0c..e8a73336 100644 --- a/src/_includes/head.html +++ b/src/_includes/head.html @@ -33,6 +33,7 @@ environment: "{{ environment }}", version: "{{ version }}", oauthScopes: "{{ oauthScopes }}", + oauthOptionalScopes: "{{ oauthOptionalScopes }}", playwright: "{{ playwright }}" === "true", oatproxyClientId: "{{ oatproxyClientId }}", }; diff --git a/src/index.html b/src/index.html index fb4eb1cc..72f457b3 100644 --- a/src/index.html +++ b/src/index.html @@ -184,6 +184,20 @@ import { PluginService } from "/js/plugins/pluginService.js"; import { MainLayout } from "/js/mainLayout.js"; + const DRAFT_SCOPES = [ + "rpc:app.bsky.draft.getDrafts", + "rpc:app.bsky.draft.createDraft", + "rpc:app.bsky.draft.updateDraft", + "rpc:app.bsky.draft.deleteDraft", + ]; + + async function checkDraftsEnabled() { + const results = await Promise.all( + DRAFT_SCOPES.map((scope) => auth.hasScope(scope)), + ); + return results.every(Boolean); + } + async function main() { // Body class for styling if (isNative()) { @@ -227,7 +241,14 @@ ? new ChatNotificationService(api) : null; const postComposerService = session - ? new PostComposerService(dataLayer, identityResolver, pluginService) + ? new PostComposerService( + dataLayer, + identityResolver, + pluginService, + { + draftsEnabled: await checkDraftsEnabled(), + }, + ) : null; const accountSwitcherService = session ? new AccountSwitcherService(dataLayer) diff --git a/src/js/auth.js b/src/js/auth.js index 6fc7a4e4..1be3ef93 100644 --- a/src/js/auth.js +++ b/src/js/auth.js @@ -301,12 +301,17 @@ export class OAuthProvider { const FORCE_LOGOUT_QUERY_PARAM = "force-logout"; -export function getMissingScopes(grantedScope, requiredScope) { +export function getMissingScopes( + grantedScope, + requiredScope, + optionalScope = "", +) { const granted = new Set(grantedScope.split(/\s+/).filter(Boolean)); + const optional = new Set(optionalScope.split(/\s+/).filter(Boolean)); return requiredScope .split(/\s+/) .filter(Boolean) - .filter((scope) => !granted.has(scope)); + .filter((scope) => !granted.has(scope) && !optional.has(scope)); } export class Auth { @@ -340,10 +345,12 @@ export class Auth { async listAccounts() { const accounts = (await this.provider.listAccounts?.()) ?? []; const requiredScope = window.env?.oauthScopes ?? ""; + const optionalScope = window.env?.oauthOptionalScopes ?? ""; // Mark accounts with out-of-date scopes as "needsReauth" return accounts.map((account) => { const scopesOutOfDate = - getMissingScopes(account.scope ?? "", requiredScope).length > 0; + getMissingScopes(account.scope ?? "", requiredScope, optionalScope) + .length > 0; return { ...account, needsReauth: account.needsReauth || scopesOutOfDate, @@ -410,10 +417,24 @@ export class Auth { return null; } + // Check for scope, ignoring query params + async hasScope(scope) { + const session = await this.provider.getSession(); + if (!session) return false; + if (!session.scope) return true; + return session.scope + .split(/\s+/) + .some((granted) => granted === scope || granted.startsWith(scope + "?")); + } + async ensureCurrentScopes() { const session = await this.provider.getSession(); if (!session?.scope) return; - const missing = getMissingScopes(session.scope, window.env.oauthScopes); + const missing = getMissingScopes( + session.scope, + window.env.oauthScopes, + window.env.oauthOptionalScopes ?? "", + ); if (missing.length === 0) return; console.warn("OAuth scopes are out of date, forcing re-auth:", missing); try { diff --git a/src/js/components/post-composer.js b/src/js/components/post-composer.js index 2ebd91bc..18b6318e 100644 --- a/src/js/components/post-composer.js +++ b/src/js/components/post-composer.js @@ -231,6 +231,10 @@ class PostComposer extends Component { this._pendingQuotedRecord ?? null, ); this._pendingQuotedRecord = null; + this.state.$draftsEnabled = new Signal.State( + this._pendingDraftsEnabled ?? false, + ); + this._pendingDraftsEnabled = null; this._disposers = [ effect(() => { this.render(); @@ -258,10 +262,24 @@ class PostComposer extends Component { this.state.$quotedRecord.set(value); } + get draftsEnabled() { + if (!this.state) return this._pendingDraftsEnabled ?? false; + return untrack(() => this.state.$draftsEnabled.get()); + } + + set draftsEnabled(value) { + if (!this.state) { + this._pendingDraftsEnabled = value; + return; + } + this.state.$draftsEnabled.set(value); + } + render() { const promptText = this.replyTo ? "Write your reply" : "What's up?"; const isSending = this.state.$isSending.get(); const isSavingDraft = this.state.$isSavingDraft.get(); + const draftsEnabled = this.state.$draftsEnabled.get(); const externalLinkEmbedData = this.state.$externalLinkEmbedData.get(); const selectedImages = this.state.$selectedImages.get(); const selectedVideo = this.state.$selectedVideo.get(); @@ -315,7 +333,7 @@ class PostComposer extends Component { > Cancel - ${!this.replyTo + ${!this.replyTo && draftsEnabled ? html` - ${groupDetails + ${groupDetails && canViewGroupDetails ? html` { diff --git a/src/oauthScopes.js b/src/oauthScopes.js index a9c088ee..2c6d9102 100644 --- a/src/oauthScopes.js +++ b/src/oauthScopes.js @@ -96,10 +96,6 @@ const BSKY_OAUTH_RPC_SCOPES = [ "rpc:app.bsky.video.getJobStatus", "rpc:app.bsky.video.getUploadLimits", "rpc:app.bsky.video.uploadVideo", - "rpc:app.bsky.draft.getDrafts", - "rpc:app.bsky.draft.createDraft", - "rpc:app.bsky.draft.updateDraft", - "rpc:app.bsky.draft.deleteDraft", ]; const BSKY_OAUTH_REPO_SCOPES = [ @@ -142,7 +138,6 @@ const CHAT_OAUTH_RPC_SCOPES = [ "rpc:chat.bsky.convo.updateRead", "rpc:chat.bsky.group.getJoinLinkPreviews", "rpc:chat.bsky.group.requestJoin", - "rpc:chat.bsky.convo.getConvoMembers", ]; const CHAT_OAUTH_REPO_SCOPES = ["repo:chat.bsky.actor.declaration"]; @@ -152,21 +147,45 @@ const ATPROTO_OAUTH_RPC_SCOPES = [ "rpc:com.atproto.repo.uploadBlob", ]; -function buildOauthScopesString() { - let scopesString = "atproto blob:*/*"; - for (let scope of [ - ...BSKY_OAUTH_RPC_SCOPES, - ...CHAT_OAUTH_RPC_SCOPES, - ...ATPROTO_OAUTH_RPC_SCOPES, - ]) { - scopesString += " " + scope + "?aud=*"; +const OPTIONAL_OAUTH_RPC_SCOPES = [ + "rpc:chat.bsky.convo.getConvoMembers", + "rpc:app.bsky.draft.getDrafts", + "rpc:app.bsky.draft.createDraft", + "rpc:app.bsky.draft.updateDraft", + "rpc:app.bsky.draft.deleteDraft", +]; + +function expandScope(scope) { + if (scope.startsWith("rpc:")) { + return [scope + "?aud=*"]; } - for (let scope of [...BSKY_OAUTH_REPO_SCOPES, ...CHAT_OAUTH_REPO_SCOPES]) { - for (let action of ["create", "update", "delete"]) { - scopesString += " " + scope + "?action=" + action; - } + if (scope.startsWith("repo:")) { + return ["create", "update", "delete"].map( + (action) => scope + "?action=" + action, + ); } - return scopesString; + return [scope]; +} + +function expandScopes(scopes) { + return scopes.flatMap(expandScope); +} + +function buildOauthScopesString() { + const scopes = [ + "atproto", + "blob:*/*", + ...expandScopes(BSKY_OAUTH_RPC_SCOPES), + ...expandScopes(CHAT_OAUTH_RPC_SCOPES), + ...expandScopes(ATPROTO_OAUTH_RPC_SCOPES), + ...expandScopes(BSKY_OAUTH_REPO_SCOPES), + ...expandScopes(CHAT_OAUTH_REPO_SCOPES), + ...expandScopes(OPTIONAL_OAUTH_RPC_SCOPES), + ]; + return scopes.join(" "); } export const OAUTH_SCOPES = buildOauthScopesString(); +export const OPTIONAL_OAUTH_SCOPES = expandScopes( + OPTIONAL_OAUTH_RPC_SCOPES, +).join(" "); diff --git a/src/src.11tydata.js b/src/src.11tydata.js index 9338d59f..e34a3ad2 100644 --- a/src/src.11tydata.js +++ b/src/src.11tydata.js @@ -1,5 +1,5 @@ import { execSync } from "child_process"; -import { OAUTH_SCOPES } from "./oauthScopes.js"; +import { OAUTH_SCOPES, OPTIONAL_OAUTH_SCOPES } from "./oauthScopes.js"; import pkg from "../package.json" with { type: "json" }; export default { @@ -11,5 +11,6 @@ export default { environment: process.env.ENVIRONMENT ?? "development", playwright: process.env.PLAYWRIGHT ? "true" : "", oauthScopes: OAUTH_SCOPES, + oauthOptionalScopes: OPTIONAL_OAUTH_SCOPES, oatproxyClientId: process.env.OATPROXY_CLIENT_ID ?? "", }; diff --git a/tests/e2e/helpers.js b/tests/e2e/helpers.js index 63d8e723..469f1fbb 100644 --- a/tests/e2e/helpers.js +++ b/tests/e2e/helpers.js @@ -11,7 +11,7 @@ export async function longPress(page, locator) { await page.mouse.up(); } -export async function login(page) { +export async function login(page, { scope = OAUTH_SCOPES } = {}) { const oauthSession = { did: userProfile.did, serviceEndpoint: "https://fake.bsky.social", @@ -22,7 +22,7 @@ export async function login(page) { authServerMetadata: { token_endpoint: `https://fake.bsky.social/oauth/token`, }, - scope: OAUTH_SCOPES, + scope, }; await page.addInitScript((session) => { localStorage.setItem("oauth_session", JSON.stringify(session)); diff --git a/tests/e2e/specs/flows/drafts.test.js b/tests/e2e/specs/flows/drafts.test.js index c22b6d64..876369a3 100644 --- a/tests/e2e/specs/flows/drafts.test.js +++ b/tests/e2e/specs/flows/drafts.test.js @@ -1,6 +1,7 @@ import { test, expect } from "../../base.js"; import { login } from "../../helpers.js"; import { MockServer } from "../../mockServer.js"; +import { OAUTH_SCOPES } from "../../../../src/oauthScopes.js"; async function openComposer(page) { const homeView = page.locator("#home-view"); @@ -80,6 +81,34 @@ test.describe("Post drafts flow", () => { ).toBeVisible({ timeout: 10000 }); }); + test("hides drafts and confirms discard when the session lacks the draft scopes", async ({ + page, + }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + + const scopeWithoutDrafts = OAUTH_SCOPES.split(" ") + .filter((scope) => !scope.includes("app.bsky.draft.")) + .join(" "); + await login(page, { scope: scopeWithoutDrafts }); + await page.goto("/"); + + const composer = await openComposer(page); + await composer.locator(".rich-text-input").click(); + await composer.locator(".rich-text-input").type("Ephemeral thought"); + await expect( + composer.locator('[data-testid="composer-drafts-button"]'), + ).toHaveCount(0); + await composer.locator(".post-composer-cancel-button").click(); + + // A plain discard confirmation replaces the save-draft choice prompt + const confirmModal = page.locator('[data-testid="confirm-modal"]'); + await expect(confirmModal).toBeVisible({ timeout: 10000 }); + await expect(page.locator('[data-testid="choice-modal"]')).toHaveCount(0); + await confirmModal.locator('[data-testid="modal-confirm-button"]').click(); + await expect(composer).not.toBeVisible({ timeout: 10000 }); + }); + test("delete a draft from the list with confirmation", async ({ page }) => { const mockServer = new MockServer(); mockServer.drafts.push({ diff --git a/tests/e2e/specs/views/chatDetail.view.test.js b/tests/e2e/specs/views/chatDetail.view.test.js index a0fa4eb4..c44e857c 100644 --- a/tests/e2e/specs/views/chatDetail.view.test.js +++ b/tests/e2e/specs/views/chatDetail.view.test.js @@ -1,5 +1,6 @@ import { test, expect } from "../../base.js"; import { login } from "../../helpers.js"; +import { OAUTH_SCOPES } from "../../../../src/oauthScopes.js"; import { MockServer } from "../../mockServer.js"; import { createConvo, @@ -747,6 +748,44 @@ test.describe("Chat detail view", () => { ).toHaveCount(0); }); + test("should not offer group chat details when the session lacks the members scope", async ({ + page, + }) => { + const mockServer = new MockServer(); + const alice = createProfile({ + did: "did:plc:alice1", + handle: "alice.bsky.social", + displayName: "Alice", + }); + const convo = createGroupConvo({ + id: "convo-1", + name: "Cool Group", + otherMembers: [alice], + }); + mockServer.addConvos([convo]); + await mockServer.setup(page); + + const scopeWithoutMembers = OAUTH_SCOPES.split(" ") + .filter((scope) => !scope.includes("chat.bsky.convo.getConvoMembers")) + .join(" "); + await login(page, { scope: scopeWithoutMembers }); + await page.goto("/messages/convo-1"); + + const chatDetailView = page.locator("#chat-detail-view"); + await expect( + chatDetailView.locator('[data-testid="header-title"]'), + ).toContainText("Cool Group", { timeout: 10000 }); + await expect(chatDetailView.locator(".header-title-link")).toHaveCount(0); + + await chatDetailView.locator('[data-testid="chat-menu-button"]').click(); + await expect( + chatDetailView.locator('[data-testid="menu-action-chat-open-in-bsky"]'), + ).toBeVisible(); + await expect( + chatDetailView.locator('[data-testid="menu-action-group-chat-details"]'), + ).toHaveCount(0); + }); + test("should add emoji reaction to a message", async ({ page }) => { const mockServer = new MockServer(); const alice = createProfile({ diff --git a/tests/unit/specs/auth.test.js b/tests/unit/specs/auth.test.js index 8a32fd34..3c085a59 100644 --- a/tests/unit/specs/auth.test.js +++ b/tests/unit/specs/auth.test.js @@ -531,6 +531,30 @@ describe("Auth account management", () => { assert.deepEqual(byDid["did:plc:carol"].needsReauth, true); }); + it("listAccounts does not flip needsReauth when only optional scopes are missing", async () => { + globalThis.window = { + ...originalWindow, + env: { + oauthScopes: "atproto rpc:a rpc:future", + oauthOptionalScopes: "rpc:future", + }, + }; + const provider = makeMultiAccountProvider({ + accounts: [ + { + did: "did:plc:alice", + handle: "alice.test", + scope: "atproto rpc:a", + needsReauth: false, + }, + ], + currentDid: "did:plc:alice", + }); + const manager = new Auth(provider); + const accounts = await manager.listAccounts(); + assert.deepEqual(accounts[0].needsReauth, false); + }); + it("listAccounts leaves needsReauth alone for providers that don't expose scope", async () => { const provider = makeMultiAccountProvider({ accounts: [ @@ -725,6 +749,83 @@ describe("getMissingScopes", () => { const result = getMissingScopes("rpc:a?aud=did:web:foo", "rpc:a?aud=*"); assert.deepEqual(result, ["rpc:a?aud=*"]); }); + + it("does not report missing scopes that are optional", () => { + const result = getMissingScopes( + "atproto rpc:a", + "atproto rpc:a rpc:future", + "rpc:future", + ); + assert.deepEqual(result.length, 0); + }); + + it("still reports missing required scopes alongside missing optional ones", () => { + const result = getMissingScopes( + "atproto rpc:a", + "atproto rpc:a rpc:b rpc:future", + "rpc:future", + ); + assert.deepEqual(result, ["rpc:b"]); + }); + + it("optional scopes that are granted change nothing", () => { + const result = getMissingScopes( + "atproto rpc:a rpc:future", + "atproto rpc:a rpc:future", + "rpc:future", + ); + assert.deepEqual(result.length, 0); + }); +}); + +describe("Auth.hasScope", () => { + it("returns false when there is no session", async () => { + const manager = new Auth(makeMockProvider()); + assert.deepEqual(await manager.hasScope("rpc:a"), false); + }); + + it("returns true when the session has no scope string (BasicAuth)", async () => { + const provider = makeMockProvider(); + provider.getSession = mock.fn(() => Promise.resolve({ scope: undefined })); + const manager = new Auth(provider); + assert.deepEqual(await manager.hasScope("rpc:a"), true); + }); + + it("returns true when the scope is granted exactly", async () => { + const provider = makeMockProvider(); + provider.getSession = mock.fn(() => + Promise.resolve({ scope: "atproto rpc:a rpc:b" }), + ); + const manager = new Auth(provider); + assert.deepEqual(await manager.hasScope("rpc:a"), true); + }); + + it("returns true when a param variant of the scope is granted", async () => { + const provider = makeMockProvider(); + provider.getSession = mock.fn(() => + Promise.resolve({ scope: "atproto rpc:a?aud=*" }), + ); + const manager = new Auth(provider); + assert.deepEqual(await manager.hasScope("rpc:a"), true); + }); + + it("returns false when the scope is not granted", async () => { + const provider = makeMockProvider(); + provider.getSession = mock.fn(() => + Promise.resolve({ scope: "atproto rpc:a?aud=*" }), + ); + const manager = new Auth(provider); + assert.deepEqual(await manager.hasScope("rpc:b"), false); + }); + + it("does not treat a scope as a prefix of a longer scope name", async () => { + const provider = makeMockProvider(); + provider.getSession = mock.fn(() => + Promise.resolve({ scope: "rpc:abc?aud=*" }), + ); + const manager = new Auth(provider); + assert.deepEqual(await manager.hasScope("rpc:a"), false); + }); }); describe("Auth.ensureCurrentScopes", () => { @@ -769,6 +870,22 @@ describe("Auth.ensureCurrentScopes", () => { assert.deepEqual(provider.logout.mock.callCount(), 0); }); + it("does nothing when only optional scopes are missing", async () => { + const capturedHrefs = mockWindowLocation(); + globalThis.window.env = { + oauthScopes: "atproto rpc:a rpc:future", + oauthOptionalScopes: "rpc:future", + }; + const provider = makeMockProvider(); + provider.getSession = mock.fn(() => + Promise.resolve({ scope: "atproto rpc:a" }), + ); + const manager = new Auth(provider); + await manager.ensureCurrentScopes(); + assert.deepEqual(capturedHrefs.length, 0); + assert.deepEqual(provider.logout.mock.callCount(), 0); + }); + it("logs out and redirects to login when a required scope is missing", async () => { const capturedHrefs = mockWindowLocation(); globalThis.window.env = { oauthScopes: "atproto rpc:a rpc:b" }; diff --git a/tests/unit/specs/components/post-composer.test.js b/tests/unit/specs/components/post-composer.test.js index b99a0ec3..7a1d2cbc 100644 --- a/tests/unit/specs/components/post-composer.test.js +++ b/tests/unit/specs/components/post-composer.test.js @@ -23,8 +23,9 @@ describe("post-composer", () => { document.body.appendChild(container); } - function createPostComposer() { + function createPostComposer({ draftsEnabled = true } = {}) { const element = document.createElement("post-composer"); + element.draftsEnabled = draftsEnabled; element.currentUser = { did: "did:plc:test", handle: "test.bsky.social", @@ -1040,6 +1041,58 @@ describe("post-composer", () => { ); }); + it("does not render the Drafts button when drafts are disabled", () => { + const element = createPostComposer({ draftsEnabled: false }); + connectElement(element); + assert.deepEqual( + element.querySelector('[data-testid="composer-drafts-button"]'), + null, + ); + }); + + it("shows the Drafts button when draftsEnabled is set after connect", async () => { + const element = createPostComposer({ draftsEnabled: false }); + connectElement(element); + assert.deepEqual( + element.querySelector('[data-testid="composer-drafts-button"]'), + null, + ); + element.draftsEnabled = true; + await nextFrame(); + assert( + element.querySelector('[data-testid="composer-drafts-button"]') !== + null, + ); + }); + + it("prompts a plain discard confirm instead of the save choice when drafts are disabled", async () => { + const element = createPostComposer({ draftsEnabled: false }); + connectElement(element); + element.handleInput({ detail: { text: "hello", facets: [] } }); + let confirmationSeen = false; + globalThis.__testConfirmation = (resolve) => { + confirmationSeen = true; + resolve(true); + }; + globalThis.__testChoice = () => { + throw new Error( + "choice prompt should not be shown when drafts are disabled", + ); + }; + const result = await element.confirmClose(); + assert.deepEqual(result, true); + assert(confirmationSeen); + }); + + it("stays open when the discard confirm is declined with drafts disabled", async () => { + const element = createPostComposer({ draftsEnabled: false }); + connectElement(element); + element.handleInput({ detail: { text: "hello", facets: [] } }); + globalThis.__testConfirmation = (resolve) => resolve(false); + const result = await element.confirmClose(); + assert.deepEqual(result, false); + }); + it("does not render the Drafts button for replies", () => { const element = createPostComposer(); element.replyTo = {