Skip to content

Commit 23b38fb

Browse files
committed
chore(PayoutMethods): disable restore
1 parent ce292c7 commit 23b38fb

File tree

6 files changed

+129
-27
lines changed

6 files changed

+129
-27
lines changed

server/graphql/common/expenses.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
omit,
2424
omitBy,
2525
pick,
26+
remove,
2627
set,
2728
size,
2829
uniq,
@@ -378,15 +379,25 @@ export const canSeeExpensePayoutMethodPrivateDetails: ExpensePermissionEvaluator
378379
return true;
379380
}
380381

381-
return remoteUserMeetsOneCondition(req, expense, [
382+
const allowedRoles = [
382383
isOwner,
383384
isOwnerAccountant,
384385
isHostAdmin,
385386
isHostAccountant,
386387
isAdminOrAccountantOfHostWhoPaidExpense,
387388
isAdminOfCollectiveWithPermissivePayoutMethodPermissions, // Some fiscal hosts rely on the collective admins to do some verifications on the payout method
388389
isAdminOfCollectiveAndExpenseIsAVirtualCardButNotManuallyCreated, // Virtual cards are created by the collective admins, but manually created ones are managed by host admins
389-
]);
390+
];
391+
392+
// Submitter can see own information until the expense is paid
393+
if (expense.status === expenseStatus.PAID) {
394+
const payoutMethod = await req.loaders.PayoutMethod.byId.load(expense.PayoutMethodId);
395+
if (!payoutMethod.isSaved) {
396+
remove(allowedRoles, [isOwner, isOwnerAccountant]);
397+
}
398+
}
399+
400+
return remoteUserMeetsOneCondition(req, expense, allowedRoles);
390401
};
391402

392403
/** Checks if the user can see expense's invoice information (the generated PDF) */

