Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
2 changes: 1 addition & 1 deletion .github/workflows/unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
- run: pnpm i

- name: Run unit tests ${{ matrix.name }}
run: pnpm --filter ${{ matrix.name }} test
run: pnpm --filter seven exec init-loaders && pnpm --filter ${{ matrix.name }} test

client:
name: '@plone/client'
Expand Down
167 changes: 166 additions & 1 deletion apps/seven/app/middleware.server.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { expect, describe, it, vi, afterEach } from 'vitest';
import config from '@plone/registry';
import { RouterContextProvider } from 'react-router';
import { jwtDecode } from 'jwt-decode';
import { getAuthFromRequest } from '@plone/react-router';
import {
fetchPloneContent,
getAPIResourceWithAuth,
Expand All @@ -9,10 +11,15 @@ import {
ploneClientContext,
ploneContentContext,
ploneSiteContext,
ploneUserContext,
} from './middleware.server';

vi.mock('jwt-decode');
vi.mock('@plone/react-router', () => ({ getAuthFromRequest: vi.fn() }));

describe('middleware', () => {
afterEach(() => {
vi.resetAllMocks();
vi.restoreAllMocks();
});

Expand Down Expand Up @@ -160,6 +167,31 @@ describe('middleware', () => {
expect(err.init.status).toEqual(404);
}
});

it('blocks requests to .well-known paths', async () => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not add this one but I found I needed to fix this because it was missing the unstable_pattern option. Now I see it as a new addition, so maybe it was removed elsewhere. Do you think this needs to be removed or kept? @sneridagh

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pnicolli in the past, this triggered an error in the server (since everything gets in), so we filtered it. I'm not sure if we removed it at some point (I did not). If it's still failing on these requests, it should be kept, I guess.

const request = new Request('http://example.com');
const context = new RouterContextProvider();
const params = {
'*': '.well-known/appspecific/com.chrome.devtools.json',
};
const nextMock = vi.fn();

try {
await otherResources(
{
request,
params,
context,
unstable_pattern:
'/.well-known/appspecific/com.chrome.devtools.json',
},
nextMock,
);
} catch (err: any) {
expect(err).toBeInstanceOf(Response);
expect(err.status).toEqual(200);
}
});
});

