Skip to content

Commit

Permalink
Manually connect via the full connect flow
Browse files Browse the repository at this point in the history
  • Loading branch information
@whymarrh
whymarrh committed May 28, 2020
1 parent 145edbe commit 5b16086
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 289 deletions.
82 changes: 16 additions & 66 deletions app/scripts/controllers/permissions/index.js
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import nanoid from 'nanoid'
import JsonRpcEngine from 'json-rpc-engine'
import asMiddleware from 'json-rpc-engine/src/asMiddleware'
import ObservableStore from 'obs-store'
Expand Down Expand Up @@ -94,7 +95,7 @@ export class PermissionsController {
getUnlockPromise: () => this._getUnlockPromise(true),
hasPermission: this.hasPermission.bind(this, origin),
requestAccountsPermission: this._requestPermissions.bind(
this, origin, { eth_accounts: {} }
this, { origin }, { eth_accounts: {} },
),
}))

Expand All @@ -105,6 +106,17 @@ export class PermissionsController {
return asMiddleware(engine)
}

/**
* Request {@code eth_accounts} permissions
* @param {string} origin - The origin
* @returns {Promise<string>} the request ID
*/
async requestAccountsPermission (origin) {
const id = nanoid()
this._requestPermissions({ origin, id }, { eth_accounts: {} })
return id
}

