Skip to content

Commit

Permalink
check /status before getting user details
Browse files Browse the repository at this point in the history
  • Loading branch information
paskal committed Mar 16, 2024
1 parent e574318 commit d432667
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 11 deletions.
12 changes: 10 additions & 2 deletions frontend/apps/remark42/app/common/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import {
Sorting,
BlockTTL,
Image,
StatusResponse,
EmailSubVerificationStatus,
} from './types';
import { apiFetcher, adminFetcher } from './fetcher';
import { apiFetcher, adminFetcher, authFetcher } from './fetcher';

/* API methods */

Expand Down Expand Up @@ -61,7 +62,14 @@ export const removeMyComment = (id: Comment['id']): Promise<void> =>
export const getPreview = (text: string): Promise<string> => apiFetcher.post('/preview', {}, { text });

export function getUser(): Promise<User | null> {
return apiFetcher.get<User | null>('/user').catch(() => null);
return authFetcher.get<StatusResponse>('/status')
.then((resp) => {

Check warning on line 66 in frontend/apps/remark42/app/common/api.ts

View check run for this annotation

Codecov / codecov/patch

frontend/apps/remark42/app/common/api.ts#L65-L66

Added lines #L65 - L66 were not covered by tests
if (resp.user) {
// verify that user is logged in first as otherwise we get an error 401 for unlogged user
return apiFetcher.get<User | null>('/user').catch(() => null);

Check warning on line 69 in frontend/apps/remark42/app/common/api.ts

View check run for this annotation

Codecov / codecov/patch

frontend/apps/remark42/app/common/api.ts#L69

Added line #L69 was not covered by tests
}
return Promise.resolve(null);

Check warning on line 71 in frontend/apps/remark42/app/common/api.ts

View check run for this annotation

Codecov / codecov/patch

frontend/apps/remark42/app/common/api.ts#L71

Added line #L71 was not covered by tests
});
}

export const uploadImage = (image: File): Promise<Image> => {
Expand Down
5 changes: 5 additions & 0 deletions frontend/apps/remark42/app/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ export type User = {
paid_sub?: boolean;
};

export interface StatusResponse {
status: string;
user?: string;
}

/** data which is used on user-info page */
export type Profile = Pick<User, 'id' | 'name' | 'picture'> & { current?: '1' };

Expand Down
5 changes: 5 additions & 0 deletions frontend/packages/api/clients/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ export interface User {
paid_sub?: boolean
}

export interface StatusResponse {
status: string;
user?: string;
}

export type OAuthProvider =
| 'apple'
| 'facebook'
Expand Down
12 changes: 10 additions & 2 deletions frontend/packages/api/clients/public.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ClientParams, Provider, User } from './index'
import type { ClientParams, Provider, User, StatusResponse } from './index'
import { createFetcher } from '../lib/fetcher'
import { API_BASE } from '../consts'

Expand Down Expand Up @@ -82,6 +82,7 @@ export type Vote = -1 | 1

export function createPublicClient({ siteId: site, baseUrl }: ClientParams) {
const fetcher = createFetcher(site, `${baseUrl}${API_BASE}`)
const authFetcher = createFetcher(site,`${baseUrl}/auth`)

/**
* Get server config
Expand All @@ -94,7 +95,14 @@ export function createPublicClient({ siteId: site, baseUrl }: ClientParams) {
* Get current authorized user
*/
async function getUser(): Promise<User | null> {
return fetcher.get<User | null>('/user').catch(() => null)
return authFetcher.get<StatusResponse>('/status')
.then((resp: StatusResponse) => {
if (resp.user) {
// verify that user is logged in first as otherwise we get an error 401 for unlogged user
return fetcher.get<User | null>('/user').catch(() => null);
}
return Promise.resolve(null);
});
}

/**
Expand Down
35 changes: 28 additions & 7 deletions frontend/packages/api/tests/clients/public.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,32 @@ describe<Context>('Public Client', (publicClient) => {
})
})

const userCases = [null, { id: '1', username: 'user' }]
userCases.forEach((user) => {
publicClient('should return user', async ({ client }) => {
mockEndpoint('/remark42/api/v1/user', { body: user })
await expect(client.getUser()).resolves.toEqual(user)
})
})
const userCases = [
{ status: 'not logged in', user: null },
{ status: 'logged in', user: { id: '1', username: 'user' } },
];

userCases.forEach((testCase) => {
publicClient(`should return user when status is ${testCase.status}`, async ({ client }) => {
if (!testCase.user) {
mockEndpoint('/remark42/auth/status', { body: { status: testCase.status } });
} else {
mockEndpoint('/remark42/auth/status', {
body: {
status: testCase.status,
user: testCase.user.username
}
});
}
const userMock = mockEndpoint('/remark42/api/v1/user', { body: testCase.user });

await expect(client.getUser()).resolves.toEqual(testCase.status === 'logged in' ? testCase.user : null);

if (testCase.status === 'not logged in') {
expect(userMock.req).not.toHaveBeenCalled();
} else {
expect(userMock.req).toBeDefined();
}
});
});
})

0 comments on commit d432667

Please sign in to comment.