Skip to content

Commit 628506c

Browse files
MoMannnMoMannn
authored
fix: is adjusment allowed check (#193)
* move isAdjustmentAllowed check from general updateContext to more specific parts where it needs to be applied * linter fix * allow for account change * update jsdocs
1 parent 248acdb commit 628506c

File tree

5 files changed

+95
-28
lines changed

5 files changed

+95
-28
lines changed

‎packages/gator-permissions-snap/src/core/permissionHandler.ts‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -234,10 +234,12 @@ export class PermissionHandler<
234234
interfaceId,
235235
initialContext,
236236
updateContext,
237+
isAdjustmentAllowed,
237238
}: {
238239
interfaceId: string;
239240
initialContext: TContext;
240241
updateContext: (args: { updatedContext: TContext }) => Promise<void>;
242+
isAdjustmentAllowed: boolean;
241243
}) => {
242244
let currentContext = initialContext;
243245
const rerender = async () => {
@@ -343,17 +345,21 @@ export class PermissionHandler<
343345
},
344346
});
345347

346-
const unbindRuleHandlers = bindRuleHandlers({
347-
rules: this.#rules,
348-
userEventDispatcher: this.#userEventDispatcher,
349-
interfaceId,
350-
getContext: () => currentContext,
351-
deriveMetadata: this.#dependencies.deriveMetadata,
352-
onContextChanged: async ({ context }) => {
353-
currentContext = context;
354-
await rerender();
355-
},
356-
});
348+
const unbindRuleHandlers = isAdjustmentAllowed
349+
? bindRuleHandlers({
350+
rules: this.#rules,
351+
userEventDispatcher: this.#userEventDispatcher,
352+
interfaceId,
353+
getContext: () => currentContext,
354+
deriveMetadata: this.#dependencies.deriveMetadata,
355+
onContextChanged: async ({ context }) => {
356+
currentContext = context;
357+
await rerender();
358+
},
359+
})
360+
: () => {
361+
// No-op function when adjustment is not allowed
362+
};
357363

358364
this.#unbindHandlers = () => {
359365
unbindRuleHandlers();

