diff --git a/e2e/deployment_test.go b/e2e/deployment_test.go index b3ec833ca8..31c1bf5578 100644 --- a/e2e/deployment_test.go +++ b/e2e/deployment_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/mxschmitt/playwright-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -41,15 +42,18 @@ func TestComment_AdminEditHasNoDeadline(t *testing.T) { postComment(t, frame, text) // the countdown is what #2001 left running. an admin has no deadline, so it should never - // have been rendered at all - // asserted on a loaded thread, not on the comment the widget has just added to the - // page. The optimistic render puts a countdown on an admin's own new comment and drops it on - // the next load, so asserting here would fail on that instead of on the deadline logic this - // case is about + // have been rendered at all. Asserted on the comment just posted and again on a loaded + // thread, because #2205 showed only on the first: right after sign-in the widget did not + // know the user was an admin + assertNoCountdown := func(frame playwright.FrameLocator, msg string) { + count, err := actions(frame, text).Locator(`[role="timer"]`).Count() + require.NoError(t, err) + assert.Zero(t, count, msg) + } + assertNoCountdown(frame, "an admin's new comment is counting down an edit window that does not apply to it") + frame = reload(t, page) - count, err := actions(frame, text).Locator(`[role="timer"]`).Count() - require.NoError(t, err) - assert.Zero(t, count, "an admin's comment is counting down an edit window that does not apply to it") + assertNoCountdown(frame, "an admin's comment is counting down an edit window that does not apply to it") // past the window every other user on this instance is held to. editWindow is EDIT_TIME on // both short-window instances in compose-e2e-test.yml diff --git a/frontend/apps/remark42/app/common/api.ts b/frontend/apps/remark42/app/common/api.ts index 1cf9381fe8..e763fdd67e 100644 --- a/frontend/apps/remark42/app/common/api.ts +++ b/frontend/apps/remark42/app/common/api.ts @@ -74,9 +74,15 @@ export async function getUser(): Promise { clearAuthCookie(XSRF_COOKIE); return null; } - return apiFetcher.get('/user').catch(() => null); + return getSignedInUser().catch(() => null); } +/** + * The current user as the API knows it, without the `/auth/status` probe `getUser` makes first. + * Rejects when nobody is signed in + */ +export const getSignedInUser = (): Promise => apiFetcher.get('/user'); + export const uploadImage = (image: File): Promise => { const data = new FormData(); data.append('file', image); diff --git a/frontend/apps/remark42/app/components/auth/auth.api.spec.ts b/frontend/apps/remark42/app/components/auth/auth.api.spec.ts index da4d397ba6..bce61ee73c 100644 --- a/frontend/apps/remark42/app/components/auth/auth.api.spec.ts +++ b/frontend/apps/remark42/app/components/auth/auth.api.spec.ts @@ -1,6 +1,9 @@ +import type { User } from 'common/types'; import { apiFetcher, JWT_HEADER } from 'common/fetcher'; +import { user } from '__stubs__/user'; +import { RequestError } from 'utils/errorUtils'; -import { logout, oauthSignin } from './auth.api'; +import { anonymousSignin, logout, oauthSignin, verifyEmailSignin, verifyTelegramSignin } from './auth.api'; /** * The OAuth flow has no message from the popup to listen for: the popup closes itself and the @@ -119,6 +122,42 @@ describe('oauthSignin', () => { }); }); +/** + * A sign-in endpoint answers with the auth library's user, which keeps the admin flag under + * `attrs`. What reaches the store has to be the API's user instead, or an admin is shown as a + * regular user until the page is reloaded (#2205) + */ +describe.each` + name | signin + ${'anonymousSignin'} | ${() => anonymousSignin('username')} + ${'verifyEmailSignin'} | ${() => verifyEmailSignin('token')} + ${'verifyTelegramSignin'} | ${() => verifyTelegramSignin('token')} +`('$name', ({ signin }: { signin: () => Promise }) => { + const json = { headers: { 'Content-Type': 'application/json' } }; + + beforeEach(() => { + fetchMock.resetMocks(); + }); + + it('resolves the user the API knows rather than the one sign-in answered with', async () => { + const admin = { ...user, admin: true }; + fetchMock.mockResponses( + [JSON.stringify({ id: user.id, name: user.name, attrs: { admin: true } }), json], + [JSON.stringify(admin), json] + ); + + await expect(signin()).resolves.toEqual(admin); + expect(fetchMock.mock.calls).toHaveLength(2); + expect(fetchMock.mock.calls[1][0]).toContain('/api/v1/user'); + }); + + it('rejects when the signed-in user cannot be loaded', async () => { + fetchMock.mockResponses(['{}', json], ['', { status: 401 }]); + + await expect(signin()).rejects.toBeInstanceOf(RequestError); + }); +}); + describe('logout', () => { beforeEach(() => { fetchMock.resetMocks(); diff --git a/frontend/apps/remark42/app/components/auth/auth.api.ts b/frontend/apps/remark42/app/components/auth/auth.api.ts index 421069da53..00dca6296e 100644 --- a/frontend/apps/remark42/app/components/auth/auth.api.ts +++ b/frontend/apps/remark42/app/components/auth/auth.api.ts @@ -2,13 +2,25 @@ import type { User } from 'common/types'; import { authFetcher, dropSession } from 'common/fetcher'; import { siteId } from 'common/settings'; -import { getUser } from 'common/api'; +import { getSignedInUser, getUser } from 'common/api'; const EMAIL_SIGNIN_ENDPOINT = '/email/login'; const TELEGRAM_SIGNIN_ENDPOINT = '/telegram/login'; +/** + * The user a sign-in endpoint answers with is the auth library's own shape, which carries the + * admin flag under `attrs` rather than as `admin`, so storing it made an admin a regular user + * until the next page load (#2205). Each sign-in loads the user from the API instead. It goes to + * `/user` directly rather than through `getUser`, whose `/auth/status` probe has nothing to tell + * straight after a sign-in has succeeded + */ +async function signin(endpoint: string, query: Record): Promise { + await authFetcher.get(endpoint, query); + return getSignedInUser(); +} + export function anonymousSignin(user: string): Promise { - return authFetcher.get('/anonymous/login', { user, aud: siteId }); + return signin('/anonymous/login', { user, aud: siteId }); } /** @@ -22,7 +34,7 @@ export function emailSignin(email: string, username: string): Promise { * Second step of two of `email` authorization */ export function verifyEmailSignin(token: string): Promise { - return authFetcher.get(EMAIL_SIGNIN_ENDPOINT, { token }); + return signin(EMAIL_SIGNIN_ENDPOINT, { token }); } /** @@ -118,7 +130,7 @@ export function getTelegramSigninParams(): Promise<{ * Second step of two of `telegram` authorization */ export function verifyTelegramSignin(token: string): Promise { - return authFetcher.get(TELEGRAM_SIGNIN_ENDPOINT, { token }); + return signin(TELEGRAM_SIGNIN_ENDPOINT, { token }); } /**