/**
* Returns the accounts that should be exposed for the given origin domain,
* if any. This method exists for when a trusted context needs to know
Expand Down Expand Up @@ -154,18 +166,18 @@ export class PermissionsController {
/**
* Submits a permissions request to rpc-cap. Internal, background use only.
*
* @param {string} origin - The origin string.
* @param {IOriginMetadata} metadata - The origin metadata.
* @param {IRequestedPermissions} permissions - The requested permissions.
*/
_requestPermissions (origin, permissions) {
_requestPermissions (metadata, permissions) {
return new Promise((resolve, reject) => {

// rpc-cap assigns an id to the request if there is none, as expected by
// requestUserApproval below
const req = { method: 'wallet_requestPermissions', params: [permissions] }
const res = {}
this.permissions.providerMiddlewareFunction(
{ origin }, req, res, () => {}, _end
metadata, req, res, () => {}, _end
)

function _end (_err) {
Expand Down Expand Up @@ -243,68 +255,6 @@ export class PermissionsController {
this._removePendingApproval(id)
}

/**
* @deprecated
* Grants the given origin the eth_accounts permission for the given account(s).
* This method should ONLY be called as a result of direct user action in the UI,
* with the intention of supporting legacy dapps that don't support EIP 1102.
*
* @param {string} origin - The origin to expose the account(s) to.
* @param {Array<string>} accounts - The account(s) to expose.
*/
async legacyExposeAccounts (origin, accounts) {

// accounts are validated by finalizePermissionsRequest
if (typeof origin !== 'string' || !origin.length) {
throw new Error('Must provide non-empty string origin.')
}

const existingAccounts = await this.getAccounts(origin)

if (existingAccounts.length > 0) {
throw new Error(
'May not call legacyExposeAccounts on origin with exposed accounts.'
)
}

const permissions = await this.finalizePermissionsRequest(
{ eth_accounts: {} }, accounts
)

try {

await new Promise((resolve, reject) => {
this.permissions.grantNewPermissions(
origin, permissions, {}, _end
)

function _end (err) {
if (err) {
reject(err)
} else {
resolve()
}
}
})

this.notifyDomain(origin, {
method: NOTIFICATION_NAMES.accountsChanged,
result: accounts,
})
this.permissionsLog.logAccountExposure(origin, accounts)

} catch (error) {

throw ethErrors.rpc.internal({
message: `Failed to add 'eth_accounts' to '${origin}'.`,
data: {
originalError: error,
accounts,
},
})
}
}

/**
* Expose an account to the given origin. Changes the eth_accounts
* permissions and emits accountsChanged.
Expand Down
27 changes: 0 additions & 27 deletions app/scripts/controllers/permissions/permissionsLog.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,33 +202,6 @@ export default class PermissionsLogController {
this.updateActivityLog(logs)
}

/**
* Record account exposure and eth_accounts permissions history for the given
* origin.
*
* @param {string} origin - The origin accounts were exposed to.
* @param {Array<string>} accounts - The accounts that were exposed.
*/
logAccountExposure (origin, accounts) {

if (
typeof origin !== 'string' || !origin.length ||
!Array.isArray(accounts) || accounts.length === 0
) {
throw new Error(
'Must provide non-empty string origin and array of accounts.'
)
}

this.logPermissionsHistory(
['eth_accounts'],
origin,
accounts,
Date.now(),
true
)
}

/**
* Create new permissions history log entries, if any, and commit them.
*
Expand Down
2 changes: 1 addition & 1 deletion app/scripts/metamask-controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ export default class MetamaskController extends EventEmitter {
removePermissionsFor: permissionsController.removePermissionsFor.bind(permissionsController),
addPermittedAccount: nodeify(permissionsController.addPermittedAccount, permissionsController),
removePermittedAccount: nodeify(permissionsController.removePermittedAccount, permissionsController),
legacyExposeAccounts: nodeify(permissionsController.legacyExposeAccounts, permissionsController),
requestAccountsPermission: nodeify(permissionsController.requestAccountsPermission, permissionsController),

getRequestAccountTabIds: (cb) => cb(null, this.getRequestAccountTabIds()),
getOpenMetamaskTabsIds: (cb) => cb(null, this.getOpenMetamaskTabsIds()),
Expand Down
22 changes: 0 additions & 22 deletions test/unit/app/controllers/permissions/mocks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,20 +366,6 @@ export const getters = deepFreeze({
},
},

legacyExposeAccounts: {
badOrigin: () => {
return {
message: 'Must provide non-empty string origin.',
}
},
forbiddenUsage: () => {
return {
name: 'Error',
message: 'May not call legacyExposeAccounts on origin with exposed accounts.',
}
},
},

_handleAccountSelected: {
invalidParams: () => {
return {
Expand Down Expand Up @@ -426,14 +412,6 @@ export const getters = deepFreeze({
},
},

logAccountExposure: {
invalidParams: () => {
return {
message: 'Must provide non-empty string origin and array of accounts.',
}
},
},

pendingApprovals: {
duplicateOriginOrId: (id, origin) => {
return {
Expand Down
123 changes: 10 additions & 113 deletions test/unit/app/controllers/permissions/permissions-controller-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,119 +715,6 @@ describe('permissions controller', function () {
})
})

describe('legacyExposeAccounts', function () {

let permController, notifications

beforeEach(function () {
notifications = initNotifications()
permController = initPermController(notifications)
})

it('successfully exposes accounts and updates permissions history', async function () {

let aAccounts = await permController.getAccounts(ORIGINS.a)
assert.deepEqual(aAccounts, [], 'origin should have no accounts')

await permController.legacyExposeAccounts(ORIGINS.a, ACCOUNT_ARRAYS.a)

aAccounts = await permController.getAccounts(ORIGINS.a)
assert.deepEqual(aAccounts, ACCOUNT_ARRAYS.a, 'origin should have correct accounts')

// now, permissions history should be updated
const permissionsHistory = permController.permissionsLog.getHistory()
const historyOrigins = Object.keys(permissionsHistory)

assert.equal(historyOrigins.length, 1, 'should have single origin')
assert.equal(historyOrigins[0], ORIGINS.a, 'should have correct origin')

assert.ok(
permissionsHistory[ORIGINS.a].eth_accounts,
'history should have eth_accounts entry'
)

assert.deepEqual(
Object.keys(permissionsHistory[ORIGINS.a].eth_accounts.accounts),
ACCOUNT_ARRAYS.a,
'should have expected eth_accounts entry accounts'
)

// notification should also have been sent
assert.deepEqual(
notifications[ORIGINS.a][0],
NOTIFICATIONS.newAccounts(ACCOUNT_ARRAYS.a),
'first origin should have correct notification'
)
})

it('throws if called on origin with existing exposed accounts', async function () {

grantPermissions(
permController, ORIGINS.a,
PERMS.finalizedRequests.eth_accounts(ACCOUNT_ARRAYS.a)
)

const aAccounts = await permController.getAccounts(ORIGINS.a)
assert.deepEqual(aAccounts, ACCOUNT_ARRAYS.a, 'origin should have correct accounts')

await assert.rejects(
permController.legacyExposeAccounts(ORIGINS.a, ACCOUNT_ARRAYS.b),
ERRORS.legacyExposeAccounts.forbiddenUsage(),
'should throw if called on origin with existing exposed accounts'
)

const permissionsHistory = permController.permissionsLog.getHistory()
assert.deepEqual(
permissionsHistory, {},
'should not have modified history'
)
assert.deepEqual(
notifications[ORIGINS.a], [],
'should not have sent notification'
)
})

it('throws if called with bad accounts', async function () {

await assert.rejects(
permController.legacyExposeAccounts(ORIGINS.a, []),
ERRORS.validatePermittedAccounts.invalidParam(),
'should throw if called with no accounts'
)

const permissionsHistory = permController.permissionsLog.getHistory()
assert.deepEqual(
permissionsHistory, {},
'should not have modified history'
)
assert.deepEqual(
notifications[ORIGINS.a], [],
'should not have sent notification'
)
})

it('throws if called with bad origin', async function () {

await assert.rejects(
permController.legacyExposeAccounts(null, ACCOUNT_ARRAYS.a),
ERRORS.legacyExposeAccounts.badOrigin(),
'should throw if called with invalid origin'
)

const permissionsHistory = permController.permissionsLog.getHistory()
assert.deepEqual(
permissionsHistory, {},
'should not have modified history'
)
Object.keys(notifications).forEach((domain) => {
assert.deepEqual(
notifications[domain], [],
'should not have sent notification'
)
})
})
})

describe('preferences state update', function () {

let permController, notifications, preferences, identities
Expand Down Expand Up @@ -1506,6 +1393,16 @@ describe('permissions controller', function () {
permController = initPermController()
})

it('requestAccountsPermission calls _requestAccountsPermission with an explicit request ID', async function () {
const _requestPermissions = sinon.stub(permController, '_requestPermissions').resolves()
await permController.requestAccountsPermission('example.com')
assert.ok(_requestPermissions.calledOnceWithExactly(
sinon.match.object.and(sinon.match.has('origin')).and(sinon.match.has('id')),
{ eth_accounts: {} },
))
_requestPermissions.restore()
})

it('_addPendingApproval: should throw if adding origin twice', function () {

const id = nanoid()
Expand Down
41 changes: 0 additions & 41 deletions test/unit/app/controllers/permissions/permissions-log-controller-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
} from './mocks'

const {
ERRORS,
PERMS,
RPC_REQUESTS,
} = getters
Expand Down Expand Up @@ -647,44 +646,4 @@ describe('permissions log', function () {
)
})
})

describe('instance method edge cases', function () {

it('logAccountExposure errors on invalid params', function () {

const permLog = initPermLog()

assert.throws(
() => {
permLog.logAccountExposure('', ACCOUNT_ARRAYS.a)
},
ERRORS.logAccountExposure.invalidParams(),
'should throw expected error'
)

assert.throws(
() => {
permLog.logAccountExposure(null, ACCOUNT_ARRAYS.a)
},
ERRORS.logAccountExposure.invalidParams(),
'should throw expected error'
)

assert.throws(
() => {
permLog.logAccountExposure('foo', {})
},
ERRORS.logAccountExposure.invalidParams(),
'should throw expected error'
)

assert.throws(
() => {
permLog.logAccountExposure('foo', [])
},
ERRORS.logAccountExposure.invalidParams(),
'should throw expected error'
)
})
})
})
Loading

0 comments on commit 5b16086

Please sign in to comment.