Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Block invalid account name creation #5784

Merged
merged 6 commits into from
Jun 7, 2017
Merged

Block invalid account name creation #5784

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1bfe8db
Additional non-empty phrase check (fromNew)
jacogr Jun 7, 2017
757a2af
Explicit canCreate check in create (not only on UI)
jacogr Jun 7, 2017
f227230
BN instance check (fixes Geth imports)
jacogr Jun 7, 2017
f1227a5
Fixup tests after better checks
jacogr Jun 7, 2017
1376427
PR comments (Thanks @tomusdrw )
jacogr Jun 7, 2017
750c427
Typo
jacogr Jun 7, 2017
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
2 changes: 0 additions & 2 deletions js/src/modals/CreateAccount/createAccount.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,6 @@ class CreateAccount extends Component {
}

onCreate = () => {
this.createStore.setBusy(true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that we don't need to setBusy(false) as well (since it's done inside createAccount in store)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1


return this.createStore
.createAccount(this.vaultStore)
.then(() => {
Expand Down
4 changes: 3 additions & 1 deletion js/src/modals/CreateAccount/createAccount.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import BigNumber from 'bignumber.js';
import sinon from 'sinon';

import Api from '~/api';
import Store from './store';

const ADDRESS = '0x00000123456789abcdef123456789abcdef123456789abcdef';
Expand Down Expand Up @@ -45,7 +46,8 @@ function createApi () {
setAccountName: sinon.stub().resolves(),
listVaults: sinon.stub().resolves([]),
listOpenedVaults: sinon.stub().resolves([])
}
},
util: Api.util
};
}

