Skip to content

Commit 09c0d64

Browse files
pyphiliapyphilia
authored
feat: allow readers to edit etherpad (#1713)
* tbd * feat: add reader permission over etherpad item * test: write tests * refactor: remove duplicate patch endpoint
1 parent 58bd1f2 commit 09c0d64

5 files changed

Lines changed: 292 additions & 20 deletions

File tree

‎src/services/item/plugins/etherpad/controller.ts‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { StatusCodes } from 'http-status-codes';
2+
13
import { FastifyPluginAsyncTypebox } from '@fastify/type-provider-typebox';
24
import { FastifyPluginAsync } from 'fastify';
35
import fp from 'fastify-plugin';
@@ -10,7 +12,7 @@ import { matchOne } from '../../../authorization';
1012
import { assertIsMember } from '../../../member/entities/member';
1113
import { validatedMemberAccountRole } from '../../../member/strategies/validatedMemberAccountRole';
1214
import { ItemService } from '../../service';
13-
import { createEtherpad, getEtherpadFromItem } from './schemas';
15+
import { createEtherpad, getEtherpadFromItem, updateEtherpad } from './schemas';
1416
import { EtherpadItemService } from './service';
1517

1618
const endpoints: FastifyPluginAsyncTypebox = async (fastify) => {
@@ -47,6 +49,33 @@ const endpoints: FastifyPluginAsyncTypebox = async (fastify) => {
4749
},
4850
);
4951

52+
/**
53+
* Etherpad update
54+
*/
55+
fastify.patch(
56+
'/:id',
57+
{
58+
schema: updateEtherpad,
59+
preHandler: [isAuthenticated, matchOne(validatedMemberAccountRole)],
60+
},
61+
async (request, reply) => {
62+
const {
63+
user,
64+
params: { id },
65+
body: { readerPermission },
66+
} = request;
67+
const member = asDefined(user?.account);
68+
assertIsMember(member);
69+
70+
await db.transaction(async (manager) => {
71+
return await etherpadItemService.patchWithOptions(member, buildRepositories(manager), id, {
72+
readerPermission,
73+
});
74+
});
75+
reply.status(StatusCodes.NO_CONTENT);
76+
},
77+
);
78+
5079
/**
5180
* Etherpad view in given mode (read or write)
5281
* Access should be granted if and only if the user has at least write

‎src/services/item/plugins/etherpad/schemas.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { StatusCodes } from 'http-status-codes';
33

44
import { FastifySchema } from 'fastify';
55

6+
import { PermissionLevel } from '@graasp/sdk';
7+
68
import { customType } from '../../../../plugins/typebox';
79
import { errorSchemaRef } from '../../../../schemas/global';
810
import { itemSchemaRef } from '../../schemas';
@@ -47,3 +49,21 @@ export const getEtherpadFromItem = {
4749
'4xx': errorSchemaRef,
4850
},
4951
};
52+
53+
export const updateEtherpad = {
54+
operationId: 'updateEtherpad',
55+
tags: ['item'],
56+
summary: 'Update etherpad',
57+
description: 'Update etherpad permission of readers.',
58+
59+
params: customType.StrictObject({
60+
id: customType.UUID(),
61+
}),
62+
body: customType.StrictObject({
63+
readerPermission: Type.Union([
64+
Type.Literal(PermissionLevel.Read),
65+
Type.Literal(PermissionLevel.Write),
66+
]),
67+
}),
68+
response: { [StatusCodes.NO_CONTENT]: Type.Null(), '4xx': errorSchemaRef },
69+
} as const satisfies FastifySchema;

‎src/services/item/plugins/etherpad/service.test.ts‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
import { v4 } from 'uuid';
2+
3+
import Etherpad, { AuthorSession } from '@graasp/etherpad-api';
4+
import { EtherpadItemFactory, FolderItemFactory, ItemType, PermissionLevel } from '@graasp/sdk';
5+
6+
import { MOCK_LOGGER } from '../../../../../test/app';
7+
import { resolveDependency } from '../../../../di/utils';
8+
import * as repositoriesUtils from '../../../../utils/repositories';
9+
import { ItemMembership } from '../../../itemMembership/entities/ItemMembership';
10+
import { ItemMembershipRepository } from '../../../itemMembership/repository';
11+
import { Member } from '../../../member/entities/member';
12+
import { ThumbnailService } from '../../../thumbnail/service';
13+
import { EtherpadItem } from '../../entities/Item';
14+
import { WrongItemTypeError } from '../../errors';
15+
import { ItemRepository } from '../../repository';
16+
import { ItemService } from '../../service';
17+
import { ItemThumbnailService } from '../thumbnail/service';
18+
import { EtherpadItemService } from './service';
19+
import { EtherpadServiceConfig } from './serviceConfig';
20+
import { PadNameFactory } from './types';
21+
22+
jest.mock('node-fetch');
23+
24+
const padNameFactory = {} as PadNameFactory;
25+
const etherPadConfig = resolveDependency(EtherpadServiceConfig);
26+
const itemService = new ItemService(
27+
{} as ThumbnailService,
28+
{} as ItemThumbnailService,
29+
MOCK_LOGGER,
30+
);
31+
const etherpad = {
32+
getReadOnlyID: () => {},
33+
createAuthorIfNotExistsFor: () => {},
34+
createSession: () => {},
35+
deleteSession: () => {},
36+
listSessionsOfAuthor: () => {},
37+
} as unknown as Etherpad;
38+
39+
const etherpadService = new EtherpadItemService(
40+
etherpad,
41+
padNameFactory,
42+
etherPadConfig,
43+
itemService,
44+
MOCK_LOGGER,
45+
);
46+
const id = v4();
47+
const MOCK_ITEM = EtherpadItemFactory({
48+
id,
49+
extra: { etherpad: { readerPermission: undefined, padID: v4(), groupID: v4() } },
50+
}) as unknown as EtherpadItem;
51+
52+
const MOCK_MEMBER = {} as Member;
53+
const repositories = {
54+
itemRepository: {
55+
getOneOrThrow: async () => {
56+
return MOCK_ITEM;
57+
},
58+
} as unknown as ItemRepository,
59+
itemMembershipRepository: {
60+
getOneOrThrow: async () => {
61+
return MOCK_ITEM;
62+
},
63+
getInherited: async () => {
64+
return {};
65+
},
66+
} as unknown as ItemMembershipRepository,
67+
} as repositoriesUtils.Repositories;
68+
69+
describe('Etherpad Service', () => {
70+
afterEach(() => {
71+
jest.clearAllMocks();
72+
});
73+
74+
describe('patchWithOptions', () => {
75+
it('throw if item is not an etherpad', async () => {
76+
const FOLDER_ITEM = FolderItemFactory();
77+
await expect(() =>
78+
etherpadService.patchWithOptions(
79+
MOCK_MEMBER,
80+
{
81+
itemRepository: {
82+
getOneOrThrow: async () => {
83+
return FOLDER_ITEM;
84+
},
85+
} as unknown as ItemRepository,
86+
} as repositoriesUtils.Repositories,
87+
FOLDER_ITEM.id,
88+
{ readerPermission: PermissionLevel.Write },
89+
),
90+
).rejects.toBeInstanceOf(WrongItemTypeError);
91+
});
92+
it('patch readerPermission', async () => {
93+
const itemServicePatchMock = jest
94+
.spyOn(ItemService.prototype, 'patch')
95+
.mockImplementation(async () => {
96+
return MOCK_ITEM;
97+
});
98+
99+
expect(MOCK_ITEM.extra.etherpad.readerPermission).toBeUndefined();
100+
101+
const readerPermission = PermissionLevel.Write;
102+
await etherpadService.patchWithOptions(MOCK_MEMBER, repositories, MOCK_ITEM.id, {
103+
readerPermission,
104+
});
105+
106+
// call to item service with initial item name
107+
expect(itemServicePatchMock).toHaveBeenCalledWith(MOCK_MEMBER, repositories, MOCK_ITEM.id, {
108+
extra: {
109+
[ItemType.ETHERPAD]: {
110+
readerPermission: PermissionLevel.Write,
111+
},
112+
},
113+
});
114+
});
115+
116+
it('Cannot update not found item given id', async () => {
117+
jest.spyOn(repositories.itemRepository, 'getOneOrThrow').mockImplementation(() => {
118+
throw new Error();
119+
});
120+
121+
await expect(() =>
122+
etherpadService.patchWithOptions(MOCK_MEMBER, repositories, v4(), {
123+
readerPermission: PermissionLevel.Write,
124+
}),
125+
).rejects.toThrow();
126+
});
127+
});
128+
describe('getEtherpadFromItem', () => {
129+
beforeEach(() => {
130+
jest.spyOn(repositoriesUtils, 'buildRepositories').mockReturnValue(repositories);
131+
jest.spyOn(etherpad, 'createAuthorIfNotExistsFor').mockResolvedValue({ authorID: v4() });
132+
jest.spyOn(etherpad, 'createSession').mockResolvedValue({ sessionID: v4() });
133+
jest.spyOn(etherpad, 'deleteSession').mockResolvedValue(null);
134+
jest
135+
.spyOn(etherpad, 'listSessionsOfAuthor')
136+
.mockResolvedValue({ id: { validUntil: 1 } as AuthorSession });
137+
});
138+
139+
it('return write for reader with write permission', async () => {
140+
// readerPermission is write
141+
jest.spyOn(itemService, 'get').mockResolvedValue(
142+
EtherpadItemFactory({
143+
extra: {
144+
etherpad: { padID: v4(), groupID: v4(), readerPermission: PermissionLevel.Write },
145+
},
146+
}) as unknown as EtherpadItem,
147+
);
148+
149+
// actor has read permission
150+
jest
151+
.spyOn(repositories.itemMembershipRepository, 'getInherited')
152+
.mockResolvedValue({ permission: PermissionLevel.Read } as unknown as ItemMembership);
153+
const getReadOnlyIDMock = jest.spyOn(etherpad, 'getReadOnlyID');
154+
155+
// actor require write
156+
await etherpadService.getEtherpadFromItem(MOCK_MEMBER, MOCK_ITEM.id, 'write');
157+
158+
// this is called only if returned mode is read, which shouldn't be the case here
159+
expect(getReadOnlyIDMock).not.toHaveBeenCalled();
160+
});
161+
it('return read for reader with no permission even if asked for write', async () => {
162+
// readerPermission is read
163+
jest.spyOn(itemService, 'get').mockResolvedValue(
164+
EtherpadItemFactory({
165+
extra: {
166+
etherpad: { padID: v4(), groupID: v4(), readerPermission: PermissionLevel.Read },
167+
},
168+
}) as unknown as EtherpadItem,
169+
);
170+
171+
// permission is read
172+
jest
173+
.spyOn(repositories.itemMembershipRepository, 'getInherited')
174+
.mockResolvedValue({ permission: PermissionLevel.Read } as unknown as ItemMembership);
175+
const getReadOnlyIDMock = jest
176+
.spyOn(etherpad, 'getReadOnlyID')
177+
.mockResolvedValue({ readOnlyID: v4() });
178+
179+
// request write
180+
await etherpadService.getEtherpadFromItem(MOCK_MEMBER, MOCK_ITEM.id, 'write');
181+
182+
// this is called only if returned mode is read, which is the case here
183+
expect(getReadOnlyIDMock).toHaveBeenCalled();
184+
});
185+
});
186+
});

‎src/services/item/plugins/etherpad/service.ts‎

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ import { BaseLogger } from '../../../../logger';
1010
import { MemberCannotWriteItem } from '../../../../utils/errors';
1111
import { Repositories, buildRepositories } from '../../../../utils/repositories';
1212
import { Account } from '../../../account/entities/account';
13-
import { validatePermission } from '../../../authorization';
1413
import { Member } from '../../../member/entities/member';
15-
import { Item, isItemType } from '../../entities/Item';
14+
import { EtherpadItem, Item, isItemType } from '../../entities/Item';
15+
import { WrongItemTypeError } from '../../errors';
1616
import { ItemService } from '../../service';
1717
import { MAX_SESSIONS_IN_COOKIE, PLUGIN_NAME } from './constants';
1818
import { EtherpadServerError, ItemMissingExtraError } from './errors';
@@ -121,31 +121,69 @@ export class EtherpadItemService {
121121
}
122122
}
123123

124+
/**
125+
* Updates Etherpad item
126+
*/
127+
public async patchWithOptions(
128+
member: Member,
129+
repositories: Repositories,
130+
itemId: Item['id'],
131+
{ readerPermission }: { readerPermission: PermissionLevel.Read | PermissionLevel.Write },
132+
) {
133+
const { itemRepository } = repositories;
134+
135+
const item = await itemRepository.getOneOrThrow(itemId);
136+
137+
// check item is link
138+
if (!isItemType(item, ItemType.ETHERPAD)) {
139+
throw new WrongItemTypeError(item.type);
140+
}
141+
142+
return this.itemService.patch(member, repositories, itemId, {
143+
extra: { [ItemType.ETHERPAD]: { readerPermission } },
144+
});
145+
}
146+
124147
/**
125148
* Helper to determine the final viewing mode of an etherpad
126149
*/
127150
private async checkMode(
151+
repositories: Repositories,
128152
requestedMode: 'read' | 'write',
129153
account: Account,
130-
item: Item,
154+
item: EtherpadItem,
131155
): Promise<'read' | 'write'> {
132156
// no specific check if read mode was requested
133-
if (requestedMode === 'read') {
157+
if (requestedMode === PermissionLevel.Read) {
134158
return 'read';
135159
}
136-
// if mode was write, check that permission is at least write
137-
try {
138-
// validatePermission will throw if user does not have write rights
139-
await validatePermission(buildRepositories(), PermissionLevel.Write, account, item);
160+
// if mode was write,
161+
// check that permission is at least write
162+
163+
const membership = await repositories.itemMembershipRepository.getInherited(
164+
item.path,
165+
account.id,
166+
true,
167+
);
168+
// allow write for admin, writers, and readers if setting is enabled
169+
if (
170+
membership &&
171+
(membership.permission == PermissionLevel.Write ||
172+
membership.permission == PermissionLevel.Admin ||
173+
(membership.permission == PermissionLevel.Read &&
174+
item.extra.etherpad.readerPermission == PermissionLevel.Write))
175+
) {
140176
return 'write';
141-
} catch (error) {
142-
// something else failed in the authorization
143-
if (!(error instanceof MemberCannotWriteItem)) {
144-
throw error;
145-
}
146-
// the user simply does not have write permission, so fallback to read
147-
return 'read';
148177
}
178+
return 'read';
179+
}
180+
catch(error) {
181+
// something else failed in the authorization
182+
if (!(error instanceof MemberCannotWriteItem)) {
183+
throw error;
184+
}
185+
// the user simply does not have write permission, so fallback to read
186+
return 'read';
149187
}
150188

151189
/**
@@ -156,11 +194,12 @@ export class EtherpadItemService {
156194
const repos = buildRepositories();
157195
const item = await this.itemService.get(account, repos, itemId);
158196

159-
const checkedMode = await this.checkMode(mode, account, item);
160-
161197
if (!isItemType(item, ItemType.ETHERPAD) || !item.extra?.etherpad) {
162198
throw new ItemMissingExtraError(item?.id);
163199
}
200+
201+
const checkedMode = await this.checkMode(repos, mode, account, item);
202+
164203
const { padID, groupID } = item.extra.etherpad;
165204

166205
let padUrl: string;

‎src/services/item/service.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -488,8 +488,6 @@ export class ItemService {
488488

489489
await validatePermission(repositories, PermissionLevel.Write, member, item);
490490

491-
// TODO: if updating a link item, fetch the new informations
492-
493491
await this.hooks.runPreHooks('update', member, repositories, { item: item });
494492

495493
const updated = await itemRepository.updateOne(item.id, body);

0 commit comments

Comments
 (0)