Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(snaps): Migration to remove snap_ManageAccounts from the snaps permissions. #28656

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
182 changes: 182 additions & 0 deletions app/scripts/migrations/132.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
import { AccountsControllerState } from '@metamask/accounts-controller';
import { cloneDeep } from 'lodash';
import { createMockInternalAccount } from '../../../test/jest/mocks';
import { migrate, version } from './132';

const oldVersion = 131;

const mockSnapControllerState = {
snaps: {
foobar: {
id: 'foobar',
},
},
};

const mockPermissionControllerState = {
subjects: {
foobar: {
permissions: {
snap_manageAccounts: {},
},
},
},
};

describe(`migration #${version}`, () => {
afterEach(() => jest.resetAllMocks());

it('updates the version metadata', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
SnapController: mockSnapControllerState,
PermissionController: mockPermissionControllerState,
},
};

const newStorage = await migrate(cloneDeep(oldStorage));

expect(newStorage.meta).toStrictEqual({ version });
});

it('does nothing if SnapController is not present', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
PermissionController: mockPermissionControllerState,
},
};

const newStorage = await migrate(cloneDeep(oldStorage));

expect(newStorage.data).toStrictEqual(oldStorage.data);
});

it('does nothing if PermissionController is not present', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
SnapController: mockSnapControllerState,
},
};

const newStorage = await migrate(cloneDeep(oldStorage));

expect(newStorage.data).toStrictEqual(oldStorage.data);
});

it('does nothing if there is no snaps', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
SnapController: {},
PermissionController: mockPermissionControllerState,
},
};

const newStorage = await migrate(cloneDeep(oldStorage));

expect(newStorage.data).toStrictEqual(oldStorage.data);
});

it('does nothing if there are no permission subjects', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
SnapController: mockSnapControllerState,
PermissionController: {},
},
};

const newStorage = await migrate(cloneDeep(oldStorage));

expect(newStorage.data).toStrictEqual(oldStorage.data);
});

it('does nothing if the snap does not have a corresponding permission subject', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
SnapController: {
snaps: {
foobar: {
id: 'foobar',
},
},
},
PermissionController: {
subjects: {},
},
},
};

const newStorage = await migrate(cloneDeep(oldStorage));

expect(newStorage.data).toStrictEqual(oldStorage.data);
});

it('does nothing if the snap does not have a snap_manageAccounts permission', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
SnapController: {
snaps: {
foobar: {
id: 'foobar',
},
},
},
PermissionController: {
subjects: {
foobar: {
permissions: {
snap_notify: {},
},
},
},
},
},
};

const newStorage = await migrate(cloneDeep(oldStorage));

expect(newStorage.data).toStrictEqual(oldStorage.data);
});

it('removes the snap_manageAccounts permission from the snap', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
SnapController: mockSnapControllerState,
PermissionController: {
subjects: {
foobar: {
permissions: {
snap_notify: {},
snap_manageAccounts: {},
},
},
},
},
},
};

const expectedData = {
SnapController: mockSnapControllerState,
PermissionController: {
subjects: {
foobar: {
permissions: {
snap_notify: {},
},
},
},
},
};

const newStorage = await migrate(cloneDeep(oldStorage));

expect(newStorage.data).toStrictEqual(expectedData);
});
});
96 changes: 96 additions & 0 deletions app/scripts/migrations/132.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import {
CaveatSpecificationConstraint,
ExtractPermission,
PermissionControllerState,
PermissionSpecificationConstraint,
} from '@metamask/permission-controller';
import { SnapControllerState } from '@metamask/snaps-controllers';
import { hasProperty, isObject } from '@metamask/utils';
import { cloneDeep } from 'lodash';

type VersionedData = {
meta: { version: number };
data: Record<string, unknown>;
};

type ActualPermissionControllerState = PermissionControllerState<
ExtractPermission<
PermissionSpecificationConstraint,
CaveatSpecificationConstraint
>
>;

export const version = 132;

/**
* Handle the `snap_manageAccounts` removal from the restricted methods of snap.
*
* @param originalVersionedData - Versioned MetaMask extension state, exactly
* what we persist to dist.
* @param originalVersionedData.meta - State metadata.
* @param originalVersionedData.meta.version - The current state version.
* @param originalVersionedData.data - The persisted MetaMask state, keyed by
* controller.
* @returns Updated versioned MetaMask extension state.
*/
export async function migrate(
originalVersionedData: VersionedData,
): Promise<VersionedData> {
const versionedData = cloneDeep(originalVersionedData);
versionedData.meta.version = version;
transformState(versionedData.data);
return versionedData;
}

function transformState(state: Record<string, unknown>): void {
if (
!hasProperty(state, 'SnapController') ||
!isObject(state.SnapController)
) {
return;
}

if (
!hasProperty(state, 'PermissionController') ||
!isObject(state.PermissionController)
) {
return;
}

const snapControllerState = state.SnapController as SnapControllerState;
const permissionControllerState =
state.PermissionController as ActualPermissionControllerState;

if (
!isObject(snapControllerState) ||
!hasProperty(snapControllerState, 'snaps')
) {
return;
}

if (
!isObject(permissionControllerState) ||
!hasProperty(permissionControllerState, 'subjects')
) {
return;
}

const { snaps } = snapControllerState;
const { subjects } = permissionControllerState;

Object.keys(snaps).forEach((snapId) => {
if (subjects[snapId]) {
const newSnapPermissions = Object.fromEntries(
Object.entries(subjects[snapId].permissions).filter(
([permissionName]) => permissionName !== 'snap_manageAccounts',
),
);

(state.PermissionController as ActualPermissionControllerState).subjects[
snapId
].permissions = newSnapPermissions;
}
});

console.log(state);
}
1 change: 1 addition & 0 deletions app/scripts/migrations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ const migrations = [
require('./129'),
require('./130'),
require('./131'),
require('./132'),
];

export default migrations;
Loading