‎packages/gator-permissions-snap/src/core/permissionRequestLifecycleOrchestrator.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -189,10 +189,6 @@ export class PermissionRequestLifecycleOrchestrator {
189189
}: {
190190
updatedContext: TContext;
191191
}) => {
192-
if (!isAdjustmentAllowed) {
193-
throw new InvalidInputError('Adjustment is not allowed');
194-
}
195-
196192
try {
197193
await updateConfirmation({
198194
newContext: updatedContext,
@@ -208,6 +204,7 @@ export class PermissionRequestLifecycleOrchestrator {
208204
interfaceId,
209205
updateContext,
210206
initialContext: context,
207+
isAdjustmentAllowed,
211208
});
212209
}
213210

‎packages/gator-permissions-snap/src/core/types.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,13 +150,18 @@ export type LifecycleOrchestrationHandlers<
150150
/**
151151
* Optional callback that is invoked when a confirmation dialog is created.
152152
* @param confirmationCreatedArgs - Arguments containing the interface ID and a function to update the context
153+
* @param confirmationCreatedArgs.interfaceId - The interface ID for the confirmation dialog
154+
* @param confirmationCreatedArgs.initialContext - The initial context for the confirmation dialog
155+
* @param confirmationCreatedArgs.updateContext - Function to update the context of the confirmation dialog
156+
* @param confirmationCreatedArgs.isAdjustmentAllowed - Whether adjustments to the confirmation dialog are allowed
153157
*/
154158
onConfirmationCreated?: (confirmationCreatedArgs: {
155159
interfaceId: string;
156160
initialContext: TContext;
157161
updateContext: (updateContextArgs: {
158162
updatedContext: TContext;
159163
}) => Promise<void>;
164+
isAdjustmentAllowed: boolean;
160165
}) => void;
161166

162167
/**

‎packages/gator-permissions-snap/test/core/permissionHandler.test.ts‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ type TestLifecycleHandlersType = LifecycleOrchestrationHandlers<
4242

4343
const mockPermissionRequest: PermissionRequest = {
4444
chainId: '0x1',
45-
expiry: 1234567890,
4645
signer: {
4746
type: 'account',
4847
data: {
@@ -58,6 +57,7 @@ const mockPermissionRequest: PermissionRequest = {
5857
},
5958
isAdjustmentAllowed: false,
6059
},
60+
rules: [],
6161
};
6262

6363
const mockContext: TestContextType = {
@@ -69,7 +69,10 @@ const mockContext: TestContextType = {
6969
},
7070
accountAddressCaip10: `eip155:1:${mockAddress}`,
7171
tokenAddressCaip19: `eip155:1/erc20:${mockAssetAddress}`,
72-
expiry: '1234567890',
72+
expiry: {
73+
timestamp: 1234567890,
74+
isAdjustmentAllowed: true,
75+
},
7376
isAdjustmentAllowed: false,
7477
};
7578

@@ -447,6 +450,7 @@ describe('PermissionHandler', () => {
447450
interfaceId: mockInterfaceId,
448451
initialContext: mockContext,
449452
updateContext,
453+
isAdjustmentAllowed: true,
450454
});
451455

452456
const accountSelectorBoundEvent = getBoundEvent({
@@ -481,6 +485,7 @@ describe('PermissionHandler', () => {
481485
interfaceId: mockInterfaceId,
482486
initialContext: mockContext,
483487
updateContext,
488+
isAdjustmentAllowed: true,
484489
});
485490

486491
expect(
@@ -508,6 +513,7 @@ describe('PermissionHandler', () => {
508513
interfaceId: mockInterfaceId,
509514
initialContext: mockContext,
510515
updateContext,
516+
isAdjustmentAllowed: true,
511517
});
512518

513519
const accountSelectorChangeHandler = getBoundEvent({
@@ -556,6 +562,7 @@ describe('PermissionHandler', () => {
556562
interfaceId: mockInterfaceId,
557563
initialContext: mockContext,
558564
updateContext,
565+
isAdjustmentAllowed: true,
559566
});
560567

561568
const accountSelectorChangeHandler = getBoundEvent({
@@ -602,6 +609,7 @@ describe('PermissionHandler', () => {
602609
interfaceId: mockInterfaceId,
603610
initialContext: mockContext,
604611
updateContext,
612+
isAdjustmentAllowed: true,
605613
});
606614

607615
const confirmationContent =
@@ -1088,6 +1096,7 @@ describe('PermissionHandler', () => {
10881096
interfaceId: mockInterfaceId,
10891097
initialContext: mockContext,
10901098
updateContext,
1099+
isAdjustmentAllowed: true,
10911100
});
10921101

10931102
const accountSelectorChangeHandler = getBoundEvent({
@@ -1626,6 +1635,7 @@ describe('PermissionHandler', () => {
16261635
interfaceId: mockInterfaceId,
16271636
initialContext: mockContext,
16281637
updateContext,
1638+
isAdjustmentAllowed: true,
16291639
});
16301640

16311641
const accountSelectorChangeHandler = getBoundEvent({
@@ -3116,6 +3126,7 @@ describe('PermissionHandler', () => {
31163126
interfaceId: mockInterfaceId,
31173127
initialContext: mockContext,
31183128
updateContext,
3129+
isAdjustmentAllowed: true,
31193130
});
31203131

31213132
const accountSelectorChangeHandler = getBoundEvent({
@@ -3168,6 +3179,7 @@ describe('PermissionHandler', () => {
31683179
interfaceId: mockInterfaceId,
31693180
initialContext: mockContext,
31703181
updateContext,
3182+
isAdjustmentAllowed: true,
31713183
});
31723184

31733185
const accountSelectorBoundEvent = getBoundEvent({
@@ -3202,6 +3214,45 @@ describe('PermissionHandler', () => {
32023214
expect(accountSelectorUnboundEvent).toBeDefined();
32033215
expect(showMoreButtonUnboundEvent).toBeDefined();
32043216
});
3217+
3218+
it('does not bind rule handlers when isAdjustmentAllowed is false but allows account selection', async () => {
3219+
const {
3220+
permissionHandler,
3221+
getLifecycleHandlers,
3222+
getBoundEvent,
3223+
updateContext,
3224+
} = setupTest();
3225+
3226+
await permissionHandler.handlePermissionRequest(mockOrigin);
3227+
3228+
const lifecycleHandlers = getLifecycleHandlers();
3229+
3230+
lifecycleHandlers.onConfirmationCreated?.({
3231+
interfaceId: mockInterfaceId,
3232+
initialContext: mockContext,
3233+
updateContext,
3234+
isAdjustmentAllowed: false, // Adjustment not allowed
3235+
});
3236+
3237+
// Try to get a rule input handler - it should not exist when adjustment is not allowed
3238+
const ruleInputHandler = getBoundEvent({
3239+
elementName: 'amountPerSecond', // Example rule input field
3240+
eventType: 'InputChangeEvent',
3241+
interfaceId: mockInterfaceId,
3242+
});
3243+
3244+
// The rule handler should not be bound when adjustment is not allowed
3245+
expect(ruleInputHandler).toBeUndefined();
3246+
3247+
// But account selector should still be bound (it's allowed even when adjustment is not allowed)
3248+
const accountSelectorHandler = getBoundEvent({
3249+
elementName: 'account-selector',
3250+
eventType: 'InputChangeEvent',
3251+
interfaceId: mockInterfaceId,
3252+
});
3253+
3254+
expect(accountSelectorHandler).toBeDefined();
3255+
});
32053256
});
32063257
});
32073258
});

‎packages/gator-permissions-snap/test/core/permissionRequestLifecycleOrchestrator.test.ts‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ describe('PermissionRequestLifecycleOrchestrator', () => {
562562
});
563563
});
564564

565-
it('throws an error when adjustment is not allowed', async () => {
565+
it('passes isAdjustmentAllowed flag to onConfirmationCreated when adjustment is not allowed', async () => {
566566
const initialContext = {
567567
foo: 'bar',
568568
expiry: '2024-12-31',
@@ -579,11 +579,11 @@ describe('PermissionRequestLifecycleOrchestrator', () => {
579579

580580
lifecycleHandlerMocks.buildContext.mockResolvedValue(initialContext);
581581

582-
let updateContextHandler: any;
582+
let onConfirmationCreatedArgs: any;
583583
expect(lifecycleHandlerMocks.onConfirmationCreated).toBeDefined();
584584
lifecycleHandlerMocks.onConfirmationCreated?.mockImplementation(
585-
({ updateContext }) => {
586-
updateContextHandler = updateContext;
585+
(args) => {
586+
onConfirmationCreatedArgs = args;
587587
},
588588
);
589589

@@ -596,14 +596,22 @@ describe('PermissionRequestLifecycleOrchestrator', () => {
596596

597597
await new Promise((resolve) => setTimeout(resolve, 0));
598598

599-
await expect(
600-
updateContextHandler({
601-
updatedContext: { ...initialContext, foo: 'updated' },
602-
}),
603-
).rejects.toThrow('Adjustment is not allowed');
599+
// Verify that isAdjustmentAllowed is passed correctly
600+
expect(onConfirmationCreatedArgs).toBeDefined();
601+
expect(onConfirmationCreatedArgs.isAdjustmentAllowed).toBe(false);
602+
expect(onConfirmationCreatedArgs.updateContext).toBeDefined();
603+
expect(onConfirmationCreatedArgs.initialContext).toStrictEqual(
604+
initialContext,
605+
);
606+
607+
// Verify that updateContext can be called without throwing (restriction is now in handlers)
608+
const updateContextPromise = onConfirmationCreatedArgs.updateContext({
609+
updatedContext: { ...initialContext, foo: 'updated' },
610+
});
611+
expect(await updateContextPromise).toBeUndefined();
604612

605-
// this is called once when the context is first resolved
606-
expect(mockConfirmationDialog.updateContent).toHaveBeenCalledTimes(1);
613+
// this is called once when the context is first resolved, and once when we call updateContext
614+
expect(mockConfirmationDialog.updateContent).toHaveBeenCalledTimes(2);
607615

608616
mockConfirmationDialog.displayConfirmationDialogAndAwaitUserDecision.mockResolvedValue(
609617
{

0 commit comments

Comments
 (0)