describe('getAPIResourceWithAuth', () => {
Expand Down Expand Up @@ -304,7 +336,7 @@ describe('middleware', () => {

describe('fetchPloneContent', () => {
afterEach(() => {
delete config.utilities['client'];
delete config.utilities['ploneClient'];
});

it('fetches content and site and sets them in context', async () => {
Expand Down Expand Up @@ -436,5 +468,138 @@ describe('middleware', () => {
expect(err.init.status).toEqual(500);
}
});

it('sets ploneUserContext to null when no token is provided', async () => {
const getContentMock = vi.fn().mockResolvedValue({ data: {} });
const getSiteMock = vi.fn().mockResolvedValue({ data: {} });
const getUserMock = vi.fn();
config.settings.apiPath = 'http://example.com';
config.registerUtility({
name: 'ploneClient',
type: 'client',
method: () => ({
config: { token: undefined },
getContent: getContentMock,
getSite: getSiteMock,
getUser: getUserMock,
}),
});
const request = new Request('http://example.com');
const context = new RouterContextProvider();
const nextMock = vi.fn();

await fetchPloneContent(
{ request, params: {}, context, unstable_pattern: '/' },
nextMock,
);

expect(getUserMock).not.toHaveBeenCalled();
expect(context.get(ploneUserContext)).toBeNull();
});

it('fetches user and sets ploneUserContext when token is valid', async () => {
const mockUser = { data: { id: 'testuser', fullname: 'Test User' } };
const getContentMock = vi.fn().mockResolvedValue({ data: {} });
const getSiteMock = vi.fn().mockResolvedValue({ data: {} });
const getUserMock = vi.fn().mockResolvedValue(mockUser);
config.settings.apiPath = 'http://example.com';
config.registerUtility({
name: 'ploneClient',
type: 'client',
method: () => ({
config: { token: undefined },
getContent: getContentMock,
getSite: getSiteMock,
getUser: getUserMock,
}),
});
vi.mocked(getAuthFromRequest).mockResolvedValue('valid.jwt.token');
vi.mocked(jwtDecode).mockReturnValue({
sub: 'testuser',
exp: 9999999999,
fullname: 'Test User',
});
const request = new Request('http://example.com');
const context = new RouterContextProvider();
const nextMock = vi.fn();

await fetchPloneContent(
{ request, params: {}, context, unstable_pattern: '/' },
nextMock,
);

expect(getUserMock).toHaveBeenCalledWith({ userId: 'testuser' });
expect(context.get(ploneUserContext)).toEqual(mockUser.data);
expect(getContentMock).toHaveBeenCalledWith({
path: '/',
expand: ['navroot', 'breadcrumbs', 'navigation', 'actions', 'types'],
});
});

it('does not fetch user when token has no sub field', async () => {
const getContentMock = vi.fn().mockResolvedValue({ data: {} });
const getSiteMock = vi.fn().mockResolvedValue({ data: {} });
const getUserMock = vi.fn();
config.settings.apiPath = 'http://example.com';
config.registerUtility({
name: 'ploneClient',
type: 'client',
method: () => ({
config: { token: undefined },
getContent: getContentMock,
getSite: getSiteMock,
getUser: getUserMock,
}),
});
vi.mocked(getAuthFromRequest).mockResolvedValue('token.without.sub');
vi.mocked(jwtDecode).mockReturnValue({ exp: 9999999999 });
const request = new Request('http://example.com');
const context = new RouterContextProvider();
const nextMock = vi.fn();

await fetchPloneContent(
{ request, params: {}, context, unstable_pattern: '/' },
nextMock,
);

expect(getUserMock).not.toHaveBeenCalled();
expect(context.get(ploneUserContext)).toBeNull();
expect(getContentMock).toHaveBeenCalledWith({
path: '/',
expand: ['navroot', 'breadcrumbs', 'navigation', 'actions'],
});
});

it('handles JWT decode errors gracefully and proceeds without user', async () => {
const getContentMock = vi.fn().mockResolvedValue({ data: {} });
const getSiteMock = vi.fn().mockResolvedValue({ data: {} });
const getUserMock = vi.fn();
config.settings.apiPath = 'http://example.com';
config.registerUtility({
name: 'ploneClient',
type: 'client',
method: () => ({
config: { token: undefined },
getContent: getContentMock,
getSite: getSiteMock,
getUser: getUserMock,
}),
});
vi.mocked(getAuthFromRequest).mockResolvedValue('malformed.token');
vi.mocked(jwtDecode).mockImplementation(() => {
throw new Error('Invalid token');
});
const request = new Request('http://example.com');
const context = new RouterContextProvider();
const nextMock = vi.fn();

await fetchPloneContent(
{ request, params: {}, context, unstable_pattern: '/' },
nextMock,
);

expect(getUserMock).not.toHaveBeenCalled();
expect(context.get(ploneUserContext)).toBeNull();
});
});
});
22 changes: 21 additions & 1 deletion apps/seven/app/middleware.server.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { jwtDecode } from 'jwt-decode';
import { data, createContext } from 'react-router';
import { flattenToAppURL } from '@plone/helpers';
import { getAuthFromRequest } from '@plone/react-router';
Expand All @@ -12,6 +13,9 @@ export const ploneContentContext =
createContext<Awaited<ReturnType<PloneClient['getContent']>>['data']>();
export const ploneSiteContext =
createContext<Awaited<ReturnType<PloneClient['getSite']>>['data']>();
export const ploneUserContext = createContext<
Awaited<ReturnType<PloneClient['getUser']>>['data'] | null
>(null);

export const installServerMiddleware: Route.MiddlewareFunction = async (
{ request, context },
Expand Down Expand Up @@ -85,17 +89,33 @@ export const fetchPloneContent: Route.MiddlewareFunction = async (

const path = `/${params['*'] || ''}`;

let userId = '';
if (token) {
try {
const decodedToken = jwtDecode<{
sub: string;
exp: number;
fullname: string | null;
}>(token);
userId = decodedToken.sub || '';
} catch {}
}

if (userId) expand.push('types');

try {
const [content, site] = await Promise.all([
const [content, site, user] = await Promise.all([
cli.getContent({ path, expand }),
cli.getSite(),
userId ? cli.getUser({ userId }) : Promise.resolve(null),
]);

migrateContent(content.data);

context.set(ploneClientContext, cli);
context.set(ploneContentContext, flattenToAppURL(content.data));
context.set(ploneSiteContext, flattenToAppURL(site.data));
context.set(ploneUserContext, user?.data ?? null);
} catch (error: any) {
throw data('Content Not Found', {
status: typeof error.status === 'number' ? error.status : 500,
Expand Down
1 change: 1 addition & 0 deletions apps/seven/news/+contextloaders.internal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Updated app test and eslint config. @pnicolli
1 change: 1 addition & 0 deletions apps/seven/news/+getuser.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added user data in the context for authenticated users @pnicolli
1 change: 1 addition & 0 deletions apps/seven/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"i18next-fs-backend": "^2.6.0",
"i18next-http-backend": "^3.0.2",
"isbot": "^5.1.27",
"jwt-decode": "^4.0.0",
"react": "catalog:",
"react-dom": "catalog:",
"react-i18next": "catalog:",
Expand Down
2 changes: 1 addition & 1 deletion apps/seven/vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export default defineConfig({
'packages/**',
'build/**',
'*.config.ts',
'registry.loader.js',
'.plone/**',
'app/entry.server.tsx',
'app/entry.client.tsx',
'app/i18next.server.ts',
Expand Down
6 changes: 4 additions & 2 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ export default tseslint.config(
project: ['packages/*/tsconfig.json', 'apps/seven/tsconfig.json'],
alwaysTryTypes: true,
},
alias: {
map: [['seven', './apps/seven']],
},
node: true,
},
},
Expand Down Expand Up @@ -181,8 +184,7 @@ export default tseslint.config(
'packages/registry/docs',
'**/.react-router/*',
'**/+types/*',
'**/registry.loader.js',
'**/registry.loader.server.js',
'**/.plone/*',
],
},
);
1 change: 1 addition & 0 deletions packages/cmsui/+contextloaders.internal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Refactored to use context in all loaders and actions. @pnicolli
1 change: 1 addition & 0 deletions packages/cmsui/news/+getuser.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Set the cookie expiration date equal to the token expiration date after login @pnicolli
1 change: 1 addition & 0 deletions packages/cmsui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
"clsx": "^2.1.1",
"jotai": "^2.12.3",
"jotai-optics": "^0.4.0",
"jwt-decode": "^4.0.0",
"optics-ts": "^2.4.1",
"react-aria": "catalog:",
"react-aria-components": "catalog:",
Expand Down
22 changes: 9 additions & 13 deletions packages/cmsui/routes/api/createContent.test.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { afterEach, describe, expect, it, vi } from 'vitest';
import config from '@plone/registry';
import { action } from './createContent';
import { RouterContextProvider } from 'react-router';
import { ploneClientContext } from 'seven/app/middleware.server';

describe('createContent API route action', () => {
afterEach(() => {
vi.restoreAllMocks();
config.settings = {};
delete config.utilities['ploneClient'];
});

it('calls createContent with wildcard path and returns response data', async () => {
Expand All @@ -18,16 +19,10 @@ describe('createContent API route action', () => {
});

config.settings.apiPath = 'http://example.com';
config.registerUtility({
name: 'ploneClient',
type: 'client',
method: () => ({
config: {
token: undefined,
},
createContent: createContentMock,
}),
});
const context = new RouterContextProvider();
context.set(ploneClientContext, {
createContent: createContentMock,
} as any);

const request = new Request('http://example.com/@createContent/folder', {
method: 'POST',
Expand All @@ -51,8 +46,9 @@ describe('createContent API route action', () => {
const response = await action({
request,
params: { '*': 'folder' },
context: {},
} as any);
context,
unstable_pattern: '/@createContent/folder',
});

expect(createContentMock).toHaveBeenCalledWith({
path: '/folder',
Expand Down
Loading
Loading