server/graphql/schemaV2.graphql

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23694,16 +23694,6 @@ type Mutation {
2369423694
Remove the given payout method. Scope: "expenses".
2369523695
"""
2369623696
removePayoutMethod(payoutMethodId: String!): PayoutMethod!
23697-
23698-
"""
23699-
Restore the given payout method. Scope: "expenses".
23700-
"""
23701-
restorePayoutMethod(
23702-
"""
23703-
Payout Method reference
23704-
"""
23705-
payoutMethod: PayoutMethodReferenceInput!
23706-
): PayoutMethod!
2370723697
editPayoutMethod(
2370823698
"""
2370923699
Payout Method reference

server/graphql/v2/mutation/PayoutMethodMutations.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { Forbidden, NotFound, Unauthorized, ValidationFailed } from '../../error
1111
import { idDecode, IDENTIFIER_TYPES } from '../identifiers';
1212
import { fetchAccountWithReference, GraphQLAccountReferenceInput } from '../input/AccountReferenceInput';
1313
import { GraphQLPayoutMethodInput } from '../input/PayoutMethodInput';
14-
import { fetchPayoutMethodWithReference, GraphQLPayoutMethodReferenceInput } from '../input/PayoutMethodReferenceInput';
14+
import { fetchPayoutMethodWithReference } from '../input/PayoutMethodReferenceInput';
1515
import GraphQLPayoutMethod from '../object/PayoutMethod';
1616

1717
const payoutMethodMutations = {
@@ -89,24 +89,16 @@ const payoutMethodMutations = {
8989
},
9090
restorePayoutMethod: {
9191
description: 'Restore the given payout method. Scope: "expenses".',
92+
deprecationReason: '2025-02-10: Payout methods cannot be restored.',
9293
type: new GraphQLNonNull(GraphQLPayoutMethod),
9394
args: {
9495
payoutMethod: {
9596
type: new GraphQLNonNull(GraphQLPayoutMethodReferenceInput),
9697
description: 'Payout Method reference',
9798
},
9899
},
99-
async resolve(_: void, args, req: express.Request): Promise<PayoutMethodModel> {
100-
checkRemoteUserCanUseExpenses(req);
101-
102-
const payoutMethod = await fetchPayoutMethodWithReference(args.payoutMethod);
103-
104-
const collective = await req.loaders.Collective.byId.load(payoutMethod.CollectiveId);
105-
if (!req.remoteUser.isAdminOfCollective(collective)) {
106-
throw new Forbidden();
107-
}
108-
109-
return payoutMethod.update({ isSaved: true });
100+
async resolve() {
101+
return null;
110102
},
111103
},
112104
editPayoutMethod: {
@@ -130,6 +122,10 @@ const payoutMethodMutations = {
130122
// Enforce 2FA
131123
await twoFactorAuthLib.enforceForAccount(req, collective);
132124

125+
if (!payoutMethod.isSaved && args.payoutMethod.isSaved === true) {
126+
throw new Forbidden('Archived payout methods cannot be restored.');
127+
}
128+
133129
if (await payoutMethod.canBeEdited()) {
134130
return await payoutMethod.update({
135131
...pick(args.payoutMethod, ['name', 'data', 'isSaved']),

server/graphql/v2/object/PayoutMethod.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ const GraphQLPayoutMethod = new GraphQLObjectType({
2424
name: {
2525
type: GraphQLString,
2626
description: 'A friendly name for users to easily find their payout methods',
27-
resolve: async (payoutMethod, _, req: express.Request): Promise<string> => {
27+
resolve: async (payoutMethod, _, req: express.Request): Promise<string | null> => {
2828
const collective = await req.loaders.Collective.byId.load(payoutMethod.CollectiveId);
2929
if (
3030
req.remoteUser?.isAdminOfCollective(collective) ||
@@ -54,10 +54,10 @@ const GraphQLPayoutMethod = new GraphQLObjectType({
5454
data: {
5555
type: GraphQLJSON,
5656
description: 'The actual data for this payout method. Content depends on the type.',
57-
resolve: async (payoutMethod, _, req: express.Request): Promise<Record<string, unknown>> => {
57+
resolve: async (payoutMethod, _, req: express.Request): Promise<Record<string, unknown> | null> => {
5858
const collective = await req.loaders.Collective.byId.load(payoutMethod.CollectiveId);
5959
if (
60-
req.remoteUser?.isAdminOfCollective(collective) ||
60+
(req.remoteUser?.isAdminOfCollective(collective) && payoutMethod.isSaved) ||
6161
getContextPermission(req, PERMISSION_TYPE.SEE_PAYOUT_METHOD_DETAILS, payoutMethod.id)
6262
) {
6363
if (checkScope(req, 'expenses')) {

test/server/graphql/v2/mutation/PayoutMethodMutations.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,5 +226,25 @@ describe('server/graphql/v2/mutation/PayoutMethodMutations', () => {
226226
expect(result.errors).to.exist;
227227
expect(result.errors[0].message).to.eql('You are authenticated but forbidden to perform this action');
228228
});
229+
230+
it('Rejects setting isSaved true on an archived payout method (restore via edit)', async () => {
231+
const archivedPayoutMethod = await fakePayoutMethod({
232+
CollectiveId: adminUser.CollectiveId,
233+
isSaved: false,
234+
name: 'Archived Bank',
235+
});
236+
await fakeExpense({ PayoutMethodId: archivedPayoutMethod.id, status: 'PAID' });
237+
const mutationArgs = {
238+
payoutMethod: {
239+
id: idEncode(archivedPayoutMethod.id, IDENTIFIER_TYPES.PAYOUT_METHOD),
240+
isSaved: true,
241+
},
242+
};
243+
const result = await graphqlQueryV2(editPayoutMethodMutation, mutationArgs, adminUser);
244+
expect(result.errors).to.exist;
245+
expect(result.errors[0].message).to.include('Archived payout methods cannot be restored');
246+
await archivedPayoutMethod.reload();
247+
expect(archivedPayoutMethod.isSaved).to.be.false;
248+
});
229249
});
230250
});

test/server/graphql/v2/object/PayoutMethod.test.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,4 +159,89 @@ describe('server/graphql/v2/object/PayoutMethod', () => {
159159
}
160160
});
161161
});
162+
163+
describe('archived payout method obfuscation', () => {
164+
it('returns null for name and data when collective admin queries an archived payout method', async () => {
165+
const user = await fakeUser();
166+
const archivedPm = await fakePayoutMethod({
167+
type: PayoutMethodTypes.BANK_ACCOUNT,
168+
name: 'Secret Bank',
169+
CollectiveId: user.CollectiveId,
170+
isSaved: false,
171+
data: {
172+
accountHolderName: 'Secret Holder',
173+
currency: 'EUR',
174+
type: 'IBAN',
175+
details: { iban: 'FR123456789' },
176+
},
177+
});
178+
await fakeExpense({ PayoutMethodId: archivedPm.id, status: 'PAID' });
179+
180+
const result = await graphqlQueryV2(
181+
gql`
182+
query Account($slug: String!) {
183+
account(slug: $slug) {
184+
id
185+
payoutMethods(includeArchived: true) {
186+
id
187+
name
188+
data
189+
isSaved
190+
type
191+
}
192+
}
193+
}
194+
`,
195+
{ slug: user.collective.slug },
196+
user,
197+
);
198+
expect(result.errors).to.not.exist;
199+
const archivedFromQuery = result.data.account.payoutMethods.find(pm => pm.id && !pm.isSaved);
200+
expect(archivedFromQuery).to.exist;
201+
expect(archivedFromQuery.name).to.be.null;
202+
expect(archivedFromQuery.data).to.be.null;
203+
expect(archivedFromQuery.isSaved).to.be.false;
204+
expect(archivedFromQuery.type).to.equal('BANK_ACCOUNT');
205+
});
206+
207+
it('returns name and data for saved payout methods when collective admin queries', async () => {
208+
const user = await fakeUser();
209+
await fakePayoutMethod({
210+
type: PayoutMethodTypes.BANK_ACCOUNT,
211+
name: 'My Bank',
212+
CollectiveId: user.CollectiveId,
213+
isSaved: true,
214+
data: {
215+
accountHolderName: 'Holder',
216+
currency: 'EUR',
217+
type: 'IBAN',
218+
details: { iban: 'FR999999999' },
219+
},
220+
});
221+
222+
const result = await graphqlQueryV2(
223+
gql`
224+
query Account($slug: String!) {
225+
account(slug: $slug) {
226+
id
227+
payoutMethods {
228+
id
229+
name
230+
data
231+
isSaved
232+
}
233+
}
234+
}
235+
`,
236+
{ slug: user.collective.slug },
237+
user,
238+
);
239+
expect(result.errors).to.not.exist;
240+
const pm = result.data.account.payoutMethods.find(p => p.name === 'My Bank');
241+
expect(pm).to.exist;
242+
expect(pm.name).to.equal('My Bank');
243+
expect(pm.data).to.exist;
244+
expect(pm.isSaved).to.be.true;
245+
});
246+
});
162247
});

0 commit comments

Comments
 (0)