Expand Down
18 changes: 15 additions & 3 deletions js/src/modals/CreateAccount/store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export default class Store {
return !(this.nameError || this.walletFileError);

case 'fromNew':
return !(this.nameError || this.passwordRepeatError) && this.hasAddress;
return !(this.nameError || this.passwordRepeatError) && this.hasAddress && this.hasPhrase;

case 'fromPhrase':
return !(this.nameError || this.passwordRepeatError || this.passPhraseError);
Expand All @@ -91,7 +91,11 @@ export default class Store {
}

@computed get hasAddress () {
return !!(this.address);
return this.address.length !== 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.address is never null/undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, will revert - extra safety. (It should be empty, but no need to muck about)

}

@computed get hasPhrase () {
return this.phrase.length !== 0;
}

@computed get passwordRepeatError () {
Expand All @@ -112,7 +116,7 @@ export default class Store {
this.passwordRepeat = '';
this.phrase = '';
this.name = '';
this.nameError = null;
this.nameError = ERRORS.noName;
this.qrAddress = null;
this.rawKey = '';
this.rawKeyError = null;
Expand Down Expand Up @@ -250,6 +254,10 @@ export default class Store {
}

@action nextStage = () => {
if (this.stage === 0) {
this.clearErrors();
}

this.stage++;
}

Expand All @@ -258,6 +266,10 @@ export default class Store {
}

createAccount = (vaultStore) => {
if (!this.canCreate) {
return false;
}

this.setBusy(true);

return this
Expand Down
57 changes: 45 additions & 12 deletions js/src/modals/CreateAccount/store.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe('modals/CreateAccount/Store', () => {
store.clearErrors();

expect(store.name).to.equal('');
expect(store.nameError).to.be.null;
expect(store.nameError).not.to.be.null;
expect(store.password).to.equal('');
expect(store.passwordRepeatError).to.be.null;
expect(store.qrAddress).to.be.null;
Expand Down Expand Up @@ -309,6 +309,7 @@ describe('modals/CreateAccount/Store', () => {
describe('createType === fromJSON/fromPresale', () => {
beforeEach(() => {
store.setCreateType('fromJSON');
store.setName('blah');
});

it('returns true on no errors', () => {
Expand All @@ -330,6 +331,8 @@ describe('modals/CreateAccount/Store', () => {
beforeEach(() => {
store.setCreateType('fromNew');
store.setAddress('0x0000000000000000000000000000000000000000');
store.setName('blah');
store.setPhrase('testing');
});

it('returns true on no errors', () => {
Expand All @@ -342,6 +345,12 @@ describe('modals/CreateAccount/Store', () => {
expect(store.canCreate).to.be.false;
});

it('returns false on no phrase', () => {
store.setPhrase('');

expect(store.canCreate).to.be.false;
});

it('returns false on passwordRepeatError', () => {
store.setPassword('testing');

Expand All @@ -352,6 +361,7 @@ describe('modals/CreateAccount/Store', () => {
describe('createType === fromPhrase', () => {
beforeEach(() => {
store.setCreateType('fromPhrase');
store.setName('name');
});

it('returns true on no errors', () => {
Expand All @@ -372,6 +382,8 @@ describe('modals/CreateAccount/Store', () => {
describe('createType === fromRaw', () => {
beforeEach(() => {
store.setCreateType('fromRaw');
store.setName('name');
store.setRawKey('0x1000000000000000000000000000000000000000000000000000000000000000');
});

it('returns true on no errors', () => {
Expand All @@ -389,7 +401,7 @@ describe('modals/CreateAccount/Store', () => {
});

it('returns false on rawKeyError', () => {
store.setRawKey('testing');
store.setRawKey('0x1');
expect(store.canCreate).to.be.false;
});
});
Expand Down Expand Up @@ -459,6 +471,9 @@ describe('modals/CreateAccount/Store', () => {
createAccountFromQrSpy = sinon.spy(store, 'createAccountFromQr');
createAccountFromRawSpy = sinon.spy(store, 'createAccountFromRaw');
busySpy = sinon.spy(store, 'setBusy');

store.setName('name');
store.setPhrase('testing');
});

afterEach(() => {
Expand All @@ -477,6 +492,8 @@ describe('modals/CreateAccount/Store', () => {

it('calls createAccountFromGeth on createType === fromGeth', () => {
store.setCreateType('fromGeth');
store.setGethAccountsAvailable(GETH_ADDRESSES);
store.selectGethAccount(GETH_ADDRESSES[0]);

return store.createAccount().then(() => {
expect(createAccountFromGethSpy).to.have.been.called;
Expand All @@ -485,6 +502,8 @@ describe('modals/CreateAccount/Store', () => {

it('calls createAccountFromWallet on createType === fromJSON', () => {
store.setCreateType('fromJSON');
store.setName('name');
store.setWalletJson('{}');

return store.createAccount().then(() => {
expect(createAccountFromWalletSpy).to.have.been.called;
Expand All @@ -493,6 +512,9 @@ describe('modals/CreateAccount/Store', () => {

it('calls createAccountFromPhrase on createType === fromNew', () => {
store.setCreateType('fromNew');
store.setName('name');
store.setPhrase('phrase');
store.setAddress('0x1234567890123456789012345678901234567890');

return store.createAccount().then(() => {
expect(createAccountFromPhraseSpy).to.have.been.called;
Expand All @@ -501,6 +523,9 @@ describe('modals/CreateAccount/Store', () => {

it('calls createAccountFromPhrase on createType === fromPhrase', () => {
store.setCreateType('fromPhrase');
store.setName('name');
store.setPhrase('phrase');
store.setAddress('0x1234567890123456789012345678901234567890');

return store.createAccount().then(() => {
expect(createAccountFromPhraseSpy).to.have.been.called;
Expand All @@ -509,6 +534,8 @@ describe('modals/CreateAccount/Store', () => {

it('calls createAccountFromWallet on createType === fromPresale', () => {
store.setCreateType('fromPresale');
store.setName('name');
store.setWalletJson('{}');

return store.createAccount().then(() => {
expect(createAccountFromWalletSpy).to.have.been.called;
Expand All @@ -517,6 +544,8 @@ describe('modals/CreateAccount/Store', () => {

it('calls createAccountFromQr on createType === fromQr', () => {
store.setCreateType('fromQr');
store.setQrAddress('0x1234567890123456789012345678901234567890');
store.setName('name');

return store.createAccount().then(() => {
expect(createAccountFromQrSpy).to.have.been.called;
Expand All @@ -525,6 +554,8 @@ describe('modals/CreateAccount/Store', () => {

it('calls createAccountFromRaw on createType === fromRaw', () => {
store.setCreateType('fromRaw');
store.setName('name');
store.setRawKey('0x1000000000000000000000000000000000000000000000000000000000000000');

return store.createAccount().then(() => {
expect(createAccountFromRawSpy).to.have.been.called;
Expand All @@ -534,6 +565,9 @@ describe('modals/CreateAccount/Store', () => {
it('moves account to vault when vaultName set', () => {
store.setCreateType('fromNew');
store.setVaultName('testing');
store.setName('name');
store.setAddress('0x1234567890123456789012345678901234567890');
store.setPhrase('phrase');

return store.createAccount(vaultStore).then(() => {
expect(vaultStore.moveAccount).to.have.been.calledWith('testing', ADDRESS);
Expand All @@ -542,6 +576,9 @@ describe('modals/CreateAccount/Store', () => {

it('sets and rests the busy flag', () => {
store.setCreateType('fromNew');
store.setName('name');
store.setAddress('0x1234567890123456789012345678901234567890');
store.setPhrase('phrase');

return store.createAccount().then(() => {
expect(busySpy).to.have.been.calledWith(true);
Expand Down Expand Up @@ -634,22 +671,18 @@ describe('modals/CreateAccount/Store', () => {
beforeEach(() => {
store.setName('some name');
store.setDescription('some desc');
store.setQrAddress('0x123');
store.setQrAddress('0x1234567890123456789012345678901234567890');
sinon.spy(store, 'setupMeta');

return store.createAccountFromQr(-1);
});

it('sets the accountInfo name', () => {
expect(api.parity.setAccountName).to.have.been.calledWith('0x123', 'some name');
afterEach(() => {
store.setupMeta.restore();
});

it('sets the meta (with extrenal flag)', () => {
expect(api.parity.setAccountMeta).to.have.been.calledWith('0x123', {
description: 'some desc',
passwordHint: '',
timestamp: -1,
external: true
});
it('sets the meta', () => {
expect(store.setupMeta).to.have.been.called;
});
});

Expand Down
2 changes: 1 addition & 1 deletion js/src/ui/Balance/balance.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export class Balance extends Component {

const isEthToken = token.native;
const isFullToken = !showOnlyEth || isEthToken;
const hasBalance = balanceValue.gt(0);
const hasBalance = (balanceValue instanceof BigNumber) && balanceValue.gt(0);

if (!hasBalance && !isEthToken) {
return null;
Expand Down