From 63ad6d52d69c77269254e3f3712c9b3e4d3d1051 Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Tue, 29 Sep 2026 12:49:51 +0200 Subject: [PATCH 01/16] Fixed timezone-dependent scheduler and email renderer tests (#31072) no ref Two Core tests failed outside UTC: the cron assertion ignored Bree's active scheduler timezone, and an email fixture used local midnight despite mocking a UTC publication. Match the scheduler's calendar and give the fixture an explicit UTC timestamp. --- .../server/adapters/jobs/in-memory-jobs-backend.test.ts | 6 +++++- .../server/services/email-service/email-renderer.test.js | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ghost/core/test/unit/server/adapters/jobs/in-memory-jobs-backend.test.ts b/ghost/core/test/unit/server/adapters/jobs/in-memory-jobs-backend.test.ts index 694ce643c0a..60dabc3d8aa 100644 --- a/ghost/core/test/unit/server/adapters/jobs/in-memory-jobs-backend.test.ts +++ b/ghost/core/test/unit/server/adapters/jobs/in-memory-jobs-backend.test.ts @@ -5,6 +5,7 @@ import InMemoryJobsBackend from '../../../../../core/server/adapters/jobs/InMemo const sinon = require('sinon'); const logging = require('@tryghost/logging'); +const later = require('@breejs/later'); runJobsBackendContractTests(() => new InMemoryJobsBackend(), { describe, it }); @@ -387,7 +388,10 @@ describe('InMemoryJobsBackend', function () { const backend = new InMemoryJobsBackend(); backend.start({ processor: async () => { - fireDays.push(new Date().getUTCDay()); + // Bree configures the shared scheduler to use local time when loaded. + // Assert the cron weekday in the calendar the scheduler actually uses. + const now = new Date(); + fireDays.push(later.date.isUTC ? now.getUTCDay() : now.getDay()); }, }); diff --git a/ghost/core/test/unit/server/services/email-service/email-renderer.test.js b/ghost/core/test/unit/server/services/email-service/email-renderer.test.js index 316fabefa65..994bea1795f 100644 --- a/ghost/core/test/unit/server/services/email-service/email-renderer.test.js +++ b/ghost/core/test/unit/server/services/email-service/email-renderer.test.js @@ -3009,7 +3009,7 @@ describe('Email renderer', function () { customSettings.locale = 'pt-PT'; const post = createModel( Object.assign({}, basePost, { - published_at: new Date(2026, 2, 19), + published_at: new Date('2026-03-19T00:00:00.000Z'), authors: [createModel({ name: "Author/Name O'Brien & Co." })], }), ); From 3bc42773332ad7e4dd2345c3fd498c4076db5b2f Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Tue, 29 Sep 2026 12:50:17 +0200 Subject: [PATCH 02/16] Fixed load-dependent timeout in deep queue test (#31073) no ref The deep-queue regression test could time out under load while waiting through 50,000 real event-loop turns. Use controlled `setImmediate` timers, keeping the full queue depth and verifying that other queued work runs before the drain completes. --- .../parent/middleware/queue-request.test.ts | 69 ++++++++++--------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/ghost/core/test/unit/server/web/parent/middleware/queue-request.test.ts b/ghost/core/test/unit/server/web/parent/middleware/queue-request.test.ts index af9e8ffbf7d..24cb12be0bb 100644 --- a/ghost/core/test/unit/server/web/parent/middleware/queue-request.test.ts +++ b/ghost/core/test/unit/server/web/parent/middleware/queue-request.test.ts @@ -5,6 +5,7 @@ import type { AddressInfo } from 'node:net'; import express from 'express'; import type { Request, Response } from 'express'; import nock from 'nock'; +import sinon from 'sinon'; import { queueRequest } from '../../../../../../core/server/web/parent/middleware/queue-request'; @@ -98,44 +99,46 @@ describe('Queue request middleware', function () { } }); - it('drains a deep queue of handlers that respond synchronously', async function () { - const middleware = queueRequest({ concurrencyLimit: 1 }); - const fakeRequest = () => { - const res = Object.assign(new EventEmitter(), { end: () => res }); - return { req: { path: '/sync' } as Request, res: res as unknown as Response }; - }; + it('drains a deep queue of handlers that respond synchronously', function () { + const depth = 50000; + // Exercise every scheduled pass without waiting for 50,000 real event-loop turns. + const clock = sinon.useFakeTimers({ toFake: ['setImmediate'], loopLimit: depth * 2 }); + try { + const middleware = queueRequest({ concurrencyLimit: 1 }); + const fakeRequest = () => { + const res = Object.assign(new EventEmitter(), { end: () => res }); + return { req: { path: '/sync' } as Request, res: res as unknown as Response }; + }; - const first = fakeRequest(); - middleware(first.req, first.res, () => {}); + const first = fakeRequest(); + middleware(first.req, first.res, () => {}); - const depth = 50000; - let completed = 0; - for (let i = 0; i < depth; i++) { - const { req, res } = fakeRequest(); - middleware(req, res, () => { - completed += 1; - res.end(); - }); - } + let completed = 0; + for (let i = 0; i < depth; i++) { + const { req, res } = fakeRequest(); + middleware(req, res, () => { + completed += 1; + res.end(); + }); + } - // a synchronous drain would recurse once per queued request and overflow the stack - first.res.end(); - let completedWhenOtherWorkRan = -1; - setImmediate(() => { - completedWhenOtherWorkRan = completed; - }); - while (completed < depth) { - await new Promise((resolve) => { - setImmediate(resolve); + // a synchronous drain would recurse once per queued request and overflow the stack + first.res.end(); + let completedWhenOtherWorkRan = -1; + setImmediate(() => { + completedWhenOtherWorkRan = completed; }); + clock.runAll(); + + assert.equal(completed, depth); + // the drain yields between passes, so other queued work gets to run part-way through + assert.ok( + completedWhenOtherWorkRan >= 0 && completedWhenOtherWorkRan < depth, + `other work only ran after ${completedWhenOtherWorkRan} requests had completed`, + ); + } finally { + clock.restore(); } - - assert.equal(completed, depth); - // the drain yields between passes, so other queued work gets to run part-way through - assert.ok( - completedWhenOtherWorkRan < depth, - `other work only ran after ${completedWhenOtherWorkRan} requests had completed`, - ); }); it('does not queue requests for static assets', async function () { From 1822985602a1135b82170f1bec4ea92413c36a18 Mon Sep 17 00:00:00 2001 From: Rob Lester Date: Tue, 29 Sep 2026 11:20:39 +0100 Subject: [PATCH 03/16] Fixed integrations being refused the custom field definitions fixes https://linear.app/ghost/issue/BER-3974/an-integration-reads-the-custom-fields-a-site-defines An integration such as Zapier has to learn which custom fields a site collects before it can write them to a member, and every integration key was refused the definitions with a 403. The Admin API decides which endpoints an integration key may reach by naming the resource from the first segment of the request path, and the definitions are served by a router mounted under /members/, which strips its mount from the path before that check runs, so the check saw a resource called "custom" and refused it. Authenticating as a route alongside the mount, where the path is still whole, restores the access integrations had before the definitions moved behind a mounted router, reads and writes alike. The check itself is left alone: it is due to be replaced as authentication moves to Better Auth, and this keeps the fix to where the routes are declared rather than reshaping something that is about to change. --- .../server/web/api/endpoints/admin/routes.js | 7 ++- .../admin/member-custom-fields.test.ts | 44 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/ghost/core/core/server/web/api/endpoints/admin/routes.js b/ghost/core/core/server/web/api/endpoints/admin/routes.js index 1d5bb904951..d51310883be 100644 --- a/ghost/core/core/server/web/api/endpoints/admin/routes.js +++ b/ghost/core/core/server/web/api/endpoints/admin/routes.js @@ -206,11 +206,14 @@ module.exports = function apiRoutes() { // guarded by being there, which is the safer way round to forget. // // Mounted before /members/:id so the literal path is not captured as an id. + // + // Authenticated as a route here rather than inside the mount: mounting strips the path + // from req.url, and the check on integration keys names the resource from its first + // segment, so inside it would see "custom" instead of "members" and refuse them. const metafieldsRouter = express.Router('admin api members metafields'); + router.all(['/members/metafields', '/members/metafields/*'], mw.authAdminApi); router.use('/members/metafields', metafieldsRouter); - metafieldsRouter.use(mw.authAdminApi); - // Reading is deliberately open: Admin asks every site for its definitions to draw // screens it renders either way, and a site that has none simply answers with an empty // list rather than a 404. diff --git a/ghost/core/test/e2e-api/admin/member-custom-fields.test.ts b/ghost/core/test/e2e-api/admin/member-custom-fields.test.ts index a5d4f3ceedd..1d2751d0e35 100644 --- a/ghost/core/test/e2e-api/admin/member-custom-fields.test.ts +++ b/ghost/core/test/e2e-api/admin/member-custom-fields.test.ts @@ -5,6 +5,7 @@ const { fixtureManager, mockManager, configUtils, + dbUtils, hostLimits, } = require('../../utils/e2e-framework'); const models = require('../../../core/server/models'); @@ -2383,6 +2384,49 @@ describe('Member Custom Fields Admin API', function () { }); }); + // Every definition route, and the member routes that carry values, as each kind of + // credential the Admin API accepts. Definitions are served by a mounted router, and + // mounting changes what the integration-key check sees of the path, so a route that + // works from a session can still refuse a key. + describe('Credentials', function () { + const credentials: Record Promise> = { + 'a staff session': () => agent.loginAsOwner(), + 'a staff access token': () => agent.useStaffTokenForOwner(), + "an integration's Admin API key": () => agent.useZapierAdminAPIKey(), + }; + + // Switching back to a session means logging in again, and this file already logs in + // close to the limit on login attempts, so clear the count before the later blocks do. + afterAll(async function () { + await dbUtils.truncate('brute'); + await agent.loginAsOwner(); + }); + + it.each(Object.entries(credentials))( + 'reaches every definition and value route with %s', + async function (_credential, useCredential) { + await useCredential(); + + const company = await createField({ name: 'Company' }); + const shirtSize = await createField({ name: 'Shirt size' }); + await agent.get('members/metafields/custom/').expectStatus(200); + await agent.get(`members/metafields/custom/${company.key}/`).expectStatus(200); + await agent + .put('members/metafields/custom/') + .body({ members_metafields: [{ key: shirtSize.key }, { key: company.key }] }) + .expectStatus(200); + + const memberId = await createMember(); + await setValues(memberId, { [company.key]: 'Ghost' }); + assert.deepEqual(await readValues(memberId), { [company.key]: 'Ghost' }); + await agent.get('members/').expectStatus(200); + + await setStatus(shirtSize.key, 'archived'); + await agent.delete(`members/metafields/custom/${shirtSize.key}/`).expectStatus(204); + }, + ); + }); + describe('Authorization', function () { // The full role matrix is pinned in migration.test.js; here we only prove // the endpoint enforces the permission — a role without it may look but not touch. From 5c18440904b6724d18783a87eec45ef3aade841c Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Tue, 29 Sep 2026 12:51:05 +0200 Subject: [PATCH 04/16] Fixed editor exit racing a create acknowledgement (#31075) no ref Flaky test patch. A new post's create acknowledgement could render before React Router published a blocked exit, replacing the intended Posts destination with the new editor URL. Added checking the synchronous navigation guard before replacing that URL, keeping cancellation from reviving stale blocked state. --- .../editor/session/use-leave-guard.test.ts | 53 ++++++++++++++++++- .../src/editor/session/use-leave-guard.ts | 4 +- .../src/hooks/use-unsaved-changes-guard.ts | 8 +-- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/apps/admin/src/editor/session/use-leave-guard.test.ts b/apps/admin/src/editor/session/use-leave-guard.test.ts index ab05f8cb993..c22461ba92d 100644 --- a/apps/admin/src/editor/session/use-leave-guard.test.ts +++ b/apps/admin/src/editor/session/use-leave-guard.test.ts @@ -1,4 +1,5 @@ -import { createElement } from 'react'; +import { createElement, useState } from 'react'; +import { flushSync } from 'react-dom'; import { act, render, waitFor } from '@testing-library/react'; import { createMemoryRouter, RouterProvider } from 'react-router'; import { describe, expect, it, vi } from 'vitest'; @@ -7,6 +8,56 @@ import { useEditorLeaveGuard, type EditorLeaveGuard } from './use-leave-guard'; import type { EditorSessionHandle } from './use-editor-session'; describe('useEditorLeaveGuard', () => { + it.each(['proceed', 'confirm'] as const)( + 'settles a blocked exit (%s) when a create renders before the router', + async (outcome) => { + const decision = deferred<'proceed' | 'confirm'>(); + let guard!: EditorLeaveGuard; + let acknowledgeCreate!: () => void; + const leaveRequested = vi.fn(() => decision.promise); + function Editor() { + const [createdId, setCreatedId] = useState(null); + acknowledgeCreate = () => setCreatedId('new789'); + guard = useEditorLeaveGuard( + { + state: { kind: 'idle' }, + createdId, + isDirty: () => !createdId, + leaveRequested, + } as unknown as EditorSessionHandle, + 'post', + ); + return createElement('main', { 'data-testid': 'editor' }); + } + const router = createMemoryRouter( + [ + { path: '/editor/post/:id?', element: createElement(Editor) }, + { path: '/posts', element: 'Posts' }, + ], + { initialEntries: ['/editor/post'] }, + ); + render(createElement(RouterProvider, { router })); + act(() => { + void router.navigate('/posts'); + // Session updates use an external store and can beat the router's transition. + flushSync(acknowledgeCreate); + }); + await act(async () => { + decision.resolve(outcome); + await decision.promise; + }); + if (outcome === 'confirm') { + await waitFor(() => expect(guard.dialogProps.open).toBe(true)); + expect(router.state.location.pathname).toBe('/editor/post'); + act(() => guard.dialogProps.onOpenChange(false)); + await waitFor(() => expect(router.state.location.pathname).toBe('/editor/post/new789')); + } else { + await waitFor(() => expect(router.state.location.pathname).toBe('/posts')); + } + router.dispose(); + }, + ); + it('keeps the confirmation open until a slow navigation unmounts the editor', async () => { const destination = deferred(); const session = { diff --git a/apps/admin/src/editor/session/use-leave-guard.ts b/apps/admin/src/editor/session/use-leave-guard.ts index 93b68098f9b..cb8effe4cf7 100644 --- a/apps/admin/src/editor/session/use-leave-guard.ts +++ b/apps/admin/src/editor/session/use-leave-guard.ts @@ -65,8 +65,9 @@ export function useEditorLeaveGuard( // Router owns one blocker target, so starting this replace while an exit is // blocked would overwrite the writer's original destination. const createdId = session.createdId; + const { hasBlockedNavigation } = guard; useEffect(() => { - if (!createdId || guard.isBlocked || isUrlSwapBlocked || isLeavingRef.current) { + if (!createdId || hasBlockedNavigation() || isUrlSwapBlocked || isLeavingRef.current) { return; } const target = `/editor/${postType}/${createdId}`; @@ -80,6 +81,7 @@ export function useEditorLeaveGuard( }, [ createdId, guard.isBlocked, + hasBlockedNavigation, isUrlSwapBlocked, location.pathname, navigate, diff --git a/apps/admin/src/hooks/use-unsaved-changes-guard.ts b/apps/admin/src/hooks/use-unsaved-changes-guard.ts index e698b1c5e00..651e9a25716 100644 --- a/apps/admin/src/hooks/use-unsaved-changes-guard.ts +++ b/apps/admin/src/hooks/use-unsaved-changes-guard.ts @@ -30,6 +30,8 @@ export interface UseUnsavedChangesGuardOptions { export interface UnsavedChangesGuard { /** A guarded navigation is currently blocked awaiting the discard dialog. */ isBlocked: boolean; + /** Reads a blocked exit synchronously, before the router has necessarily rerendered. */ + hasBlockedNavigation: () => boolean; /** Wiring for Shade's `DirtyConfirmDialog`: ``. */ dialogProps: { open: boolean; @@ -115,9 +117,6 @@ export function useUnsavedChangesGuard({ const isBlockedByIntercept = blocker.state === 'blocked' && blockedByInterceptRef.current; const isBlocked = (blocker.state === 'blocked' && !blockedByInterceptRef.current) || anchorGuard.isBlocked; - if (isBlocked) { - blockedNavigationRef.current = true; - } // One-shot state is scoped to the current route target. React.useEffect(() => { @@ -143,6 +142,8 @@ export function useUnsavedChangesGuard({ bypassRef.current = true; }, []); + const hasBlockedNavigation = React.useCallback(() => blockedNavigationRef.current, []); + const resumeBlockedNavigationAfterSave = React.useCallback(() => { if (!blockedNavigationRef.current) { return false; @@ -153,6 +154,7 @@ export function useUnsavedChangesGuard({ return { isBlocked, + hasBlockedNavigation, dialogProps: { open: isBlocked && !isSaving, onConfirm: () => { From 0c9852e4d4b00d33240deca10655de7f26cc20b3 Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Tue, 29 Sep 2026 13:01:36 +0200 Subject: [PATCH 05/16] Fixed signed-out Admin loads racing the signin redirect (#31066) no ref Admin reloads to /ghost/ when an API request says the session has expired. That also happened on page loads where nobody was signed in yet, and the reload could beat Ember to saving the route the visitor asked for, so they landed on the dashboard after signing in instead of where they were going. The user's path should now be preserved through auth. --- apps/admin-x-framework/src/helpers.ts | 1 + .../src/hooks/use-handle-error.ts | 6 +- .../src/utils/api/fetch-api.ts | 16 +++- .../admin-x-framework/src/utils/auth-paths.ts | 16 ++++ .../test/unit/hooks/use-handle-error.test.tsx | 6 +- .../api/permission-session-expiry.test.tsx | 34 ++++++-- .../unit/utils/api/session-expiry.test.tsx | 81 ++++++++++++++++++- .../test/unit/utils/auth-paths.test.ts | 27 +++++++ 8 files changed, 171 insertions(+), 16 deletions(-) create mode 100644 apps/admin-x-framework/src/utils/auth-paths.ts create mode 100644 apps/admin-x-framework/test/unit/utils/auth-paths.test.ts diff --git a/apps/admin-x-framework/src/helpers.ts b/apps/admin-x-framework/src/helpers.ts index 29ad3d388d0..b8e1c43729b 100644 --- a/apps/admin-x-framework/src/helpers.ts +++ b/apps/admin-x-framework/src/helpers.ts @@ -1,2 +1,3 @@ export * from './utils/helpers'; +export { isAuthPath } from './utils/auth-paths'; export { apiUrl } from './utils/api/fetch-api'; diff --git a/apps/admin-x-framework/src/hooks/use-handle-error.ts b/apps/admin-x-framework/src/hooks/use-handle-error.ts index eecc0209099..523c1ea7504 100644 --- a/apps/admin-x-framework/src/hooks/use-handle-error.ts +++ b/apps/admin-x-framework/src/hooks/use-handle-error.ts @@ -49,9 +49,9 @@ const useHandleError = () => { // but still clear lingering toasts that would block clicks the same way. toast.dismiss(); } else if (error instanceof SessionExpiredError) { - // A redirecting request unloads the page, so a toast would only flash; - // one that opted out of the redirect reports the expiry itself. - toast.dismiss(); + // Either the page is reloading to signin or nobody is signed in yet, so + // there is nothing to report; toasts the signin flow shows must survive. + return; } else if (error instanceof APIError) { showErrorToast(getErrorMessage(error, error.message)); } else { diff --git a/apps/admin-x-framework/src/utils/api/fetch-api.ts b/apps/admin-x-framework/src/utils/api/fetch-api.ts index ce7ab47ce2b..b96b30db5de 100644 --- a/apps/admin-x-framework/src/utils/api/fetch-api.ts +++ b/apps/admin-x-framework/src/utils/api/fetch-api.ts @@ -9,6 +9,7 @@ import { TimeoutError, UnauthorizedError, } from '../errors'; +import { isAuthPath } from '../auth-paths'; import { getGhostPaths } from '../helpers'; import handleResponse, { ResponseType } from './handle-response'; @@ -60,8 +61,11 @@ const xhrToFetchResponse = (xhr: Readonly): Response => const GHOST_API_REQUEST = /\/ghost\/api\//; const SESSION_API_REQUEST = /\/ghost\/api\/admin\/session([/?#]|$)/; -const UNAUTHENTICATED_ADMIN_ROUTE = /^#\/(?:reset|setup|signin|signup)(?:[/?]|$)/; +const CURRENT_USER_REQUEST = /\/ghost\/api\/admin\/users\/me\/([?#]|$)/; +// A session can only expire once this page load has seen it work; failures +// before that are the signed-out state, which the signin flow handles. +let sessionConfirmed = false; let sessionExpiryHandled = false; const isUnauthenticatedAdminRoute = (adminRoot: string) => { @@ -69,7 +73,7 @@ const isUnauthenticatedAdminRoute = (adminRoot: string) => { window.location.pathname === adminRoot && (!window.location.hash || window.location.hash === '#/' || - UNAUTHENTICATED_ADMIN_ROUTE.test(window.location.hash)) + isAuthPath(window.location.hash.slice(1))) ); }; @@ -85,7 +89,7 @@ const isSessionExpiry = (endpoint: string | URL) => { const redirectOnSessionExpiry = () => { const { adminRoot } = getGhostPaths(); - if (!sessionExpiryHandled && !isUnauthenticatedAdminRoute(adminRoot)) { + if (sessionConfirmed && !sessionExpiryHandled && !isUnauthenticatedAdminRoute(adminRoot)) { sessionExpiryHandled = true; window.location.replace(adminRoot); } @@ -232,7 +236,11 @@ export const useFetchApi = () => { try { const response = await fetchFn(endpoint, requestInit); // Awaited so response errors reject inside the try/catch - return (await handleResponse(response, { responseType })) as ResponseData; + const data = (await handleResponse(response, { responseType })) as ResponseData; + if (CURRENT_USER_REQUEST.test(endpoint.toString())) { + sessionConfirmed = true; + } + return data; } catch (error) { retryingMs = Date.now() - startTime; diff --git a/apps/admin-x-framework/src/utils/auth-paths.ts b/apps/admin-x-framework/src/utils/auth-paths.ts new file mode 100644 index 00000000000..08257aa4810 --- /dev/null +++ b/apps/admin-x-framework/src/utils/auth-paths.ts @@ -0,0 +1,16 @@ +// Admin routes that serve the signed-out and session flows. `/setup/onboarding` +// is a signed-in screen, so only the bare `/setup` counts. +const AUTH_PATH_PATTERNS = [ + /^\/signin\/?$/, + /^\/signin\/verify\/?$/, + /^\/signout\/?$/, + /^\/signup\/[^/]+\/?$/, + /^\/reset\/[^/]+\/?$/, + /^\/setup\/?$/, +]; + +/** Whether an Admin route path (optionally with a query string) is an authentication screen. */ +export function isAuthPath(path: string): boolean { + const [pathname] = path.split('?'); + return AUTH_PATH_PATTERNS.some((pattern) => pattern.test(pathname)); +} diff --git a/apps/admin-x-framework/test/unit/hooks/use-handle-error.test.tsx b/apps/admin-x-framework/test/unit/hooks/use-handle-error.test.tsx index 690cec043c2..e82ea532775 100644 --- a/apps/admin-x-framework/test/unit/hooks/use-handle-error.test.tsx +++ b/apps/admin-x-framework/test/unit/hooks/use-handle-error.test.tsx @@ -211,10 +211,10 @@ describe('useHandleError', () => { result.current(error); - // The fetch layer redirects to signin on session expiry, so the - // error handler must not flash a toast over the unloading page + // Signed-out boots report every read as expired; the signin flow's own + // toasts must survive those reports expect(toast.error).not.toHaveBeenCalled(); - expect(toast.dismiss).toHaveBeenCalled(); + expect(toast.dismiss).not.toHaveBeenCalled(); }); it('shows toast for unauthorized errors that do not trigger a redirect', () => { diff --git a/apps/admin-x-framework/test/unit/utils/api/permission-session-expiry.test.tsx b/apps/admin-x-framework/test/unit/utils/api/permission-session-expiry.test.tsx index bb8242eab32..4ed7dd2f97d 100644 --- a/apps/admin-x-framework/test/unit/utils/api/permission-session-expiry.test.tsx +++ b/apps/admin-x-framework/test/unit/utils/api/permission-session-expiry.test.tsx @@ -20,12 +20,30 @@ const readCurrentUser = (mock: { calls: unknown[][] }) => // FrameworkProvider comes from the previous registry and its context is a // different object than the one the hooks module reads. const loadModules = async () => { - const [{ createInfiniteQuery, createQuery, createQueryWithId }, testUtils] = await Promise.all([ + const [ + { createInfiniteQuery, createQuery, createQueryWithId }, + { apiUrl, useFetchApi }, + testUtils, + ] = await Promise.all([ import('../../../../src/utils/api/hooks'), + import('../../../../src/utils/api/fetch-api'), import('../../../../src/test/test-utils'), ]); + // The redirect only applies once the page has seen the session work + const confirmSession = () => + withMockFetch( + { json: { users: [{ id: '1' }] }, headers: { 'content-type': 'application/json' } }, + async () => { + const { result } = testUtils.renderHookWithProviders(() => useFetchApi(), { + queryClient: testUtils.createTestQueryClient(), + }); + await result.current(apiUrl('/users/me/', { include: 'roles' })); + }, + ); + return { + confirmSession, useTestQuery: createQuery({ dataType: 'test', path: '/test/' }), useTestInfiniteQuery: createInfiniteQuery({ dataType: 'test-infinite', @@ -52,7 +70,9 @@ describe('permission read session expiry', () => { }); it('redirects on an expired session when a query omits the opt-out', async () => { - const { useTestQuery, createTestQueryClient, renderHookWithProviders } = await loadModules(); + const { confirmSession, useTestQuery, createTestQueryClient, renderHookWithProviders } = + await loadModules(); + await confirmSession(); await withMockFetch(unauthorized, async (mock) => { const { result } = renderHookWithProviders( @@ -68,7 +88,9 @@ describe('permission read session expiry', () => { }); it('leaves an expired session to the caller when a query opts out', async () => { - const { useTestQuery, createTestQueryClient, renderHookWithProviders } = await loadModules(); + const { confirmSession, useTestQuery, createTestQueryClient, renderHookWithProviders } = + await loadModules(); + await confirmSession(); await withMockFetch(unauthorized, async (mock) => { const { result } = renderHookWithProviders(() => useTestQuery(optedOut), { @@ -83,8 +105,9 @@ describe('permission read session expiry', () => { }); it('leaves an expired session to the caller when an infinite query opts out', async () => { - const { useTestInfiniteQuery, createTestQueryClient, renderHookWithProviders } = + const { confirmSession, useTestInfiniteQuery, createTestQueryClient, renderHookWithProviders } = await loadModules(); + await confirmSession(); await withMockFetch(unauthorized, async (mock) => { const { result } = renderHookWithProviders(() => useTestInfiniteQuery(optedOut), { @@ -99,8 +122,9 @@ describe('permission read session expiry', () => { }); it('leaves an expired session to the caller when a query by id opts out', async () => { - const { useTestQueryWithId, createTestQueryClient, renderHookWithProviders } = + const { confirmSession, useTestQueryWithId, createTestQueryClient, renderHookWithProviders } = await loadModules(); + await confirmSession(); await withMockFetch(unauthorized, async (mock) => { const { result } = renderHookWithProviders(() => useTestQueryWithId('abc123', optedOut), { diff --git a/apps/admin-x-framework/test/unit/utils/api/session-expiry.test.tsx b/apps/admin-x-framework/test/unit/utils/api/session-expiry.test.tsx index 37ac6e5829e..3506ed0d22b 100644 --- a/apps/admin-x-framework/test/unit/utils/api/session-expiry.test.tsx +++ b/apps/admin-x-framework/test/unit/utils/api/session-expiry.test.tsx @@ -53,6 +53,16 @@ const forbidden = () => ); const server = setupServer( + http.get('http://localhost:3000/ghost/api/admin/users/me/', () => + HttpResponse.json({ users: [{ id: '1' }] }), + ), + http.get('http://localhost:3000/blog/ghost/api/admin/users/me/', () => + HttpResponse.json({ users: [{ id: '1' }] }), + ), + http.get('http://localhost:3000/ghost/api/admin/site/', () => HttpResponse.json({ site: {} })), + http.get('http://localhost:3000/ghost/api/admin/users/me/token/', () => + HttpResponse.json({ apiKey: {} }), + ), http.get('http://localhost:3000/ghost/api/admin/posts/', expiredSession), http.get('http://localhost:3000/ghost/api/admin/posts/401/', unauthorized), http.get('http://localhost:3000/ghost/api/admin/members/', forbidden), @@ -77,6 +87,12 @@ const loadModules = async () => { return { useFetchApi, SessionExpiredError, UnauthorizedError, ValidationError }; }; +type FetchApi = ReturnType>['useFetchApi']>; + +// The redirect only applies once the page has seen the session work +const confirmSession = (fetchApi: FetchApi) => + fetchApi('http://localhost:3000/ghost/api/admin/users/me/?include=roles', { retry: false }); + describe('session expiry handling', () => { beforeEach(() => { vi.resetModules(); @@ -90,6 +106,7 @@ describe('session expiry handling', () => { it('redirects to the admin root when an API request returns 403 Authorization failed', async () => { const { useFetchApi, SessionExpiredError } = await loadModules(); const { result } = renderHook(() => useFetchApi()); + await confirmSession(result.current); await expect( result.current('http://localhost:3000/ghost/api/admin/posts/', { retry: false }), @@ -101,6 +118,7 @@ describe('session expiry handling', () => { it('redirects to the admin root when an API request returns 401', async () => { const { useFetchApi, SessionExpiredError } = await loadModules(); const { result } = renderHook(() => useFetchApi()); + await confirmSession(result.current); const error = await result .current('http://localhost:3000/ghost/api/admin/posts/401/', { retry: false }) @@ -114,6 +132,7 @@ describe('session expiry handling', () => { it('redirects only once when multiple in-flight requests return session expiry errors', async () => { const { useFetchApi, SessionExpiredError } = await loadModules(); const { result } = renderHook(() => useFetchApi()); + await confirmSession(result.current); const results = await Promise.allSettled([ result.current('http://localhost:3000/ghost/api/admin/posts/', { retry: false }), @@ -129,18 +148,75 @@ describe('session expiry handling', () => { expect(window.location.replace).toHaveBeenCalledTimes(1); }); + it('does not redirect while signed out before the page has seen the session work', async () => { + server.use(http.get('http://localhost:3000/ghost/api/admin/users/me/', expiredSession)); + const { useFetchApi, SessionExpiredError } = await loadModules(); + const { result } = renderHook(() => useFetchApi()); + + await expect(confirmSession(result.current)).rejects.toBeInstanceOf(SessionExpiredError); + await expect( + result.current('http://localhost:3000/ghost/api/admin/posts/', { retry: false }), + ).rejects.toBeInstanceOf(SessionExpiredError); + + expect(window.location.replace).not.toHaveBeenCalled(); + }); + + it('does not treat a successful request to a public endpoint as a confirmed session', async () => { + const { useFetchApi, SessionExpiredError } = await loadModules(); + const { result } = renderHook(() => useFetchApi()); + + await result.current('http://localhost:3000/ghost/api/admin/site/', { retry: false }); + await result.current('http://localhost:3000/ghost/api/admin/users/me/token/', { retry: false }); + await expect( + result.current('http://localhost:3000/ghost/api/admin/posts/', { retry: false }), + ).rejects.toBeInstanceOf(SessionExpiredError); + + expect(window.location.replace).not.toHaveBeenCalled(); + }); + + it('confirms the session from the current user under a subdirectory install', async () => { + const { useFetchApi, SessionExpiredError } = await loadModules(); + const { result } = renderHook(() => useFetchApi()); + + await result.current('http://localhost:3000/blog/ghost/api/admin/users/me/?include=roles', { + retry: false, + }); + await expect( + result.current('http://localhost:3000/ghost/api/admin/posts/', { retry: false }), + ).rejects.toBeInstanceOf(SessionExpiredError); + + expect(window.location.replace).toHaveBeenCalledExactlyOnceWith('/ghost/'); + }); + + it('redirects from the signed-in onboarding route under /setup', async () => { + (window as any).location.hash = '#/setup/onboarding?returnTo=/analytics'; + const { useFetchApi, SessionExpiredError } = await loadModules(); + const { result } = renderHook(() => useFetchApi()); + await confirmSession(result.current); + + await expect( + result.current('http://localhost:3000/ghost/api/admin/posts/', { retry: false }), + ).rejects.toBeInstanceOf(SessionExpiredError); + + expect(window.location.replace).toHaveBeenCalledExactlyOnceWith('/ghost/'); + }); + it.each([ '', '#/', '#/signin', '#/signin/verify', + '#/signin?labs=authReact', + '#/signout', '#/signup/invitation-token', - '#/setup/one', + '#/setup', '#/reset/reset-token', + '#/reset/reset-token/', ])('does not redirect from unauthenticated Admin route %s', async (hash) => { (window as any).location.hash = hash; const { useFetchApi, SessionExpiredError } = await loadModules(); const { result } = renderHook(() => useFetchApi()); + await confirmSession(result.current); await expect( result.current('http://localhost:3000/ghost/api/admin/posts/', { retry: false }), @@ -152,6 +228,7 @@ describe('session expiry handling', () => { it('does not redirect when the session endpoint returns 401', async () => { const { useFetchApi, SessionExpiredError, UnauthorizedError } = await loadModules(); const { result } = renderHook(() => useFetchApi()); + await confirmSession(result.current); await expect( result.current('http://localhost:3000/ghost/api/admin/session/', { @@ -192,6 +269,7 @@ describe('session expiry handling', () => { it('does not redirect when a non-Ghost API request returns 401', async () => { const { useFetchApi, SessionExpiredError, UnauthorizedError } = await loadModules(); const { result } = renderHook(() => useFetchApi()); + await confirmSession(result.current); const error = await result .current('http://localhost:3000/external/data/', { retry: false }) @@ -205,6 +283,7 @@ describe('session expiry handling', () => { it('does not redirect for other 403 permission errors', async () => { const { useFetchApi, SessionExpiredError, ValidationError } = await loadModules(); const { result } = renderHook(() => useFetchApi()); + await confirmSession(result.current); const error = await result .current('http://localhost:3000/ghost/api/admin/members/', { retry: false }) diff --git a/apps/admin-x-framework/test/unit/utils/auth-paths.test.ts b/apps/admin-x-framework/test/unit/utils/auth-paths.test.ts new file mode 100644 index 00000000000..bf379fe8021 --- /dev/null +++ b/apps/admin-x-framework/test/unit/utils/auth-paths.test.ts @@ -0,0 +1,27 @@ +import { isAuthPath } from '../../../src/utils/auth-paths'; + +describe('isAuthPath', () => { + it.each([ + '/signin', + '/signin/', + '/signin/verify', + '/signin?labs=authReact', + '/signout', + '/signup/aW52aXRl', + '/reset/cmVzZXQ/', + '/setup', + ])('matches %s', (path) => { + expect(isAuthPath(path)).toBe(true); + }); + + it.each([ + '/', + '/setup/onboarding', + '/setup/onboarding?returnTo=/analytics', + '/signup', + '/posts', + '/signin-help', + ])('does not match %s', (path) => { + expect(isAuthPath(path)).toBe(false); + }); +}); From 2797262154eca0eda3dc8eeb8ca7dc07b4d42813 Mon Sep 17 00:00:00 2001 From: Austin Burdine Date: Tue, 29 Sep 2026 07:08:28 -0400 Subject: [PATCH 06/16] Added settings for rotating the site signing keys (#31053) no ref Sites created before 2048-bit key generation still sign member and staff tokens with 1024-bit RSA keys, and jsonwebtoken 9 won't sign with those. These rows let a later change rotate each keypair without breaking verifiers: the next key is published before it's used to sign, and the previous public key stays published briefly after the switch. The next public key is derived from its private key, and row `updated_at` records when each was written, so neither needs its own row. The active key stays in the existing settings rows, so downgrading keeps working. --- ...58-53-add-signing-key-rotation-settings.js | 20 +++++++++++++++++++ .../default-settings/default-settings.json | 16 +++++++++++++++ ghost/core/package.json | 2 +- .../integration/settings/settings.test.js | 4 ++++ .../test/legacy/models/model-settings.test.js | 2 +- .../unit/server/data/schema/integrity.test.js | 2 +- .../test/utils/fixtures/default-settings.json | 16 +++++++++++++++ 7 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 ghost/core/core/server/data/migrations/versions/6.66/2026-09-28-18-58-53-add-signing-key-rotation-settings.js diff --git a/ghost/core/core/server/data/migrations/versions/6.66/2026-09-28-18-58-53-add-signing-key-rotation-settings.js b/ghost/core/core/server/data/migrations/versions/6.66/2026-09-28-18-58-53-add-signing-key-rotation-settings.js new file mode 100644 index 00000000000..418a6b7c80e --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/6.66/2026-09-28-18-58-53-add-signing-key-rotation-settings.js @@ -0,0 +1,20 @@ +const { combineTransactionalMigrations, addSetting } = require('../../utils'); + +// Rows for rotating the signing keypairs; empty until a rotation is in progress +const keys = [ + 'ghost_next_private_key', + 'ghost_previous_public_key', + 'members_next_private_key', + 'members_previous_public_key', +]; + +module.exports = combineTransactionalMigrations( + ...keys.map((key) => + addSetting({ + key, + value: null, + type: 'string', + group: 'core', + }), + ), +); diff --git a/ghost/core/core/server/data/schema/default-settings/default-settings.json b/ghost/core/core/server/data/schema/default-settings/default-settings.json index ac2b997f549..b887961c4ab 100644 --- a/ghost/core/core/server/data/schema/default-settings/default-settings.json +++ b/ghost/core/core/server/data/schema/default-settings/default-settings.json @@ -36,6 +36,14 @@ "defaultValue": null, "type": "string" }, + "ghost_next_private_key": { + "defaultValue": null, + "type": "string" + }, + "ghost_previous_public_key": { + "defaultValue": null, + "type": "string" + }, "members_public_key": { "defaultValue": null, "type": "string" @@ -44,6 +52,14 @@ "defaultValue": null, "type": "string" }, + "members_next_private_key": { + "defaultValue": null, + "type": "string" + }, + "members_previous_public_key": { + "defaultValue": null, + "type": "string" + }, "members_email_auth_secret": { "defaultValue": null, "type": "string" diff --git a/ghost/core/package.json b/ghost/core/package.json index 8817446591d..badc5f5d81a 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -1,6 +1,6 @@ { "name": "ghost", - "version": "6.65.1-rc.0", + "version": "6.66.0-rc.0", "description": "The professional publishing platform", "keywords": [ "blog", diff --git a/ghost/core/test/integration/settings/settings.test.js b/ghost/core/test/integration/settings/settings.test.js index a04fadfa807..ff76f440078 100644 --- a/ghost/core/test/integration/settings/settings.test.js +++ b/ghost/core/test/integration/settings/settings.test.js @@ -25,8 +25,12 @@ describe('Settings', function () { 'theme_session_secret', 'ghost_public_key', 'ghost_private_key', + 'ghost_next_private_key', + 'ghost_previous_public_key', 'members_public_key', 'members_private_key', + 'members_next_private_key', + 'members_previous_public_key', 'members_email_auth_secret', 'members_stripe_webhook_id', 'members_stripe_webhook_secret', diff --git a/ghost/core/test/legacy/models/model-settings.test.js b/ghost/core/test/legacy/models/model-settings.test.js index a1b76632509..61e5761b32b 100644 --- a/ghost/core/test/legacy/models/model-settings.test.js +++ b/ghost/core/test/legacy/models/model-settings.test.js @@ -5,7 +5,7 @@ const db = require('../../../core/server/data/db'); // Stuff we are testing const models = require('../../../core/server/models'); -const SETTINGS_LENGTH = 119; +const SETTINGS_LENGTH = 123; describe('Settings Model', function () { // Create the schema once, then empty every table before each test — these diff --git a/ghost/core/test/unit/server/data/schema/integrity.test.js b/ghost/core/test/unit/server/data/schema/integrity.test.js index 93936f05e33..bf1f911f23f 100644 --- a/ghost/core/test/unit/server/data/schema/integrity.test.js +++ b/ghost/core/test/unit/server/data/schema/integrity.test.js @@ -40,7 +40,7 @@ describe('DB version integrity', function () { // Only these variables should need updating const currentSchemaHash = 'b3467bb26d2c4ef862382718ca6ed6e8'; const currentFixturesHash = '5718e0d4eb037f159c312369e949829a'; - const currentSettingsHash = '6ea42a00cca61a1ba87f66eb6e25a78a'; + const currentSettingsHash = 'ad77752f31c6a7f174c04c499214b975'; const currentRoutesHash = 'd8c25fa01bf6d22a2bcb05ba0de70dc1'; // If this test is failing, then it is likely a change has been made that requires a DB version bump, diff --git a/ghost/core/test/utils/fixtures/default-settings.json b/ghost/core/test/utils/fixtures/default-settings.json index 041e9cfcca6..db960868890 100644 --- a/ghost/core/test/utils/fixtures/default-settings.json +++ b/ghost/core/test/utils/fixtures/default-settings.json @@ -36,6 +36,14 @@ "defaultValue": null, "type": "string" }, + "ghost_next_private_key": { + "defaultValue": null, + "type": "string" + }, + "ghost_previous_public_key": { + "defaultValue": null, + "type": "string" + }, "members_public_key": { "defaultValue": null, "type": "string" @@ -44,6 +52,14 @@ "defaultValue": null, "type": "string" }, + "members_next_private_key": { + "defaultValue": null, + "type": "string" + }, + "members_previous_public_key": { + "defaultValue": null, + "type": "string" + }, "members_email_auth_secret": { "defaultValue": null, "type": "string" From c22701c22b89359e97691cb91632bfffde060e43 Mon Sep 17 00:00:00 2001 From: Peter Zimon Date: Tue, 29 Sep 2026 13:45:33 +0200 Subject: [PATCH 07/16] Improved React editor title and feature image layout (#31074) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improves the React editor’s title and feature image layout. Titles match the Ember editor’s typography and resize to fit their content, and feature images keep their original aspect ratio instead of being clipped at 480px. Fixes [PLA-447 — Editor header refinements](https://linear.app/ghost/issue/PLA-447/editor-header-refinements). ## Changes - Match title typography across desktop and mobile, and remove inherited textarea constraints that prevented titles from growing and shrinking correctly. - Place feature image upload and Unsplash controls side by side with a 13px label and 20px gap. Keep the Unsplash icon at 14px inside a circular 32 × 32px button. - Use consistent 13px sans-serif typography for feature image captions, placeholders, and alt text, with a black-and-white selected Alt toggle. - Display full feature images at their original proportions, including tall portrait images. - Reset virtual-list row windows before layout effects so opening Members from the sidebar reliably starts at the top, while Back and breadcrumb navigation retain their saved position. ## Validation - [CI passed](https://github.com/TryGhost/Ghost/actions/runs/36559439696) on `58ec72537c`: all 12 E2E shards, both Admin acceptance shards, both unit-test jobs, lint, and build checks passed. CodeRabbit reported no actionable findings. - Fixed the CI failure in the members virtual-window E2E test. Both Back and breadcrumb flows passed three repeated runs each; all 20 focused virtual-window and scroll-restoration unit tests passed, including a new regression for reset timing. - Post editor, feature image, and X card acceptance suites passed, with regressions covering title resizing and portrait/landscape image clipping. All 12 feature-image tests passed after the final button adjustment. - Admin TypeScript, focused ESLint, formatting, and pre-commit checks passed. - Visually checked the editor at 1280px, 600px, and 390px widths, and verified the Unsplash button’s 32 × 32px dimensions and circular shape. - Full `pnpm check` passed formatting and lint, but the Admin unit suite encountered a Koenig module-resolution failure in `engine/__fixtures__/after-load.test.tsx` and a timeout in the members custom-field country filter test. Neither failing file is changed by this PR. --- .../editor-feature-image.acceptance.test.tsx | 21 ++++++++ .../src/editor/feature-image-caption.tsx | 7 +-- apps/admin/src/editor/feature-image.tsx | 6 +-- apps/admin/src/editor/image-field.tsx | 29 +++++++---- .../editor/post-editor.acceptance.test.tsx | 32 ++++++++++++ apps/admin/src/editor/post-editor.tsx | 2 +- apps/admin/src/editor/unsplash-picker.tsx | 13 ++++- .../use-virtual-list-window.test.ts | 30 ++++++++++- .../virtual-list/use-virtual-list-window.ts | 50 +++++++++---------- 9 files changed, 145 insertions(+), 45 deletions(-) diff --git a/apps/admin/src/editor/editor-feature-image.acceptance.test.tsx b/apps/admin/src/editor/editor-feature-image.acceptance.test.tsx index 7510fe21b32..a974e2c714f 100644 --- a/apps/admin/src/editor/editor-feature-image.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-feature-image.acceptance.test.tsx @@ -40,6 +40,27 @@ function fakeSavablePost(overrides: Partial = {}) { * save engine the body uses. */ describe('Post editor feature image', () => { + it.each([ + { orientation: 'portrait', width: 800, height: 1200 }, + { orientation: 'landscape', width: 1200, height: 800 }, + ])( + 'shows the full $orientation image at its original aspect ratio', + async ({ width, height }) => { + const svg = ``; + fakeSavablePost({ feature_image: `data:image/svg+xml,${encodeURIComponent(svg)}` }); + await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); + + await expect.element(editorScreen.featureImage()).toBeVisible(); + const image = editorScreen.featureImage().element().querySelector('img')!; + await expect.poll(() => image.naturalWidth).toBe(width); + + const imageBounds = image.getBoundingClientRect(); + const containerBounds = image.closest('[data-slot="image-upload"]')!.getBoundingClientRect(); + expect(imageBounds.height).toBeCloseTo((imageBounds.width * height) / width, 0); + expect(containerBounds.height).toBeCloseTo(imageBounds.height, 0); + }, + ); + it('saves an uploaded image as soon as it lands', async () => { const saveApi = fakeSavablePost(); const uploadApi = fakeAdminEndpoint('POST', '/images/upload/', { diff --git a/apps/admin/src/editor/feature-image-caption.tsx b/apps/admin/src/editor/feature-image-caption.tsx index d2d92096580..23ca2680793 100644 --- a/apps/admin/src/editor/feature-image-caption.tsx +++ b/apps/admin/src/editor/feature-image-caption.tsx @@ -51,11 +51,12 @@ function CaptionMount({ onError={reportKoenigError} > +
diff --git a/apps/admin/src/editor/feature-image.tsx b/apps/admin/src/editor/feature-image.tsx index 5ea59c0af4a..b575d5c6fb6 100644 --- a/apps/admin/src/editor/feature-image.tsx +++ b/apps/admin/src/editor/feature-image.tsx @@ -117,7 +117,7 @@ export function FeatureImage({ {isEditingAlt ? ( onAltChange(event.target.value)} /> ) : ( -
+
+ @@ -100,11 +107,12 @@ export function ImageField({ disabled={isUploading} enabled={unsplashEnabled} label={`Select ${subject} from Unsplash`} + variant={variant === 'bar' ? 'inline' : 'overlay'} onSelect={(picked) => onUnsplashSelect ? onUnsplashSelect(picked) : onChange(picked.src) } /> - + ); } @@ -121,7 +129,10 @@ export function ImageField({ if (!children) { return ( - + {preview} ); @@ -129,7 +140,7 @@ export function ImageField({ return ( - {preview} + {preview} {children} ); diff --git a/apps/admin/src/editor/post-editor.acceptance.test.tsx b/apps/admin/src/editor/post-editor.acceptance.test.tsx index cc9e02a7300..0e6de177783 100644 --- a/apps/admin/src/editor/post-editor.acceptance.test.tsx +++ b/apps/admin/src/editor/post-editor.acceptance.test.tsx @@ -15,6 +15,7 @@ import { post, renderAdminApp, staffRole, + withoutAutosave, type RenderAdminAppOptions, } from '@test-utils/acceptance'; import { editorScreen } from '@/editor/editor.screen'; @@ -83,6 +84,37 @@ function pasteText(content: string) { * editor-save.acceptance.test.tsx. */ describe('Post editor', () => { + it('grows and shrinks the title with its text under the Ember host constraints', async () => { + // The acceptance host omits Ember's global form CSS, which still surrounds + // the React editor in production. + const hostStyles = document.createElement('style'); + hostStyles.textContent = 'textarea { min-height: 10rem; max-width: 500px; }'; + document.head.appendChild(hostStyles); + + try { + fakeEditorPost({ title: 'Short title' }); + await renderAdminApp(`/editor/post/${POST_ID}`, withoutAutosave(FLAG_ON)); + + const title = editorScreen.titleInput(); + await expect.element(title).toHaveValue('Short title'); + const height = () => title.element().getBoundingClientRect().height; + const singleLineHeight = height(); + const lineHeight = parseFloat(getComputedStyle(title.element()).lineHeight); + expect(singleLineHeight).toBeLessThan(lineHeight * 2); + expect(title.element().getBoundingClientRect().width).toBeGreaterThan(500); + + await title.fill( + 'A long post title that wraps across several lines in the writing area '.repeat(3), + ); + await expect.poll(height).toBeGreaterThan(singleLineHeight * 2); + + await title.fill('Short title'); + await expect.poll(height).toBe(singleLineHeight); + } finally { + hostStyles.remove(); + } + }); + it('loads the post into the title and body', async () => { const postsApi = fakeEditorPost(); await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); diff --git a/apps/admin/src/editor/post-editor.tsx b/apps/admin/src/editor/post-editor.tsx index 835329530b8..89d6796be7f 100644 --- a/apps/admin/src/editor/post-editor.tsx +++ b/apps/admin/src/editor/post-editor.tsx @@ -316,7 +316,7 @@ export function PostEditor({ autoFocus={autofocusTitle} className={cn( fieldClassName, - 'heading-font-features mb-4 text-4xl leading-tight font-bold tracking-tight text-foreground placeholder:font-bold placeholder:text-muted-foreground', + 'heading-font-features mb-4 min-h-0 max-w-none min-w-0 pb-1 text-[4.8rem] leading-[1.1] font-bold tracking-[-0.017em] text-foreground placeholder:font-bold placeholder:text-muted-foreground max-[769px]:text-[3.6rem] max-[501px]:text-[2.8rem]', )} data-testid={editorTitleInput} placeholder={`${capitalize(postType)} title`} diff --git a/apps/admin/src/editor/unsplash-picker.tsx b/apps/admin/src/editor/unsplash-picker.tsx index 33bc700c329..1e196261bf3 100644 --- a/apps/admin/src/editor/unsplash-picker.tsx +++ b/apps/admin/src/editor/unsplash-picker.tsx @@ -22,6 +22,7 @@ export interface UnsplashPickerProps { /** Names the button, e.g. `Select feature image from Unsplash`. */ label: string; disabled?: boolean; + variant?: 'overlay' | 'inline'; /** Places the button over the dropzone it sits on. */ className?: string; onSelect: (image: UnsplashSelection) => void; @@ -35,6 +36,7 @@ export function UnsplashPicker({ enabled, label, disabled, + variant = 'overlay', className, onSelect, }: UnsplashPickerProps) { @@ -78,15 +80,22 @@ export function UnsplashPicker({ diff --git a/apps/admin/src/shared/virtual-list/use-virtual-list-window.test.ts b/apps/admin/src/shared/virtual-list/use-virtual-list-window.test.ts index 02cdf2e1b67..8326dcdc969 100644 --- a/apps/admin/src/shared/virtual-list/use-virtual-list-window.test.ts +++ b/apps/admin/src/shared/virtual-list/use-virtual-list-window.test.ts @@ -1,5 +1,5 @@ -import React from 'react'; -import { MemoryRouter, useSearchParams } from 'react-router'; +import React, { useLayoutEffect } from 'react'; +import { MemoryRouter, useNavigate, useSearchParams } from 'react-router'; import { act, renderHook } from '@testing-library/react'; import { beforeEach, describe, expect, it } from 'vitest'; import type { ReactNode } from 'react'; @@ -68,6 +68,32 @@ describe('useVirtualListWindow', () => { }); }); + it('commits the reset window before layout effects run for a fresh sidebar entry', () => { + const committedCounts: number[] = []; + const { result } = renderHook( + () => { + const state = useVirtualListWindow(5000); + const navigate = useNavigate(); + useLayoutEffect(() => { + committedCounts.push(state.visibleItemCount); + }); + return { ...state, navigate }; + }, + { wrapper: createWrapper('/members') }, + ); + + act(() => result.current.loadMore()); + expect(result.current.visibleItemCount).toBe(2000); + committedCounts.length = 0; + + act(() => { + window.history.replaceState({}, ''); + void result.current.navigate('/members'); + }); + + expect(committedCounts).toEqual([1000]); + }); + it('shows all items when the total is below the cap', () => { const { result } = renderHook(() => useVirtualListWindow(125), { wrapper: createWrapper('/members?filter=members'), diff --git a/apps/admin/src/shared/virtual-list/use-virtual-list-window.ts b/apps/admin/src/shared/virtual-list/use-virtual-list-window.ts index ac7a4410436..3b427ecd8de 100644 --- a/apps/admin/src/shared/virtual-list/use-virtual-list-window.ts +++ b/apps/admin/src/shared/virtual-list/use-virtual-list-window.ts @@ -1,5 +1,5 @@ import { readListReturnState, rememberListReturnState } from './list-return-state'; -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useState } from 'react'; import { useLocation } from '@tryghost/admin-x-framework'; const DEFAULT_VIRTUAL_LIST_WINDOW_SIZE = 1000; @@ -117,38 +117,35 @@ export function useVirtualListWindow( const { key: locationEntryKey, pathname, search } = useLocation(); const effectiveResetKey = resetKey ?? search; const historyKey = getVirtualListWindowHistoryKey(pathname, effectiveResetKey); - const [unlockedItemCount, setUnlockedItemCount] = useState(() => { - return getStoredUnlockedItemCount( + const readUnlockedItemCount = () => + getStoredUnlockedItemCount( getCurrentHistoryState(), historyKey, readListReturnState(getCurrentHistoryState(), pathname + search)?.unlockedItemCount ?? windowSize, ); - }); - const previousHistoryKeyRef = useRef(historyKey); - const previousEntryKeyRef = useRef(locationEntryKey); + const [windowState, setWindowState] = useState(() => ({ + historyKey, + entryKey: locationEntryKey, + unlockedItemCount: readUnlockedItemCount(), + })); + let { unlockedItemCount } = windowState; + + // Commit the new window together with navigation. Resetting it in an effect + // lets the virtualizer measure old rows after scroll restoration, which can + // move a fresh sidebar entry back down the list. + if ( + windowState.historyKey !== historyKey || + (resetKey === undefined && windowState.entryKey !== locationEntryKey) + ) { + unlockedItemCount = readUnlockedItemCount(); + setWindowState({ historyKey, entryKey: locationEntryKey, unlockedItemCount }); + } useEffect(() => { - if ( - previousHistoryKeyRef.current !== historyKey || - (resetKey === undefined && previousEntryKeyRef.current !== locationEntryKey) - ) { - previousEntryKeyRef.current = locationEntryKey; - previousHistoryKeyRef.current = historyKey; - setUnlockedItemCount( - getStoredUnlockedItemCount( - getCurrentHistoryState(), - historyKey, - readListReturnState(getCurrentHistoryState(), pathname + search)?.unlockedItemCount ?? - windowSize, - ), - ); - return; - } - setStoredUnlockedItemCount(getCurrentHistoryState(), historyKey, unlockedItemCount); rememberListReturnState(pathname + search, { unlockedItemCount }); - }, [historyKey, locationEntryKey, unlockedItemCount, windowSize, pathname, search, resetKey]); + }, [historyKey, locationEntryKey, unlockedItemCount, pathname, search]); const { visibleItemCount, canLoadMore } = getVirtualListWindowState({ totalItems, @@ -159,6 +156,9 @@ export function useVirtualListWindow( visibleItemCount, canLoadMore, loadMore: () => - setUnlockedItemCount((current) => getNextUnlockedItemCount(current, windowSize)), + setWindowState((current) => ({ + ...current, + unlockedItemCount: getNextUnlockedItemCount(current.unlockedItemCount, windowSize), + })), }; } From 1d7c38e053a4a18c3a09b8d34c8991d895c2a8c5 Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Tue, 29 Sep 2026 14:08:16 +0200 Subject: [PATCH 08/16] Fixed lists jumping back to their old position after a sidebar click (#31078) no ref Flaky test fix. After returning to a scrolled list, clicking its sidebar link could leave the fresh list at the old position instead of the top (flaky `e2e/tests/admin/members/virtual-window.test.ts`). The reset wrote `scrollTop` directly, but the virtualizer only picks up the new offset from the next frame's `scroll` event. If the shorter fresh list rendered first, it corrected row sizes against the old offset and scrolled back. Scroll writes now dispatch the `scroll` event themselves, and the departing entry's listener ignores it so Back still restores that entry. --- ...ers-scroll-restoration.acceptance.test.tsx | 42 +++++++++++++++++++ apps/admin/src/members/members.screen.ts | 17 ++++++++ .../use-scroll-restoration.test.tsx | 17 ++++++++ .../virtual-list/use-scroll-restoration.tsx | 15 ++++++- .../pages/admin/members/members-page.ts | 11 +++-- .../test-data/src/selectors/members.ts | 2 + 6 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 apps/admin/src/members/members-scroll-restoration.acceptance.test.tsx diff --git a/apps/admin/src/members/members-scroll-restoration.acceptance.test.tsx b/apps/admin/src/members/members-scroll-restoration.acceptance.test.tsx new file mode 100644 index 00000000000..4f2b816d457 --- /dev/null +++ b/apps/admin/src/members/members-scroll-restoration.acceptance.test.tsx @@ -0,0 +1,42 @@ +import { describe, expect, it } from 'vitest'; + +import { fakeMembers, fakeTags, member, renderAdminApp } from '@test-utils/acceptance'; +import { sidebarScreen } from '@/layout/sidebar.screen'; +import { membersScreen } from './members.screen'; + +describe('Members list scroll restoration', () => { + it('opens a fresh list at the top from the sidebar after restoring a scrolled one', async () => { + fakeMembers(Array.from({ length: 1200 }, (_, index) => member({ name: `Member ${index}` }))); + fakeTags([]); + await renderAdminApp('/members'); + + // Scroll past the first 1000-row window, beyond the end of a fresh list. + await membersScreen.loadMoreButton().click(); + await expect.element(membersScreen.loadMoreButton()).not.toBeInTheDocument(); + membersScreen.listScrollElement()?.scrollTo({ top: Number.MAX_SAFE_INTEGER }); + await expect.poll(membersScreen.lastRenderedRowIndex).toBeGreaterThan(1000); + + await sidebarScreen.navLink('Tags').click(); + await expect(membersScreen.memberRows()).toHaveCount(0); + window.history.back(); + await expect.poll(membersScreen.lastRenderedRowIndex).toBeGreaterThan(1000); + + // On a busy main thread the reset's scroll event lands after the list + // re-renders; holding the browser's own scroll events back forces that order. + const holdScrollEvent = (event: Event) => { + if (event.isTrusted) { + event.stopImmediatePropagation(); + } + }; + window.addEventListener('scroll', holdScrollEvent, true); + try { + await sidebarScreen.navLink('Members').click(); + + await expect.element(membersScreen.loadMoreButton()).toBeVisible(); + await expect.element(membersScreen.link('Member 0')).toBeVisible(); + } finally { + window.removeEventListener('scroll', holdScrollEvent, true); + } + await expect.poll(() => membersScreen.listScrollElement()?.scrollTop).toBe(0); + }); +}); diff --git a/apps/admin/src/members/members.screen.ts b/apps/admin/src/members/members.screen.ts index 4492c123933..4d9ffc01857 100644 --- a/apps/admin/src/members/members.screen.ts +++ b/apps/admin/src/members/members.screen.ts @@ -1,9 +1,12 @@ import { page } from 'vitest/browser'; +import { getScrollParent } from '@tryghost/shade/utils'; import { addFilterButton, filterButton, + loadMoreButton, membersActions, membersListItem, + membersListScrollRoot, newMemberLink, noResultsText, searchLabel, @@ -22,6 +25,7 @@ export const membersScreen = { showAllButton: () => page.getByRole('button', { name: showAllButton }), emptyState: () => page.getByText('Start building your audience'), actionsButton: () => page.getByTestId(membersActions), + loadMoreButton: () => page.getByRole('button', { name: loadMoreButton }), dialog: () => page.getByRole('dialog'), menuItem: (name: string | RegExp) => page.getByRole('menuitem', { name }), @@ -51,6 +55,19 @@ export const membersScreen = { } }, + /** The shell element whose scroll position drives the virtualized rows. */ + listScrollElement: () => + getScrollParent(document.querySelector(`[data-testid="${membersListScrollRoot}"]`)), + + /** The highest row index the virtualizer has rendered with data (-1 when none). */ + lastRenderedRowIndex: () => + Math.max( + -1, + ...Array.from(document.querySelectorAll(`[data-testid="${membersListItem}"]`), (row) => + Number(row.getAttribute('data-index')), + ), + ), + multiselectOption: (name: string) => page.getByRole('option', { name: new RegExp(`^${name}\\b`) }), diff --git a/apps/admin/src/shared/virtual-list/use-scroll-restoration.test.tsx b/apps/admin/src/shared/virtual-list/use-scroll-restoration.test.tsx index 77147796b2a..0ea4dcbe552 100644 --- a/apps/admin/src/shared/virtual-list/use-scroll-restoration.test.tsx +++ b/apps/admin/src/shared/virtual-list/use-scroll-restoration.test.tsx @@ -108,6 +108,23 @@ describe('list scroll restoration', () => { expect(getListReturnNavigationState('/posts')?.listReturn.scrollPosition).toBe(0); }); + it('keeps the previous entry position when a new entry resets to the top', async () => { + const { rerender } = mount(true); + const previousKey = location.key; + scrollContainer.scrollTop = 1500; + await act(() => scrollContainer.dispatchEvent(new Event('scroll'))); + entry += 1; + location.key = `entry-${entry}`; + window.history.replaceState({ key: location.key }, ''); + rerender({ isLoading: false }); + expect(scrollContainer.scrollTop).toBe(0); + + location.key = previousKey; + window.history.replaceState({ key: previousKey }, ''); + rerender({ isLoading: false }); + expect(scrollContainer.scrollTop).toBe(1500); + }); + it('keeps in-place Comments thread navigation at its existing position', () => { location.pathname = '/comments'; const { rerender } = mount(); diff --git a/apps/admin/src/shared/virtual-list/use-scroll-restoration.tsx b/apps/admin/src/shared/virtual-list/use-scroll-restoration.tsx index 5f468210dba..2d98bee9de1 100644 --- a/apps/admin/src/shared/virtual-list/use-scroll-restoration.tsx +++ b/apps/admin/src/shared/virtual-list/use-scroll-restoration.tsx @@ -88,6 +88,13 @@ function setStoredScrollPosition( window.history.replaceState(nextState, ''); } +function setScrollPosition(element: HTMLElement, position: number) { + element.scrollTop = position; + // The scroll event for a programmatic write lands next frame; until then the + // virtualizer corrects row resizes against its old offset and scrolls back. + element.dispatchEvent(new Event('scroll')); +} + interface UseScrollRestorationOptions { /** Reference to the element whose scroll parent should be tracked */ parentRef: RefObject; @@ -226,6 +233,10 @@ export function useScrollRestoration({ }; const handleScroll = () => { + // Until cleanup, the entry navigated away from still hears the next entry's reset. + if (getHistoryEntryKey(getCurrentHistoryState()) !== sourceHistoryEntryKey) { + return; + } latestScrollPositionRef.current = scrollContainer.scrollTop; rememberListReturnState(key, { scrollPosition: scrollContainer.scrollTop }); queuePersistScrollPosition(); @@ -307,7 +318,7 @@ export function useScrollRestoration({ // Restore the position if (Math.abs(savedPosition - currentScroll) > 5) { const targetPosition = Math.min(savedPosition, maxScroll); - scrollContainer.scrollTop = targetPosition; + setScrollPosition(scrollContainer, targetPosition); } rememberListReturnState(key, { scrollPosition: scrollContainer.scrollTop }); }; @@ -320,7 +331,7 @@ export function useScrollRestoration({ // visited earlier. Only Back and explicit breadcrumb state restore it. if (savedPosition === undefined && previousEntryRef.current !== entryKey) { if (resetOnNavigation) { - scrollContainer.scrollTop = 0; + setScrollPosition(scrollContainer, 0); } rememberListReturnState(key, { scrollPosition: scrollContainer.scrollTop }); } diff --git a/e2e/helpers/pages/admin/members/members-page.ts b/e2e/helpers/pages/admin/members/members-page.ts index 53f7d229055..ce02cca5ded 100644 --- a/e2e/helpers/pages/admin/members/members-page.ts +++ b/e2e/helpers/pages/admin/members/members-page.ts @@ -1,6 +1,11 @@ import { AdminPage } from '@/admin-pages'; import { JSHandle, Locator, Page } from '@playwright/test'; -import { membersListItem, newMemberLink } from '@tryghost/test-data/selectors/members'; +import { + loadMoreButton, + membersListItem, + membersListScrollRoot, + newMemberLink, +} from '@tryghost/test-data/selectors/members'; export class MembersPage extends AdminPage { readonly newMemberButton: Locator; @@ -14,8 +19,8 @@ export class MembersPage extends AdminPage { this.newMemberButton = page.getByRole('link', { name: newMemberLink }); - this.loadMoreButton = page.getByRole('button', { name: 'Load more' }); - this.membersListScrollRoot = page.getByTestId('members-list-scroll-root'); + this.loadMoreButton = page.getByRole('button', { name: loadMoreButton }); + this.membersListScrollRoot = page.getByTestId(membersListScrollRoot); this.memberListItems = page.getByTestId(membersListItem); } diff --git a/packages/testing/test-data/src/selectors/members.ts b/packages/testing/test-data/src/selectors/members.ts index a68fd38f48e..92792ef5540 100644 --- a/packages/testing/test-data/src/selectors/members.ts +++ b/packages/testing/test-data/src/selectors/members.ts @@ -5,6 +5,7 @@ // testids export const membersListItem = 'members-list-item'; +export const membersListScrollRoot = 'members-list-scroll-root'; export const membersSearchInput = 'members-search-input'; export const membersActions = 'members-actions'; export const memberDetail = 'member-detail'; @@ -28,6 +29,7 @@ export const newMemberLink = 'New member'; export const showAllButton = 'Show all members'; export const addYourselfButton = 'Add yourself as a member'; export const importCsvLink = 'Import with CSV'; +export const loadMoreButton = 'Load more'; // accessible-name prefixes (the import mapping table names controls per CSV column, // and member detail names its per-field edit buttons) From e5b6c6b98dccb7dbddc62299e8de12836da9c319 Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Tue, 29 Sep 2026 14:39:58 +0200 Subject: [PATCH 09/16] Added React auth screens behind the authReact flag (#31067) no ref Adds React versions of Admin's sign in, 2FA verification, password reset, staff invite signup, setup and sign out. They are served when the new private Labs flag `authReact` is on, and by Ember otherwise. Behaviour, copy and validation match the Ember screens, though there's a visual change that'll need to be reviewed. The purpose is to make these screens Better Auth 'shaped' so that when we're able to turn to implementing that, that there's less churn needed. --- .../src/api/authentication.ts | 97 +++++++ .../admin-x-framework/src/api/current-user.ts | 3 + apps/admin-x-framework/src/api/session.ts | 14 + apps/admin-x-framework/src/api/site.ts | 2 + apps/admin-x-framework/src/hooks.ts | 1 + .../test/unit/api/authentication.test.tsx | 147 ++++++++++ .../test/unit/api/session.test.tsx | 40 +++ apps/admin/src/app.tsx | 13 +- apps/admin/src/auth/README.md | 62 ++++ apps/admin/src/auth/api.ts | 9 + apps/admin/src/auth/auth-layout.tsx | 100 +++++++ apps/admin/src/auth/auth-notice.ts | 40 +++ apps/admin/src/auth/auth-route.tsx | 90 ++++++ apps/admin/src/auth/auth-routes.tsx | 19 ++ apps/admin/src/auth/auth.screen.ts | 29 ++ apps/admin/src/auth/client/auth-client.ts | 110 ++++++++ .../src/auth/client/ghost-auth-client.ts | 265 ++++++++++++++++++ apps/admin/src/auth/password-rules.test.ts | 47 ++++ apps/admin/src/auth/password-rules.ts | 65 +++++ apps/admin/src/auth/reload.ts | 10 + apps/admin/src/auth/reset.acceptance.test.tsx | 82 ++++++ apps/admin/src/auth/reset.tsx | 115 ++++++++ apps/admin/src/auth/setup.acceptance.test.tsx | 189 +++++++++++++ apps/admin/src/auth/setup.tsx | 236 ++++++++++++++++ .../auth/signed-in-auth.acceptance.test.tsx | 81 ++++++ apps/admin/src/auth/signin-redirect.ts | 30 ++ .../auth/signin-verify.acceptance.test.tsx | 95 +++++++ apps/admin/src/auth/signin-verify.tsx | 142 ++++++++++ .../admin/src/auth/signin.acceptance.test.tsx | 257 +++++++++++++++++ apps/admin/src/auth/signin.tsx | 172 ++++++++++++ apps/admin/src/auth/signout.tsx | 30 ++ .../admin/src/auth/signup.acceptance.test.tsx | 129 +++++++++ apps/admin/src/auth/signup.tsx | 178 ++++++++++++ apps/admin/src/auth/use-auth-screens-owner.ts | 31 ++ apps/admin/src/routes.tsx | 19 +- .../advanced/labs/private-features.tsx | 6 + apps/admin/test-utils/acceptance/README.md | 1 + apps/admin/test-utils/acceptance/auth.ts | 57 ++++ apps/admin/test-utils/acceptance/index.ts | 1 + apps/ember-admin/app/routes/authenticated.js | 9 + apps/ember-admin/app/routes/reset.js | 7 +- apps/ember-admin/app/routes/setup.js | 8 +- apps/ember-admin/app/routes/signout.js | 12 + apps/ember-admin/app/routes/signup.js | 7 +- .../ember-admin/app/routes/unauthenticated.js | 11 +- apps/ember-admin/app/services/feature.js | 11 + e2e/tests/admin/auth-react.test.ts | 130 +++++++++ e2e/tests/admin/staff-role-smoke.test.ts | 212 +++++++------- .../api/endpoints/utils/public-config/site.js | 3 + .../utils/serializers/output/site.js | 1 + ghost/core/core/shared/labs.js | 1 + .../admin/__snapshots__/site.test.js.snap | 1 + .../members/__snapshots__/site.test.js.snap | 3 + .../utils/public-config/site.test.js | 12 + .../testing/test-data/src/selectors/auth.ts | 30 ++ 55 files changed, 3357 insertions(+), 115 deletions(-) create mode 100644 apps/admin-x-framework/src/api/authentication.ts create mode 100644 apps/admin-x-framework/test/unit/api/authentication.test.tsx create mode 100644 apps/admin/src/auth/README.md create mode 100644 apps/admin/src/auth/api.ts create mode 100644 apps/admin/src/auth/auth-layout.tsx create mode 100644 apps/admin/src/auth/auth-notice.ts create mode 100644 apps/admin/src/auth/auth-route.tsx create mode 100644 apps/admin/src/auth/auth-routes.tsx create mode 100644 apps/admin/src/auth/auth.screen.ts create mode 100644 apps/admin/src/auth/client/auth-client.ts create mode 100644 apps/admin/src/auth/client/ghost-auth-client.ts create mode 100644 apps/admin/src/auth/password-rules.test.ts create mode 100644 apps/admin/src/auth/password-rules.ts create mode 100644 apps/admin/src/auth/reload.ts create mode 100644 apps/admin/src/auth/reset.acceptance.test.tsx create mode 100644 apps/admin/src/auth/reset.tsx create mode 100644 apps/admin/src/auth/setup.acceptance.test.tsx create mode 100644 apps/admin/src/auth/setup.tsx create mode 100644 apps/admin/src/auth/signed-in-auth.acceptance.test.tsx create mode 100644 apps/admin/src/auth/signin-redirect.ts create mode 100644 apps/admin/src/auth/signin-verify.acceptance.test.tsx create mode 100644 apps/admin/src/auth/signin-verify.tsx create mode 100644 apps/admin/src/auth/signin.acceptance.test.tsx create mode 100644 apps/admin/src/auth/signin.tsx create mode 100644 apps/admin/src/auth/signout.tsx create mode 100644 apps/admin/src/auth/signup.acceptance.test.tsx create mode 100644 apps/admin/src/auth/signup.tsx create mode 100644 apps/admin/src/auth/use-auth-screens-owner.ts create mode 100644 apps/admin/test-utils/acceptance/auth.ts create mode 100644 e2e/tests/admin/auth-react.test.ts create mode 100644 packages/testing/test-data/src/selectors/auth.ts diff --git a/apps/admin-x-framework/src/api/authentication.ts b/apps/admin-x-framework/src/api/authentication.ts new file mode 100644 index 00000000000..24219ad60f0 --- /dev/null +++ b/apps/admin-x-framework/src/api/authentication.ts @@ -0,0 +1,97 @@ +import { createMutation, createQuery } from '../utils/api/hooks'; + +// Types + +export interface SetupStatusResponseType { + setup: Array<{ + status: boolean; + // Prefill values from Ghost's config, only present before setup + title?: string; + name?: string; + email?: string; + }>; +} + +export interface InvitationStatusResponseType { + invitation: Array<{ valid: boolean }>; +} + +export interface PasswordResetResponseType { + password_reset: Array<{ message: string }>; +} + +export interface CompletePasswordResetPayload { + token: string; + newPassword: string; + ne2Password: string; +} + +export interface AcceptInvitationPayload { + token: string; + name: string; + email: string; + password: string; +} + +export interface CompleteSetupPayload { + name: string; + email: string; + password: string; + blogTitle: string; +} + +// Every write here consumes something single-use (a reset token, an invite, +// the unset-up site), so a retried request after a lost response would fail +// against its own first attempt. None of these endpoints needs a session, so +// their 401s are answers rather than an expired session. +const authenticationRequestOptions = { retry: false, sessionExpiryRedirect: false } as const; + +// Requests + +export const useSetupStatus = createQuery({ + dataType: 'SetupStatusResponseType', + path: '/authentication/setup/', +}); + +/** Whether a sent, unaccepted invitation exists for the email; the server does not check expiry here. */ +export const useInvitationStatus = createQuery({ + dataType: 'InvitationStatusResponseType', + path: '/authentication/invitation/', +}); + +export const useRequestPasswordReset = createMutation( + { + method: 'POST', + path: () => '/authentication/password_reset/', + body: ({ email }) => ({ password_reset: [{ email }] }), + ...authenticationRequestOptions, + }, +); + +// On success the server also signs the user in with an already verified session. +export const useCompletePasswordReset = createMutation< + PasswordResetResponseType, + CompletePasswordResetPayload +>({ + method: 'PUT', + path: () => '/authentication/password_reset/', + body: (payload) => ({ password_reset: [payload] }), + ...authenticationRequestOptions, +}); + +// Creates the account without signing in; `email` is ignored by current servers +// (the invite's own address is used) but required by older ones. +export const useAcceptInvitation = createMutation({ + method: 'POST', + path: () => '/authentication/invitation/', + body: (payload) => ({ invitation: [payload] }), + ...authenticationRequestOptions, +}); + +// Creates the owner account without signing in. +export const useCompleteSetup = createMutation({ + method: 'POST', + path: () => '/authentication/setup/', + body: (payload) => ({ setup: [payload] }), + ...authenticationRequestOptions, +}); diff --git a/apps/admin-x-framework/src/api/current-user.ts b/apps/admin-x-framework/src/api/current-user.ts index e22fc6d856c..91d7fa3e36b 100644 --- a/apps/admin-x-framework/src/api/current-user.ts +++ b/apps/admin-x-framework/src/api/current-user.ts @@ -24,6 +24,9 @@ export const useCurrentUser = ({ requestOptions }: CurrentUserOptions = {}) => { queryKey: currentUserQueryKey, queryFn: () => fetchApi(currentUserUrl, requestOptions), select: (data) => data.users[0], + // Every query hook reads the current user for permissions, so each new + // screen would otherwise re-ask a signed-out server; signing in reloads. + retryOnMount: false, }); useEffect(() => { diff --git a/apps/admin-x-framework/src/api/session.ts b/apps/admin-x-framework/src/api/session.ts index 6647d45f5a4..fcbac3adaf1 100644 --- a/apps/admin-x-framework/src/api/session.ts +++ b/apps/admin-x-framework/src/api/session.ts @@ -10,11 +10,16 @@ export interface SessionVerification { token: string; } +// Each call is single-use on the server (a new session, a rotated or consumed +// code), so a retried request after a lost response would undo the first one. +const sessionRequestOptions = { retry: false } as const; + // The server replies 201 Created with only the status text ("Created") as a text/plain body. export const useAddSession = createMutation({ method: 'POST', path: () => '/session/', body: (credentials) => credentials, + ...sessionRequestOptions, }); // The server replies 200 OK with only the status text ("OK") as a text/plain body; a wrong code is a bare 401. @@ -22,12 +27,21 @@ export const useVerifySession = createMutation({ method: 'PUT', path: () => '/session/verify/', body: ({ token }) => ({ token }), + ...sessionRequestOptions, +}); + +// Emails a fresh sign-in code (invalidating the previous one); the server replies 200 "OK" as text/plain. +export const useSendSessionVerification = createMutation({ + method: 'POST', + path: () => '/session/verify/', + ...sessionRequestOptions, }); // The server replies 204 No Content on sign-out, so the mutation resolves with no data. export const useDeleteSession = createMutation({ method: 'DELETE', path: () => '/session/', + ...sessionRequestOptions, }); const twoFactorRequiredCodes = ['2FA_TOKEN_REQUIRED', '2FA_NEW_DEVICE_DETECTED']; diff --git a/apps/admin-x-framework/src/api/site.ts b/apps/admin-x-framework/src/api/site.ts index d1b4ad7fbd0..6613c1f7e15 100644 --- a/apps/admin-x-framework/src/api/site.ts +++ b/apps/admin-x-framework/src/api/site.ts @@ -14,6 +14,8 @@ export type SiteData = { locale: string; version: string; site_uuid: string; + /** Whether Admin serves its React auth screens; absent on servers before the flag existed. */ + authReact?: boolean; }; export interface SiteResponseType { diff --git a/apps/admin-x-framework/src/hooks.ts b/apps/admin-x-framework/src/hooks.ts index adffcf95203..3fb874b01ba 100644 --- a/apps/admin-x-framework/src/hooks.ts +++ b/apps/admin-x-framework/src/hooks.ts @@ -11,6 +11,7 @@ export type { } from './hooks/use-form'; export { default as useHandleError } from './hooks/use-handle-error'; export { useFeatureFlag } from './hooks/use-feature-flag'; +export { useFeatureFlagOverrides } from './providers/feature-flag-overrides-context'; export { useHostLimits } from './hooks/use-host-limits'; export type { HostLimits } from './hooks/use-host-limits'; export { useLimiter } from './hooks/use-limiter'; diff --git a/apps/admin-x-framework/test/unit/api/authentication.test.tsx b/apps/admin-x-framework/test/unit/api/authentication.test.tsx new file mode 100644 index 00000000000..a09d39c93c8 --- /dev/null +++ b/apps/admin-x-framework/test/unit/api/authentication.test.tsx @@ -0,0 +1,147 @@ +import { act, waitFor } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import { renderHookWithProviders } from '../../../src/test/test-utils'; +import { + useAcceptInvitation, + useCompletePasswordReset, + useCompleteSetup, + useInvitationStatus, + useRequestPasswordReset, + useSetupStatus, +} from '../../../src/api/authentication'; +import { UnauthorizedError } from '../../../src/utils/errors'; +import { withMockFetch } from '../../utils/mock-fetch'; + +const json = { 'content-type': 'application/json' }; + +const requestedUrls = (mock: { calls: unknown[][] }) => mock.calls.map(([url]) => String(url)); + +describe('authentication api', () => { + it('reads the setup status', async () => { + await withMockFetch({ json: { setup: [{ status: true }] }, headers: json }, async (mock) => { + const { result } = renderHookWithProviders(() => useSetupStatus()); + + await waitFor(() => expect(result.current.data).toEqual({ setup: [{ status: true }] })); + expect(requestedUrls(mock)).toContain( + 'http://localhost:3000/ghost/api/admin/authentication/setup/', + ); + }); + }); + + it('checks an invitation by email', async () => { + await withMockFetch( + { json: { invitation: [{ valid: true }] }, headers: json }, + async (mock) => { + const { result } = renderHookWithProviders(() => + useInvitationStatus({ searchParams: { email: 'staff@example.com' } }), + ); + + await waitFor(() => expect(result.current.data?.invitation?.[0].valid).toBe(true)); + expect(requestedUrls(mock)).toContain( + 'http://localhost:3000/ghost/api/admin/authentication/invitation/?email=staff%40example.com', + ); + }, + ); + }); + + it.each([ + [ + 'requests a password reset', + () => useRequestPasswordReset(), + { email: 'owner@example.com' }, + 'POST', + '/authentication/password_reset/', + { password_reset: [{ email: 'owner@example.com' }] }, + ], + [ + 'completes a password reset', + () => useCompletePasswordReset(), + { token: 'dG9rZW4', newPassword: 'a-long-password', ne2Password: 'a-long-password' }, + 'PUT', + '/authentication/password_reset/', + { + password_reset: [ + { token: 'dG9rZW4', newPassword: 'a-long-password', ne2Password: 'a-long-password' }, + ], + }, + ], + [ + 'accepts an invitation', + () => useAcceptInvitation(), + { token: 'dG9rZW4', name: 'Jamie', email: 'staff@example.com', password: 'a-long-password' }, + 'POST', + '/authentication/invitation/', + { + invitation: [ + { + token: 'dG9rZW4', + name: 'Jamie', + email: 'staff@example.com', + password: 'a-long-password', + }, + ], + }, + ], + [ + 'completes setup', + () => useCompleteSetup(), + { name: 'Jamie', email: 'owner@example.com', password: 'a-long-password', blogTitle: 'Blog' }, + 'POST', + '/authentication/setup/', + { + setup: [ + { + name: 'Jamie', + email: 'owner@example.com', + password: 'a-long-password', + blogTitle: 'Blog', + }, + ], + }, + ], + ])('%s', async (_name, useHook, payload, method, path, body) => { + await withMockFetch({ json: {}, headers: json }, async (mock) => { + const { result } = renderHookWithProviders( + useHook as () => { mutateAsync: (value: object) => Promise }, + ); + + await act(async () => { + await result.current.mutateAsync(payload); + }); + + expect(mock.calls[0][0]).toBe(`http://localhost:3000/ghost/api/admin${path}`); + expect(mock.calls[0][1].method).toBe(method); + expect(JSON.parse(mock.calls[0][1].body)).toEqual(body); + }); + }); + + it('keeps the error body of a rejected reset link', async () => { + await withMockFetch( + { + json: { + errors: [{ message: 'Cannot reset password.', context: 'Invalid password reset link.' }], + }, + headers: json, + ok: false, + status: 401, + }, + async () => { + const { result } = renderHookWithProviders(() => useCompletePasswordReset()); + + let error: unknown; + await act(async () => { + try { + await result.current.mutateAsync({ token: 'bad', newPassword: 'x', ne2Password: 'x' }); + } catch (caught) { + error = caught; + } + }); + + expect(error).toBeInstanceOf(UnauthorizedError); + expect((error as UnauthorizedError).data).toEqual({ + errors: [{ message: 'Cannot reset password.', context: 'Invalid password reset link.' }], + }); + }, + ); + }); +}); diff --git a/apps/admin-x-framework/test/unit/api/session.test.tsx b/apps/admin-x-framework/test/unit/api/session.test.tsx index 70bc6d5464d..4195ffdb17e 100644 --- a/apps/admin-x-framework/test/unit/api/session.test.tsx +++ b/apps/admin-x-framework/test/unit/api/session.test.tsx @@ -5,6 +5,7 @@ import { isTwoFactorRequiredError, useAddSession, useDeleteSession, + useSendSessionVerification, useVerifySession, } from '../../../src/api/session'; import { @@ -15,6 +16,8 @@ import { } from '../../../src/utils/errors'; import { withMockFetch } from '../../utils/mock-fetch'; +const originalFetch = globalThis.fetch; + const passwordIncorrectResponse = { errors: [ { @@ -197,4 +200,41 @@ describe('session api', () => { expect(response).toBeUndefined(); }); }); + it('emails a new sign-in code via POST to the verify endpoint', async () => { + await withMockFetch( + { status: 200, headers: { 'content-type': 'text/plain; charset=utf-8' } }, + async (mock) => { + const { result } = renderHookWithProviders(() => useSendSessionVerification()); + + await act(async () => { + await result.current.mutateAsync(null); + }); + + expect(mock.calls[0][0]).toBe('http://localhost:3000/ghost/api/admin/session/verify/'); + expect(mock.calls[0][1].method).toBe('POST'); + expect(mock.calls[0][1].body).toBeUndefined(); + }, + ); + }); + + it('does not replay a sign-in whose response was lost', async () => { + const mockFetch = vi.fn(() => + Promise.reject(new TypeError('offline')), + ); + globalThis.fetch = mockFetch; + + try { + const { result } = renderHookWithProviders(() => useAddSession()); + + await act(async () => { + await expect( + result.current.mutateAsync({ username: 'owner@example.com', password: 'hunter22' }), + ).rejects.toBeDefined(); + }); + + expect(mockFetch).toHaveBeenCalledTimes(1); + } finally { + globalThis.fetch = originalFetch; + } + }); }); diff --git a/apps/admin/src/app.tsx b/apps/admin/src/app.tsx index 7378bbc8d0a..c5d398fb447 100644 --- a/apps/admin/src/app.tsx +++ b/apps/admin/src/app.tsx @@ -6,9 +6,14 @@ import { AdminLayout } from './layout/admin-layout'; import { useEmberAuthSync, useEmberDataSync, useEmberListReturnSync } from './ember-bridge'; import { DocsBotWidgetHost } from './docsbot-widget-host'; import { useAccentColorProperties } from './hooks/use-accent-color-properties'; +import { SignedOutApp, useAuthNotice, useAuthScreensOwner } from './auth/api'; function App() { - const { data: currentUser } = useCurrentUser(); + const { data: currentUser, errorUpdatedAt } = useCurrentUser(); + // Not `isError`: every new observer of the failed query refetches it and + // reports it pending meanwhile, which would unmount the signed-out screens. + const isSignedOut = !currentUser && errorUpdatedAt > 0; + const authScreensOwner = useAuthScreensOwner(); // Warm the settings cache at boot (as the removed AppProvider did): screens // hold on settings, and resolving it before routes mount keeps route guards // (e.g. force-upgrade) ahead of screen-level data fetches. @@ -17,6 +22,7 @@ function App() { useEmberAuthSync(); useEmberDataSync(); useEmberListReturnSync(); + useAuthNotice(Boolean(currentUser)); return ( @@ -26,6 +32,11 @@ function App() { + ) : isSignedOut && authScreensOwner === 'react' ? ( + <> + + + ) : ( <> diff --git a/apps/admin/src/auth/README.md b/apps/admin/src/auth/README.md new file mode 100644 index 00000000000..399955b96b4 --- /dev/null +++ b/apps/admin/src/auth/README.md @@ -0,0 +1,62 @@ +# Auth screens + +Sign in, sign-in verification, password reset, staff invite signup, first-run +setup and sign out, served at `/signin`, `/signin/verify`, `/reset/:token`, +`/signup/:token`, `/setup` and `/signout`. + +## Who serves them + +The screens render before anyone is signed in, so the authenticated `/config/` +Labs payload is unavailable. `useAuthScreensOwner` decides from inputs that are +public: the `authReact` field of `GET /site/`, or an `authReact` Labs URL +override (`/ghost/#/signin?labs=authReact`). A server without the field serves +the Ember screens. The answer is held for the page's lifetime. + +When React owns them, a signed-out visitor gets `SignedOutApp` in place of the +admin shell: auth routes render, any other route is remembered and replaced by +`/signin`, and a site that has not been set up sends every auth screen except +sign out to `/setup`. A signed-in visitor on an auth route goes home (with a +warning on reset and signup), except `/signout`. + +## The client contract + +Screens call `useAuthClient()` and the read hooks from `client/auth-client.ts` +and nothing else. The contract follows the BetterAuth client: + +- calls resolve to `{data, error}`; a request that reached the server never + throws, while a transport failure (and the global upgrade/maintenance states) + does; +- `error.code` is set only where a screen branches on it + (`USER_NOT_FOUND`, `INVALID_PASSWORD`, `PASSWORD_RESET_REQUIRED`, + `INVALID_CODE`); `error.message` is the text to show; +- sign in reports a required emailed code as data + (`{twoFactorRedirect: true}`), with Ghost's reason as an extension field; +- `invitation`, `setup`, `useSetupStatus`, `useInvitation` and + `getResetTokenEmail` are Ghost extensions. + +`client/ghost-auth-client.ts` implements it over Ghost's session and +authentication endpoints (through the framework hooks, which never retry these +single-use writes). Replacing the implementation means exporting a different +`useAuthClient` from `client/auth-client.ts`. Two server behaviours the screens +rely on and a replacement must keep: the first verification code is emailed +during sign in (the verify screen only calls `sendOtp` from Resend), and a +password reset may or may not sign the user in (the screen reloads either way, +landing on the admin or on sign in). + +## Session changes reload the page + +Every successful sign in, verification, reset, signup and setup, and every +sign out, ends in `reloadAdmin()`: the admin still boots a hidden Ember app +for the screens it serves, and it has to boot with the new session. The +reload lands directly on the destination: the route remembered in +`sessionStorage['ghost-signin-redirect']` (written by whichever shell sent the +visitor to sign in), `/` for role-based landing, `/?firstStart=true` after +setup, or `/signin` after signing out. A password reset leaves its confirmation +in `sessionStorage` for the reloaded admin to show. + +## Tests + +Acceptance specs boot signed out with `renderAdminApp(route, signedOut({authReact: true}))` +and fake `/authentication/setup/` with `fakeSetupStatus()`; `reloadAdmin` is +mocked. Signed-in specs live in their own file: once a page load has seen the +session work, a later 403 would trigger the session-expiry redirect. diff --git a/apps/admin/src/auth/api.ts b/apps/admin/src/auth/api.ts new file mode 100644 index 00000000000..0732423dc2d --- /dev/null +++ b/apps/admin/src/auth/api.ts @@ -0,0 +1,9 @@ +/** + * Public surface of the auth domain, consumed by the admin shell + * (apps/admin/src/app.tsx and routes.tsx). Everything else in this domain is + * internal. + */ +export { authRoutes, type AuthRouteHandle } from './auth-routes'; +export { SignedOutApp } from './auth-route'; +export { useAuthNotice } from './auth-notice'; +export { useAuthScreensOwner } from './use-auth-screens-owner'; diff --git a/apps/admin/src/auth/auth-layout.tsx b/apps/admin/src/auth/auth-layout.tsx new file mode 100644 index 00000000000..c2dbbafb776 --- /dev/null +++ b/apps/admin/src/auth/auth-layout.tsx @@ -0,0 +1,100 @@ +import { type CSSProperties, type ReactNode } from 'react'; +import { useBrowseSite } from '@tryghost/admin-x-framework/api/site'; +import { Button, LoadingIndicator } from '@tryghost/shade/components'; +import { Stack } from '@tryghost/shade/primitives'; +import { cn } from '@tryghost/shade/utils'; + +const GHOST_ORB = 'https://static.ghost.org/v4.0.0/images/ghost-orb-2.png'; + +/** + * Full-page frame for the auth screens. Settings are unreadable before sign + * in, so the site's accent colour comes from the public site payload here. + */ +export function AuthLayout({ children }: { children: ReactNode }) { + const { data } = useBrowseSite({ defaultErrorHandler: false }); + const accentColor = data?.site.accent_color; + + return ( +
+ + {children} + +
+ ); +} + +export function AuthHeader({ title, children }: { title: ReactNode; children?: ReactNode }) { + const { data } = useBrowseSite({ defaultErrorHandler: false }); + + return ( +
+ +

{title}

+ {children} +
+ ); +} + +/** The line under a form that carries its error, or a confirmation. */ +export function FlowMessage({ error, children }: { error?: boolean; children?: ReactNode }) { + return ( +

+ {children}  +

+ ); +} + +export type SubmitState = 'idle' | 'running' | 'failed'; + +/** + * The form's submit button: a spinner while the request runs (kept through a + * successful submit until the page reloads), and "Retry" after a failure. + * `accent` buttons use the site accent colour and turn red on failure. + */ +export function SubmitButton({ + state, + label, + runningLabel, + accent = false, + showRetry = true, +}: { + state: SubmitState; + label: string; + runningLabel?: string; + accent?: boolean; + showRetry?: boolean; +}) { + const failed = showRetry && state === 'failed'; + + return ( + + ); +} diff --git a/apps/admin/src/auth/auth-notice.ts b/apps/admin/src/auth/auth-notice.ts new file mode 100644 index 00000000000..193de03d440 --- /dev/null +++ b/apps/admin/src/auth/auth-notice.ts @@ -0,0 +1,40 @@ +import { useEffect } from 'react'; +import { toast } from 'sonner'; + +// Carries a confirmation across the reload that follows a session change. +const AUTH_NOTICE_KEY = 'ghost-admin:auth-notice'; + +const NOTICES = { + 'password-updated': 'Password updated', +} as const; + +type AuthNotice = keyof typeof NOTICES; + +const isAuthNotice = (value: string | null): value is AuthNotice => + value !== null && Object.hasOwn(NOTICES, value); + +export function leaveAuthNotice(notice: AuthNotice): void { + try { + window.sessionStorage.setItem(AUTH_NOTICE_KEY, notice); + } catch { + // Storage can be unavailable; the confirmation is then skipped. + } +} + +/** Shows a confirmation left before the last reload, once. */ +export function useAuthNotice(enabled: boolean): void { + useEffect(() => { + if (!enabled) { + return; + } + try { + const notice = window.sessionStorage.getItem(AUTH_NOTICE_KEY); + window.sessionStorage.removeItem(AUTH_NOTICE_KEY); + if (isAuthNotice(notice)) { + toast.info(NOTICES[notice]); + } + } catch { + // Nothing was left behind. + } + }, [enabled]); +} diff --git a/apps/admin/src/auth/auth-route.tsx b/apps/admin/src/auth/auth-route.tsx new file mode 100644 index 00000000000..d8c9b815755 --- /dev/null +++ b/apps/admin/src/auth/auth-route.tsx @@ -0,0 +1,90 @@ +import { createContext, lazy, Suspense, useContext, useEffect } from 'react'; +import { Navigate, Outlet, useLocation } from '@tryghost/admin-x-framework'; +import { isAuthPath } from '@tryghost/admin-x-framework/helpers'; +import { toast } from 'sonner'; +import { EmberFallback } from '@/ember-bridge'; +import { useSetupStatus } from './client/auth-client'; +import { rememberSigninRedirect } from './signin-redirect'; +import { useAuthScreensOwner } from './use-auth-screens-owner'; + +const screens = { + signin: lazy(() => import('./signin')), + signinVerify: lazy(() => import('./signin-verify')), + signout: lazy(() => import('./signout')), + signup: lazy(() => import('./signup')), + reset: lazy(() => import('./reset')), + setup: lazy(() => import('./setup')), +}; + +export type AuthScreen = keyof typeof screens; + +// Set by the signed-out shell; auth routes rendered anywhere else are being +// visited by someone who is signed in. +const SignedOutContext = createContext(false); + +const SIGNED_IN_WARNINGS: Partial> = { + reset: "You can't reset your password while you're signed in.", + signup: 'You need to sign out to register as a new user.', +}; + +function SignedInRedirect({ screen }: { screen: AuthScreen }) { + const warning = SIGNED_IN_WARNINGS[screen]; + + useEffect(() => { + if (warning) { + toast.warning(warning, { id: 'auth-signed-in' }); + } + }, [warning]); + + return ; +} + +/** Serves an auth route from React or Ember, whichever owns the auth screens. */ +export function AuthRoute({ screen }: { screen: AuthScreen }) { + const owner = useAuthScreensOwner(); + const signedOut = useContext(SignedOutContext); + const Screen = screens[screen]; + + if (owner === 'pending') { + return null; + } + if (owner === 'ember') { + return ; + } + if (!signedOut && screen !== 'signout') { + return ; + } + return ( + + + + ); +} + +/** + * The whole app for a signed-out visitor while React owns the auth screens: + * auth routes render, anything else is remembered for after sign in and + * sends the visitor to sign in. A site that isn't set up yet sends every + * auth screen but sign out to setup. + */ +export function SignedOutApp() { + const { pathname, search } = useLocation(); + const setup = useSetupStatus(); + const route = pathname.replace(/\/$/, '') || '/'; + + if (!isAuthPath(pathname)) { + rememberSigninRedirect(pathname + search); + return ; + } + if (setup.isPending) { + return null; + } + if (setup.data && !setup.data.isSetup && route !== '/setup' && route !== '/signout') { + return ; + } + return ( + + + + ); +} diff --git a/apps/admin/src/auth/auth-routes.tsx b/apps/admin/src/auth/auth-routes.tsx new file mode 100644 index 00000000000..3b1240f9e47 --- /dev/null +++ b/apps/admin/src/auth/auth-routes.tsx @@ -0,0 +1,19 @@ +import type { RouteObject } from '@tryghost/admin-x-framework'; +import { AuthRoute, type AuthScreen } from './auth-route'; + +export type AuthRouteHandle = { authScreen: AuthScreen }; + +const authRoute = (path: string, screen: AuthScreen): RouteObject => ({ + path, + element: , + handle: { authScreen: screen } satisfies AuthRouteHandle, +}); + +export const authRoutes: RouteObject[] = [ + authRoute('/signin', 'signin'), + authRoute('/signin/verify', 'signinVerify'), + authRoute('/signout', 'signout'), + authRoute('/signup/:token', 'signup'), + authRoute('/reset/:token', 'reset'), + authRoute('/setup', 'setup'), +]; diff --git a/apps/admin/src/auth/auth.screen.ts b/apps/admin/src/auth/auth.screen.ts new file mode 100644 index 00000000000..e1007bef84c --- /dev/null +++ b/apps/admin/src/auth/auth.screen.ts @@ -0,0 +1,29 @@ +import { page } from 'vitest/browser'; +import * as sel from '@tryghost/test-data/selectors/auth'; + +/** Auth screen locators and gestures for acceptance specs; no assertions. */ +export const authScreen = { + heading: (name: string) => page.getByRole('heading', { name }), + emailInput: () => page.getByLabelText(sel.emailLabel, { exact: true }), + passwordInput: () => page.getByLabelText(sel.passwordLabel, { exact: true }), + signInButton: () => page.getByRole('button', { name: sel.signInButton }), + forgotButton: () => page.getByRole('button', { name: sel.forgotButton }), + codeInput: () => page.getByLabelText(sel.verificationCodeLabel), + verifyButton: () => page.getByRole('button', { name: sel.verifyButton }), + resendButton: () => page.getByRole('button', { name: sel.resendButton }), + newPasswordInput: () => page.getByLabelText(sel.newPasswordLabel, { exact: true }), + confirmPasswordInput: () => page.getByLabelText(sel.confirmPasswordLabel), + saveNewPasswordButton: () => page.getByRole('button', { name: sel.saveNewPasswordButton }), + fullNameInput: () => page.getByLabelText(sel.fullNameLabel), + createAccountButton: () => page.getByRole('button', { name: sel.createAccountButton }), + siteTitleInput: () => page.getByLabelText(sel.siteTitleLabel), + startPublishingButton: () => page.getByRole('button', { name: sel.startPublishingButton }), + retryButton: () => page.getByRole('button', { name: sel.retryButton }), + text: (text: string | RegExp) => page.getByText(text, { exact: typeof text === 'string' }), + + async signIn(email: string, password: string): Promise { + await authScreen.emailInput().fill(email); + await authScreen.passwordInput().fill(password); + await authScreen.signInButton().click(); + }, +}; diff --git a/apps/admin/src/auth/client/auth-client.ts b/apps/admin/src/auth/client/auth-client.ts new file mode 100644 index 00000000000..052ffc378c6 --- /dev/null +++ b/apps/admin/src/auth/client/auth-client.ts @@ -0,0 +1,110 @@ +/** + * The client contract the auth screens are written against. It mirrors the + * BetterAuth client (`better-auth/react`): calls resolve to `{data, error}` + * instead of throwing, transport failures still throw, and errors carry a + * `code` the screens branch on. `invitation`, `setup` and the reads below are + * Ghost extensions with no BetterAuth equivalent. `describeUnexpectedError` + * turns a thrown failure into the text to show. + * + * The implementation is chosen here; screens import only from this module. + */ + +export type AuthErrorCode = + /** Sign in or forgot password: no staff user has that email address. */ + | 'USER_NOT_FOUND' + /** Sign in: the password does not match. */ + | 'INVALID_PASSWORD' + /** Sign in: the account is locked and a reset email has already been sent. */ + | 'PASSWORD_RESET_REQUIRED' + /** Verification: the emailed code is wrong or has expired. */ + | 'INVALID_CODE'; + +export interface AuthError { + status: number; + statusText: string; + code?: AuthErrorCode; + /** Text to show the user, when the server supplied one. */ + message?: string; +} + +export type AuthResult = { data: T; error: null } | { data: null; error: AuthError }; + +export type SignInData = + | { redirect: false } + | { + twoFactorRedirect: true; + twoFactorMethods: ['otp']; + /** + * Ghost extension: why the code was asked for, which changes the copy + * on the verification screen. The code has already been emailed. + */ + twoFactorReason?: 'required' | 'new-device'; + }; + +export interface AuthClient { + signIn: { + email(body: { email: string; password: string }): Promise>; + }; + twoFactor: { + /** Emails a new code, replacing the one sent at sign-in. */ + sendOtp(): Promise>; + verifyOtp(body: { code: string }): Promise>; + }; + requestPasswordReset(body: { email: string }): Promise>; + /** On Ghost the reset also signs the user in; screens reload either way. */ + resetPassword(body: { + newPassword: string; + token: string; + }): Promise>; + signOut(): Promise>; + invitation: { + /** Creates the invited staff user without signing in. */ + accept(body: { + token: string; + name: string; + password: string; + }): Promise>; + }; + setup: { + /** Creates the owner account without signing in. */ + create(body: { + name: string; + email: string; + password: string; + blogTitle: string; + }): Promise>; + }; + /** The email a password reset link was issued for, when the token carries one. */ + getResetTokenEmail(token: string): string | null; +} + +export interface QueryState { + data: T | undefined; + isPending: boolean; + isError: boolean; +} + +export interface SetupStatusState extends QueryState { + refetch: () => Promise; +} + +export interface SetupStatus { + isSetup: boolean; + /** Prefill values configured for a new site. */ + title?: string; + name?: string; + email?: string; +} + +export interface Invitation { + /** Null when the link is malformed. */ + email: string | null; + valid: boolean; +} + +export { + describeUnexpectedError, + useGhostAuthClient as useAuthClient, + useGhostInvitation as useInvitation, + useGhostSetupStatus as useSetupStatus, +} from './ghost-auth-client'; diff --git a/apps/admin/src/auth/client/ghost-auth-client.ts b/apps/admin/src/auth/client/ghost-auth-client.ts new file mode 100644 index 00000000000..5420d4fa02b --- /dev/null +++ b/apps/admin/src/auth/client/ghost-auth-client.ts @@ -0,0 +1,265 @@ +import { useMemo } from 'react'; +import { + useAcceptInvitation, + useCompletePasswordReset, + useCompleteSetup, + useInvitationStatus, + useRequestPasswordReset, + useSetupStatus as useSetupStatusQuery, +} from '@tryghost/admin-x-framework/api/authentication'; +import { + isTwoFactorRequiredError, + useAddSession, + useDeleteSession, + useSendSessionVerification, + useVerifySession, +} from '@tryghost/admin-x-framework/api/session'; +import { + APIError, + type ErrorResponse, + MaintenanceError, + UnauthorizedError, + VersionMismatchError, +} from '@tryghost/admin-x-framework/errors'; +import type { + AuthClient, + AuthError, + AuthErrorCode, + AuthResult, + Invitation, + QueryState, + SetupStatusState, +} from './auth-client'; + +type GhostError = Partial; + +interface Translation { + code?: AuthErrorCode; + message?: string; +} + +const firstGhostError = (error: APIError): GhostError | undefined => { + const { data } = error; + if (data && typeof data === 'object' && 'errors' in data && Array.isArray(data.errors)) { + return data.errors[0] as GhostError | undefined; + } + return undefined; +}; + +// Ghost's answers become an AuthError. Anything else throws: no response, a +// response without Ghost's error body (e.g. a proxy's 502), and the +// upgrade/maintenance states, which have their own copy. +function toAuthError( + error: unknown, + translate: (ghostError: GhostError, status: number) => Translation, +): AuthError { + const ghostError = error instanceof APIError ? firstGhostError(error) : undefined; + if ( + !(error instanceof APIError) || + !error.response || + !ghostError || + error instanceof VersionMismatchError || + error instanceof MaintenanceError + ) { + throw error; + } + const { status, statusText } = error.response; + return { status, statusText, ...translate(ghostError, status) }; +} + +/** The text for a failure that never produced an AuthError, falling back to the caller's own. */ +export function describeUnexpectedError(error: unknown, fallback: string): string { + if (error instanceof VersionMismatchError) { + return 'Ghost has been upgraded, please copy any unsaved data and refresh the page to continue.'; + } + if (error instanceof MaintenanceError) { + return 'Sorry, Ghost is currently undergoing maintenance, please wait a moment then try again.'; + } + return fallback; +} + +async function settle( + request: Promise, + toData: (response: unknown) => T, + translate: (ghostError: GhostError, status: number) => Translation, +): Promise> { + try { + return { data: toData(await request), error: null }; + } catch (error) { + return { data: null, error: toAuthError(error, translate) }; + } +} + +const translateSignIn = (ghost: GhostError, status: number): Translation => { + let code: AuthErrorCode | undefined; + if (status === 404) { + code = 'USER_NOT_FOUND'; + } else if (ghost.code === 'PASSWORD_INCORRECT') { + code = 'INVALID_PASSWORD'; + } else if (ghost.type === 'PasswordResetRequiredError') { + code = 'PASSWORD_RESET_REQUIRED'; + } + const message = + ghost.type === 'TooManyRequestsError' ? ghost.message : ghost.context || ghost.message; + return { code, message: message || undefined }; +}; + +const messageOnly = ({ message }: GhostError): Translation => ({ message: message || undefined }); + +const joined = (...parts: Array) => + parts.filter(Boolean).join(' ') || undefined; + +/** Invite and reset tokens are base64url of `expiry|email|hash`. */ +function decodeTokenEmail(token: string): string | null { + try { + const base64 = token.replace(/-/g, '+').replace(/_/g, '/'); + return window.atob(base64.padEnd(Math.ceil(base64.length / 4) * 4, '=')).split('|')[1] || null; + } catch { + return null; + } +} + +const INVITE_TOKEN = /^(?:[A-Za-z0-9_-]{4})*(?:[A-Za-z0-9_-]{2}|[A-Za-z0-9_-]{3})?$/; + +export function useGhostAuthClient(): AuthClient { + const { mutateAsync: addSession } = useAddSession(); + const { mutateAsync: verifySession } = useVerifySession(); + const { mutateAsync: sendVerification } = useSendSessionVerification(); + const { mutateAsync: deleteSession } = useDeleteSession(); + const { mutateAsync: requestReset } = useRequestPasswordReset(); + const { mutateAsync: completeReset } = useCompletePasswordReset(); + const { mutateAsync: acceptInvitation } = useAcceptInvitation(); + const { mutateAsync: completeSetup } = useCompleteSetup(); + + return useMemo( + () => ({ + signIn: { + async email({ email, password }) { + try { + await addSession({ username: email, password }); + return { data: { redirect: false }, error: null }; + } catch (error) { + if (isTwoFactorRequiredError(error)) { + const code = error.data?.errors?.[0]?.code; + return { + data: { + twoFactorRedirect: true, + twoFactorMethods: ['otp'], + twoFactorReason: code === '2FA_TOKEN_REQUIRED' ? 'required' : 'new-device', + }, + error: null, + }; + } + return { data: null, error: toAuthError(error, translateSignIn) }; + } + }, + }, + twoFactor: { + sendOtp: () => + settle(sendVerification(null), () => ({ status: true as const }), messageOnly), + async verifyOtp({ code }) { + try { + await verifySession({ token: code }); + return { data: { redirect: false }, error: null }; + } catch (error) { + // A wrong or expired code is a bare 401 with a text body. + if (error instanceof UnauthorizedError && error.response?.status === 401) { + const { status, statusText } = error.response; + return { data: null, error: { status, statusText, code: 'INVALID_CODE' } }; + } + return { data: null, error: toAuthError(error, messageOnly) }; + } + }, + }, + requestPasswordReset: ({ email }) => + settle( + requestReset({ email }), + () => ({ status: true as const }), + (ghost, status) => ({ + ...messageOnly(ghost), + code: status === 404 ? 'USER_NOT_FOUND' : undefined, + }), + ), + resetPassword: ({ newPassword, token }) => + settle( + completeReset({ token, newPassword, ne2Password: newPassword }), + () => ({ status: true as const }), + (ghost) => ({ + message: + ghost.context === ghost.message + ? ghost.message + : joined(ghost.message, ghost.context), + }), + ), + signOut: () => settle(deleteSession(null), () => ({ success: true as const }), messageOnly), + invitation: { + accept: ({ token, name, password }) => + settle( + acceptInvitation({ token, name, password, email: decodeTokenEmail(token) ?? '' }), + () => ({ status: true as const }), + messageOnly, + ), + }, + setup: { + create: (body) => + settle( + completeSetup(body), + () => ({ status: true as const }), + (ghost) => ({ + message: joined(ghost.message, ghost.context), + }), + ), + }, + getResetTokenEmail: decodeTokenEmail, + }), + [ + acceptInvitation, + addSession, + completeReset, + completeSetup, + deleteSession, + requestReset, + sendVerification, + verifySession, + ], + ); +} + +const decodeApostrophes = (value?: string) => value?.replace(/'/gi, "'"); + +export function useGhostSetupStatus(): SetupStatusState { + const { data, isLoading, isError, refetch } = useSetupStatusQuery({ defaultErrorHandler: false }); + const setup = data?.setup?.[0]; + + return { + data: setup && { + isSetup: setup.status === true, + title: decodeApostrophes(setup.title), + name: decodeApostrophes(setup.name), + email: setup.email, + }, + isPending: isLoading, + isError, + refetch, + }; +} + +export function useGhostInvitation(token: string): QueryState { + const email = INVITE_TOKEN.test(token) ? decodeTokenEmail(token) : null; + const { data, isLoading, isError } = useInvitationStatus({ + searchParams: { email: email ?? '' }, + enabled: email !== null, + defaultErrorHandler: false, + }); + + if (email === null) { + return { data: { email: null, valid: false }, isPending: false, isError: false }; + } + + // An unanswered check still shows the form; accepting reports a bad invite. + return { + data: data || isError ? { email, valid: data?.invitation?.[0]?.valid !== false } : undefined, + isPending: isLoading, + isError, + }; +} diff --git a/apps/admin/src/auth/password-rules.test.ts b/apps/admin/src/auth/password-rules.test.ts new file mode 100644 index 00000000000..db7ddc2b150 --- /dev/null +++ b/apps/admin/src/auth/password-rules.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest'; +import { passwordProblems } from './password-rules'; + +const context = { + email: 'jamie@example.com', + siteTitle: 'The Daily Awesome', + siteUrl: 'https://daily.example.com/', +}; + +describe('passwordProblems', () => { + it('accepts a long, unrelated password', () => { + expect(passwordProblems('correct horse battery', context)).toEqual([]); + }); + + it('counts characters, not UTF-16 units, for the length rule', () => { + expect(passwordProblems('🔒🔒🔒🔒🔒abcd', context)).toContain( + 'Password must be at least 10 characters long.', + ); + }); + + it('stops at the length rule', () => { + expect(passwordProblems('ghost', context)).toEqual([ + 'Password must be at least 10 characters long.', + ]); + }); + + it.each([ + ['1234567890', 'Sorry, you cannot use an insecure password.'], + ['JAMIE@example.com', 'Sorry, you cannot use the email as your password.'], + ['myghostpassword1', 'Sorry, you cannot use a password including common phrases.'], + ['the daily awesome', 'Sorry, you cannot use the blog title as your password.'], + ['daily.example.com', 'Sorry, you cannot use the blog URL as your password.'], + ['daily.example.com/', 'Sorry, you cannot use the blog URL as your password.'], + ['aaaaaaaaaabc', 'Sorry, you cannot use an insecure password.'], + ])('rejects %s', (password, problem) => { + expect(passwordProblems(password, context)).toContain(problem); + }); + + it('lists every broken rule in check order', () => { + expect(passwordProblems('passwordpassword', context)).toEqual([ + 'Sorry, you cannot use a password including common phrases.', + ]); + expect(passwordProblems('aaaaaaaaaa', context)).toEqual([ + 'Sorry, you cannot use an insecure password.', + ]); + }); +}); diff --git a/apps/admin/src/auth/password-rules.ts b/apps/admin/src/auth/password-rules.ts new file mode 100644 index 00000000000..74e920dc204 --- /dev/null +++ b/apps/admin/src/auth/password-rules.ts @@ -0,0 +1,65 @@ +import validator from 'validator'; + +const INSECURE_PASSWORDS = [ + '1234567890', + 'qwertyuiop', + 'qwertzuiop', + 'asdfghjkl;', + 'abcdefghij', + '0987654321', + '1q2w3e4r5t', + '12345asdfg', +]; +const COMMON_PHRASES = ['ghost', 'password', 'passw0rd']; + +interface PasswordContext { + email: string; + siteTitle?: string; + /** The site URL, compared without its protocol; defaults to the admin's host. */ + siteUrl?: string; +} + +/** Half or more of the characters being the same one. */ +const isRepetitive = (password: string) => { + const counts = new Map(); + for (const char of password.split('')) { + counts.set(char, (counts.get(char) ?? 0) + 1); + } + return [...counts.values()].some((count) => count >= password.length / 2); +}; + +/** + * Every rule the password breaks, in the order they are checked; empty when it + * is acceptable. The server enforces its own copy of these rules. + */ +export function passwordProblems(password: string, { email, siteTitle, siteUrl }: PasswordContext) { + if (!validator.isLength(password, { min: 10 })) { + return ['Password must be at least 10 characters long.']; + } + + const lower = password.toLowerCase(); + const url = (siteUrl ?? window.location.host).replace(/^https?:\/\//, '').replace(/\/$/, ''); + const urlWithSlash = url.endsWith('/') ? url : `${url}/`; + const problems: string[] = []; + + if (INSECURE_PASSWORDS.includes(password)) { + problems.push('Sorry, you cannot use an insecure password.'); + } + if (lower === email.toLowerCase()) { + problems.push('Sorry, you cannot use the email as your password.'); + } + if (COMMON_PHRASES.some((phrase) => lower.includes(phrase))) { + problems.push('Sorry, you cannot use a password including common phrases.'); + } + if (siteTitle && lower === siteTitle.trim().toLowerCase()) { + problems.push('Sorry, you cannot use the blog title as your password.'); + } + if (lower === url || lower === urlWithSlash) { + problems.push('Sorry, you cannot use the blog URL as your password.'); + } + if (isRepetitive(password)) { + problems.push('Sorry, you cannot use an insecure password.'); + } + + return problems; +} diff --git a/apps/admin/src/auth/reload.ts b/apps/admin/src/auth/reload.ts new file mode 100644 index 00000000000..261c23099f4 --- /dev/null +++ b/apps/admin/src/auth/reload.ts @@ -0,0 +1,10 @@ +/** + * Loads the admin afresh at a route. Every session change goes through here: + * the hidden Ember app has to boot with the new session for the screens it + * still serves. replaceState is not a navigation, so the reload is the only + * one and neither router sees an intermediate route. + */ +export function reloadAdmin(route: string): void { + window.history.replaceState(null, '', `#${route}`); + window.location.reload(); +} diff --git a/apps/admin/src/auth/reset.acceptance.test.tsx b/apps/admin/src/auth/reset.acceptance.test.tsx new file mode 100644 index 00000000000..4f0a464e2cb --- /dev/null +++ b/apps/admin/src/auth/reset.acceptance.test.tsx @@ -0,0 +1,82 @@ +import { beforeEach, expect, it, vi } from 'vitest'; +import { + authToken, + fakeAdminEndpoint, + fakeSetupStatus, + renderAdminApp, + signedOut, +} from '@test-utils/acceptance'; +import { authScreen } from './auth.screen'; +import { reloadAdmin } from './reload'; + +vi.mock('./reload', () => ({ reloadAdmin: vi.fn() })); + +const token = authToken('owner@example.com'); + +beforeEach(() => { + vi.mocked(reloadAdmin).mockClear(); + window.sessionStorage.clear(); + fakeSetupStatus(); +}); + +it('asks for a new password, focused', async () => { + await renderAdminApp(`/reset/${token}`, signedOut({ authReact: true })); + + await expect.element(authScreen.heading('Reset your password.')).toBeVisible(); + await expect.element(authScreen.newPasswordInput()).toHaveFocus(); +}); + +it.each([ + ['', '', 'Please enter a password.'], + ['correct horse battery', 'correct horse', "The two new passwords don't match."], + ['short', 'short', 'Password must be at least 10 characters long.'], + ['owner@example.com', 'owner@example.com', 'Sorry, you cannot use the email as your password.'], +])('checks %j before saving', async (newPassword, confirmation, message) => { + await renderAdminApp(`/reset/${token}`, signedOut({ authReact: true })); + + await authScreen.newPasswordInput().fill(newPassword); + await authScreen.confirmPasswordInput().fill(confirmation); + await authScreen.saveNewPasswordButton().click(); + + await expect.element(authScreen.text(message)).toBeVisible(); + await expect.element(authScreen.retryButton()).toBeVisible(); +}); + +it('saves the password and reloads signed in, confirming once loaded', async () => { + const resetApi = fakeAdminEndpoint('PUT', '/authentication/password_reset/', { + password_reset: [{ message: 'Password updated' }], + }); + await renderAdminApp(`/reset/${token}`, signedOut({ authReact: true })); + + await authScreen.newPasswordInput().fill('correct horse battery'); + await authScreen.confirmPasswordInput().fill('correct horse battery'); + await authScreen.saveNewPasswordButton().click(); + + await expect.poll(() => vi.mocked(reloadAdmin).mock.calls).toEqual([['/']]); + expect(resetApi.lastRequest?.body).toEqual({ + password_reset: [ + { token, newPassword: 'correct horse battery', ne2Password: 'correct horse battery' }, + ], + }); + expect(window.sessionStorage.getItem('ghost-admin:auth-notice')).toBe('password-updated'); +}); + +it.each([ + [401, 'Invalid password reset link.', 'Cannot reset password. Invalid password reset link.'], + [400, 'Password reset link expired.', 'Cannot reset password. Password reset link expired.'], +])('reports a rejected link (%i)', async (status, context, message) => { + fakeAdminEndpoint( + 'PUT', + '/authentication/password_reset/', + { errors: [{ type: 'BadRequestError', message: 'Cannot reset password.', context }] }, + { status }, + ); + await renderAdminApp(`/reset/${token}`, signedOut({ authReact: true })); + + await authScreen.newPasswordInput().fill('correct horse battery'); + await authScreen.confirmPasswordInput().fill('correct horse battery'); + await authScreen.saveNewPasswordButton().click(); + + await expect.element(authScreen.text(message)).toBeVisible(); + expect(reloadAdmin).not.toHaveBeenCalled(); +}); diff --git a/apps/admin/src/auth/reset.tsx b/apps/admin/src/auth/reset.tsx new file mode 100644 index 00000000000..1f496a3b433 --- /dev/null +++ b/apps/admin/src/auth/reset.tsx @@ -0,0 +1,115 @@ +import { type FormEvent, useState } from 'react'; +import { useParams } from '@tryghost/admin-x-framework'; +import { useBrowseSite } from '@tryghost/admin-x-framework/api/site'; +import { Input } from '@tryghost/shade/components'; +import { Stack } from '@tryghost/shade/primitives'; +import { toast } from 'sonner'; +import { describeUnexpectedError, useAuthClient } from './client/auth-client'; +import { AuthHeader, AuthLayout, FlowMessage, SubmitButton, type SubmitState } from './auth-layout'; +import { leaveAuthNotice } from './auth-notice'; +import { passwordProblems } from './password-rules'; +import { reloadAdmin } from './reload'; +import { takeSigninRedirect } from './signin-redirect'; + +export default function Reset() { + const authClient = useAuthClient(); + const { token = '' } = useParams(); + const { data: siteData } = useBrowseSite({ defaultErrorHandler: false }); + + const [newPassword, setNewPassword] = useState(''); + const [confirmPassword, setConfirmPassword] = useState(''); + const [invalid, setInvalid] = useState<{ newPassword?: boolean; confirmPassword?: boolean }>({}); + const [flowError, setFlowError] = useState(''); + const [submitState, setSubmitState] = useState('idle'); + + const clearErrors = () => { + setFlowError(''); + setInvalid({}); + }; + + const validate = () => { + const blank = !newPassword.trim(); + const newPasswordErrors = blank ? ['Please enter a password.'] : []; + const mismatch = !blank && newPassword !== confirmPassword; + newPasswordErrors.push( + ...passwordProblems(newPassword, { + email: authClient.getResetTokenEmail(token) ?? '', + siteTitle: siteData?.site.title, + siteUrl: siteData?.site.url, + }), + ); + setInvalid({ newPassword: newPasswordErrors.length > 0, confirmPassword: Boolean(mismatch) }); + return mismatch ? "The two new passwords don't match." : newPasswordErrors[0]; + }; + + const save = async (event: FormEvent) => { + event.preventDefault(); + setFlowError(''); + + const problem = validate(); + if (problem) { + setFlowError(problem); + setSubmitState('failed'); + return; + } + + setSubmitState('running'); + try { + const { error } = await authClient.resetPassword({ newPassword, token }); + if (error) { + toast.error(error.message ?? 'An unexpected error occurred, please try again.', { + id: 'password-reset', + }); + setSubmitState('failed'); + return; + } + leaveAuthNotice('password-updated'); + reloadAdmin(takeSigninRedirect()); + } catch (error) { + toast.error( + describeUnexpectedError(error, 'An unexpected error occurred, please try again.'), + { id: 'password-reset' }, + ); + setSubmitState('failed'); + } + }; + + return ( + +
void save(event)}> + + + { + clearErrors(); + setNewPassword(event.target.value); + }} + /> + { + clearErrors(); + setConfirmPassword(event.target.value); + }} + /> + + +
+ {flowError} +
+ ); +} diff --git a/apps/admin/src/auth/setup.acceptance.test.tsx b/apps/admin/src/auth/setup.acceptance.test.tsx new file mode 100644 index 00000000000..97dfaaab454 --- /dev/null +++ b/apps/admin/src/auth/setup.acceptance.test.tsx @@ -0,0 +1,189 @@ +import { beforeEach, expect, it, vi } from 'vitest'; +import { + currentRoute, + fakeAdminEndpoint, + fakeSetupStatus, + plainText, + renderAdminApp, + signedOut, +} from '@test-utils/acceptance'; +import { authScreen } from './auth.screen'; +import { reloadAdmin } from './reload'; + +vi.mock('./reload', () => ({ reloadAdmin: vi.fn() })); + +beforeEach(() => { + vi.mocked(reloadAdmin).mockClear(); + window.sessionStorage.clear(); +}); + +it('sends a set-up site to sign in', async () => { + fakeSetupStatus({ status: true }); + await renderAdminApp('/setup', signedOut({ authReact: true })); + + await expect.poll(currentRoute).toBe('/signin'); +}); + +it('prefills the configured site details, focused on the title', async () => { + fakeSetupStatus({ + status: false, + title: 'Jamie's Blog', + name: 'Jamie O'Neil', + email: 'jamie@example.com', + }); + await renderAdminApp('/setup', signedOut({ authReact: true })); + + await expect.element(authScreen.siteTitleInput()).toHaveValue("Jamie's Blog"); + await expect.element(authScreen.siteTitleInput()).toHaveFocus(); + await expect.element(authScreen.fullNameInput()).toHaveValue("Jamie O'Neil"); + await expect.element(authScreen.emailInput()).toHaveValue('jamie@example.com'); +}); + +it('checks every field on submit', async () => { + fakeSetupStatus({ status: false }); + await renderAdminApp('/setup', signedOut({ authReact: true })); + + await authScreen.startPublishingButton().click(); + + await expect.element(authScreen.text('Please enter a site title.')).toBeVisible(); + await expect.element(authScreen.text('Please enter a name.')).toBeVisible(); + await expect.element(authScreen.text('Please enter an email.')).toBeVisible(); + await expect + .element(authScreen.text('Please fill out every field correctly to set up your site.')) + .toBeVisible(); +}); + +it('creates the owner, signs in and starts onboarding', async () => { + fakeSetupStatus({ status: false }); + const setupApi = fakeAdminEndpoint( + 'POST', + '/authentication/setup/', + { users: [{}] }, + { status: 201 }, + ); + fakeAdminEndpoint('POST', '/session/', plainText('Created'), { + status: 201, + contentType: 'text/plain; charset=utf-8', + }); + await renderAdminApp('/setup', signedOut({ authReact: true })); + + await authScreen.siteTitleInput().fill(' The Daily Awesome '); + await authScreen.fullNameInput().fill('Jamie Larson'); + await authScreen.emailInput().fill('jamie@example.com'); + await authScreen.passwordInput().fill('correct horse battery'); + await authScreen.startPublishingButton().click(); + + await expect.poll(() => vi.mocked(reloadAdmin).mock.calls).toEqual([['/?firstStart=true']]); + expect(setupApi.lastRequest?.body).toEqual({ + setup: [ + { + blogTitle: 'The Daily Awesome', + name: 'Jamie Larson', + email: 'jamie@example.com', + password: 'correct horse battery', + }, + ], + }); +}); + +it('shows why the server refused the details', async () => { + fakeSetupStatus({ status: false }); + fakeAdminEndpoint( + 'POST', + '/authentication/setup/', + { + errors: [ + { + type: 'ValidationError', + message: 'Sorry, you cannot use an insecure password.', + context: null, + }, + ], + }, + { status: 422 }, + ); + await renderAdminApp('/setup', signedOut({ authReact: true })); + + await authScreen.siteTitleInput().fill('The Daily Awesome'); + await authScreen.fullNameInput().fill('Jamie Larson'); + await authScreen.emailInput().fill('jamie@example.com'); + await authScreen.passwordInput().fill('correct horse battery'); + await authScreen.startPublishingButton().click(); + + await expect + .element(authScreen.text('Sorry, you cannot use an insecure password.')) + .toBeVisible(); + expect(reloadAdmin).not.toHaveBeenCalled(); +}); + +it('sends the new owner to sign in when signing in right after setup fails', async () => { + let setupDone = false; + fakeAdminEndpoint('GET', '/authentication/setup/', () => ({ setup: [{ status: setupDone }] })); + const setupApi = fakeAdminEndpoint('POST', '/authentication/setup/', () => { + setupDone = true; + return { users: [{}] }; + }); + fakeAdminEndpoint( + 'POST', + '/session/', + { errors: [{ type: 'UnauthorizedError', message: 'Access Denied.' }] }, + { status: 401 }, + ); + await renderAdminApp('/setup', signedOut({ authReact: true })); + + await authScreen.siteTitleInput().fill('The Daily Awesome'); + await authScreen.fullNameInput().fill('Jamie Larson'); + await authScreen.emailInput().fill('jamie@example.com'); + await authScreen.passwordInput().fill('correct horse battery'); + await authScreen.startPublishingButton().click(); + + await expect.element(authScreen.text('Access Denied.')).toBeVisible(); + await expect.poll(currentRoute).toBe('/signin'); + await expect.element(authScreen.signInButton()).toBeVisible(); + expect(setupApi.requests).toHaveLength(1); +}); + +it('moves on to sign in when the server reports the site is already set up', async () => { + let setupDone = false; + fakeAdminEndpoint('GET', '/authentication/setup/', () => ({ setup: [{ status: setupDone }] })); + fakeAdminEndpoint( + 'POST', + '/authentication/setup/', + () => { + setupDone = true; + return { + errors: [{ type: 'NoPermissionError', message: 'Setup has already been completed.' }], + }; + }, + { status: 403 }, + ); + await renderAdminApp('/setup', signedOut({ authReact: true })); + + await authScreen.siteTitleInput().fill('The Daily Awesome'); + await authScreen.fullNameInput().fill('Jamie Larson'); + await authScreen.emailInput().fill('jamie@example.com'); + await authScreen.passwordInput().fill('correct horse battery'); + await authScreen.startPublishingButton().click(); + + await expect.element(authScreen.text('Setup has already been completed.')).toBeVisible(); + await expect.poll(currentRoute).toBe('/signin'); +}); + +it('lets the owner submit again when neither setup nor the server answered', async () => { + fakeSetupStatus({ status: false }); + fakeAdminEndpoint('POST', '/authentication/setup/', plainText('Bad Gateway'), { + status: 502, + contentType: 'text/html', + }); + await renderAdminApp('/setup', signedOut({ authReact: true })); + + await authScreen.siteTitleInput().fill('The Daily Awesome'); + await authScreen.fullNameInput().fill('Jamie Larson'); + await authScreen.emailInput().fill('jamie@example.com'); + await authScreen.passwordInput().fill('correct horse battery'); + await authScreen.startPublishingButton().click(); + + await expect.element(authScreen.text('There was a problem on the server.')).toBeVisible(); + await expect.element(authScreen.startPublishingButton()).toBeEnabled(); + expect(currentRoute()).toBe('/setup'); +}); diff --git a/apps/admin/src/auth/setup.tsx b/apps/admin/src/auth/setup.tsx new file mode 100644 index 00000000000..71af19358a0 --- /dev/null +++ b/apps/admin/src/auth/setup.tsx @@ -0,0 +1,236 @@ +import { type ComponentProps, type FormEvent, useState } from 'react'; +import { Navigate, useNavigate } from '@tryghost/admin-x-framework'; +import { useBrowseSite } from '@tryghost/admin-x-framework/api/site'; +import { Field, FieldError, FieldLabel, GhostOrb, Input } from '@tryghost/shade/components'; +import { Stack } from '@tryghost/shade/primitives'; +import { toast } from 'sonner'; +import validator from 'validator'; +import { + describeUnexpectedError, + type SetupStatus, + useAuthClient, + useSetupStatus, +} from './client/auth-client'; +import { AuthLayout, FlowMessage, SubmitButton, type SubmitState } from './auth-layout'; +import { passwordProblems } from './password-rules'; +import { reloadAdmin } from './reload'; + +type SetupField = 'blogTitle' | 'name' | 'email' | 'password'; +type SetupValues = Record; + +export default function Setup() { + const { data: status, isPending, refetch } = useSetupStatus(); + + if (isPending) { + return null; + } + if (status?.isSetup) { + return ; + } + return ; +} + +const problemWith = ( + field: SetupField, + values: SetupValues, + site?: { title?: string; url?: string }, +) => { + switch (field) { + case 'blogTitle': + if (!values.blogTitle) { + return 'Please enter a site title.'; + } + return validator.isLength(values.blogTitle, { max: 150 }) ? undefined : 'Title is too long'; + case 'name': + return values.name ? undefined : 'Please enter a name.'; + case 'email': + if (!values.email.trim()) { + return 'Please enter an email.'; + } + return validator.isEmail(values.email) ? undefined : 'Invalid Email.'; + case 'password': + return passwordProblems(values.password, { + email: values.email, + siteTitle: values.blogTitle || site?.title, + siteUrl: site?.url, + })[0]; + } +}; + +const FIELDS: SetupField[] = ['blogTitle', 'name', 'email', 'password']; + +function SetupForm({ + prefill, + recheckSetup, +}: { + prefill?: SetupStatus; + /** Re-reads the setup status, which sends a now set-up site on to sign in. */ + recheckSetup: () => Promise; +}) { + const authClient = useAuthClient(); + const navigate = useNavigate(); + const { data: siteData } = useBrowseSite({ defaultErrorHandler: false }); + + const [values, setValues] = useState({ + blogTitle: prefill?.title ?? '', + name: prefill?.name ?? '', + email: prefill?.email ?? '', + password: '', + }); + const [errors, setErrors] = useState>>({}); + const [flowError, setFlowError] = useState(''); + const [submitState, setSubmitState] = useState('idle'); + + const setValue = (field: SetupField, value: string) => + setValues((current) => ({ ...current, [field]: value })); + + // Checked on leaving a field only once something was typed, so tabbing + // through the empty form stays quiet. + const checkOnBlur = (field: SetupField, nextValues = values) => { + if (nextValues[field]) { + setErrors((current) => ({ + ...current, + [field]: problemWith(field, nextValues, siteData?.site), + })); + } + }; + + const submit = async (event: FormEvent) => { + event.preventDefault(); + setFlowError(''); + + const nextErrors = Object.fromEntries( + FIELDS.map((field) => [field, problemWith(field, values, siteData?.site)]), + ); + setErrors(nextErrors); + if (Object.values(nextErrors).some(Boolean)) { + setFlowError('Please fill out every field correctly to set up your site.'); + return; + } + + setSubmitState('running'); + const { blogTitle, name, email, password } = values; + try { + const { error: createError } = await authClient.setup.create({ + blogTitle, + name, + email, + password, + }); + if (createError?.status === 422) { + setFlowError(createError.message ?? ''); + setSubmitState('idle'); + return; + } + if (createError) { + toast.error(createError.message ?? 'An unexpected error occurred, please try again.', { + id: 'setup', + }); + await recheckSetup(); + setSubmitState('idle'); + return; + } + + const { data, error } = await authClient.signIn.email({ email, password }); + if (data && !('twoFactorRedirect' in data)) { + reloadAdmin('/?firstStart=true'); + return; + } + // The owner exists now, so what is left is signing in. + await recheckSetup(); + if (data) { + navigate('/signin/verify', { state: { twoFactorReason: data.twoFactorReason } }); + return; + } + toast.error(error.message ?? 'There was a problem on the server.', { id: 'setup' }); + setSubmitState('idle'); + } catch (error) { + toast.error(describeUnexpectedError(error, 'There was a problem on the server.'), { + id: 'setup', + }); + // The server may have finished setting up before the failure reached us. + await recheckSetup(); + setSubmitState('idle'); + } + }; + + const field = ( + name: SetupField, + label: string, + input: Omit, 'value' | 'onChange' | 'onBlur'>, + trimOnBlur = false, + ) => ( + + {label} + { + const nextValues = trimOnBlur ? { ...values, [name]: values[name].trim() } : values; + setValues(nextValues); + checkOnBlur(name, nextValues); + }} + onChange={(event) => setValue(name, event.target.value)} + {...input} + /> + {errors[name]} + + ); + + return ( + +
+ +

+ Welcome to Ghost. +

+

+ All over the world, people have started 3,000,000+ incredible sites with Ghost. Today, + we’re starting yours. +

+
+
void submit(event)}> + + {field( + 'blogTitle', + 'Site title', + { + autoFocus: true, + id: 'blog-title', + name: 'blog-title', + placeholder: 'The Daily Awesome', + }, + true, + )} + {field('name', 'Full name', { + autoComplete: 'name', + id: 'name', + name: 'name', + placeholder: 'Jamie Larson', + })} + {field('email', 'Email address', { + autoComplete: 'username', + id: 'email', + name: 'email', + placeholder: 'jamie@example.com', + type: 'email', + })} + {field('password', 'Password', { + autoComplete: 'new-password', + id: 'password', + name: 'password', + placeholder: 'At least 10 characters', + type: 'password', + })} + + +
+ {flowError && {flowError}} +
+ ); +} diff --git a/apps/admin/src/auth/signed-in-auth.acceptance.test.tsx b/apps/admin/src/auth/signed-in-auth.acceptance.test.tsx new file mode 100644 index 00000000000..d07afcad6f3 --- /dev/null +++ b/apps/admin/src/auth/signed-in-auth.acceptance.test.tsx @@ -0,0 +1,81 @@ +import { beforeEach, expect, it, vi } from 'vitest'; +import { siteResponse } from '@tryghost/test-data'; +import { + allowUnhandledRequests, + authToken, + currentRoute, + fakeAdminEndpoint, + renderAdminApp, + type RenderAdminAppOptions, +} from '@test-utils/acceptance'; +import { authScreen } from './auth.screen'; +import { reloadAdmin } from './reload'; + +// Signed-in boots live apart from the signed-out specs: once this page load +// has seen the session work, a later 403 would take the test page away. + +vi.mock('./reload', () => ({ reloadAdmin: vi.fn() })); + +const withAuthReact = (authReact: boolean): RenderAdminAppOptions => { + const site = siteResponse(); + return { boot: { browseSite: { response: { site: { ...site.site, authReact } } } } }; +}; + +const emberFrameHidden = () => document.getElementById('ember-app')?.parentElement?.hidden; + +beforeEach(() => { + vi.mocked(reloadAdmin).mockClear(); + window.sessionStorage.clear(); +}); + +it('sends a signed-in user away from sign in', async () => { + // The analytics dashboard it lands on owns its request graph. + allowUnhandledRequests(); + await renderAdminApp('/signin', withAuthReact(true)); + + await expect.poll(currentRoute).toBe('/analytics'); +}); + +it.each([ + [ + 'reset', + `/reset/${authToken('owner@example.com')}`, + "You can't reset your password while you're signed in.", + ], + [ + 'signup', + `/signup/${authToken('staff@example.com')}`, + 'You need to sign out to register as a new user.', + ], +])('warns a signed-in user off %s', async (_screen, route, warning) => { + // The analytics dashboard it lands on owns its request graph. + allowUnhandledRequests(); + await renderAdminApp(route, withAuthReact(true)); + + await expect.element(authScreen.text(warning)).toBeVisible(); + await expect.poll(currentRoute).toBe('/analytics'); +}); + +it('signs out and reloads onto sign in', async () => { + const sessionApi = fakeAdminEndpoint('DELETE', '/session/', null, { status: 204 }); + await renderAdminApp('/signout', withAuthReact(true)); + + await expect.poll(() => vi.mocked(reloadAdmin).mock.calls).toEqual([['/signin']]); + expect(sessionApi.requests).toHaveLength(1); +}); + +it('leaves signed-in auth routes to Ember when the flag is off', async () => { + await renderAdminApp('/signin', withAuthReact(false)); + + await expect.poll(emberFrameHidden).toBe(false); + expect(currentRoute()).toBe('/signin'); +}); + +it('confirms a password reset once the admin has reloaded', async () => { + window.sessionStorage.setItem('ghost-admin:auth-notice', 'password-updated'); + await renderAdminApp('/tags', withAuthReact(true)); + fakeAdminEndpoint('GET', /^\/tags\//, { tags: [], meta: { pagination: { next: null } } }); + + await expect.element(authScreen.text('Password updated')).toBeVisible(); + expect(window.sessionStorage.getItem('ghost-admin:auth-notice')).toBeNull(); +}); diff --git a/apps/admin/src/auth/signin-redirect.ts b/apps/admin/src/auth/signin-redirect.ts new file mode 100644 index 00000000000..6523b6f64d5 --- /dev/null +++ b/apps/admin/src/auth/signin-redirect.ts @@ -0,0 +1,30 @@ +import { isAuthPath } from '@tryghost/admin-x-framework/helpers'; + +// Shared with Ember's authenticated route and session service, so either shell +// can store the route a signed-out visitor asked for and the other can use it. +const SIGNIN_REDIRECT_KEY = 'ghost-signin-redirect'; + +const isRedirectTarget = (route: string | null): route is string => + Boolean(route) && route!.split('?')[0] !== '/' && !isAuthPath(route!); + +/** Remembers where a signed-out visitor was going; the latest attempt wins. */ +export function rememberSigninRedirect(route: string): void { + try { + if (isRedirectTarget(route)) { + window.sessionStorage.setItem(SIGNIN_REDIRECT_KEY, route); + } + } catch { + // Storage can be unavailable; signing in then lands on the home route. + } +} + +/** The route to open after signing in, cleared so it is used once. */ +export function takeSigninRedirect(): string { + try { + const route = window.sessionStorage.getItem(SIGNIN_REDIRECT_KEY); + window.sessionStorage.removeItem(SIGNIN_REDIRECT_KEY); + return isRedirectTarget(route) ? route : '/'; + } catch { + return '/'; + } +} diff --git a/apps/admin/src/auth/signin-verify.acceptance.test.tsx b/apps/admin/src/auth/signin-verify.acceptance.test.tsx new file mode 100644 index 00000000000..b01aa7dd92f --- /dev/null +++ b/apps/admin/src/auth/signin-verify.acceptance.test.tsx @@ -0,0 +1,95 @@ +import { beforeEach, expect, it, vi } from 'vitest'; +import { page } from 'vitest/browser'; +import { + fakeAdminEndpoint, + fakeSetupStatus, + plainText, + renderAdminApp, + signedOut, +} from '@test-utils/acceptance'; +import { authScreen } from './auth.screen'; +import { reloadAdmin } from './reload'; + +vi.mock('./reload', () => ({ reloadAdmin: vi.fn() })); + +const textReply = { contentType: 'text/plain; charset=utf-8' }; + +beforeEach(() => { + vi.mocked(reloadAdmin).mockClear(); + window.sessionStorage.clear(); + fakeSetupStatus(); +}); + +it('shows the new-device copy when opened directly', async () => { + await renderAdminApp('/signin/verify', signedOut({ authReact: true })); + + await expect.element(authScreen.heading("Verify it's really you")).toBeVisible(); + await expect.element(authScreen.text(/signing in from a new device/)).toBeVisible(); +}); + +it.each([ + ['', 'Verification code is required'], + ['12345', 'Verification code must be 6 numbers'], +])('checks the code %j before sending it', async (code, message) => { + await renderAdminApp('/signin/verify', signedOut({ authReact: true })); + + await authScreen.codeInput().fill(code); + await authScreen.verifyButton().click(); + + await expect.element(authScreen.text(message)).toBeVisible(); + await expect.element(authScreen.codeInput()).toHaveAttribute('aria-invalid', 'true'); + await expect.element(authScreen.retryButton()).toBeVisible(); +}); + +it('says a rejected code is incorrect, and clears that when typing again', async () => { + fakeAdminEndpoint('PUT', '/session/verify/', plainText('Unauthorized'), { + status: 401, + ...textReply, + }); + await renderAdminApp('/signin/verify', signedOut({ authReact: true })); + + await authScreen.codeInput().fill('123456'); + await authScreen.verifyButton().click(); + + await expect.element(authScreen.text('Your verification code is incorrect.')).toBeVisible(); + await authScreen.codeInput().fill('654321'); + await expect(authScreen.text('Your verification code is incorrect.')).toHaveCount(0); + await expect.element(authScreen.verifyButton()).toBeVisible(); +}); + +it('verifies the code and reloads onto the remembered route', async () => { + window.sessionStorage.setItem('ghost-signin-redirect', '/members'); + const verifyApi = fakeAdminEndpoint('PUT', '/session/verify/', plainText('OK'), textReply); + await renderAdminApp('/signin/verify', signedOut({ authReact: true })); + + await authScreen.codeInput().fill(' 123456 '); + await authScreen.verifyButton().click(); + + await expect.poll(() => vi.mocked(reloadAdmin).mock.calls).toEqual([['/members']]); + expect(verifyApi.lastRequest?.body).toEqual({ token: '123456' }); +}); + +it('resends the code, then holds the button while the new one arrives', async () => { + const resendApi = fakeAdminEndpoint('POST', '/session/verify/', plainText('OK'), textReply); + await renderAdminApp('/signin/verify', signedOut({ authReact: true })); + + await authScreen.resendButton().click(); + + await expect.element(page.getByRole('button', { name: 'Sent' })).toBeDisabled(); + expect(resendApi.requests).toHaveLength(1); +}); + +it('shows why a resend failed', async () => { + fakeAdminEndpoint( + 'POST', + '/session/verify/', + { errors: [{ type: 'TooManyRequestsError', message: 'Too many attempts.' }] }, + { status: 429 }, + ); + await renderAdminApp('/signin/verify', signedOut({ authReact: true })); + + await authScreen.resendButton().click(); + + await expect.element(authScreen.text('Too many attempts.')).toBeVisible(); + await expect.element(authScreen.resendButton()).toBeEnabled(); +}); diff --git a/apps/admin/src/auth/signin-verify.tsx b/apps/admin/src/auth/signin-verify.tsx new file mode 100644 index 00000000000..5b76623e268 --- /dev/null +++ b/apps/admin/src/auth/signin-verify.tsx @@ -0,0 +1,142 @@ +import { type FormEvent, useEffect, useState } from 'react'; +import { useLocation } from '@tryghost/admin-x-framework'; +import { + Field, + FieldLabel, + InputGroup, + InputGroupAddon, + InputGroupButton, + InputGroupInput, + LoadingIndicator, +} from '@tryghost/shade/components'; +import { Stack } from '@tryghost/shade/primitives'; +import { describeUnexpectedError, useAuthClient } from './client/auth-client'; +import { AuthHeader, AuthLayout, FlowMessage, SubmitButton, type SubmitState } from './auth-layout'; +import { reloadAdmin } from './reload'; +import { takeSigninRedirect } from './signin-redirect'; + +const RESEND_COOLDOWN_MS = 15_000; + +export default function SigninVerify() { + const authClient = useAuthClient(); + const { state } = useLocation() as { state: { twoFactorReason?: string } | null }; + const twoFactorRequired = state?.twoFactorReason === 'required'; + + const [code, setCode] = useState(''); + const [codeError, setCodeError] = useState(''); + const [flowError, setFlowError] = useState(''); + const [submitState, setSubmitState] = useState('idle'); + const [resendState, setResendState] = useState<'idle' | 'sending' | 'sent'>('idle'); + + useEffect(() => { + if (resendState !== 'sent') { + return; + } + const timeout = setTimeout(() => setResendState('idle'), RESEND_COOLDOWN_MS); + return () => clearTimeout(timeout); + }, [resendState]); + + const failWith = (message: string, setMessage: (message: string) => void) => { + setMessage(message); + setSubmitState('failed'); + }; + + const verify = async (event: FormEvent) => { + event.preventDefault(); + setFlowError(''); + setCodeError(''); + + const trimmed = code.trim(); + if (!trimmed) { + return failWith('Verification code is required', setCodeError); + } + if (!/^\d{6}$/.test(trimmed)) { + return failWith('Verification code must be 6 numbers', setCodeError); + } + + setSubmitState('running'); + try { + const { error } = await authClient.twoFactor.verifyOtp({ code: trimmed }); + if (!error) { + reloadAdmin(takeSigninRedirect()); + } else if (error.code === 'INVALID_CODE') { + failWith('Your verification code is incorrect.', setCodeError); + } else { + failWith(error.message ?? '', setFlowError); + } + } catch (error) { + failWith( + describeUnexpectedError(error, 'There was a problem verifying the code. Please try again.'), + setFlowError, + ); + } + }; + + const resend = async () => { + setResendState('sending'); + try { + const { error } = await authClient.twoFactor.sendOtp(); + if (error) { + setFlowError(error.message ?? ''); + setResendState('idle'); + } else { + setResendState('sent'); + } + } catch (error) { + setFlowError( + describeUnexpectedError(error, 'There was a problem resending the verification token.'), + ); + setResendState('idle'); + } + }; + + return ( + +
void verify(event)}> + + +

+ {twoFactorRequired + ? 'Enter the sign-in verification code sent to your email.' + : "It looks like you're signing in from a new device. A 6-digit sign-in verification code has been sent to your email to keep your account safe."} +

+
+ + Verification code + + { + setCode(event.target.value); + setCodeError(''); + setFlowError(''); + setSubmitState('idle'); + }} + /> + + void resend()}> + {resendState === 'sending' && } + {resendState === 'idle' && 'Resend'} + {resendState === 'sending' && 'Sending'} + {resendState === 'sent' && 'Sent'} + + + + + +
+
+ {flowError || codeError} +
+ ); +} diff --git a/apps/admin/src/auth/signin.acceptance.test.tsx b/apps/admin/src/auth/signin.acceptance.test.tsx new file mode 100644 index 00000000000..5184cc009d1 --- /dev/null +++ b/apps/admin/src/auth/signin.acceptance.test.tsx @@ -0,0 +1,257 @@ +import { beforeEach, expect, it, vi } from 'vitest'; +import { + currentRoute, + fakeAdminEndpoint, + fakeSetupStatus, + plainText, + renderAdminApp, + signedOut, +} from '@test-utils/acceptance'; +import { authScreen } from './auth.screen'; +import { reloadAdmin } from './reload'; + +vi.mock('./reload', () => ({ reloadAdmin: vi.fn() })); + +const SIGNIN_REDIRECT_KEY = 'ghost-signin-redirect'; + +const ghostError = (status: number, error: Record) => + [{ errors: [error] }, { status }] as const; + +const emberFrameHidden = () => document.getElementById('ember-app')?.parentElement?.hidden; + +beforeEach(() => { + vi.mocked(reloadAdmin).mockClear(); + window.sessionStorage.clear(); +}); + +it('serves sign in from React when the site hands the auth screens over', async () => { + fakeSetupStatus(); + await renderAdminApp('/signin', signedOut({ authReact: true })); + + await expect.element(authScreen.emailInput()).toBeVisible(); + await expect.element(authScreen.signInButton()).toBeVisible(); + expect(emberFrameHidden()).toBe(true); +}); + +it.each([ + ['a server that predates the flag', undefined], + ['the flag off', false], +])('leaves sign in to Ember on %s', async (_case, authReact) => { + await renderAdminApp('/signin', signedOut({ authReact })); + + await expect.poll(emberFrameHidden).toBe(false); + await expect(authScreen.signInButton()).toHaveCount(0); +}); + +it('serves sign in from React with the Labs URL override', async () => { + fakeSetupStatus(); + await renderAdminApp('/signin?labs=authReact', signedOut()); + + await expect.element(authScreen.signInButton()).toBeVisible(); +}); + +it('sends a signed-out visitor to sign in and remembers where they were going', async () => { + fakeSetupStatus(); + await renderAdminApp('/settings/newsletters?verifyEmail=abc', signedOut({ authReact: true })); + + await expect.poll(currentRoute).toBe('/signin'); + expect(window.sessionStorage.getItem(SIGNIN_REDIRECT_KEY)).toBe( + '/settings/newsletters?verifyEmail=abc', + ); +}); + +it('remembers the latest route a signed-out visitor asked for', async () => { + fakeSetupStatus(); + window.sessionStorage.setItem(SIGNIN_REDIRECT_KEY, '/tags'); + await renderAdminApp('/members', signedOut({ authReact: true })); + + await expect.poll(currentRoute).toBe('/signin'); + expect(window.sessionStorage.getItem(SIGNIN_REDIRECT_KEY)).toBe('/members'); +}); + +it('sends every auth screen to setup on a site that is not set up', async () => { + fakeSetupStatus({ status: false }); + await renderAdminApp('/signin', signedOut({ authReact: true })); + + await expect.poll(currentRoute).toBe('/setup'); +}); + +it('signs in and reloads onto the route the visitor was going to', async () => { + fakeSetupStatus(); + window.sessionStorage.setItem(SIGNIN_REDIRECT_KEY, '/tags'); + const sessionApi = fakeAdminEndpoint('POST', '/session/', plainText('Created'), { + status: 201, + contentType: 'text/plain; charset=utf-8', + }); + await renderAdminApp('/signin', signedOut({ authReact: true })); + + await authScreen.signIn('owner@example.com', 'correct horse battery'); + + await expect.poll(() => vi.mocked(reloadAdmin).mock.calls).toEqual([['/tags']]); + expect(sessionApi.lastRequest?.body).toEqual({ + username: 'owner@example.com', + password: 'correct horse battery', + }); + expect(window.sessionStorage.getItem(SIGNIN_REDIRECT_KEY)).toBeNull(); +}); + +it('asks for the whole form before signing in', async () => { + fakeSetupStatus(); + await renderAdminApp('/signin', signedOut({ authReact: true })); + + await authScreen.signInButton().click(); + + await expect.element(authScreen.text('Please fill out the form to sign in.')).toBeVisible(); + await expect.element(authScreen.retryButton()).toBeVisible(); + await expect.element(authScreen.emailInput()).toHaveAttribute('aria-invalid', 'true'); + await expect.element(authScreen.passwordInput()).toHaveAttribute('aria-invalid', 'true'); +}); + +it('marks the password when Core rejects it', async () => { + fakeSetupStatus(); + fakeAdminEndpoint( + 'POST', + '/session/', + ...ghostError(422, { + type: 'ValidationError', + code: 'PASSWORD_INCORRECT', + message: 'Your password is incorrect.', + context: 'Your password is incorrect.', + }), + ); + await renderAdminApp('/signin', signedOut({ authReact: true })); + + await authScreen.signIn('owner@example.com', 'wrong password here'); + + await expect.element(authScreen.text('Your password is incorrect.')).toBeVisible(); + await expect.element(authScreen.passwordInput()).toHaveAttribute('aria-invalid', 'true'); + await expect.element(authScreen.emailInput()).not.toHaveAttribute('aria-invalid'); + await expect.element(authScreen.retryButton()).toBeVisible(); +}); + +it('shows the full rate-limit message', async () => { + fakeSetupStatus(); + fakeAdminEndpoint( + 'POST', + '/session/', + ...ghostError(429, { + type: 'TooManyRequestsError', + message: + 'Too many login attempts. Please wait 10 minutes before trying again, or reset your password.', + context: 'Too many login attempts.', + }), + ); + await renderAdminApp('/signin', signedOut({ authReact: true })); + + await authScreen.signIn('owner@example.com', 'correct horse battery'); + + await expect + .element( + authScreen.text( + 'Too many login attempts. Please wait 10 minutes before trying again, or reset your password.', + ), + ) + .toBeVisible(); +}); + +it.each([ + ['2FA_TOKEN_REQUIRED', '2FA confirmation'], + ['2FA_NEW_DEVICE_DETECTED', "Verify it's really you"], +])('asks for the emailed code when Core answers %s', async (code, heading) => { + fakeSetupStatus(); + fakeAdminEndpoint( + 'POST', + '/session/', + ...ghostError(403, { + type: 'Needs2FAError', + code, + message: 'User must verify session to login.', + context: + 'A 6-digit sign-in verification code has been sent to your email to keep your account safe.', + }), + ); + await renderAdminApp('/signin', signedOut({ authReact: true })); + + await authScreen.signIn('owner@example.com', 'correct horse battery'); + + await expect.poll(currentRoute).toBe('/signin/verify'); + await expect.element(authScreen.heading(heading)).toBeVisible(); + expect(reloadAdmin).not.toHaveBeenCalled(); +}); + +it('explains that a locked account has been sent a reset email', async () => { + fakeSetupStatus(); + fakeAdminEndpoint( + 'POST', + '/session/', + ...ghostError(401, { + type: 'PasswordResetRequiredError', + message: + 'For security, you need to create a new password. An email has been sent to you with instructions!', + }), + ); + await renderAdminApp('/signin', signedOut({ authReact: true })); + + await authScreen.signIn('owner@example.com', 'correct horse battery'); + + await expect.element(authScreen.heading('Update your password.')).toBeVisible(); + await expect(authScreen.signInButton()).toHaveCount(0); +}); + +it('needs a valid email before sending a password reset', async () => { + fakeSetupStatus(); + await renderAdminApp('/signin', signedOut({ authReact: true })); + + await authScreen.forgotButton().click(); + + await expect + .element(authScreen.text('We need your email address to reset your password.')) + .toBeVisible(); + await expect.element(authScreen.emailInput()).toHaveAttribute('aria-invalid', 'true'); +}); + +it('sends a password reset email for the entered address', async () => { + fakeSetupStatus(); + const resetApi = fakeAdminEndpoint('POST', '/authentication/password_reset/', { + password_reset: [{ message: 'Check your email for further instructions.' }], + }); + await renderAdminApp('/signin', signedOut({ authReact: true })); + + await authScreen.emailInput().fill('owner@example.com'); + await authScreen.forgotButton().click(); + + await expect + .element(authScreen.text('An email with password reset instructions has been sent.')) + .toBeVisible(); + expect(resetApi.lastRequest?.body).toEqual({ password_reset: [{ email: 'owner@example.com' }] }); +}); + +it('marks the email when no staff user has it', async () => { + fakeSetupStatus(); + fakeAdminEndpoint( + 'POST', + '/authentication/password_reset/', + ...ghostError(404, { type: 'NotFoundError', message: 'User not found.' }), + ); + await renderAdminApp('/signin', signedOut({ authReact: true })); + + await authScreen.emailInput().fill('nobody@example.com'); + await authScreen.forgotButton().click(); + + await expect.element(authScreen.text('User not found.')).toBeVisible(); + await expect.element(authScreen.emailInput()).toHaveAttribute('aria-invalid', 'true'); +}); + +it('reports a failure without an answer from Ghost as a server problem', async () => { + fakeSetupStatus(); + fakeAdminEndpoint('POST', '/session/', plainText('Bad Gateway'), { + status: 502, + contentType: 'text/html', + }); + await renderAdminApp('/signin', signedOut({ authReact: true })); + + await authScreen.signIn('owner@example.com', 'correct horse battery'); + + await expect.element(authScreen.text('There was a problem on the server.')).toBeVisible(); + await expect.element(authScreen.retryButton()).toBeVisible(); +}); diff --git a/apps/admin/src/auth/signin.tsx b/apps/admin/src/auth/signin.tsx new file mode 100644 index 00000000000..38f610959a7 --- /dev/null +++ b/apps/admin/src/auth/signin.tsx @@ -0,0 +1,172 @@ +import { type FormEvent, useState } from 'react'; +import { useNavigate } from '@tryghost/admin-x-framework'; +import { useBrowseSite } from '@tryghost/admin-x-framework/api/site'; +import { + Field, + FieldLabel, + Input, + InputGroup, + InputGroupAddon, + InputGroupButton, + InputGroupInput, + LoadingIndicator, +} from '@tryghost/shade/components'; +import { Stack } from '@tryghost/shade/primitives'; +import { toast } from 'sonner'; +import validator from 'validator'; +import { describeUnexpectedError, useAuthClient } from './client/auth-client'; +import { AuthHeader, AuthLayout, FlowMessage, SubmitButton, type SubmitState } from './auth-layout'; +import { reloadAdmin } from './reload'; +import { takeSigninRedirect } from './signin-redirect'; + +export default function Signin() { + const authClient = useAuthClient(); + const navigate = useNavigate(); + const { data: siteData } = useBrowseSite({ defaultErrorHandler: false }); + + const [email, setEmail] = useState(''); + const [password, setPassword] = useState(''); + const [invalid, setInvalid] = useState<{ email?: boolean; password?: boolean }>({}); + const [flowError, setFlowError] = useState(''); + const [flowNotice, setFlowNotice] = useState(''); + const [submitState, setSubmitState] = useState('idle'); + const [isSendingReset, setIsSendingReset] = useState(false); + const [resetRequired, setResetRequired] = useState(false); + + const signIn = async (event: FormEvent) => { + event.preventDefault(); + setFlowError(''); + + const emailInvalid = !email.trim() || !validator.isEmail(email); + const passwordBlank = !password.trim(); + if (emailInvalid || passwordBlank) { + setInvalid({ email: emailInvalid, password: passwordBlank }); + setFlowError('Please fill out the form to sign in.'); + setSubmitState('failed'); + return; + } + + setInvalid({}); + setSubmitState('running'); + try { + const { data, error } = await authClient.signIn.email({ email, password }); + if (data && 'twoFactorRedirect' in data) { + navigate('/signin/verify', { state: { twoFactorReason: data.twoFactorReason } }); + } else if (data) { + reloadAdmin(takeSigninRedirect()); + } else { + setInvalid({ password: error.code === 'INVALID_PASSWORD' }); + setFlowError(error.message ?? ''); + setResetRequired(error.code === 'PASSWORD_RESET_REQUIRED'); + setSubmitState('failed'); + } + } catch (error) { + toast.error(describeUnexpectedError(error, 'There was a problem on the server.'), { + id: 'signin', + }); + setSubmitState('failed'); + } + }; + + const sendPasswordReset = async () => { + setFlowError(''); + setFlowNotice(''); + + if (!validator.isEmail(email)) { + setInvalid({ email: true }); + setFlowError('We need your email address to reset your password.'); + return; + } + + setInvalid({}); + setIsSendingReset(true); + try { + const { error } = await authClient.requestPasswordReset({ email }); + if (error) { + setInvalid({ email: error.code === 'USER_NOT_FOUND' }); + setFlowError(error.message ?? ''); + } else { + setFlowNotice('An email with password reset instructions has been sent.'); + } + } catch (error) { + toast.error( + describeUnexpectedError(error, 'There was a problem with the reset, please try again.'), + { id: 'forgot-password' }, + ); + } finally { + setIsSendingReset(false); + } + }; + + if (resetRequired) { + return ( + + +

+ For security, you need to create a new password. An email has been sent to you with + instructions. +

+
+
+ ); + } + + return ( + +
void signIn(event)}> + + + + Email address + + setInvalid((current) => ({ + ...current, + email: email.trim() !== '' && !validator.isEmail(email), + })) + } + onChange={(event) => setEmail(event.target.value)} + /> + + + Password + + setPassword(event.target.value)} + /> + + void sendPasswordReset()} + > + {isSendingReset ? : 'Forgot?'} + + + + + + +
+ {flowError || flowNotice} +
+ ); +} diff --git a/apps/admin/src/auth/signout.tsx b/apps/admin/src/auth/signout.tsx new file mode 100644 index 00000000000..1e791729537 --- /dev/null +++ b/apps/admin/src/auth/signout.tsx @@ -0,0 +1,30 @@ +import { useEffect, useRef } from 'react'; +import { useAuthClient } from './client/auth-client'; +import { reloadAdmin } from './reload'; +import { takeSigninRedirect } from './signin-redirect'; + +export default function Signout() { + const authClient = useAuthClient(); + const started = useRef(false); + + useEffect(() => { + // StrictMode mounts effects twice; one sign out is enough. + if (started.current) { + return; + } + started.current = true; + + const signOut = async () => { + try { + await authClient.signOut(); + } catch { + // Already signed out, or unreachable: the reload shows which. + } + takeSigninRedirect(); + reloadAdmin('/signin'); + }; + void signOut(); + }, [authClient]); + + return null; +} diff --git a/apps/admin/src/auth/signup.acceptance.test.tsx b/apps/admin/src/auth/signup.acceptance.test.tsx new file mode 100644 index 00000000000..da6293337dd --- /dev/null +++ b/apps/admin/src/auth/signup.acceptance.test.tsx @@ -0,0 +1,129 @@ +import { beforeEach, expect, it, vi } from 'vitest'; +import { + authToken, + currentRoute, + fakeAdminEndpoint, + fakeSetupStatus, + plainText, + renderAdminApp, + signedOut, +} from '@test-utils/acceptance'; +import { authScreen } from './auth.screen'; +import { reloadAdmin } from './reload'; + +vi.mock('./reload', () => ({ reloadAdmin: vi.fn() })); + +const token = authToken('staff@example.com'); + +const fakeInvitation = (valid: boolean) => + fakeAdminEndpoint('GET', /^\/authentication\/invitation\/\?email=/, { invitation: [{ valid }] }); + +beforeEach(() => { + vi.mocked(reloadAdmin).mockClear(); + window.sessionStorage.clear(); + fakeSetupStatus(); +}); + +it('checks the invitation for the email in the link and prefills it', async () => { + const invitationApi = fakeInvitation(true); + await renderAdminApp(`/signup/${token}`, signedOut({ authReact: true })); + + await expect.element(authScreen.heading('Create your account.')).toBeVisible(); + await expect.element(authScreen.emailInput()).toHaveValue('staff@example.com'); + await expect.element(authScreen.emailInput()).toBeDisabled(); + expect(invitationApi.lastRequest?.url).toContain('email=staff%40example.com'); +}); + +it('sends a malformed link to sign in', async () => { + await renderAdminApp('/signup/not*a*token', signedOut({ authReact: true })); + + await expect.poll(currentRoute).toBe('/signin'); + await expect.element(authScreen.text('Invalid token.')).toBeVisible(); +}); + +it('sends a used or revoked invitation to sign in', async () => { + fakeInvitation(false); + await renderAdminApp(`/signup/${token}`, signedOut({ authReact: true })); + + await expect.poll(currentRoute).toBe('/signin'); + await expect + .element(authScreen.text('The invitation does not exist or is no longer valid.')) + .toBeVisible(); +}); + +it('checks each field as it is left, and the whole form on submit', async () => { + fakeInvitation(true); + await renderAdminApp(`/signup/${token}`, signedOut({ authReact: true })); + + await authScreen.passwordInput().fill('short'); + (authScreen.passwordInput().element() as HTMLElement).blur(); + await expect + .element(authScreen.text('Password must be at least 10 characters long.')) + .toBeVisible(); + + await authScreen.createAccountButton().click(); + await expect.element(authScreen.text('Please enter a name.')).toBeVisible(); + await expect + .element(authScreen.text('Please fill out the form to complete your signup')) + .toBeVisible(); +}); + +it('creates the account, signs in and reloads', async () => { + fakeInvitation(true); + const acceptApi = fakeAdminEndpoint('POST', '/authentication/invitation/', { + invitation: [{ message: 'Invitation accepted.' }], + }); + const sessionApi = fakeAdminEndpoint('POST', '/session/', plainText('Created'), { + status: 201, + contentType: 'text/plain; charset=utf-8', + }); + await renderAdminApp(`/signup/${token}`, signedOut({ authReact: true })); + + await authScreen.fullNameInput().fill(' Jamie Larson '); + await authScreen.passwordInput().fill('correct horse battery'); + await authScreen.createAccountButton().click(); + + await expect.poll(() => vi.mocked(reloadAdmin).mock.calls).toEqual([['/']]); + expect(acceptApi.lastRequest?.body).toEqual({ + invitation: [ + { + token, + name: 'Jamie Larson', + password: 'correct horse battery', + email: 'staff@example.com', + }, + ], + }); + expect(sessionApi.lastRequest?.body).toEqual({ + username: 'staff@example.com', + password: 'correct horse battery', + }); +}); + +it('reports why the invitation could not be accepted', async () => { + fakeInvitation(true); + fakeAdminEndpoint( + 'POST', + '/authentication/invitation/', + { + errors: [ + { + type: 'ValidationError', + message: 'Could not create an account, email is already in use.', + context: 'Attempting to create an account with existing email address.', + }, + ], + }, + { status: 422 }, + ); + await renderAdminApp(`/signup/${token}`, signedOut({ authReact: true })); + + await authScreen.fullNameInput().fill('Jamie Larson'); + await authScreen.passwordInput().fill('correct horse battery'); + await authScreen.createAccountButton().click(); + + await expect + .element(authScreen.text('Could not create an account, email is already in use.')) + .toBeVisible(); + await expect.element(authScreen.retryButton()).toBeVisible(); +}); diff --git a/apps/admin/src/auth/signup.tsx b/apps/admin/src/auth/signup.tsx new file mode 100644 index 00000000000..294f49b3b39 --- /dev/null +++ b/apps/admin/src/auth/signup.tsx @@ -0,0 +1,178 @@ +import { type FormEvent, useEffect, useState } from 'react'; +import { Navigate, useNavigate, useParams } from '@tryghost/admin-x-framework'; +import { useBrowseSite } from '@tryghost/admin-x-framework/api/site'; +import { Field, FieldError, FieldLabel, Input } from '@tryghost/shade/components'; +import { Stack } from '@tryghost/shade/primitives'; +import { toast } from 'sonner'; +import { describeUnexpectedError, useAuthClient, useInvitation } from './client/auth-client'; +import { AuthHeader, AuthLayout, FlowMessage, SubmitButton, type SubmitState } from './auth-layout'; +import { passwordProblems } from './password-rules'; +import { reloadAdmin } from './reload'; +import { takeSigninRedirect } from './signin-redirect'; + +type SignupField = 'name' | 'password'; + +export default function Signup() { + const { token = '' } = useParams(); + const { data: invitation, isPending } = useInvitation(token); + + const invalidToken = invitation?.email === null; + const invalidInvitation = invitation?.email && !invitation.valid; + const rejected = invalidToken || invalidInvitation; + + useEffect(() => { + if (invalidToken) { + toast.error('Invalid token.', { id: 'signup-rejected' }); + } else if (invalidInvitation) { + toast.warning('The invitation does not exist or is no longer valid.', { + id: 'signup-rejected', + }); + } + }, [invalidToken, invalidInvitation]); + + if (isPending || !invitation) { + return null; + } + if (rejected || !invitation.email) { + return ; + } + return ; +} + +function SignupForm({ email, token }: { email: string; token: string }) { + const authClient = useAuthClient(); + const navigate = useNavigate(); + const { data: siteData } = useBrowseSite({ defaultErrorHandler: false }); + + const [name, setName] = useState(''); + const [password, setPassword] = useState(''); + const [errors, setErrors] = useState>>({}); + const [flowError, setFlowError] = useState(''); + const [submitState, setSubmitState] = useState('idle'); + + const problemWith = (field: SignupField, values = { name, password }) => { + if (field === 'name') { + return values.name ? undefined : 'Please enter a name.'; + } + return passwordProblems(values.password, { email, siteTitle: siteData?.site.title })[0]; + }; + + const validateField = (field: SignupField, values?: { name: string; password: string }) => + setErrors((current) => ({ ...current, [field]: problemWith(field, values) })); + + const submit = async (event: FormEvent) => { + event.preventDefault(); + setFlowError(''); + + const nextErrors = { name: problemWith('name'), password: problemWith('password') }; + setErrors(nextErrors); + if (nextErrors.name || nextErrors.password) { + setFlowError('Please fill out the form to complete your signup'); + setSubmitState('failed'); + return; + } + + setSubmitState('running'); + let accountCreated = false; + try { + const accepted = await authClient.invitation.accept({ token, name, password }); + if (accepted.error) { + setFlowError(accepted.error.message ?? ''); + setSubmitState('failed'); + return; + } + accountCreated = true; + + const { data, error } = await authClient.signIn.email({ email, password }); + if (data && 'twoFactorRedirect' in data) { + navigate('/signin/verify', { state: { twoFactorReason: data.twoFactorReason } }); + } else if (data) { + reloadAdmin(takeSigninRedirect()); + } else { + toast.error(error.message ?? 'An unexpected error occurred, please try again.', { + id: 'signup', + }); + navigate('/signin', { replace: true }); + } + } catch (error) { + toast.error( + describeUnexpectedError(error, 'An unexpected error occurred, please try again.'), + { id: 'signup' }, + ); + // The account exists now, so submitting again could only fail. + if (accountCreated) { + navigate('/signin', { replace: true }); + } else { + setSubmitState('failed'); + } + } + }; + + return ( + + +
void submit(event)}> + + + Full name + { + const trimmed = name.trim(); + setName(trimmed); + validateField('name', { name: trimmed, password }); + }} + onChange={(event) => setName(event.target.value)} + /> + {errors.name} + + + Email address + + + + Password + validateField('password')} + onChange={(event) => setPassword(event.target.value)} + /> + {errors.password} + + + +
+ {flowError && {flowError}} +
+ ); +} diff --git a/apps/admin/src/auth/use-auth-screens-owner.ts b/apps/admin/src/auth/use-auth-screens-owner.ts new file mode 100644 index 00000000000..d395ef94578 --- /dev/null +++ b/apps/admin/src/auth/use-auth-screens-owner.ts @@ -0,0 +1,31 @@ +import { useQueryClient } from '@tanstack/react-query'; +import { useBrowseSite } from '@tryghost/admin-x-framework/api/site'; +import { useFeatureFlagOverrides } from '@tryghost/admin-x-framework/hooks'; + +export type AuthScreensOwner = 'react' | 'ember' | 'pending'; + +// One decision per app instance (its query client): later `/site/` refetches, +// e.g. after a Labs change, must not move screens Ember has already parked. +const decisions = new WeakMap(); + +/** + * Who serves the auth screens. Decided before anyone signs in, so it reads the + * public site payload (older servers omit the field: Ember) and URL overrides, + * the same inputs Ember's feature service uses once at boot. + */ +export function useAuthScreensOwner(): AuthScreensOwner { + const queryClient = useQueryClient(); + const { enabledFlags } = useFeatureFlagOverrides(); + const { data, isError } = useBrowseSite({ defaultErrorHandler: false }); + + if (!decisions.has(queryClient)) { + if (enabledFlags.includes('authReact') || data?.site.authReact === true) { + decisions.set(queryClient, 'react'); + } else if (data) { + decisions.set(queryClient, 'ember'); + } + } + + // A failed read defers to Ember without deciding, so a later success can still decide. + return decisions.get(queryClient) ?? (isError ? 'ember' : 'pending'); +} diff --git a/apps/admin/src/routes.tsx b/apps/admin/src/routes.tsx index 84f3cef4cc4..8163287fd64 100644 --- a/apps/admin/src/routes.tsx +++ b/apps/admin/src/routes.tsx @@ -46,18 +46,11 @@ import { } from '@tryghost/admin-x-framework/api/users'; import { NotFound } from './shared/not-found'; +import { type AuthRouteHandle, authRoutes, useAuthScreensOwner } from './auth/api'; // Routes handled by the Ember admin app. React delegates these to Ember via // EmberFallback. When migrating a route to React, remove its entry from here. -const EMBER_ROUTES: string[] = [ - '/setup', - '/signin/*', - '/signout', - '/signup/*', - '/reset/*', - '/pro/*', - '/restore', -]; +const EMBER_ROUTES: string[] = ['/pro/*', '/restore']; const emberFallbackHandle = { allowInForceUpgrade: true } satisfies AdminRouteHandle; @@ -224,6 +217,8 @@ const appRoutes: RouteObject[] = [ ]; export const routes: RouteObject[] = [ + // Outside the guards: signed-out visitors have no user or settings to check. + ...authRoutes, { // ForceUpgradeGuard wraps all routes to redirect to /pro when in force upgrade mode. // Routes with handle.allowInForceUpgrade: true bypass this protection. @@ -250,6 +245,7 @@ export function useEmberOwnedRouteMatcher(): (pathname: string) => boolean { const postsListOwner = useFlagGatedRouteOwner('postsListReact'); const editorOwner = useFlagGatedRouteOwner('editorReact'); const memberActivityOwner = useFlagGatedRouteOwner('membersActivityReact'); + const authScreensOwner = useAuthScreensOwner(); return useCallback( (pathname: string) => { @@ -266,9 +262,12 @@ export function useEmberOwnedRouteMatcher(): (pathname: string) => boolean { if (leaf.Component === MemberActivityGate) { return memberActivityOwner !== 'react'; } + if ((leaf.handle as AuthRouteHandle | undefined)?.authScreen) { + return authScreensOwner !== 'react'; + } return EMBER_ROUTE_COMPONENTS.has(leaf.Component); }, - [postsListOwner, editorOwner, memberActivityOwner], + [postsListOwner, editorOwner, memberActivityOwner, authScreensOwner], ); } diff --git a/apps/admin/src/settings/advanced/labs/private-features.tsx b/apps/admin/src/settings/advanced/labs/private-features.tsx index ed752316a72..c1fb445e8cf 100644 --- a/apps/admin/src/settings/advanced/labs/private-features.tsx +++ b/apps/admin/src/settings/advanced/labs/private-features.tsx @@ -139,6 +139,12 @@ const features: Feature[] = [ description: 'Preview the new member activity screen.', flag: 'membersActivityReact', }, + { + title: 'React sign-in screens', + description: + 'Serves sign in, 2FA verification, password reset, staff invite signup, setup and sign out from the React app instead of the Ember screens. Takes effect on the next page load.', + flag: 'authReact', + }, { title: 'Self-serve archives', description: diff --git a/apps/admin/test-utils/acceptance/README.md b/apps/admin/test-utils/acceptance/README.md index 1be27b6bac1..5bbe7e71efd 100644 --- a/apps/admin/test-utils/acceptance/README.md +++ b/apps/admin/test-utils/acceptance/README.md @@ -140,3 +140,4 @@ runs the full suite. ## Known limitations - **5xx boot overrides leave a retry ticking.** The framework's fetch layer retries `ServerUnreachableError`/`MaintenanceError` (503)/`TypeError` with 500/1000ms backoff whenever `MODE !== 'development'` — and vitest runs with `MODE === 'test'`, so retries are ACTIVE here. A spec that overrides a boot response with a 503 leaves a pending retry that outlives the teardown quiet window and can fire mid-next-test. Prefer non-retryable 4xx statuses for error-shape specs; proper 5xx-retry semantics need a retry-disable seam in admin-x-framework — tracked as [PLA-242](https://linear.app/ghost/issue/PLA-242). +- **Signed-out boots share a file only with other signed-out boots.** The framework's session-expiry redirect arms once `/users/me/` has succeeded, and that state lives in the module for the whole spec file. A signed-in spec followed by a signed-out one in the same file triggers the redirect and takes the test page away; keep signed-in and signed-out specs in separate files (see `src/auth/`). diff --git a/apps/admin/test-utils/acceptance/auth.ts b/apps/admin/test-utils/acceptance/auth.ts new file mode 100644 index 00000000000..67d8d537949 --- /dev/null +++ b/apps/admin/test-utils/acceptance/auth.ts @@ -0,0 +1,57 @@ +import { siteResponse } from '@tryghost/test-data'; +import type { RenderAdminAppOptions } from './render-admin-app'; +import { type EndpointCapture, fakeAdminEndpoint } from './worker'; + +const authorizationFailed = { + errors: [ + { + type: 'NoPermissionError', + message: 'Authorization failed', + context: + 'Unable to determine the authenticated user or integration. Check that cookies are being passed through if using session authentication.', + }, + ], +}; + +/** + * Boots as a visitor with no session: Core answers every signed-in read with + * 403 "Authorization failed". `authReact` is the public site field that hands + * the auth screens to React; leave it out to boot against a server that + * predates it. + */ +export function signedOut({ authReact }: { authReact?: boolean } = {}): RenderAdminAppOptions { + const site = siteResponse(); + const forbidden = { response: authorizationFailed, responseStatus: 403 }; + + return { + boot: { + browseMe: forbidden, + browseSettings: forbidden, + browseConfig: forbidden, + browseSite: { + response: authReact === undefined ? site : { site: { ...site.site, authReact } }, + }, + }, + }; +} + +/** The setup check every signed-out auth screen makes; set-up sites by default. */ +export function fakeSetupStatus( + setup: { status: boolean; title?: string; name?: string; email?: string } = { status: true }, +): EndpointCapture { + return fakeAdminEndpoint('GET', '/authentication/setup/', { setup: [setup] }); +} + +/** The session endpoints answer with bare status text rather than JSON. */ +export function plainText(body: string): ArrayBuffer { + return new TextEncoder().encode(body).buffer; +} + +/** A base64url invite or reset token carrying `email`, shaped like Core's `expiry|email|hash`. */ +export function authToken(email: string): string { + return window + .btoa(`${Date.now() + 86_400_000}|${email}|hash`) + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=+$/, ''); +} diff --git a/apps/admin/test-utils/acceptance/index.ts b/apps/admin/test-utils/acceptance/index.ts index 32ed8ac7c9d..cb798d17ab5 100644 --- a/apps/admin/test-utils/acceptance/index.ts +++ b/apps/admin/test-utils/acceptance/index.ts @@ -1,4 +1,5 @@ /** Acceptance-harness public surface — see README.md for the spec anatomy. */ +export { authToken, fakeSetupStatus, plainText, signedOut } from './auth'; export { fakeAnalyticsOverview } from './analytics'; export { UNSPLASH_PICKED, diff --git a/apps/ember-admin/app/routes/authenticated.js b/apps/ember-admin/app/routes/authenticated.js index 4a02bb186d5..67af80b8e40 100644 --- a/apps/ember-admin/app/routes/authenticated.js +++ b/apps/ember-admin/app/routes/authenticated.js @@ -4,6 +4,7 @@ import windowProxy from 'ghost-admin/utils/window-proxy'; import {inject as service} from '@ember/service'; export default class AuthenticatedRoute extends Route { + @service feature; @service session; async beforeModel(transition) { @@ -12,6 +13,14 @@ export default class AuthenticatedRoute extends Route { if (url) { window.sessionStorage.setItem('ghost-signin-redirect', url); } + + // React's signin screen takes over from here; the reload below + // would restart the page it is rendering. A cached user means the + // session can still be restored, which requireAuthentication does. + if (this.feature.isAuthReact() && !this.session.user) { + transition.abort(); + return; + } } else { window.sessionStorage.removeItem('ghost-signin-redirect'); } diff --git a/apps/ember-admin/app/routes/reset.js b/apps/ember-admin/app/routes/reset.js index f68ff4de5ed..21d56fc31c2 100644 --- a/apps/ember-admin/app/routes/reset.js +++ b/apps/ember-admin/app/routes/reset.js @@ -5,7 +5,12 @@ export default class ResetRoute extends UnauthenticatedRoute { @service notifications; @service session; - beforeModel() { + beforeModel(transition) { + if (this.feature.isAuthReact()) { + transition.abort(); + return; + } + if (this.session.isAuthenticated) { this.notifications.showAlert('You can\'t reset your password while you\'re signed in.', {type: 'warn', delayed: true, key: 'password.reset.signed-in'}); } diff --git a/apps/ember-admin/app/routes/setup.js b/apps/ember-admin/app/routes/setup.js index 681c42838e2..5cbba8069fb 100644 --- a/apps/ember-admin/app/routes/setup.js +++ b/apps/ember-admin/app/routes/setup.js @@ -3,6 +3,7 @@ import {inject} from 'ghost-admin/decorators/inject'; import {inject as service} from '@ember/service'; export default class SetupRoute extends Route { + @service feature; @service ghostPaths; @service session; @service ajax; @@ -11,9 +12,14 @@ export default class SetupRoute extends Route { // use the beforeModel hook to check to see whether or not setup has been // previously completed. If it has, stop the transition into the setup page. - beforeModel() { + beforeModel(transition) { super.beforeModel(...arguments); + if (this.feature.isAuthReact()) { + transition.abort(); + return; + } + if (this.session.isAuthenticated) { return this.transitionTo('index'); } diff --git a/apps/ember-admin/app/routes/signout.js b/apps/ember-admin/app/routes/signout.js index beacb9e6b27..107cda0cabd 100644 --- a/apps/ember-admin/app/routes/signout.js +++ b/apps/ember-admin/app/routes/signout.js @@ -2,8 +2,20 @@ import AuthenticatedRoute from 'ghost-admin/routes/authenticated'; import {inject as service} from '@ember/service'; export default class SignoutRoute extends AuthenticatedRoute { + @service feature; @service notifications; + // React signs out when it owns the auth screens; a second DELETE here + // would race its reload. + beforeModel(transition) { + if (this.feature.isAuthReact()) { + transition.abort(); + return; + } + + return super.beforeModel(...arguments); + } + afterModel/*model, transition*/() { this.notifications.clearAll(); this.session.invalidate(); diff --git a/apps/ember-admin/app/routes/signup.js b/apps/ember-admin/app/routes/signup.js index 6ffffe146a6..8e8eb473f99 100644 --- a/apps/ember-admin/app/routes/signup.js +++ b/apps/ember-admin/app/routes/signup.js @@ -27,7 +27,12 @@ export default class SignupRoute extends UnauthenticatedRoute { @inject config; - beforeModel() { + beforeModel(transition) { + if (this.feature.isAuthReact()) { + transition.abort(); + return; + } + if (this.session.isAuthenticated) { this.notifications.showAlert('You need to sign out to register as a new user.', {type: 'warn', delayed: true, key: 'signup.create.already-authenticated'}); } diff --git a/apps/ember-admin/app/routes/unauthenticated.js b/apps/ember-admin/app/routes/unauthenticated.js index afc42d1de9c..797e3e97384 100644 --- a/apps/ember-admin/app/routes/unauthenticated.js +++ b/apps/ember-admin/app/routes/unauthenticated.js @@ -3,10 +3,19 @@ import {inject as service} from '@ember/service'; export default class UnauthenticatedRoute extends Route { @service ajax; + @service feature; @service ghostPaths; @service session; - beforeModel() { + beforeModel(transition) { + // React owns the auth screens when the flag is on. Aborting keeps this + // hidden app from checking setup or redirecting signed-in users over + // the URL React is navigating. + if (this.feature.isAuthReact()) { + transition.abort(); + return; + } + const authUrl = this.ghostPaths.url.api('authentication', 'setup'); // check the state of the setup process via the API diff --git a/apps/ember-admin/app/services/feature.js b/apps/ember-admin/app/services/feature.js index 7a325907c9e..a16c2c0cc85 100644 --- a/apps/ember-admin/app/services/feature.js +++ b/apps/ember-admin/app/services/feature.js @@ -106,6 +106,17 @@ export default class FeatureService extends Service { @feature('globalSearchReact') globalSearchReact; @feature('improveSendingUI') improveSendingUI; @feature('dunningWarnings') dunningWarnings; + + // React's auth screens decide before anyone signs in, so both shells read + // the public /site/ field (copied onto config) and URL overrides, never Labs. + // Decided once (first asked after /site/ loads), as React holds its answer. + isAuthReact() { + if (this._authReact === undefined) { + this._authReact = getStoredFeatureFlagOverrides().includes('authReact') || this.config.authReact === true; + } + return this._authReact; + } + _user = null; _featureFlagOverridesRevision = 0; diff --git a/e2e/tests/admin/auth-react.test.ts b/e2e/tests/admin/auth-react.test.ts new file mode 100644 index 00000000000..f06d7fcdd76 --- /dev/null +++ b/e2e/tests/admin/auth-react.test.ts @@ -0,0 +1,130 @@ +import { + AnalyticsOverviewPage, + InviteSignupPage, + LoginPage, + LoginVerifyPage, + PasswordResetPage, + PostsPage, + SettingsPage, +} from '@/admin-pages'; +import { EmailClient, EmailMessage, MailPit } from '@/helpers/services/email/mail-pit'; +import { expect, test, withIsolatedPage } from '@/helpers/playwright'; +import { extractInviteLink, extractPasswordResetLink } from '@/helpers/services/email/utils'; +import { usePerTestIsolation } from '@/helpers/playwright/isolation'; + +usePerTestIsolation(); + +// Journeys through the React auth screens. Each one ends on a screen the +// hidden Ember app or the React shell renders, which only works when the +// post-auth reload booted both with the new session. +test.describe('Ghost Admin - React auth screens', () => { + test.use({ labs: { authReact: true } }); + + const emailClient: EmailClient = new MailPit(); + + const codeFrom = (message: EmailMessage) => { + const code = message.Subject.match(/\d{6}/)?.[0]; + if (!code) { + throw new Error(`No verification code found in subject: ${message.Subject}`); + } + return code; + }; + + test('signs in with a resent 2FA code', async ({ page, browser, baseURL, ghostAccountOwner }) => { + await page.waitForLoadState(); + + await withIsolatedPage(browser, { baseURL }, async ({ page: freshPage }) => { + const loginPage = new LoginPage(freshPage); + await loginPage.goto(); + await loginPage.signIn(ghostAccountOwner.email, ghostAccountOwner.password); + + const verifyPage = new LoginVerifyPage(freshPage); + await expect(verifyPage.twoFactorTokenField).toBeVisible(); + await verifyPage.resendTwoFactorCodeButton.click(); + await expect(verifyPage.sentTwoFactorCodeButton).toBeVisible(); + + const messages = await emailClient.search( + { subject: 'verification code', to: ghostAccountOwner.email }, + { numberOfMessages: 2 }, + ); + await verifyPage.twoFactorTokenField.fill(codeFrom(messages[0])); + await verifyPage.twoFactorVerifyButton.click(); + + await expect(new AnalyticsOverviewPage(freshPage).header).toBeVisible(); + }); + }); + + test('resets a forgotten password and lands signed in', async ({ page, ghostAccountOwner }) => { + const loginPage = new LoginPage(page); + await loginPage.logout(); + + await loginPage.requestPasswordReset(ghostAccountOwner.email); + await expect(loginPage.body).toContainText( + 'An email with password reset instructions has been sent.', + ); + + const messages = await emailClient.search({ + subject: 'Reset Password', + to: ghostAccountOwner.email, + }); + const resetUrl = extractPasswordResetLink(await emailClient.getMessageDetailed(messages[0])); + await loginPage.goto(resetUrl); + + const newPassword = 'test@lginSecure@123'; + await new PasswordResetPage(page).resetPassword(newPassword, newPassword); + + await expect(new AnalyticsOverviewPage(page).header).toBeVisible(); + await expect(page.getByText('Password updated')).toBeVisible(); + }); + + test('a new staff member signs up from an invite link', async ({ page, browser, baseURL }) => { + const testEmail = `test-invite-${Date.now()}@example.com`; + + const settingsPage = new SettingsPage(page); + await settingsPage.staffSection.goto(); + await settingsPage.staffSection.inviteUser(testEmail); + + const messages = await emailClient.search({ + subject: 'has invited you to join', + to: testEmail, + }); + const inviteUrl = extractInviteLink(await emailClient.getMessageDetailed(messages[0])); + + await withIsolatedPage(browser, { baseURL }, async ({ page: signupPage }) => { + const inviteSignup = new InviteSignupPage(signupPage); + await signupPage.goto(inviteUrl); + await expect(inviteSignup.emailField).toHaveValue(testEmail); + await inviteSignup.acceptInvite('Test Invite User', 'test123456'); + + await signupPage.getByRole('button', { name: 'Open user menu' }).click(); + await expect(signupPage.getByText(testEmail)).toBeVisible(); + }); + }); +}); + +// The same round trip with either implementation of the auth screens: a +// cold load of a deep link while signed out, sign in, and back to the link +// (an Ember-owned screen, with its query string). +for (const { screens, authReact } of [ + { screens: 'Ember', authReact: false }, + { screens: 'React', authReact: true }, +] as const) { + test.describe(`Ghost Admin - signed-out deep link (${screens} auth screens)`, () => { + test.use({ labs: { authReact } }); + + test('returns to the link after signing in', async ({ page, ghostAccountOwner }) => { + const loginPage = new LoginPage(page); + await loginPage.logout(); + + // Leave the admin first, so opening the link is a cold load. + await page.goto('about:blank'); + await page.goto('/ghost/#/posts?type=draft'); + + await expect(loginPage.signInButton).toBeVisible(); + await loginPage.signIn(ghostAccountOwner.email, ghostAccountOwner.password); + + await new PostsPage(page).waitForList(); + await expect(page).toHaveURL(/#\/posts\/?\?type=draft$/); + }); + }); +} diff --git a/e2e/tests/admin/staff-role-smoke.test.ts b/e2e/tests/admin/staff-role-smoke.test.ts index 3e44571d8fe..7ae7a098232 100644 --- a/e2e/tests/admin/staff-role-smoke.test.ts +++ b/e2e/tests/admin/staff-role-smoke.test.ts @@ -9,119 +9,131 @@ import { import { Page } from '@playwright/test'; import { expect, test, withIsolatedPage } from '@/helpers/playwright'; -test.describe('Ghost Admin - Staff role smoke', () => { - async function signInFirstTime(page: Page, account: { email: string; password: string }) { - const loginPage = new LoginPage(page); - await loginPage.goto(); - await loginPage.signIn(account.email, account.password); - } - - async function expectNavigation( - sidebarPage: SidebarPage, - { visible, hidden }: { visible: string[]; hidden: string[] }, - ) { - await expect(sidebarPage.sidebar).toBeVisible(); - for (const name of visible) { - await expect(sidebarPage.getNavLink(name)).toBeVisible(); - } - for (const name of hidden) { - await expect(sidebarPage.getNavLink(name)).toHaveCount(0); - } - } - - test('administrator first login - lands on analytics with full navigation', async ({ - browser, - baseURL, - ghostAccountAdministrator, - }) => { - await withIsolatedPage(browser, { baseURL }, async ({ page }) => { - await signInFirstTime(page, ghostAccountAdministrator); - - const analyticsPage = new AnalyticsOverviewPage(page); - await expect(analyticsPage.header).toBeVisible(); - - const sidebarPage = new SidebarPage(page); - await expect(sidebarPage.adminSidebar).toBeVisible(); - await expectNavigation(sidebarPage, { - visible: ['Analytics', 'View site', 'Posts', 'Pages', 'Tags', 'Members', 'Settings'], - hidden: [], - }); +for (const { screens, authReact } of [ + { screens: 'Ember', authReact: false }, + { screens: 'React', authReact: true }, +] as const) { + test.describe(`Ghost Admin - Staff role smoke (${screens} auth screens)`, () => { + test.use({ labs: { authReact } }); + + // Labs flags are applied by the page fixture, which these tests don't otherwise use. + test.beforeEach(async ({ page }) => { + await page.waitForLoadState(); }); - }); - test('editor first login - lands on site view with content navigation', async ({ - browser, - baseURL, - ghostAccountEditor, - }) => { - await withIsolatedPage(browser, { baseURL }, async ({ page }) => { - await signInFirstTime(page, ghostAccountEditor); + async function signInFirstTime(page: Page, account: { email: string; password: string }) { + const loginPage = new LoginPage(page); + await loginPage.goto(); + await loginPage.signIn(account.email, account.password); + } - const sitePage = new SitePage(page); - await sitePage.waitForPageToFullyLoad(); + async function expectNavigation( + sidebarPage: SidebarPage, + { visible, hidden }: { visible: string[]; hidden: string[] }, + ) { + await expect(sidebarPage.sidebar).toBeVisible(); + for (const name of visible) { + await expect(sidebarPage.getNavLink(name)).toBeVisible(); + } + for (const name of hidden) { + await expect(sidebarPage.getNavLink(name)).toHaveCount(0); + } + } - await expectNavigation(new SidebarPage(page), { - visible: ['Posts', 'Pages', 'Tags', 'Settings'], - hidden: ['Analytics', 'View site', 'Members'], + test('administrator first login - lands on analytics with full navigation', async ({ + browser, + baseURL, + ghostAccountAdministrator, + }) => { + await withIsolatedPage(browser, { baseURL }, async ({ page }) => { + await signInFirstTime(page, ghostAccountAdministrator); + + const analyticsPage = new AnalyticsOverviewPage(page); + await expect(analyticsPage.header).toBeVisible(); + + const sidebarPage = new SidebarPage(page); + await expect(sidebarPage.adminSidebar).toBeVisible(); + await expectNavigation(sidebarPage, { + visible: ['Analytics', 'View site', 'Posts', 'Pages', 'Tags', 'Members', 'Settings'], + hidden: [], + }); }); }); - }); - - test('super editor first login - lands on site view with members navigation', async ({ - browser, - baseURL, - ghostAccountSuperEditor, - }) => { - await withIsolatedPage(browser, { baseURL }, async ({ page }) => { - await signInFirstTime(page, ghostAccountSuperEditor); - const sitePage = new SitePage(page); - await sitePage.waitForPageToFullyLoad(); - - await expectNavigation(new SidebarPage(page), { - visible: ['Posts', 'Pages', 'Tags', 'Members', 'Settings'], - hidden: ['Analytics', 'View site'], + test('editor first login - lands on site view with content navigation', async ({ + browser, + baseURL, + ghostAccountEditor, + }) => { + await withIsolatedPage(browser, { baseURL }, async ({ page }) => { + await signInFirstTime(page, ghostAccountEditor); + + const sitePage = new SitePage(page); + await sitePage.waitForPageToFullyLoad(); + + await expectNavigation(new SidebarPage(page), { + visible: ['Posts', 'Pages', 'Tags', 'Settings'], + hidden: ['Analytics', 'View site', 'Members'], + }); }); }); - }); - test('author first login - lands on site view with posts and pages navigation', async ({ - browser, - baseURL, - ghostAccountAuthor, - }) => { - await withIsolatedPage(browser, { baseURL }, async ({ page }) => { - await signInFirstTime(page, ghostAccountAuthor); - - const sitePage = new SitePage(page); - await sitePage.waitForPageToFullyLoad(); + test('super editor first login - lands on site view with members navigation', async ({ + browser, + baseURL, + ghostAccountSuperEditor, + }) => { + await withIsolatedPage(browser, { baseURL }, async ({ page }) => { + await signInFirstTime(page, ghostAccountSuperEditor); + + const sitePage = new SitePage(page); + await sitePage.waitForPageToFullyLoad(); + + await expectNavigation(new SidebarPage(page), { + visible: ['Posts', 'Pages', 'Tags', 'Members', 'Settings'], + hidden: ['Analytics', 'View site'], + }); + }); + }); - await expectNavigation(new SidebarPage(page), { - visible: ['Posts', 'Pages'], - hidden: ['Analytics', 'View site', 'Tags', 'Members', 'Settings'], + test('author first login - lands on site view with posts and pages navigation', async ({ + browser, + baseURL, + ghostAccountAuthor, + }) => { + await withIsolatedPage(browser, { baseURL }, async ({ page }) => { + await signInFirstTime(page, ghostAccountAuthor); + + const sitePage = new SitePage(page); + await sitePage.waitForPageToFullyLoad(); + + await expectNavigation(new SidebarPage(page), { + visible: ['Posts', 'Pages'], + hidden: ['Analytics', 'View site', 'Tags', 'Members', 'Settings'], + }); }); }); - }); - test('contributor first login - lands on posts list with floating user menu instead of sidebar', async ({ - browser, - baseURL, - ghostAccountContributor, - }) => { - await withIsolatedPage(browser, { baseURL }, async ({ page }) => { - await signInFirstTime(page, ghostAccountContributor); - - const postsPage = new PostsPage(page); - await postsPage.waitForPageToFullyLoad(); - - const sidebarPage = new SidebarPage(page); - await expect(sidebarPage.adminSidebar).toHaveCount(0); - - const contributorMenu = new ContributorUserMenu(page); - await contributorMenu.open(); - await expect(contributorMenu.postsMenuItem).toBeVisible(); - await expect(contributorMenu.viewSiteMenuItem).toBeVisible(); - await expect(contributorMenu.profileMenuItem).toBeVisible(); + test('contributor first login - lands on posts list with floating user menu instead of sidebar', async ({ + browser, + baseURL, + ghostAccountContributor, + }) => { + await withIsolatedPage(browser, { baseURL }, async ({ page }) => { + await signInFirstTime(page, ghostAccountContributor); + + const postsPage = new PostsPage(page); + await postsPage.waitForPageToFullyLoad(); + + const sidebarPage = new SidebarPage(page); + await expect(sidebarPage.adminSidebar).toHaveCount(0); + + const contributorMenu = new ContributorUserMenu(page); + await contributorMenu.open(); + await expect(contributorMenu.postsMenuItem).toBeVisible(); + await expect(contributorMenu.viewSiteMenuItem).toBeVisible(); + await expect(contributorMenu.profileMenuItem).toBeVisible(); + }); }); }); -}); +} diff --git a/ghost/core/core/server/api/endpoints/utils/public-config/site.js b/ghost/core/core/server/api/endpoints/utils/public-config/site.js index 3cc7db2e7d0..e02442a99eb 100644 --- a/ghost/core/core/server/api/endpoints/utils/public-config/site.js +++ b/ghost/core/core/server/api/endpoints/utils/public-config/site.js @@ -2,6 +2,7 @@ const ghostVersion = require('@tryghost/version'); const settingsCache = require('../../../../../shared/settings-cache'); const config = require('../../../../../shared/config'); const urlUtils = require('../../../../../shared/url-utils').default; +const labs = require('../../../../../shared/labs'); module.exports = function getSiteProperties() { const siteProperties = { @@ -22,6 +23,8 @@ module.exports = function getSiteProperties() { settingsCache.get('portal_signup_terms_html') ), site_uuid: settingsCache.get('site_uuid'), + // Admin's auth screens render before a session exists, so they can't read /config/ labs + authReact: labs.isSet('authReact'), }; if (config.get('client_sentry') && !config.get('client_sentry').disabled) { diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/site.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/site.js index 3c5e8092e03..15a50872627 100644 --- a/ghost/core/core/server/api/endpoints/utils/serializers/output/site.js +++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/site.js @@ -21,6 +21,7 @@ module.exports = { 'sentry_dsn', 'sentry_env', 'site_uuid', + 'authReact', ]), }; }, diff --git a/ghost/core/core/shared/labs.js b/ghost/core/core/shared/labs.js index 995f7ca1742..2893d44f600 100644 --- a/ghost/core/core/shared/labs.js +++ b/ghost/core/core/shared/labs.js @@ -65,6 +65,7 @@ const PRIVATE_FEATURES = [ 'postsListReact', 'membersActivityReact', 'editorReact', + 'authReact', 'globalSearchReact', 'dunningWarnings', ]; diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/site.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/site.test.js.snap index 6465571ef8d..7132d353bfd 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/site.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/site.test.js.snap @@ -5,6 +5,7 @@ Object { "site": Object { "accent_color": "#FF1A75", "allow_external_signup": true, + "authReact": false, "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg", "description": "Thoughts, stories and ideas", "icon": null, diff --git a/ghost/core/test/e2e-api/members/__snapshots__/site.test.js.snap b/ghost/core/test/e2e-api/members/__snapshots__/site.test.js.snap index 797aedbbbc2..f6866cc9f3d 100644 --- a/ghost/core/test/e2e-api/members/__snapshots__/site.test.js.snap +++ b/ghost/core/test/e2e-api/members/__snapshots__/site.test.js.snap @@ -5,6 +5,7 @@ Object { "site": Object { "accent_color": "#FF1A75", "allow_external_signup": true, + "authReact": true, "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg", "description": "Thoughts, stories and ideas", "icon": null, @@ -37,6 +38,7 @@ Object { "site": Object { "accent_color": "#FF1A75", "allow_external_signup": false, + "authReact": true, "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg", "description": "Thoughts, stories and ideas", "icon": null, @@ -69,6 +71,7 @@ Object { "site": Object { "accent_color": "#FF1A75", "allow_external_signup": false, + "authReact": true, "cover_image": "https://static.ghost.org/v5.0.0/images/publication-cover.jpg", "description": "Thoughts, stories and ideas", "icon": null, diff --git a/ghost/core/test/unit/server/api/endpoints/utils/public-config/site.test.js b/ghost/core/test/unit/server/api/endpoints/utils/public-config/site.test.js index 912c107b5b7..2d20cdef0bf 100644 --- a/ghost/core/test/unit/server/api/endpoints/utils/public-config/site.test.js +++ b/ghost/core/test/unit/server/api/endpoints/utils/public-config/site.test.js @@ -3,6 +3,7 @@ const sinon = require('sinon'); const configUtils = require('../../../../../../utils/config-utils'); const getSiteProperties = require('../../../../../../../core/server/api/endpoints/utils/public-config/site'); const settingsCache = require('../../../../../../../core/shared/settings-cache'); +const labs = require('../../../../../../../core/shared/labs'); describe('Public-config response builders', function () { describe('Site Properties', function () { @@ -27,6 +28,17 @@ describe('Public-config response builders', function () { assert.equal(siteProperties.timezone, 'America/Los_Angeles'); }); + it('exposes whether Admin serves the React auth screens', function () { + const isSet = sinon.stub(labs, 'isSet').returns(false); + isSet.withArgs('authReact').returns(true); + + assert.equal(getSiteProperties().authReact, true); + + isSet.withArgs('authReact').returns(false); + + assert.equal(getSiteProperties().authReact, false); + }); + describe('Sentry', function () { const fakeDSN = 'https://aaabbbccc000111222333444555667@sentry.io/1234567'; diff --git a/packages/testing/test-data/src/selectors/auth.ts b/packages/testing/test-data/src/selectors/auth.ts new file mode 100644 index 00000000000..4602557a93e --- /dev/null +++ b/packages/testing/test-data/src/selectors/auth.ts @@ -0,0 +1,30 @@ +/** + * Auth screen selector strings (sign in, verification, password reset, staff + * invite signup, setup), consumed by the admin screen helpers and the e2e page + * objects. Source of truth: apps/admin/src/auth. + */ + +// accessible names +export const emailLabel = 'Email address'; +export const passwordLabel = 'Password'; +export const signInButton = 'Sign in →'; +export const forgotButton = 'Forgot?'; +export const verificationCodeLabel = 'Verification code'; +export const verifyButton = 'Verify →'; +export const resendButton = 'Resend'; +export const newPasswordLabel = 'New password'; +export const confirmPasswordLabel = 'Confirm new password'; +export const saveNewPasswordButton = 'Save new password'; +export const fullNameLabel = 'Full name'; +export const createAccountButton = 'Create Account →'; +export const siteTitleLabel = 'Site title'; +export const startPublishingButton = 'Create account & start publishing →'; +export const retryButton = 'Retry'; + +// headings +export const resetPasswordHeading = 'Reset your password.'; +export const updatePasswordHeading = 'Update your password.'; +export const twoFactorHeading = '2FA confirmation'; +export const newDeviceHeading = "Verify it's really you"; +export const createAccountHeading = 'Create your account.'; +export const welcomeHeading = 'Welcome to Ghost.'; From c412b60f96a55f86f45b00615db4b5e3d2c79eb9 Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Tue, 29 Sep 2026 14:40:46 +0200 Subject: [PATCH 10/16] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20post=20history=20c?= =?UTF-8?q?rashing=20for=20posts=20without=20revisions=20(#31080)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no ref Opening Post history on a post with no saved revisions (e.g. created via the API or an import and never saved in the editor) crashed the editor with `Cannot read properties of undefined (reading 'feature_image_caption')`. Since 6.54.1 the `selectedRevision` getter sanitizes the caption without checking that a revision exists; it now returns early, so the preview falls back to the post's own title as before. Covered by a new Ember acceptance test that fails without the fix. --- .../app/components/modal-post-history.js | 3 +++ .../acceptance/editor/post-revisions-test.js | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/apps/ember-admin/app/components/modal-post-history.js b/apps/ember-admin/app/components/modal-post-history.js index f7a05202841..a61da4bbfd7 100644 --- a/apps/ember-admin/app/components/modal-post-history.js +++ b/apps/ember-admin/app/components/modal-post-history.js @@ -42,6 +42,9 @@ export default class ModalPostHistory extends Component { get selectedRevision() { const revision = this.revisionList[this.selectedRevisionIndex]; + if (!revision) { + return undefined; + } revision.feature_image_caption = DOMPurify.sanitize(revision.feature_image_caption, { ALLOWED_TAGS: ['a', 'b', 'i', 'span'], ALLOWED_ATTR: ['href', 'style'], diff --git a/apps/ember-admin/tests/acceptance/editor/post-revisions-test.js b/apps/ember-admin/tests/acceptance/editor/post-revisions-test.js index a5e839c3c2f..4f009589888 100644 --- a/apps/ember-admin/tests/acceptance/editor/post-revisions-test.js +++ b/apps/ember-admin/tests/acceptance/editor/post-revisions-test.js @@ -17,6 +17,21 @@ describe('Acceptance: Post revisions', function () { await loginAsRole('Administrator', this.server); }); + it('can open history for a post without revisions', async function () { + const post = this.server.create('post', { + title: 'Current Title', + status: 'draft' + }); + + await visit(`/editor/post/${post.id}`); + + await click('[data-test-psm-trigger]'); + await click('[data-test-toggle="post-history"]'); + + expect(findAll('[data-test-revision-item]').length).to.equal(0); + expect(find('[data-test-post-history-preview-title]')).to.have.trimmed.text('Current Title'); + }); + it('can restore a draft post revision', async function () { const post = this.server.create('post', { title: 'Current Title', From 40cdab8b4d1aab787cefde8897f2538558f75b1e Mon Sep 17 00:00:00 2001 From: Peter Zimon Date: Tue, 29 Sep 2026 15:24:19 +0200 Subject: [PATCH 11/16] Improved react editor settings and disabled primary button styling (#31079) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refines the React editor settings sidebar so its fields, navigation and metadata tools match the intended editor layout. Fixes [PLA-449 — Sidebar refinements](https://linear.app/ghost/issue/PLA-449/sidebar-refinements). - Keeps every settings pane at the sidebar width, with larger headings, circular back buttons with the same hover background and icon stroke weight as the sidebar toggle, a separator above history and consistent navigation rows with medium-weight labels. - Adds the URL icon and a published-post link, and gives publish date/time equal space with icons and an inline timezone. Adds a shared Shade TimePicker, also used by publishing schedules. - Adds validated canonical URLs (rejecting incomplete schemes and malformed hosts while preserving root-relative paths), restores the Google-style search preview, and gives code injection fields a white background. - Shares token-field sizing between tags, authors and member labels; tag/author chevrons stay at the top right as pills wrap. - Gives X and Facebook image uploaders a white background, subtle dashed border and centered upload icon above the label, matching tag details. - Places the primary header action last before the sidebar toggle; Unpublish and Unschedule precede Update and use ghost styling. - Fixes keyboard-shortcut labels clipped by legacy definition-list CSS. Uses individual Shade keycaps, compact hover rows and larger underlined group headings. Adds a reusable `Kbd variant="contrast"` with a slightly darker background and applies it in the sidebar. - Confirms the existing inline-excerpt flag already hides the sidebar excerpt. Also updates Shade’s disabled primary buttons globally: an opaque light grey surface with softened grey text, with semantic dark-mode colors. This applies in both host modes without a feature flag. The Button stories compare enabled and disabled controls in light/dark and current/legacy modes. Validation: focused editor acceptance suites (including scheduling, all 32 header tests and 9 shortcut tests with Mac/Windows legacy-host CSS regression coverage), 135 focused Admin unit tests, all 284 Shade tests, Admin typecheck, repository lint/boundary checks and commit hooks passed. The canonical URL follow-up passed 36 focused unit tests and 19 metadata acceptance tests, including invalid-URL rejection and recovery. Visually checked desktop and 390px mobile layouts, time-picker states and wrapping tokens in Storybook. Full `pnpm check` encountered Ghost Core test timeouts and temporary-file errors. Two Admin tests also timed out under the full run; both passed on an isolated rerun (44 tests). Koenig browser tests then loaded a different local app occupying port 5174, so the remaining full run was stopped. These failures are outside the sidebar changes. - [x] I've read and followed the Contributor Guide - [x] I've explained my change - [x] I've written automated tests for changed behavior --- apps/admin/src/editor/date-time-picker.tsx | 80 ++++++++++--------- .../src/editor/editor-header-actions.tsx | 37 ++++----- ...ettings-code-injection.acceptance.test.tsx | 7 +- ...ngs-keyboard-shortcuts.acceptance.test.tsx | 44 ++++++++++ ...tor-settings-meta-data.acceptance.test.tsx | 40 ++++++++++ ...-settings-publish-date.acceptance.test.tsx | 17 +++- .../editor-settings-url.acceptance.test.tsx | 23 ++++++ .../editor/editor-shell.acceptance.test.tsx | 6 +- apps/admin/src/editor/image-field.tsx | 23 ++++-- .../editor/session/settings-fields.test.ts | 28 +++++++ .../src/editor/session/settings-fields.ts | 23 +++++- apps/admin/src/editor/settings/README.md | 46 ++++++----- .../settings/code-injection-section.tsx | 3 +- .../settings/keyboard-shortcuts-section.tsx | 18 +++-- .../src/editor/settings/meta-data-section.tsx | 42 ++++++++-- .../editor/settings/post-history-section.tsx | 22 ++--- .../editor/settings/post-settings-sidebar.tsx | 17 ++-- .../settings/settings-navigation-row.tsx | 26 ++++++ .../settings/settings-subview-context.ts | 2 - .../src/editor/settings/settings-subview.tsx | 30 ++++--- .../editor/settings/social-card-section.tsx | 1 - .../admin/src/editor/settings/url-section.tsx | 53 ++++++++---- .../settings/use-settings-field.test.ts | 1 + .../src/editor/settings/use-settings-field.ts | 2 +- .../src/members/label-picker/label-picker.tsx | 13 +-- apps/admin/src/shared/pickers/chip-picker.tsx | 21 ++--- apps/shade/src/components.ts | 3 + .../src/components/ui/button.stories.tsx | 39 ++++++++- apps/shade/src/components/ui/button.tsx | 3 +- apps/shade/src/components/ui/kbd.stories.tsx | 33 +++++++- apps/shade/src/components/ui/kbd.tsx | 22 ++++- .../src/components/ui/time-picker.stories.tsx | 52 ++++++++++++ apps/shade/src/components/ui/time-picker.tsx | 36 +++++++++ .../src/components/ui/token-field.stories.tsx | 44 ++++++++++ apps/shade/src/components/ui/token-field.ts | 15 ++++ apps/shade/src/docs/recipes-guide.mdx | 8 ++ apps/shade/tailwind.theme.css | 2 + .../test/unit/components/ui/button.test.tsx | 2 +- apps/shade/theme-variables.css | 4 + 39 files changed, 703 insertions(+), 185 deletions(-) create mode 100644 apps/admin/src/editor/settings/settings-navigation-row.tsx create mode 100644 apps/shade/src/components/ui/time-picker.stories.tsx create mode 100644 apps/shade/src/components/ui/time-picker.tsx create mode 100644 apps/shade/src/components/ui/token-field.stories.tsx create mode 100644 apps/shade/src/components/ui/token-field.ts diff --git a/apps/admin/src/editor/date-time-picker.tsx b/apps/admin/src/editor/date-time-picker.tsx index 843eec59181..60c7b122693 100644 --- a/apps/admin/src/editor/date-time-picker.tsx +++ b/apps/admin/src/editor/date-time-picker.tsx @@ -1,12 +1,15 @@ import moment from 'moment-timezone'; import { Calendar, - Input, + InputGroup, + InputGroupAddon, + InputGroupInput, + TimePicker, Popover, PopoverContent, PopoverTrigger, } from '@tryghost/shade/components'; -import { Inline, Text } from '@tryghost/shade/primitives'; +import { Grid } from '@tryghost/shade/primitives'; import { LucideIcon } from '@tryghost/shade/utils'; import { useState } from 'react'; import { siteCalendarDay } from '@/editor/publish/publish-copy'; @@ -105,50 +108,51 @@ export function DateTimePicker({ const describedByProps = describedBy ? { 'aria-describedby': describedBy } : {}; return ( - - - - - - - - - - + + + + + + + + + + + + + + commitTime(event.target.value)} onChange={(event) => setTimeDraft(event.target.value)} {...invalidProps} {...describedByProps} /> - - - - {current.format('z')} - - - + ); } diff --git a/apps/admin/src/editor/editor-header-actions.tsx b/apps/admin/src/editor/editor-header-actions.tsx index 0e4cd6535ec..824f28d341a 100644 --- a/apps/admin/src/editor/editor-header-actions.tsx +++ b/apps/admin/src/editor/editor-header-actions.tsx @@ -232,13 +232,6 @@ function PublishActions({ <> {isDraft ? ( <> - {inputs.error ? ( <> ) : null} + ) : ( <> - {/* Ember routes a sent post to the update flow from its status line, not the header. */} {post.status === 'sent' ? null : ( - + )} + )} diff --git a/apps/admin/src/editor/editor-settings-code-injection.acceptance.test.tsx b/apps/admin/src/editor/editor-settings-code-injection.acceptance.test.tsx index fe90fcbef51..42bc9419124 100644 --- a/apps/admin/src/editor/editor-settings-code-injection.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-settings-code-injection.acceptance.test.tsx @@ -35,7 +35,6 @@ const PUBLISHED_AT = '2025-12-01T10:00:00.000Z'; const PAGE_ROUTE = new RegExp(`^/pages/${POST_ID}/\\?`); // The space reserved for the floating panel, including its outer padding. const PANEL_WIDTH = 350; -const WIDE_PANEL_WIDTH = 500; const POLL = { timeout: 10_000 }; @@ -134,16 +133,16 @@ describe('Post settings code injection', () => { await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); await openCodeInjection(); - // The pane replaces the list it was opened from, in a widened panel. + // The pane replaces the list it was opened from, without resizing the panel. await expect(editorScreen.settingsExcerpt()).toHaveCount(0); - await expect.poll(sidebarWidthPx).toBe(WIDE_PANEL_WIDTH); + await expect.poll(sidebarWidthPx).toBe(PANEL_WIDTH); await editorScreen.settingsSubviewBack(settingsCodeInjectionBackButton).click(); await expect(editorScreen.settingsSubviewPane()).toHaveCount(0); await expect.element(editorScreen.settingsExcerpt()).toBeVisible(); await expect.element(editorScreen.settingsSubviewRow(settingsCodeInjectionRow)).toBeVisible(); - // The panel goes back to the width the section list is shown at. + // Returning to the section list keeps the same width. await expect.poll(sidebarWidthPx).toBe(PANEL_WIDTH); }); diff --git a/apps/admin/src/editor/editor-settings-keyboard-shortcuts.acceptance.test.tsx b/apps/admin/src/editor/editor-settings-keyboard-shortcuts.acceptance.test.tsx index 7d40c536e4c..de39a6c5a8d 100644 --- a/apps/admin/src/editor/editor-settings-keyboard-shortcuts.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-settings-keyboard-shortcuts.acceptance.test.tsx @@ -74,6 +74,50 @@ async function openShortcuts() { * slash command the editor answers to, in the writer's own platform glyphs. */ describe('Post settings keyboard shortcuts', () => { + it.each([MAC_AGENT, WINDOWS_AGENT])( + 'keeps labels readable under legacy host styles (%s)', + async (agent) => { + onPlatform(agent); + const initialViewport = { width: window.innerWidth, height: window.innerHeight }; + onTestFinished(() => page.viewport(initialViewport.width, initialViewport.height)); + const hostStyles = document.createElement('style'); + // Ember's global definition-list and keycap rules also surround the embedded editor. + hostStyles.textContent = ` + dl { margin: 1.6em 0; } + dl dt { float: left; clear: left; overflow: hidden; margin-bottom: 1em; width: 180px; text-align: right; text-overflow: ellipsis; white-space: nowrap; font-weight: bold; } + dl dd { margin-bottom: 1em; margin-left: 200px; } + kbd { margin-bottom: 0.4em; padding: 1px 8px; border: 1px solid; box-shadow: 0 1px 0; } + `; + document.head.append(hostStyles); + onTestFinished(() => hostStyles.remove()); + fakeEditablePost(); + await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); + await openShortcuts(); + + for (const width of [1280, 390]) { + await page.viewport(width, 844); + const pane = editorScreen.settingsSubviewPane().element(); + for (const label of pane.querySelectorAll('dt')) { + expect(getComputedStyle(label).whiteSpace).toBe('normal'); + expect(label.scrollWidth).toBeLessThanOrEqual(label.clientWidth + 1); + } + for (const definition of pane.querySelectorAll('dd')) { + expect(getComputedStyle(definition).marginLeft).toBe('0px'); + } + for (const group of pane.querySelectorAll('[data-slot="kbd-group"]')) { + expect(getComputedStyle(group).borderTopWidth).toBe('0px'); + const capHeight = Math.max( + ...[...group.querySelectorAll('[data-slot="kbd"]')].map( + (cap) => cap.getBoundingClientRect().height, + ), + ); + expect(group.getBoundingClientRect().height).toBeLessThanOrEqual(capHeight + 1); + } + expect(pane.scrollWidth).toBeLessThanOrEqual(pane.clientWidth + 1); + } + }, + ); + it('opens the pane over the section list and comes back from it', async () => { fakeEditablePost(); await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); diff --git a/apps/admin/src/editor/editor-settings-meta-data.acceptance.test.tsx b/apps/admin/src/editor/editor-settings-meta-data.acceptance.test.tsx index 22e9691c089..f60f5961b4d 100644 --- a/apps/admin/src/editor/editor-settings-meta-data.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-settings-meta-data.acceptance.test.tsx @@ -76,6 +76,46 @@ function countdownIsOver(): boolean { * given instead of the post's own, and the result they produce. */ describe('Post settings meta data', () => { + it('saves and clears the canonical URL and uses it in the search preview', async () => { + const saveApi = fakeSavablePost(); + await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); + await openMetaData(); + const canonical = page.getByRole('textbox', { name: 'Canonical URL' }); + await canonical.fill('https://original.example.com/story/'); + await userEvent.tab(); + await expect(saveApi).toHaveSavedFields({ + canonical_url: 'https://original.example.com/story/', + }); + await expect + .element(editorScreen.settingsSerpPreview()) + .toHaveTextContent('original.example.com › story'); + await canonical.fill(''); + await userEvent.tab(); + await expect(saveApi).toHaveSavedFields({ canonical_url: null }); + }); + + it.each(['not a url', 'https://'])( + 'keeps invalid canonical URL %s unsaved until corrected', + async (invalidUrl) => { + const saveApi = fakeSavablePost(); + await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); + await openMetaData(); + const canonical = page.getByRole('textbox', { name: 'Canonical URL' }); + await canonical.fill(invalidUrl); + await userEvent.tab(); + await expect.element(canonical).toHaveAttribute('aria-invalid', 'true'); + await expect + .element(page.getByText('Please enter a valid URL', { exact: true })) + .toBeVisible(); + expect(saveApi.requests).toHaveLength(0); + await canonical.fill('https://original.example.com/story/'); + await userEvent.tab(); + await expect(saveApi).toHaveSavedFields({ + canonical_url: 'https://original.example.com/story/', + }); + }, + ); + it('opens the pane over the section list and comes back from it', async () => { fakeSavablePost(); await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); diff --git a/apps/admin/src/editor/editor-settings-publish-date.acceptance.test.tsx b/apps/admin/src/editor/editor-settings-publish-date.acceptance.test.tsx index 25ba6ffefb7..eb013626d10 100644 --- a/apps/admin/src/editor/editor-settings-publish-date.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-settings-publish-date.acceptance.test.tsx @@ -91,10 +91,23 @@ async function openPublishDate() { await expect.element(editorScreen.settingsPublishDate()).toBeVisible(); } +async function leaveTimeField() { + // Native time controls tab through their hour/minute (and locale-specific period) + // segments before leaving the input. A save commits when the whole field blurs. + for ( + let segment = 0; + segment < 4 && document.activeElement === editorScreen.settingsPublishTime().element(); + segment++ + ) { + await userEvent.tab(); + } + expect(document.activeElement).not.toBe(editorScreen.settingsPublishTime().element()); +} + async function setTime(value: string) { await editorScreen.settingsPublishTime().fill(value); // The field commits on blur, as the publish flow's does. - await userEvent.tab(); + await leaveTimeField(); } /** The sidebar's Publish date section: when the post is published, in site time. */ @@ -172,7 +185,7 @@ describe('Post settings publish date', () => { // Tabbing through the field leaves the stored timestamp alone. await editorScreen.settingsPublishTime().click(); - await userEvent.tab(); + await leaveTimeField(); await expect.element(editorScreen.updateButton()).toBeDisabled(); // A move away and back lands on that minute again, seconds intact. diff --git a/apps/admin/src/editor/editor-settings-url.acceptance.test.tsx b/apps/admin/src/editor/editor-settings-url.acceptance.test.tsx index 28a8319af32..a07c8bf18e3 100644 --- a/apps/admin/src/editor/editor-settings-url.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-settings-url.acceptance.test.tsx @@ -48,6 +48,29 @@ async function openSidebar() { * through the slug machine rather than written as a settings field. */ describe('Post settings URL', () => { + it('links to a published post’s saved URL, including its custom route', async () => { + fakeSlugs(); + fakeSavablePost({ + status: 'published', + published_at: PUBLISHED_AT, + url: 'https://example.com/journal/saved-post/', + }); + await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); + await openSidebar(); + const link = page.getByRole('link', { name: 'View post' }); + await expect.element(link).toHaveAttribute('href', 'https://example.com/journal/saved-post/'); + await editorScreen.settingsSlug().fill('unsaved-slug'); + await userEvent.tab(); + await expect.element(link).toHaveAttribute('href', 'https://example.com/journal/saved-post/'); + }); + + it('does not offer a public link for a draft', async () => { + fakeSavablePost({ status: 'draft' }); + await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); + await openSidebar(); + await expect(page.getByRole('link', { name: 'View post' })).toHaveCount(0); + }); + it('saves the focused URL edit with Cmd-S while generation is pending', async () => { const generated = deferred<{ slugs: { slug: string }[] }>(); const slugApi = fakeAdminEndpoint('GET', /^\/slugs\/post\//, () => generated.promise); diff --git a/apps/admin/src/editor/editor-shell.acceptance.test.tsx b/apps/admin/src/editor/editor-shell.acceptance.test.tsx index b7bd8559569..fc525a651ed 100644 --- a/apps/admin/src/editor/editor-shell.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-shell.acceptance.test.tsx @@ -420,7 +420,7 @@ describe('Floating editor shell', () => { await expect.element(editorScreen.settingsSubviewPane()).toBeVisible(); const sidebar = editorScreen.settingsSidebar().element(); await expect.poll(() => sidebar.getBoundingClientRect().right).toBe(window.innerWidth - 8); - expect(sidebar.getBoundingClientRect().width).toBe(492); + expect(sidebar.getBoundingClientRect().width).toBe(342); await expect(editorScreen.settingsToggle()).toHaveCount(1); expect(editorScreen.settingsToggle().element()).toBe(toggle); expect(toggle.getBoundingClientRect()).toEqual(toggleBefore); @@ -433,7 +433,7 @@ describe('Floating editor shell', () => { expect(document.activeElement).toBe(editorScreen.settingsToggle().element()); }); - it('keeps a full-width image inside the writing pane beside normal and wide settings', async () => { + it('keeps a full-width image inside the writing pane beside the settings list and its subpanels', async () => { const imageUrl = URL.createObjectURL( new Blob( [ @@ -476,7 +476,7 @@ describe('Floating editor shell', () => { expect(pane.getBoundingClientRect().right).toBe(sidebar.getBoundingClientRect().left); await editorScreen.settingsSubviewRow('Code injection').click(); - await expect.poll(() => sidebar.parentElement!.getBoundingClientRect().width).toBe(500); + await expect.poll(() => sidebar.parentElement!.getBoundingClientRect().width).toBe(350); await expect .poll(() => image.element().getBoundingClientRect().right) .toBeCloseTo(pane.getBoundingClientRect().right - 12, 0); diff --git a/apps/admin/src/editor/image-field.tsx b/apps/admin/src/editor/image-field.tsx index fdad203a66e..872772fef2d 100644 --- a/apps/admin/src/editor/image-field.tsx +++ b/apps/admin/src/editor/image-field.tsx @@ -21,10 +21,21 @@ const VARIANTS = { empty: 'h-14', dropzone: 'group/dropzone w-auto border-0 bg-transparent px-0 shadow-none hover:bg-transparent', prompt: 'transition-colors group-hover/dropzone:text-foreground', + icon: LucideIcon.Plus, + iconClassName: 'size-4', label: 'text-base', unsplash: 'static', }, - panel: { empty: 'h-[120px]', dropzone: '', prompt: '', label: 'text-sm', unsplash: '' }, + panel: { + empty: 'h-[120px]', + dropzone: + 'group/dropzone border-dashed border-border-default bg-surface-elevated transition-colors', + prompt: 'transition-colors group-hover/dropzone:text-foreground', + icon: LucideIcon.Upload, + iconClassName: 'size-6 stroke-[1.5px]', + label: 'text-sm', + unsplash: '', + }, }; export type ImageFieldVariant = keyof typeof VARIANTS; @@ -70,6 +81,8 @@ export function ImageField({ const { isUploading, onUpload } = upload; const styles = VARIANTS[variant]; const EmptyContainer = variant === 'bar' ? Inline : ImageUpload; + const PromptContainer = variant === 'bar' ? Inline : Stack; + const PromptIcon = styles.icon; const addLabel = `Add ${subject}`; if (!src) { @@ -91,15 +104,15 @@ export function ImageField({ {isUploading ? ( ) : ( - - + + )} { } }); }); + +describe('canonical URL validation', () => { + it.each(['https://example.com/original/', 'http://localhost:2368/story/', '/original/', ''])( + 'accepts %s', + (canonicalUrl) => { + expect( + settingsFieldErrorFor('canonical_url', { ...VALID, canonical_url: canonicalUrl }), + ).toBeNull(); + }, + ); + it.each([ + 'example.com/path', + 'https://example.com/a b', + 'https://', + 'https://[invalid]', + 'https://example.com:invalid', + ])('refuses %s', (canonicalUrl) => { + expect(settingsFieldErrorFor('canonical_url', { ...VALID, canonical_url: canonicalUrl })).toBe( + 'Please enter a valid URL', + ); + }); + it('enforces the URL column limit', () => { + expect( + settingsFieldErrorFor('canonical_url', { ...VALID, canonical_url: '/' + 'a'.repeat(2000) }), + ).toBe('Canonical URL is too long, max 2000 chars'); + }); +}); diff --git a/apps/admin/src/editor/session/settings-fields.ts b/apps/admin/src/editor/session/settings-fields.ts index bd4bf1f2d0d..724a31a5156 100644 --- a/apps/admin/src/editor/session/settings-fields.ts +++ b/apps/admin/src/editor/session/settings-fields.ts @@ -127,6 +127,7 @@ export const VALIDATED_SETTINGS_FIELD_KEYS = [ 'tiers', 'meta_title', 'meta_description', + 'canonical_url', 'og_title', 'og_description', 'twitter_title', @@ -145,7 +146,7 @@ export function validatedFieldsOf(fields: ValidatedSettingsFields): ValidatedSet /** The width each text field is held to, and what it says when it is past it. */ const LENGTH_RULES: Record< - Exclude, + Exclude, { max: number; message: string } > = { meta_title: { max: META_TITLE_MAX, message: META_TITLE_TOO_LONG }, @@ -168,6 +169,26 @@ export function settingsFieldErrorFor( if (key === 'tiers') { return tiersIncomplete(fields) ? TIERS_REQUIRED : null; } + if (key === 'canonical_url') { + const url = fields.canonical_url; + if (!url) { + return null; + } + if (/\s/.test(url)) { + return 'Please enter a valid URL'; + } + // Root-relative paths are supported; absolute URLs must have a valid host. + if (!url.startsWith('/')) { + try { + if (!new URL(url).hostname) { + return 'Please enter a valid URL'; + } + } catch { + return 'Please enter a valid URL'; + } + } + return overLength(url, 2000) ? 'Canonical URL is too long, max 2000 chars' : null; + } const { max, message } = LENGTH_RULES[key]; return overLength(fields[key], max) ? message : null; } diff --git a/apps/admin/src/editor/settings/README.md b/apps/admin/src/editor/settings/README.md index f7dcbd56856..8c282291776 100644 --- a/apps/admin/src/editor/settings/README.md +++ b/apps/admin/src/editor/settings/README.md @@ -42,11 +42,9 @@ write is the entry the map leaves out. The prose below follows that order. Some sections are a row that opens a pane over the rest of the panel rather than fields in the list. `SettingsSubview` in `settings-subview.tsx` is both halves: give it the section's own id, an icon and a label for the row, a title and a -back-button label for the pane, and the pane's fields as children. Two props -adjust the shell around them: `wide` widens the panel for a pane that needs the -room, and `contentClassName` overrides the pane body's default padding for a pane -that runs full-bleed. Without either, the shell renders the pane as it renders -this one. +back-button label for the pane, and the pane's fields as children. +`contentClassName` overrides the pane body's default padding for a pane that +runs full-bleed. Every pane keeps the same width as the section list. Only one pane is open at a time. While it is, the panel shows that section alone: its heading, the other sections and their rows are all out of the way, @@ -88,8 +86,8 @@ editor entry. There is no keyboard shortcut for it. Below the `lg` breakpoint the panel overlays the editor from the right rather than narrowing it, and below 500px it takes the full width. Above it the panel -sits in the flow beside the editor at a fixed 350px, widening to 500px for a -pane that asks for the room. +sits in the flow beside the editor at a fixed 350px, including while a subview +is open. ## URL @@ -109,17 +107,18 @@ navigation and tab-close guards ask about it; the session owns [that wait](../session/README.md#the-slug). The preview under the input is the site URL without its scheme, then the slug, -both slash-terminated. The section is the slug and that preview and nothing -else: it does not link out to a published post, and a sent post previews its -site URL like any other rather than the separate email URL it also has. +both slash-terminated. Published posts also show a View post link beside the +label. It uses the saved record's URL, preserving custom routes and avoiding links to an unsaved slug. +A sent post previews its site URL like any other rather than its separate email URL. ## Publish date When the post is published, edited in the site's timezone and carried as a UTC instant. A post that has no publish time yet shows the current moment, and only an edit stages a value, so an untouched draft still leaves the time to the -server. The date is chosen from a calendar and the time typed as `HH:mm`; an -unparseable time returns to the value already held. Both fields commit at minute +server. The fields share the row equally, with calendar and clock icons and the +timezone inside the time field. The date is chosen from a calendar and the time entered +through Shade's native `TimePicker`, whose value is `HH:mm`; an unparseable time returns to the value already held. Both fields commit at minute granularity, and the seconds a publish stamped are kept whenever the committed minute is the one already saved. Tabbing through an untouched time, retyping it, or choosing the displayed calendar day does not commit a value. @@ -339,16 +338,21 @@ still closes the pane. ## Meta data -Meta data is a pane, and every role that can open the panel can open it. A meta -field cleared back to empty is stored as no value, as the excerpt is. +Meta data is a pane containing the title, description and canonical URL, and every +role that can open the panel can open it. A meta field cleared back to empty is stored as no value, as the excerpt is. -Neither field is required, and the character counts beside them are a -recommendation rather than a limit: 60 for the title, 145 for the description, +Neither title nor description is required, and the character counts beside them +are a recommendation rather than a limit: 60 for the title, 145 for the description, counted as symbols so a multibyte character counts once, and coloured once the writer is past the recommendation. -The preview under them is the result the post would produce. Each line falls -back rather than emptying: the title is the meta title, else the title the +The optional canonical URL accepts root-relative paths or absolute URLs with a +valid host, rejects whitespace, and keeps Ember's 2,000-character limit. Invalid +values stay staged and block saves until corrected. Clearing the field stores +no canonical override. + +The preview under them is the result the post would produce, with a Google logo, +search bar and blue result title. Each line falls back rather than emptying: the title is the meta title, else the title the writer is looking at, else `(Untitled)`; the description is the meta description, else the post's excerpt, else a sentence explaining that search engines will compose their own. The address is the canonical URL when the post @@ -396,6 +400,12 @@ user agent as the pane renders. Hovering a glyph names the key it stands for; a key already shown as its name carries no tooltip. A slash command reads the same wherever it is typed. +The reference uses compact rows with a shared hover background, wrapping labels, +and underlined group headings. Definition-list spacing is reset locally so Ember's +global list styles cannot indent or truncate the labels. Shade's `KbdGroup` only +lays out the individual `Kbd` caps; it does not draw another cap around them. +The caps use `variant="contrast"` to stand out against the sidebar background. + ## Delete Deleting is the one thing in the panel that does not go through the session: it diff --git a/apps/admin/src/editor/settings/code-injection-section.tsx b/apps/admin/src/editor/settings/code-injection-section.tsx index 994dca306b2..26c2bd9b6e1 100644 --- a/apps/admin/src/editor/settings/code-injection-section.tsx +++ b/apps/admin/src/editor/settings/code-injection-section.tsx @@ -56,9 +56,9 @@ export function CodeInjectionSection({ session, postType }: CodeInjectionSection id="code-injection" label="Code injection" title="Code injection" - wide > } @@ -68,6 +68,7 @@ export function CodeInjectionSection({ session, postType }: CodeInjectionSection onChange={(value) => session.stageSettings({ codeinjection_head: value || null })} /> } diff --git a/apps/admin/src/editor/settings/keyboard-shortcuts-section.tsx b/apps/admin/src/editor/settings/keyboard-shortcuts-section.tsx index 2c712e9293a..f0d0851f2e3 100644 --- a/apps/admin/src/editor/settings/keyboard-shortcuts-section.tsx +++ b/apps/admin/src/editor/settings/keyboard-shortcuts-section.tsx @@ -36,6 +36,7 @@ function KeyCap({ token }: { token: ShortcutKey }) { token.tooltip && 'pointer-events-auto', )} role={token.tooltip ? 'img' : undefined} + variant="contrast" > {token.text} @@ -58,13 +59,18 @@ function KeyCap({ token }: { token: ShortcutKey }) { function ShortcutRow({ shortcut }: { shortcut: Shortcut }) { return ( - -
+ +
{shortcut.label}
-
+
{shortcut.keys.map((token) => ( @@ -88,11 +94,11 @@ export function KeyboardShortcutsSection() { title="Keyboard shortcuts" > {groups.map((group) => ( - - + + {group.title} -
+
{group.shortcuts.map((shortcut) => ( ))} diff --git a/apps/admin/src/editor/settings/meta-data-section.tsx b/apps/admin/src/editor/settings/meta-data-section.tsx index 5d8fe1e5554..303e317b8b4 100644 --- a/apps/admin/src/editor/settings/meta-data-section.tsx +++ b/apps/admin/src/editor/settings/meta-data-section.tsx @@ -1,6 +1,13 @@ import { useId } from 'react'; -import { Field, FieldError, FieldLabel, Input, Textarea } from '@tryghost/shade/components'; -import { Stack, Text } from '@tryghost/shade/primitives'; +import { + Field, + FieldError, + FieldLabel, + GoogleLogo, + Input, + Textarea, +} from '@tryghost/shade/components'; +import { Inline, Stack, Text } from '@tryghost/shade/primitives'; import { LucideIcon, cn, formatNumber } from '@tryghost/shade/utils'; import { settingsMetaDescriptionInput, @@ -49,17 +56,27 @@ function SearchPreview({ }) { return ( - + + + {url} - + {serpTitle(title)} - + {serpDate(new Date())} — {serpDescription(description)} @@ -79,6 +96,7 @@ export function MetaDataSection({ session, siteUrl }: MetaDataSectionProps) { const titleHintId = useId(); const descriptionHintId = useId(); + const canonical = useSettingsField(session, 'canonical_url'); const title = useSettingsField(session, 'meta_title', titleHintId); const description = useSettingsField(session, 'meta_description', descriptionHintId); @@ -100,7 +118,6 @@ export function MetaDataSection({ session, siteUrl }: MetaDataSectionProps) { id="meta-data" label="Meta data" title="Meta data" - wide > Meta title @@ -129,6 +146,17 @@ export function MetaDataSection({ session, siteUrl }: MetaDataSectionProps) { {description.error} + + Canonical URL + + {canonical.error} + + Search Engine Result Preview diff --git a/apps/admin/src/editor/settings/post-history-section.tsx b/apps/admin/src/editor/settings/post-history-section.tsx index 2f6431ebd19..108686291fc 100644 --- a/apps/admin/src/editor/settings/post-history-section.tsx +++ b/apps/admin/src/editor/settings/post-history-section.tsx @@ -1,5 +1,4 @@ import { useMemo, useRef, useState } from 'react'; -import { Inline, Text } from '@tryghost/shade/primitives'; import { LucideIcon } from '@tryghost/shade/utils'; import { useFocusContext } from '@tryghost/shade/app'; import { settingsPostHistoryButton } from '@tryghost/test-data/selectors/editor'; @@ -9,7 +8,7 @@ import { useSiteTimezone } from '@/editor/use-editor-settings'; import type { EditorSettingsPort } from './editor-settings-port'; import { canViewPostHistory, revisionEntries, type RevisionEntry } from './post-history'; import { PostHistoryModal } from './post-history-modal'; -import { SettingsSection } from './settings-section'; +import { SettingsNavigationRow } from './settings-navigation-row'; export interface PostHistorySectionProps { session: EditorSettingsPort; @@ -60,22 +59,15 @@ export function PostHistorySection({ }); return ( - - + {postType === 'page' ? 'Page' : 'Post'} history + {open ? ( ) : null} - + ); } diff --git a/apps/admin/src/editor/settings/post-settings-sidebar.tsx b/apps/admin/src/editor/settings/post-settings-sidebar.tsx index c25e522cc03..fbf02b055aa 100644 --- a/apps/admin/src/editor/settings/post-settings-sidebar.tsx +++ b/apps/admin/src/editor/settings/post-settings-sidebar.tsx @@ -1,7 +1,6 @@ import { Fragment, memo, type ReactNode, useEffect, useId } from 'react'; -import { Label, Switch, Textarea } from '@tryghost/shade/components'; +import { Label, Separator, Switch, Textarea } from '@tryghost/shade/components'; import { Box, Inline, Text } from '@tryghost/shade/primitives'; -import { cn } from '@tryghost/shade/utils'; import { canAccessSettings, isAuthorOrContributor, @@ -187,12 +186,7 @@ export function PostSettingsSidebar({ return ( - + diff --git a/apps/admin/src/editor/settings/settings-navigation-row.tsx b/apps/admin/src/editor/settings/settings-navigation-row.tsx new file mode 100644 index 00000000000..6efbc7d831e --- /dev/null +++ b/apps/admin/src/editor/settings/settings-navigation-row.tsx @@ -0,0 +1,26 @@ +import { forwardRef, type ComponentProps, type ReactNode } from 'react'; +import { Text } from '@tryghost/shade/primitives'; +import { LucideIcon, cn } from '@tryghost/shade/utils'; + +export const SettingsNavigationRow = forwardRef< + HTMLButtonElement, + ComponentProps<'button'> & { icon: ReactNode } +>(function SettingsNavigationRow({ icon, children, className, ...props }, ref) { + return ( + + ); +}); diff --git a/apps/admin/src/editor/settings/settings-subview-context.ts b/apps/admin/src/editor/settings/settings-subview-context.ts index 35e93c08141..2619530215c 100644 --- a/apps/admin/src/editor/settings/settings-subview-context.ts +++ b/apps/admin/src/editor/settings/settings-subview-context.ts @@ -5,8 +5,6 @@ export interface OpenSubview { id: SettingsSectionId; /** The pane's heading, which names the panel while the pane is open. */ title: string; - /** The pane needs more room than the section list does. */ - wide: boolean; } export interface SubviewController { diff --git a/apps/admin/src/editor/settings/settings-subview.tsx b/apps/admin/src/editor/settings/settings-subview.tsx index 636b85700f1..939e0572019 100644 --- a/apps/admin/src/editor/settings/settings-subview.tsx +++ b/apps/admin/src/editor/settings/settings-subview.tsx @@ -2,6 +2,7 @@ import { type ReactNode, useEffect, useRef } from 'react'; import { Button } from '@tryghost/shade/components'; import { Box, Inline, Stack, Text } from '@tryghost/shade/primitives'; import { LucideIcon, cn } from '@tryghost/shade/utils'; +import { SettingsNavigationRow } from './settings-navigation-row'; import { settingsSubviewPane } from '@tryghost/test-data/selectors/editor'; import type { SettingsSectionId } from './sections'; import { useSubviews } from './settings-subview-context'; @@ -16,7 +17,6 @@ export interface SettingsSubviewProps { title: string; /** The accessible name of the pane's back button. */ closeLabel: string; - wide?: boolean; /** Overrides the pane body's default padding, for a pane that runs full-bleed. */ contentClassName?: string; children: ReactNode; @@ -32,7 +32,6 @@ export function SettingsSubview({ label, title, closeLabel, - wide = false, contentClassName, children, }: SettingsSubviewProps) { @@ -59,10 +58,18 @@ export function SettingsSubview({ <>
- - + {title}