diff --git a/packages/permission-controller/ARCHITECTURE.md b/packages/permission-controller/ARCHITECTURE.md index 09d8b6a7c8..2fb0332c1e 100644 --- a/packages/permission-controller/ARCHITECTURE.md +++ b/packages/permission-controller/ARCHITECTURE.md @@ -30,23 +30,6 @@ Permission system concepts correspond to components of the MetaMask stack as fol | Caveats | Caveat objects | | Permission system | The `PermissionController` and its `json-rpc-engine` middleware | -### Target Keys and Target Names - -When consuming or reading the `PermissionController`, you will encounter the concepts of "target keys" and "target names". -As described in the previous section, a permission grants a subject access to a restricted resource, called a _target_, which is some string. -Targets are referred to by consumers by their _names_, and internally (in the `PermissionController`) by their _keys_. -This distinction exists to enable namespaced targets, specifically namespaced JSON-RPC methods. - -All targets have a single key. -If a target _is not_ namespaced, the key is identical to its name. -If a target _is_ namespaced, it may have any number of names, all of which are distinct from its key. -Permissions are always requested and invoked by their target name(s). - -For example, for the non-namespaced restricted method `eth_accounts`, both the key and the name is `eth_accounts`. -On the other hand, the namespaced restricted method `wallet_getSecret_*` has the key `wallet_getSecret_*`, and any number of names where the `*` wildcard character is substituted for some valid string per the method implementation. - -See [below](#construction) for a concrete example. - ### Permission / Target Types In practice, targets can be different things, necessitating distinct implementations in order to enforce the logic of the permission system. @@ -121,14 +104,14 @@ const caveatSpecifications = { }, }; -// The property names of this object must be target keys. +// The property names of this object must be target names. const permissionSpecifications = { // This is a plain restricted method. wallet_getSecretArray: { // Every permission must have this field. permissionType: PermissionType.RestrictedMethod, // i.e. the restricted method name - targetKey: 'wallet_getSecretArray', + targetName: 'wallet_getSecretArray', allowedCaveats: ['filterArrayResponse'], // Every restricted method must specify its implementation in its // specification. @@ -139,31 +122,11 @@ const permissionSpecifications = { }, }, - // This is a namespaced restricted method. - 'wallet_getSecret_*': { - permissionType: PermissionType.RestrictedMethod, - targetKey: 'wallet_getSecret_*', - methodImplementation: ( - args: RestrictedMethodOptions, - ) => { - // The "method" is the string method name that was externally requested, - // and the "*" in the target key for this method will be replaced with - // some string whose value should affect the behavior of this method. - // - // "context" contains the origin of the requester and anything attached - // by the host during permission request processing. - const { method, context } = args; - - const secretName = method.replace('wallet_getSecret_', ''); - return context.getSecret(secretName); - }, - }, - // This is an endowment. secretEndowment: { permissionType: PermissionType.Endowment, // Naming conventions for endowments are yet to be established. - targetKey: 'endowment:globals', + targetName: 'endowment:globals', // This function will be called to retrieve the subject's endowment(s). // Here we imagine that these are the names of globals that will be made // available to a SES Compartment. diff --git a/packages/permission-controller/src/Permission.ts b/packages/permission-controller/src/Permission.ts index a09162b2b1..9434892afb 100644 --- a/packages/permission-controller/src/Permission.ts +++ b/packages/permission-controller/src/Permission.ts @@ -82,13 +82,12 @@ export type PermissionConstraint = { * * See the README for details. * - * @template TargetKey - They key of the permission target that the permission - * corresponds to. + * @template Name - The name of the permission that the target corresponds to. * @template AllowedCaveat - A union of the allowed {@link Caveat} types * for the permission. */ export type ValidPermission< - TargetKey extends TargetName, + Name extends TargetName, AllowedCaveat extends CaveatConstraint, > = PermissionConstraint & { // TODO:TS4.4 Make optional @@ -105,59 +104,9 @@ export type ValidPermission< * A pointer to the resource that possession of the capability grants * access to, for example a JSON-RPC method or endowment. */ - readonly parentCapability: ExtractPermissionTargetNames; + readonly parentCapability: Name; }; -/** - * A utility type for ensuring that the given permission target name conforms to - * our naming conventions. - * - * See the README for the distinction between target names and keys. - */ -type ValidTargetName = Name extends `${string}*` - ? never - : Name extends `${string}_` - ? never - : Name; - -/** - * A utility type for extracting permission target names from a union of target - * keys. - * - * See the README for the distinction between target names and keys. - * - * @template Key - The target key type to extract target names from. - */ -export type ExtractPermissionTargetNames = ValidTargetName< - Key extends `${infer Base}_*` ? `${Base}_${string}` : Key ->; - -/** - * Extracts the permission key of a particular name from a union of keys. - * An internal utility type used in {@link ExtractPermissionTargetKey}. - * - * @template Key - The target key type to extract from. - * @template Name - The name whose key to extract. - */ -type KeyOfTargetName< - Key extends string, - Name extends string, -> = Name extends ExtractPermissionTargetNames ? Key : never; - -/** - * A utility type for finding the permission target key corresponding to a - * target name. In a way, the inverse of {@link ExtractPermissionTargetNames}. - * - * See the README for the distinction between target names and keys. - * - * @template Key - The target key type to extract from. - * @template Name - The name whose key to extract. - */ -export type ExtractPermissionTargetKey< - Key extends string, - Name extends string, -> = Key extends Name ? Key : Extract>; - /** * Internal utility for extracting the members types of an array. The type * evalutes to `never` if the specified type is the empty tuple or neither @@ -392,22 +341,6 @@ export type PermissionSideEffect< onFailure?: SideEffectHandler; }; -/** - * A utility type for ensuring that the given permission target key conforms to - * our naming conventions. - * - * See the README for the distinction between target names and keys. - * - * @template Key - The target key string to apply the constraint to. - */ -type ValidTargetKey = Key extends `${string}_*` - ? Key - : Key extends `${string}_` - ? never - : Key extends `${string}*` - ? never - : Key; - /** * The different possible types of permissions. */ @@ -442,12 +375,9 @@ type PermissionSpecificationBase = { permissionType: Type; /** - * The target resource of the permission. The shape of this string depends on - * the permission type. For example, a restricted method target key will - * consist of either a complete method name or the prefix of a namespaced - * method, e.g. `wallet_snap_*`. + * The name of the target resource of the permission. */ - targetKey: string; + targetName: string; /** * An array of the caveat types that may be added to instances of this @@ -549,7 +479,7 @@ type PermissionSpecificationBuilderOptions< MethodHooks extends Record, ValidatorHooks extends Record, > = { - targetKey?: string; + targetName?: string; allowedCaveats?: Readonly> | null; factoryHooks?: FactoryHooks; methodHooks?: MethodHooks; @@ -575,7 +505,7 @@ export type PermissionSpecificationBuilder< * {@link PermissionSpecificationBuilder} function and "hook name" objects. */ export type PermissionSpecificationBuilderExportConstraint = { - targetKey: string; + targetName: string; specificationBuilder: PermissionSpecificationBuilder< PermissionType, PermissionSpecificationBuilderOptions, @@ -602,9 +532,7 @@ type ValidRestrictedMethodSpecification< */ export type ValidPermissionSpecification< Specification extends PermissionSpecificationConstraint, -> = Specification['targetKey'] extends ValidTargetKey< - Specification['targetKey'] -> +> = Specification['targetName'] extends TargetName ? Specification['permissionType'] extends PermissionType.Endowment ? Specification : Specification['permissionType'] extends PermissionType.RestrictedMethod @@ -644,8 +572,8 @@ export function hasSpecificationType< export type PermissionSpecificationMap< Specification extends PermissionSpecificationConstraint, > = { - [TargetKey in Specification['targetKey']]: Specification extends { - targetKey: TargetKey; + [Name in Specification['targetName']]: Specification extends { + targetName: Name; } ? Specification : never; @@ -656,13 +584,13 @@ export type PermissionSpecificationMap< * permission specifications. * * @template Specification - The specification union type to extract from. - * @template TargetKey - The `targetKey` of the specification to extract. + * @template Name - The `targetName` of the specification to extract. */ export type ExtractPermissionSpecification< Specification extends PermissionSpecificationConstraint, - TargetKey extends Specification['targetKey'], + Name extends Specification['targetName'], > = Specification extends { - targetKey: TargetKey; + targetName: Name; } ? Specification : never; diff --git a/packages/permission-controller/src/PermissionController.test.ts b/packages/permission-controller/src/PermissionController.test.ts index 2cc520202b..f1d8ff9a97 100644 --- a/packages/permission-controller/src/PermissionController.test.ts +++ b/packages/permission-controller/src/PermissionController.test.ts @@ -191,20 +191,15 @@ const PermissionKeys = { 'wallet_noopWithPermittedAndFailureSideEffects2', wallet_noopWithPermittedSideEffects: 'wallet_noopWithPermittedSideEffects', wallet_noopWithValidator: 'wallet_noopWithValidator', + wallet_noopWithRequiredCaveat: 'wallet_noopWithRequiredCaveat', wallet_noopWithFactory: 'wallet_noopWithFactory', - 'wallet_getSecret_*': 'wallet_getSecret_*', snap_foo: 'snap_foo', - endowmentPermission1: 'endowmentPermission1', - endowmentPermission2: 'endowmentPermission2', + endowmentAnySubject: 'endowmentAnySubject', + endowmentSnapsOnly: 'endowmentSnapsOnly', } as const; -// wallet_getSecret_* -// We only define types for permissions with factories. -// Other permission types are extracted from the permission specifications in -// the permission controller. - -type SecretNamespacedPermission = ValidPermission< - typeof PermissionKeys['wallet_getSecret_*'], +type NoopWithRequiredCaveat = ValidPermission< + typeof PermissionKeys['wallet_noopWithRequiredCaveat'], NoopCaveat >; @@ -214,8 +209,7 @@ type NoopWithFactoryPermission = ValidPermission< >; /** - * Permission name (as opposed to keys) constants and getters. Since one of the - * permissions are namespaced, it's a getter function. + * Permission name (as opposed to keys) constants and getters. */ const PermissionNames = { wallet_doubleNumber: PermissionKeys.wallet_doubleNumber, @@ -229,11 +223,11 @@ const PermissionNames = { PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2, wallet_noopWithPermittedSideEffects: PermissionKeys.wallet_noopWithPermittedSideEffects, + wallet_noopWithRequiredCaveat: PermissionKeys.wallet_noopWithRequiredCaveat, wallet_noopWithFactory: PermissionKeys.wallet_noopWithFactory, snap_foo: PermissionKeys.snap_foo, - endowmentPermission1: PermissionKeys.endowmentPermission1, - endowmentPermission2: PermissionKeys.endowmentPermission2, - wallet_getSecret_: (str: string) => `wallet_getSecret_${str}` as const, + endowmentAnySubject: PermissionKeys.endowmentAnySubject, + endowmentSnapsOnly: PermissionKeys.endowmentSnapsOnly, } as const; /** @@ -246,7 +240,7 @@ function getDefaultPermissionSpecifications() { return { [PermissionKeys.wallet_getSecretArray]: { permissionType: PermissionType.RestrictedMethod, - targetKey: PermissionKeys.wallet_getSecretArray, + targetName: PermissionKeys.wallet_getSecretArray, allowedCaveats: [ CaveatTypes.filterArrayResponse, CaveatTypes.reverseArrayResponse, @@ -257,7 +251,7 @@ function getDefaultPermissionSpecifications() { }, [PermissionKeys.wallet_getSecretObject]: { permissionType: PermissionType.RestrictedMethod, - targetKey: PermissionKeys.wallet_getSecretObject, + targetName: PermissionKeys.wallet_getSecretObject, allowedCaveats: [ CaveatTypes.filterObjectResponse, CaveatTypes.noopCaveat, @@ -275,32 +269,9 @@ function getDefaultPermissionSpecifications() { ); }, }, - [PermissionKeys['wallet_getSecret_*']]: { - permissionType: PermissionType.RestrictedMethod, - targetKey: PermissionKeys['wallet_getSecret_*'], - allowedCaveats: [CaveatTypes.noopCaveat], - methodImplementation: (args: RestrictedMethodOptions) => { - return `Hello, secret friend "${args.method.replace( - 'wallet_getSecret_', - '', - )}"!`; - }, - factory: (options: PermissionOptions) => - constructPermission({ - ...options, - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], - }), - validator: (permission: PermissionConstraint) => { - assert.deepStrictEqual( - permission.caveats, - [{ type: CaveatTypes.noopCaveat, value: null }], - 'getSecret_* permission validation failed', - ); - }, - }, [PermissionKeys.wallet_doubleNumber]: { permissionType: PermissionType.RestrictedMethod, - targetKey: PermissionKeys.wallet_doubleNumber, + targetName: PermissionKeys.wallet_doubleNumber, allowedCaveats: null, methodImplementation: ({ params }: RestrictedMethodOptions<[number]>) => { if (!Array.isArray(params)) { @@ -313,7 +284,7 @@ function getDefaultPermissionSpecifications() { }, [PermissionKeys.wallet_noop]: { permissionType: PermissionType.RestrictedMethod, - targetKey: PermissionKeys.wallet_noop, + targetName: PermissionKeys.wallet_noop, allowedCaveats: null, methodImplementation: (_args: RestrictedMethodOptions) => { return null; @@ -321,7 +292,7 @@ function getDefaultPermissionSpecifications() { }, [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects]: { permissionType: PermissionType.RestrictedMethod, - targetKey: PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects, + targetName: PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects, allowedCaveats: null, methodImplementation: (_args: RestrictedMethodOptions) => { return null; @@ -333,7 +304,7 @@ function getDefaultPermissionSpecifications() { }, [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2]: { permissionType: PermissionType.RestrictedMethod, - targetKey: PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2, + targetName: PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2, allowedCaveats: null, methodImplementation: (_args: RestrictedMethodOptions) => { return null; @@ -345,7 +316,7 @@ function getDefaultPermissionSpecifications() { }, [PermissionKeys.wallet_noopWithPermittedSideEffects]: { permissionType: PermissionType.RestrictedMethod, - targetKey: PermissionKeys.wallet_noopWithPermittedSideEffects, + targetName: PermissionKeys.wallet_noopWithPermittedSideEffects, allowedCaveats: null, methodImplementation: (_args: RestrictedMethodOptions) => { return null; @@ -357,7 +328,7 @@ function getDefaultPermissionSpecifications() { // This one exists to check some permission validator logic [PermissionKeys.wallet_noopWithValidator]: { permissionType: PermissionType.RestrictedMethod, - targetKey: PermissionKeys.wallet_noopWithValidator, + targetName: PermissionKeys.wallet_noopWithValidator, methodImplementation: (_args: RestrictedMethodOptions) => { return null; }, @@ -372,11 +343,45 @@ function getDefaultPermissionSpecifications() { } }, }, + [PermissionKeys.wallet_noopWithRequiredCaveat]: { + permissionType: PermissionType.RestrictedMethod, + targetName: PermissionKeys.wallet_noopWithRequiredCaveat, + methodImplementation: (_args: RestrictedMethodOptions) => { + return null; + }, + allowedCaveats: [CaveatTypes.noopCaveat], + factory: ( + options: PermissionOptions, + _requestData?: Record, + ) => { + return constructPermission({ + ...options, + caveats: [ + { + type: CaveatTypes.noopCaveat, + value: null, + }, + ], + }); + }, + validator: (permission: PermissionConstraint) => { + if ( + permission.caveats?.length !== 1 || + !permission.caveats?.some( + ({ type }) => type === CaveatTypes.noopCaveat, + ) + ) { + throw new Error( + 'noopWithRequiredCaveat permission validation failed', + ); + } + }, + }, // This one exists just to check that permission factories can use the // requestData of approved permission requests [PermissionKeys.wallet_noopWithFactory]: { permissionType: PermissionType.RestrictedMethod, - targetKey: PermissionKeys.wallet_noopWithFactory, + targetName: PermissionKeys.wallet_noopWithFactory, methodImplementation: (_args: RestrictedMethodOptions) => { return null; }, @@ -402,22 +407,22 @@ function getDefaultPermissionSpecifications() { }, [PermissionKeys.snap_foo]: { permissionType: PermissionType.RestrictedMethod, - targetKey: PermissionKeys.snap_foo, + targetName: PermissionKeys.snap_foo, allowedCaveats: null, methodImplementation: (_args: RestrictedMethodOptions) => { return null; }, subjectTypes: [SubjectType.Snap], }, - [PermissionKeys.endowmentPermission1]: { + [PermissionKeys.endowmentAnySubject]: { permissionType: PermissionType.Endowment, - targetKey: PermissionKeys.endowmentPermission1, + targetName: PermissionKeys.endowmentAnySubject, endowmentGetter: (_options: EndowmentGetterParams) => ['endowment1'], allowedCaveats: null, }, - [PermissionKeys.endowmentPermission2]: { + [PermissionKeys.endowmentSnapsOnly]: { permissionType: PermissionType.Endowment, - targetKey: PermissionKeys.endowmentPermission2, + targetName: PermissionKeys.endowmentSnapsOnly, endowmentGetter: (_options: EndowmentGetterParams) => ['endowment2'], allowedCaveats: [CaveatTypes.endowmentCaveat], subjectTypes: [SubjectType.Snap], @@ -637,29 +642,29 @@ describe('PermissionController', () => { }); }); - it('throws if a permission specification targetKey is invalid', () => { - ['', 'foo_', 'foo*'].forEach((invalidTargetKey) => { - expect( - () => - new PermissionController< - DefaultPermissionSpecifications, - DefaultCaveatSpecifications - >( - getPermissionControllerOptions({ - permissionSpecifications: { - ...getDefaultPermissionSpecifications(), - [invalidTargetKey]: { - permissionType: PermissionType.Endowment, - targetKey: invalidTargetKey, - }, + it('throws if a permission specification targetName is invalid', () => { + const invalidTargetName = ''; + + expect( + () => + new PermissionController< + DefaultPermissionSpecifications, + DefaultCaveatSpecifications + >( + getPermissionControllerOptions({ + permissionSpecifications: { + ...getDefaultPermissionSpecifications(), + [invalidTargetName]: { + permissionType: PermissionType.Endowment, + targetName: invalidTargetName, }, - }), - ), - ).toThrow(`Invalid permission target key: "${invalidTargetKey}"`); - }); + }, + }), + ), + ).toThrow(`Invalid permission target name: "${invalidTargetName}"`); }); - it('throws if a permission specification map key does not match its "targetKey" value', () => { + it('throws if a permission specification map key does not match its "targetName" value', () => { expect( () => new PermissionController< @@ -671,13 +676,13 @@ describe('PermissionController', () => { ...getDefaultPermissionSpecifications(), foo: { permissionType: PermissionType.Endowment, - targetKey: 'bar', + targetName: 'bar', }, }, }), ), ).toThrow( - `Invalid permission specification: key "foo" must match specification.target value "bar".`, + `Invalid permission specification: target name "foo" must match specification.targetName value "bar".`, ); }); @@ -713,7 +718,7 @@ describe('PermissionController', () => { ...getDefaultPermissionSpecifications(), foo: { permissionType: PermissionType.Endowment, - targetKey: 'foo', + targetName: 'foo', allowedCaveats: [defaultCaveats.reverseArrayResponse.type], }, }, @@ -737,7 +742,7 @@ describe('PermissionController', () => { ...getDefaultPermissionSpecifications(), foo: { permissionType: PermissionType.RestrictedMethod, - targetKey: 'foo', + targetName: 'foo', allowedCaveats: [defaultCaveats.endowmentCaveat.type], }, }, @@ -777,25 +782,11 @@ describe('PermissionController', () => { ).toStrictEqual(['a', 'b', 'c']); }); - it('gets the implementation of a namespaced restricted method', async () => { - const controller = getDefaultPermissionController(); - const method = controller.getRestrictedMethod( - PermissionNames.wallet_getSecret_('foo'), - ); - - expect( - await method({ - method: 'wallet_getSecret_foo', - context: { origin: 'github.com' }, - }), - ).toStrictEqual('Hello, secret friend "foo"!'); - }); - it('throws an error if the requested permission target is not a restricted method', () => { const controller = getDefaultPermissionController(); expect(() => - controller.getRestrictedMethod(PermissionNames.endowmentPermission1), - ).toThrow(errors.methodNotFound(PermissionNames.endowmentPermission1)); + controller.getRestrictedMethod(PermissionNames.endowmentAnySubject), + ).toThrow(errors.methodNotFound(PermissionNames.endowmentAnySubject)); }); it('throws an error if the method does not exist', () => { @@ -907,31 +898,6 @@ describe('PermissionController', () => { ), ).toStrictEqual(false); }); - - it('correctly indicates whether an origin has a namespaced permission', () => { - const controller = getDefaultPermissionController(); - - controller.grantPermissions({ - subject: { origin: 'metamask.io' }, - approvedPermissions: { - wallet_getSecret_kabob: {}, - }, - }); - - expect( - controller.hasPermission( - 'metamask.io', - PermissionNames.wallet_getSecret_('kabob'), - ), - ).toStrictEqual(true); - - expect( - controller.hasPermission( - 'metamask.io', - PermissionNames.wallet_getSecret_('falafel'), - ), - ).toStrictEqual(false); - }); }); describe('hasPermissions', () => { @@ -955,7 +921,7 @@ describe('PermissionController', () => { subject: { origin: 'foo' }, approvedPermissions: { [PermissionNames.wallet_getSecretArray]: {}, - [PermissionNames.wallet_getSecret_('bar')]: {}, + [PermissionNames.wallet_doubleNumber]: {}, }, }); @@ -1027,39 +993,6 @@ describe('PermissionController', () => { }); }); - it('revokes a namespaced permission', () => { - const controller = getDefaultPermissionControllerWithState(); - expect(controller.state).toStrictEqual(getExistingPermissionState()); - const origin = 'metamask.io'; - - controller.grantPermissions({ - subject: { origin }, - approvedPermissions: { - [PermissionNames.wallet_getSecret_('foo')]: {}, - }, - }); - - controller.revokePermission( - origin, - PermissionNames.wallet_getSecret_('foo'), - ); - - expect(controller.state).toStrictEqual({ - subjects: { - [origin]: { - origin, - permissions: { - [PermissionNames.wallet_getSecretArray]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecretArray, - caveats: null, - invoker: origin, - }), - }, - }, - }, - }); - }); - it('throws an error if the specified subject has no permissions', () => { const controller = getDefaultPermissionController(); expect(() => @@ -1114,7 +1047,6 @@ describe('PermissionController', () => { approvedPermissions: { [PermissionNames.wallet_getSecretArray]: {}, [PermissionNames.wallet_getSecretObject]: {}, - [PermissionNames.wallet_getSecret_('foo')]: {}, }, }); @@ -1122,7 +1054,7 @@ describe('PermissionController', () => { subject: { origin: origin3 }, approvedPermissions: { [PermissionNames.wallet_getSecretArray]: {}, - [PermissionNames.wallet_getSecret_('foo')]: {}, + [PermissionNames.endowmentAnySubject]: {}, }, }); @@ -1131,21 +1063,16 @@ describe('PermissionController', () => { approvedPermissions: { [PermissionNames.wallet_getSecretArray]: {}, [PermissionNames.wallet_getSecretObject]: {}, - [PermissionNames.wallet_getSecret_('bar')]: {}, }, }); controller.revokePermissions({ [origin0]: [PermissionNames.wallet_getSecretArray], - [origin2]: [ - PermissionNames.wallet_getSecretArray, - PermissionNames.wallet_getSecret_('foo'), - ], + [origin2]: [PermissionNames.wallet_getSecretArray], [origin3]: [PermissionNames.wallet_getSecretArray], [origin4]: [ PermissionNames.wallet_getSecretArray, PermissionNames.wallet_getSecretObject, - PermissionNames.wallet_getSecret_('bar'), ], }); @@ -1174,9 +1101,9 @@ describe('PermissionController', () => { [origin3]: { origin: origin3, permissions: { - [PermissionNames.wallet_getSecret_('foo')]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecret_('foo'), - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], + [PermissionNames.endowmentAnySubject]: getPermissionMatcher({ + parentCapability: PermissionNames.endowmentAnySubject, + caveats: null, invoker: origin3, }), }, @@ -1245,7 +1172,6 @@ describe('PermissionController', () => { approvedPermissions: { [PermissionNames.wallet_getSecretArray]: {}, [PermissionNames.wallet_getSecretObject]: {}, - [PermissionNames.wallet_getSecret_('foo')]: {}, }, }); @@ -1253,7 +1179,7 @@ describe('PermissionController', () => { subject: { origin: origin3 }, approvedPermissions: { [PermissionNames.wallet_getSecretArray]: {}, - [PermissionNames.wallet_getSecret_('foo')]: {}, + [PermissionNames.endowmentAnySubject]: {}, }, }); @@ -1262,7 +1188,6 @@ describe('PermissionController', () => { approvedPermissions: { [PermissionNames.wallet_getSecretArray]: {}, [PermissionNames.wallet_getSecretObject]: {}, - [PermissionNames.wallet_getSecret_('bar')]: {}, }, }); @@ -1290,19 +1215,14 @@ describe('PermissionController', () => { caveats: null, invoker: origin2, }), - [PermissionNames.wallet_getSecret_('foo')]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecret_('foo'), - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], - invoker: origin2, - }), }, }, [origin3]: { origin: origin3, permissions: { - [PermissionNames.wallet_getSecret_('foo')]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecret_('foo'), - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], + [PermissionNames.endowmentAnySubject]: getPermissionMatcher({ + parentCapability: PermissionNames.endowmentAnySubject, + caveats: null, invoker: origin3, }), }, @@ -1315,11 +1235,6 @@ describe('PermissionController', () => { caveats: null, invoker: origin4, }), - [PermissionNames.wallet_getSecret_('bar')]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecret_('bar'), - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], - invoker: origin4, - }), }, }, }, @@ -1341,7 +1256,6 @@ describe('PermissionController', () => { { type: CaveatTypes.filterObjectResponse, value: ['kaplar'] }, ], }, - [PermissionNames.wallet_getSecret_('foo')]: {}, }, }); @@ -1360,14 +1274,6 @@ describe('PermissionController', () => { CaveatTypes.filterObjectResponse, ), ).toStrictEqual(true); - - expect( - controller.hasCaveat( - origin, - PermissionNames.wallet_getSecret_('foo'), - CaveatTypes.noopCaveat, - ), - ).toStrictEqual(true); }); it('throws an error if no corresponding permission exists', () => { @@ -1401,7 +1307,6 @@ describe('PermissionController', () => { { type: CaveatTypes.filterObjectResponse, value: ['kaplar'] }, ], }, - [PermissionNames.wallet_getSecret_('foo')]: {}, }, }); @@ -1423,14 +1328,6 @@ describe('PermissionController', () => { type: CaveatTypes.filterObjectResponse, value: ['kaplar'], }); - - expect( - controller.getCaveat( - origin, - PermissionNames.wallet_getSecret_('foo'), - CaveatTypes.noopCaveat, - ), - ).toStrictEqual({ type: CaveatTypes.noopCaveat, value: null }); }); it('throws an error if no corresponding permission exists', () => { @@ -1738,7 +1635,7 @@ describe('PermissionController', () => { controller.grantPermissions({ subject: { origin }, approvedPermissions: { - [PermissionNames.wallet_getSecret_('foo')]: { + [PermissionNames.wallet_getSecretObject]: { caveats: [{ type: CaveatTypes.noopCaveat, value: null }], }, }, @@ -1747,7 +1644,7 @@ describe('PermissionController', () => { expect(() => controller.updateCaveat( origin, - PermissionNames.wallet_getSecret_('foo'), + PermissionNames.wallet_getSecretObject, CaveatTypes.noopCaveat, 'bar' as any, ), @@ -1938,7 +1835,7 @@ describe('PermissionController', () => { controller.grantPermissions({ subject: { origin }, approvedPermissions: { - [PermissionNames.wallet_getSecret_('foo')]: { + [PermissionNames.wallet_noopWithRequiredCaveat]: { caveats: [{ type: CaveatTypes.noopCaveat, value: null }], }, }, @@ -1947,10 +1844,12 @@ describe('PermissionController', () => { expect(() => controller.removeCaveat( origin, - PermissionNames.wallet_getSecret_('foo'), + PermissionNames.wallet_noopWithRequiredCaveat, CaveatTypes.noopCaveat, ), - ).toThrow(new Error('getSecret_* permission validation failed')); + ).toThrow( + new Error('noopWithRequiredCaveat permission validation failed'), + ); }); }); @@ -1993,7 +1892,7 @@ describe('PermissionController', () => { { type: CaveatTypes.noopCaveat, value: null }, ], }, - [PermissionNames.wallet_getSecret_('foo')]: { + [PermissionNames.wallet_noopWithRequiredCaveat]: { caveats: [{ type: CaveatTypes.noopCaveat, value: null }], }, [PermissionNames.wallet_doubleNumber]: {}, @@ -2006,7 +1905,7 @@ describe('PermissionController', () => { [PermissionNames.wallet_getSecretObject]: { caveats: [{ type: CaveatTypes.filterObjectResponse, value: ['c'] }], }, - [PermissionNames.wallet_getSecret_('bar')]: { + [PermissionNames.wallet_noopWithRequiredCaveat]: { caveats: [{ type: CaveatTypes.noopCaveat, value: null }], }, }, @@ -2055,11 +1954,13 @@ describe('PermissionController', () => { ], invoker: MultiCaveatOrigins.b, }), - [PermissionNames.wallet_getSecret_('foo')]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecret_('foo'), - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], - invoker: MultiCaveatOrigins.b, - }), + [PermissionNames.wallet_noopWithRequiredCaveat]: + getPermissionMatcher({ + parentCapability: + PermissionNames.wallet_noopWithRequiredCaveat, + caveats: [{ type: CaveatTypes.noopCaveat, value: null }], + invoker: MultiCaveatOrigins.b, + }), [PermissionNames.wallet_doubleNumber]: getPermissionMatcher({ parentCapability: PermissionNames.wallet_doubleNumber, caveats: null, @@ -2079,11 +1980,13 @@ describe('PermissionController', () => { ], invoker: MultiCaveatOrigins.c, }), - [PermissionNames.wallet_getSecret_('bar')]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecret_('bar'), - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], - invoker: MultiCaveatOrigins.c, - }), + [PermissionNames.wallet_noopWithRequiredCaveat]: + getPermissionMatcher({ + parentCapability: + PermissionNames.wallet_noopWithRequiredCaveat, + caveats: [{ type: CaveatTypes.noopCaveat, value: null }], + invoker: MultiCaveatOrigins.c, + }), ...overrides[MultiCaveatOrigins.c], }, }, @@ -2303,7 +2206,7 @@ describe('PermissionController', () => { controller.updatePermissionsByCaveat(CaveatTypes.noopCaveat, () => { return { operation: CaveatMutatorOperation.deleteCaveat }; }), - ).toThrow('getSecret_* permission validation failed'); + ).toThrow('noopWithRequiredCaveat permission validation failed'); }); it('throws if mutator returns unrecognized operation', () => { @@ -2377,32 +2280,6 @@ describe('PermissionController', () => { }); }); - it('grants new permissions (namespaced, with factory and validator)', () => { - const controller = getDefaultPermissionController(); - const origin = 'metamask.io'; - - controller.grantPermissions({ - subject: { origin }, - approvedPermissions: { - wallet_getSecret_kabob: {}, - }, - }); - - expect(controller.state).toStrictEqual({ - subjects: { - [origin]: { - origin, - permissions: { - wallet_getSecret_kabob: getPermissionMatcher({ - parentCapability: 'wallet_getSecret_kabob', - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], - }), - }, - }, - }, - }); - }); - it('grants new permissions (multiple origins)', () => { const controller = getDefaultPermissionController(); const origin1 = 'metamask.io'; @@ -2411,7 +2288,7 @@ describe('PermissionController', () => { controller.grantPermissions({ subject: { origin: origin1 }, approvedPermissions: { - wallet_getSecret_kabob: {}, + wallet_getSecretObject: {}, }, }); @@ -2427,9 +2304,9 @@ describe('PermissionController', () => { [origin1]: { origin: origin1, permissions: { - wallet_getSecret_kabob: getPermissionMatcher({ - parentCapability: 'wallet_getSecret_kabob', - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], + wallet_getSecretObject: getPermissionMatcher({ + parentCapability: 'wallet_getSecretObject', + caveats: null, invoker: origin1, }), }, @@ -2470,7 +2347,7 @@ describe('PermissionController', () => { controller.grantPermissions({ subject: { origin }, approvedPermissions: { - [PermissionNames.endowmentPermission2]: { + [PermissionNames.endowmentSnapsOnly]: { caveats: [ { type: CaveatTypes.endowmentCaveat, @@ -2488,9 +2365,9 @@ describe('PermissionController', () => { [origin]: { origin, permissions: { - [PermissionNames.endowmentPermission2]: getPermissionMatcher({ + [PermissionNames.endowmentSnapsOnly]: getPermissionMatcher({ invoker: origin, - parentCapability: PermissionNames.endowmentPermission2, + parentCapability: PermissionNames.endowmentSnapsOnly, caveats: [ { type: CaveatTypes.endowmentCaveat, @@ -2896,7 +2773,7 @@ describe('PermissionController', () => { controller.grantPermissions({ subject: { origin }, approvedPermissions: { - [PermissionNames.wallet_getSecret_('foo')]: { + [PermissionNames.wallet_getSecretObject]: { caveats: [ { type: CaveatTypes.noopCaveat, @@ -3463,7 +3340,6 @@ describe('PermissionController', () => { { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, ], }, - [PermissionNames.wallet_getSecret_('foo')]: {}, }, ), ).toMatchObject([ @@ -3480,11 +3356,6 @@ describe('PermissionController', () => { ], invoker: origin, }), - [PermissionNames.wallet_getSecret_('foo')]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecret_('foo'), - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], - invoker: origin, - }), }, { id: expect.any(String), origin }, ]); @@ -3504,7 +3375,6 @@ describe('PermissionController', () => { { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, ], }, - [PermissionNames.wallet_getSecret_('foo')]: {}, }, }, type: MethodNames.requestPermissions, @@ -3524,10 +3394,10 @@ describe('PermissionController', () => { const [, { requestData }] = args; return { metadata: { ...requestData.metadata }, - // wallet_getSecret_foo is added to the request + // endowmentAnySubject is added to the request permissions: { ...requestData.permissions, - [PermissionNames.wallet_getSecret_('foo')]: {}, + [PermissionNames.endowmentAnySubject]: {}, }, }; }); @@ -3559,9 +3429,9 @@ describe('PermissionController', () => { ], invoker: origin, }), - [PermissionNames.wallet_getSecret_('foo')]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecret_('foo'), - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], + [PermissionNames.endowmentAnySubject]: getPermissionMatcher({ + parentCapability: PermissionNames.endowmentAnySubject, + caveats: null, invoker: origin, }), }, @@ -3620,7 +3490,7 @@ describe('PermissionController', () => { { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, ], }, - [PermissionNames.wallet_getSecret_('foo')]: {}, + [PermissionNames.endowmentAnySubject]: {}, }, ), ).toMatchObject([ @@ -3632,9 +3502,9 @@ describe('PermissionController', () => { ], invoker: origin, }), - [PermissionNames.wallet_getSecret_('foo')]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecret_('foo'), - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], + [PermissionNames.endowmentAnySubject]: getPermissionMatcher({ + parentCapability: PermissionNames.endowmentAnySubject, + caveats: null, invoker: origin, }), }, @@ -3656,7 +3526,7 @@ describe('PermissionController', () => { { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, ], }, - [PermissionNames.wallet_getSecret_('foo')]: {}, + [PermissionNames.endowmentAnySubject]: {}, }, }, type: MethodNames.requestPermissions, @@ -3698,7 +3568,7 @@ describe('PermissionController', () => { { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, ], }, - [PermissionNames.wallet_getSecret_('foo')]: {}, + [PermissionNames.endowmentAnySubject]: {}, }, ), ).toMatchObject([ @@ -3715,9 +3585,9 @@ describe('PermissionController', () => { ], invoker: origin, }), - [PermissionNames.wallet_getSecret_('foo')]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecret_('foo'), - caveats: [{ type: CaveatTypes.noopCaveat, value: null }], + [PermissionNames.endowmentAnySubject]: getPermissionMatcher({ + parentCapability: PermissionNames.endowmentAnySubject, + caveats: null, invoker: origin, }), }, @@ -3739,7 +3609,7 @@ describe('PermissionController', () => { { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, ], }, - [PermissionNames.wallet_getSecret_('foo')]: {}, + [PermissionNames.endowmentAnySubject]: {}, }, }, type: MethodNames.requestPermissions, @@ -4629,14 +4499,14 @@ describe('PermissionController', () => { controller.grantPermissions({ subject: { origin }, approvedPermissions: { - [PermissionNames.endowmentPermission1]: {}, + [PermissionNames.endowmentAnySubject]: {}, }, }); expect( await controller.getEndowments( origin, - PermissionNames.endowmentPermission1, + PermissionNames.endowmentAnySubject, ), ).toStrictEqual(['endowment1']); }); @@ -4670,10 +4540,10 @@ describe('PermissionController', () => { const origin = 'metamask.io'; await expect( - controller.getEndowments(origin, PermissionNames.endowmentPermission1), + controller.getEndowments(origin, PermissionNames.endowmentAnySubject), ).rejects.toThrow( errors.unauthorized({ - data: { origin, targetName: PermissionNames.endowmentPermission1 }, + data: { origin, targetName: PermissionNames.endowmentAnySubject }, }), ); }); @@ -4719,25 +4589,6 @@ describe('PermissionController', () => { ).toStrictEqual(20); }); - it('executes a namespaced restricted method', async () => { - const controller = getDefaultPermissionController(); - const origin = 'metamask.io'; - - controller.grantPermissions({ - subject: { origin }, - approvedPermissions: { - [PermissionNames.wallet_getSecret_('foo')]: {}, - }, - }); - - expect( - await controller.executeRestrictedMethod( - origin, - PermissionNames.wallet_getSecret_('foo'), - ), - ).toStrictEqual('Hello, secret friend "foo"!'); - }); - it('executes a restricted method with a caveat', async () => { const controller = getDefaultPermissionController(); const origin = 'metamask.io'; @@ -4885,13 +4736,13 @@ describe('PermissionController', () => { messenger.call( 'PermissionController:getEndowments', 'foo', - PermissionNames.endowmentPermission1, + PermissionNames.endowmentAnySubject, ), ).rejects.toThrow( errors.unauthorized({ data: { origin: 'foo', - targetName: PermissionNames.endowmentPermission1, + targetName: PermissionNames.endowmentAnySubject, }, }), ); @@ -4899,7 +4750,7 @@ describe('PermissionController', () => { controller.grantPermissions({ subject: { origin: 'foo' }, approvedPermissions: { - [PermissionNames.endowmentPermission1]: {}, + [PermissionNames.endowmentAnySubject]: {}, }, }); @@ -4907,7 +4758,7 @@ describe('PermissionController', () => { await messenger.call( 'PermissionController:getEndowments', 'foo', - PermissionNames.endowmentPermission1, + PermissionNames.endowmentAnySubject, ), ).toStrictEqual(['endowment1']); @@ -4915,7 +4766,7 @@ describe('PermissionController', () => { await messenger.call( 'PermissionController:getEndowments', 'foo', - PermissionNames.endowmentPermission1, + PermissionNames.endowmentAnySubject, { arbitrary: 'requestData' }, ), ).toStrictEqual(['endowment1']); @@ -4924,21 +4775,21 @@ describe('PermissionController', () => { expect(getEndowmentsSpy).toHaveBeenNthCalledWith( 1, 'foo', - PermissionNames.endowmentPermission1, + PermissionNames.endowmentAnySubject, undefined, ); expect(getEndowmentsSpy).toHaveBeenNthCalledWith( 2, 'foo', - PermissionNames.endowmentPermission1, + PermissionNames.endowmentAnySubject, undefined, ); expect(getEndowmentsSpy).toHaveBeenNthCalledWith( 3, 'foo', - PermissionNames.endowmentPermission1, + PermissionNames.endowmentAnySubject, { arbitrary: 'requestData' }, ); }); diff --git a/packages/permission-controller/src/PermissionController.ts b/packages/permission-controller/src/PermissionController.ts index 6b959f1fc8..6fb33cc167 100644 --- a/packages/permission-controller/src/PermissionController.ts +++ b/packages/permission-controller/src/PermissionController.ts @@ -416,7 +416,7 @@ export type ExtractPermission< ControllerCaveatSpecification extends CaveatSpecificationConstraint, > = ControllerPermissionSpecification extends ValidPermissionSpecification ? ValidPermission< - ControllerPermissionSpecification['targetKey'], + ControllerPermissionSpecification['targetName'], ExtractCaveats > : never; @@ -621,18 +621,18 @@ export class PermissionController< /** * Gets a permission specification. * - * @param targetKey - The target key of the permission specification to get. - * @returns The permission specification with the specified target key. + * @param targetName - The name of the permission specification to get. + * @returns The permission specification with the specified target name. */ private getPermissionSpecification< - TargetKey extends ControllerPermissionSpecification['targetKey'], + TargetName extends ControllerPermissionSpecification['targetName'], >( - targetKey: TargetKey, + targetName: TargetName, ): ExtractPermissionSpecification< ControllerPermissionSpecification, - TargetKey + TargetName > { - return this._permissionSpecifications[targetKey]; + return this._permissionSpecifications[targetName]; } /** @@ -648,9 +648,7 @@ export class PermissionController< } /** - * Constructor helper for validating permission specifications. This is - * intended to prevent the use of invalid target keys which, while impossible - * to add in TypeScript, could rather easily occur in plain JavaScript. + * Constructor helper for validating permission specifications. * * Throws an error if validation fails. * @@ -667,22 +665,20 @@ export class PermissionController< permissionSpecifications, ).forEach( ([ - targetKey, - { permissionType, targetKey: innerTargetKey, allowedCaveats }, + targetName, + { permissionType, targetName: innerTargetName, allowedCaveats }, ]) => { if (!permissionType || !hasProperty(PermissionType, permissionType)) { throw new Error(`Invalid permission type: "${permissionType}"`); } - // Check if the target key is the empty string, ends with "_", or ends - // with "*" but not "_*" - if (!targetKey || /_$/u.test(targetKey) || /[^_]\*$/u.test(targetKey)) { - throw new Error(`Invalid permission target key: "${targetKey}"`); + if (!targetName) { + throw new Error(`Invalid permission target name: "${targetName}"`); } - if (targetKey !== innerTargetKey) { + if (targetName !== innerTargetName) { throw new Error( - `Invalid permission specification: key "${targetKey}" must match specification.target value "${innerTargetKey}".`, + `Invalid permission specification: target name "${targetName}" must match specification.targetName value "${innerTargetName}".`, ); } @@ -843,12 +839,11 @@ export class PermissionController< requestingOrigin, ); - const targetKey = this.getTargetKey(targetName); - if (!targetKey) { + if (!this.targetExists(targetName)) { throw failureError; } - const specification = this.getPermissionSpecification(targetKey); + const specification = this.getPermissionSpecification(targetName); if (!hasSpecificationType(specification, permissionType)) { throw failureError; } @@ -1292,7 +1287,7 @@ export class PermissionController< permission.caveats = [caveat] as any; } - this.validateModifiedPermission(permission, origin, target); + this.validateModifiedPermission(permission, origin); }); } @@ -1365,12 +1360,7 @@ export class PermissionController< break; case CaveatMutatorOperation.deleteCaveat: - this.deleteCaveat( - permission, - targetCaveatType, - subject.origin, - permission.parentCapability, - ); + this.deleteCaveat(permission, targetCaveatType, subject.origin); break; case CaveatMutatorOperation.revokePermission: @@ -1427,7 +1417,7 @@ export class PermissionController< throw new CaveatDoesNotExistError(origin, target, caveatType); } - this.deleteCaveat(permission, caveatType, origin, target); + this.deleteCaveat(permission, caveatType, origin); }); } @@ -1442,23 +1432,21 @@ export class PermissionController< * @param permission - The permission whose caveat to delete. * @param caveatType - The type of the caveat to delete. * @param origin - The origin the permission subject. - * @param target - The name of the permission target. */ private deleteCaveat< - TargetName extends ExtractPermission< - ControllerPermissionSpecification, - ControllerCaveatSpecification - >['parentCapability'], CaveatType extends ExtractCaveats['type'], >( permission: Draft, caveatType: CaveatType, origin: OriginString, - target: TargetName, ): void { /* istanbul ignore if: not possible in our usage */ if (!permission.caveats) { - throw new CaveatDoesNotExistError(origin, target, caveatType); + throw new CaveatDoesNotExistError( + origin, + permission.parentCapability, + caveatType, + ); } const caveatIndex = permission.caveats.findIndex( @@ -1466,7 +1454,11 @@ export class PermissionController< ); if (caveatIndex === -1) { - throw new CaveatDoesNotExistError(origin, target, caveatType); + throw new CaveatDoesNotExistError( + origin, + permission.parentCapability, + caveatType, + ); } if (permission.caveats.length === 1) { @@ -1475,7 +1467,7 @@ export class PermissionController< permission.caveats.splice(caveatIndex, 1); } - this.validateModifiedPermission(permission, origin, target); + this.validateModifiedPermission(permission, origin); } /** @@ -1483,80 +1475,41 @@ export class PermissionController< * on a permission after its caveats have been modified. * * Just like {@link PermissionController.validatePermission}, except that the - * corresponding target key and specification are retrieved first, and an - * error is thrown if the target key does not exist. + * corresponding target name and specification are retrieved first, and an + * error is thrown if the target name does not exist. * * @param permission - The modified permission to validate. * @param origin - The origin associated with the permission. - * @param targetName - The target name name of the permission. */ private validateModifiedPermission( permission: Draft, origin: OriginString, - targetName: ExtractPermission< - ControllerPermissionSpecification, - ControllerCaveatSpecification - >['parentCapability'], ): void { - const targetKey = this.getTargetKey(permission.parentCapability); /* istanbul ignore if: this should be impossible */ - if (!targetKey) { + if (!this.targetExists(permission.parentCapability)) { throw new Error( - `Fatal: Existing permission target key "${targetKey}" has no specification.`, + `Fatal: Existing permission target "${permission.parentCapability}" has no specification.`, ); } this.validatePermission( - this.getPermissionSpecification(targetKey), + this.getPermissionSpecification(permission.parentCapability), permission as PermissionConstraint, origin, - targetName, ); } /** - * Gets the key for the specified permission target. - * - * Used to support our namespaced permission target feature, which is used - * to implement namespaced restricted JSON-RPC methods. + * Verifies the existence the specified permission target, i.e. whether it has + * a specification. * * @param target - The requested permission target. - * @returns The internal key of the permission target. + * @returns Whether the permission target exists. */ - private getTargetKey( + private targetExists( target: string, - ): ControllerPermissionSpecification['targetKey'] | undefined { - if (hasProperty(this._permissionSpecifications, target)) { - return target; - } - - const namespacedTargetsWithoutWildcard: Record = {}; - for (const targetKey of Object.keys(this._permissionSpecifications)) { - const wildCardMatch = targetKey.match(/(.+)\*$/u); - if (wildCardMatch) { - namespacedTargetsWithoutWildcard[wildCardMatch[1]] = true; - } - } - - // Check for potentially nested namespaces: - // Ex: wildzone_ - // Ex: eth_plugin_ - const segments = target.split('_'); - let targetKey = ''; - - while ( - segments.length > 0 && - !hasProperty(this._permissionSpecifications, targetKey) && - !namespacedTargetsWithoutWildcard[targetKey] - ) { - targetKey += `${segments.shift()}_`; - } - - if (namespacedTargetsWithoutWildcard[targetKey]) { - return `${targetKey}*`; - } - - return undefined; + ): target is ControllerPermissionSpecification['targetName'] { + return hasProperty(this._permissionSpecifications, target); } /** @@ -1616,8 +1569,7 @@ export class PermissionController< for (const [requestedTarget, approvedPermission] of Object.entries( approvedPermissions, )) { - const targetKey = this.getTargetKey(requestedTarget); - if (!targetKey) { + if (!this.targetExists(requestedTarget)) { throw methodNotFound(requestedTarget); } @@ -1632,13 +1584,13 @@ export class PermissionController< ); } - // The requested target must be a valid target name if we found its key. - // We reassign it to change its type. + // We have verified that the target exists, and reassign it to change its + // type. const targetName = requestedTarget as ExtractPermission< ControllerPermissionSpecification, ControllerCaveatSpecification >['parentCapability']; - const specification = this.getPermissionSpecification(targetKey); + const specification = this.getPermissionSpecification(targetName); // The requested caveats are validated here. const caveats = this.constructCaveats( @@ -1663,14 +1615,14 @@ export class PermissionController< // Full caveat and permission validation is performed here since the // factory function can arbitrarily modify the entire permission object, // including its caveats. - this.validatePermission(specification, permission, origin, targetName); + this.validatePermission(specification, permission, origin); } else { permission = constructPermission(permissionOptions); // We do not need to validate caveats in this case, because the plain // permission constructor function does not modify the caveats, which // were already validated by `constructCaveats` above. - this.validatePermission(specification, permission, origin, targetName, { + this.validatePermission(specification, permission, origin, { invokePermissionValidator: true, performCaveatValidation: false, }); @@ -1696,7 +1648,6 @@ export class PermissionController< * @param specification - The specification of the permission. * @param permission - The permission to validate. * @param origin - The origin associated with the permission. - * @param targetName - The target name of the permission. * @param validationOptions - Validation options. * @param validationOptions.invokePermissionValidator - Whether to invoke the * permission's consumer-specified validator function, if any. @@ -1708,16 +1659,12 @@ export class PermissionController< specification: PermissionSpecificationConstraint, permission: PermissionConstraint, origin: OriginString, - targetName: ExtractPermission< - ControllerPermissionSpecification, - ControllerCaveatSpecification - >['parentCapability'], { invokePermissionValidator, performCaveatValidation } = { invokePermissionValidator: true, performCaveatValidation: true, }, ): void { - const { allowedCaveats, validator } = specification; + const { allowedCaveats, validator, targetName } = specification; if ( specification.subjectTypes?.length && @@ -2003,9 +1950,8 @@ export class PermissionController< for (const targetName of Object.keys(requestedPermissions)) { const permission = requestedPermissions[targetName]; - const targetKey = this.getTargetKey(targetName); - if (!targetKey) { + if (!this.targetExists(targetName)) { throw methodNotFound(targetName, { origin, requestedPermissions }); } @@ -2023,11 +1969,10 @@ export class PermissionController< // Here we validate the permission without invoking its validator, if any. // The validator will be invoked after the permission has been approved. this.validatePermission( - this.getPermissionSpecification(targetKey), + this.getPermissionSpecification(targetName), // Typecast: The permission is still a "PlainObject" here. permission as PermissionConstraint, origin, - targetName, { invokePermissionValidator: false, performCaveatValidation: true }, ); } @@ -2067,10 +2012,8 @@ export class PermissionController< private getSideEffects(permissions: RequestedPermissions) { return Object.keys(permissions).reduce( (sideEffectList, targetName) => { - const targetKey = this.getTargetKey(targetName); - - if (targetKey) { - const specification = this.getPermissionSpecification(targetKey); + if (this.targetExists(targetName)) { + const specification = this.getPermissionSpecification(targetName); if (specification.sideEffect) { sideEffectList.permittedHandlers[targetName] =