Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions e2e/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"
"time"

"github.com/mxschmitt/playwright-go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion frontend/apps/remark42/app/common/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,15 @@ export async function getUser(): Promise<User | null> {
clearAuthCookie(XSRF_COOKIE);
return null;
}
return apiFetcher.get<User | null>('/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<User> => apiFetcher.get<User>('/user');

export const uploadImage = (image: File): Promise<Image> => {
const data = new FormData();
data.append('file', image);
Expand Down
41 changes: 40 additions & 1 deletion frontend/apps/remark42/app/components/auth/auth.api.spec.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<User> }) => {
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();
Expand Down
20 changes: 16 additions & 4 deletions frontend/apps/remark42/app/components/auth/auth.api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>): Promise<User> {
await authFetcher.get(endpoint, query);
return getSignedInUser();
}

export function anonymousSignin(user: string): Promise<User> {
return authFetcher.get<User>('/anonymous/login', { user, aud: siteId });
return signin('/anonymous/login', { user, aud: siteId });
}

/**
Expand All @@ -22,7 +34,7 @@ export function emailSignin(email: string, username: string): Promise<unknown> {
* Second step of two of `email` authorization
*/
export function verifyEmailSignin(token: string): Promise<User> {
return authFetcher.get(EMAIL_SIGNIN_ENDPOINT, { token });
return signin(EMAIL_SIGNIN_ENDPOINT, { token });
}

/**
Expand Down Expand Up @@ -118,7 +130,7 @@ export function getTelegramSigninParams(): Promise<{
* Second step of two of `telegram` authorization
*/
export function verifyTelegramSignin(token: string): Promise<User> {
return authFetcher.get(TELEGRAM_SIGNIN_ENDPOINT, { token });
return signin(TELEGRAM_SIGNIN_ENDPOINT, { token });
}

/**
Expand Down
Loading