Skip to content

Commit

Permalink
Merge pull request #254 from getsafle/feature-add-parameter-validations
Browse files Browse the repository at this point in the history 
Feature add parameter validations
  • Loading branch information
@sshubhamagg
sshubhamagg authored Aug 29, 2023
2 parents cee2330 + 69b7fab commit 3b35cfc
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 53 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -529,4 +529,9 @@

### 1.30.04 (2023-08-28) Nightly version : only for experimental use

* Updated the logs for label update & delete account
* Updated the logs for label update & delete account
* Removed unused encryption key parameter in get accounts
* Added validation for pin parameter in export private key, restore keyring state & current pin parameter in change pin
* Added validation for encryption key in add account, sign message, delete account, get vault details & update label
* Sync the pin validation steps with other methods in import wallet
* Updated tests wrt changes in vault generation and parameter validations
80 changes: 66 additions & 14 deletions src/lib/keyring.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ class Keyring {
return { response: true };
}

async getAccounts(encryptionKey) {
async getAccounts() {

const decryptedVault = this.decryptedVault;

let chain = (Chains.evmChains.hasOwnProperty(this.chain) || this.chain === 'ethereum') ? 'eth' : this.chain;
Expand Down Expand Up @@ -153,6 +154,12 @@ class Keyring {
return { error: ERROR_MESSAGE.INCORRECT_PIN_TYPE };
}

const response = await this.validatePin(pin);

if (response.response == false || response.error) {
return { error: ERROR_MESSAGE.INCORRECT_PIN };
};

const chain = (Chains.evmChains.hasOwnProperty(this.chain) || this.chain === 'ethereum') ? 'eth' : this.chain;
const importedChain = (chain === 'eth') ? 'evmChains' : chain;

Expand All @@ -166,12 +173,6 @@ class Keyring {
return { error: ERROR_MESSAGE.ADDRESS_NOT_PRESENT };
}

const response = await this.validatePin(pin);

if (response.response == false || response.error) {
return { error: ERROR_MESSAGE.INCORRECT_PIN };
};

if (isImportedAddress) {
const privateKey = (chain === 'eth') ? this.decryptedVault.importedWallets.evmChains.data.find(element => element.address === address).privateKey : this.decryptedVault.importedWallets[chain].data.find(element => element.address === address).privateKey;

Expand Down Expand Up @@ -206,7 +207,13 @@ class Keyring {
return { error: ERROR_MESSAGE.INCORRECT_PIN };
};

const acc = await this.getAccounts(encryptionKey);
const { error } = helper.validateEncryptionKey(this.vault, JSON.stringify(encryptionKey));

if (error) {
return { error }
}

const acc = await this.getAccounts();

if (Chains.evmChains.hasOwnProperty(this.chain) || this.chain === 'ethereum') {
const accounts = await this.keyringInstance.getAccounts();
Expand Down Expand Up @@ -272,6 +279,13 @@ class Keyring {
if(res.response == false || res.error) {
return { error: ERROR_MESSAGE.INCORRECT_PIN };
};

const err = helper.validateEncryptionKey(this.vault, JSON.stringify(encryptionKey));

if (err.error) {
return { error : err.error }
}

const { error, response } = await this.exportPrivateKey(address, pin);

if (error) {
Expand Down Expand Up @@ -303,7 +317,7 @@ class Keyring {

}
else{
const accounts = await this.getAccounts(encryptionKey);
const accounts = await this.getAccounts();

if(accounts.response.filter(e => e.address === address).length < 1) {
return { error: ERROR_MESSAGE.NONEXISTENT_KEYRING_ACCOUNT };
Expand Down Expand Up @@ -374,6 +388,12 @@ class Keyring {
return { error: ERROR_MESSAGE.INCORRECT_PIN_TYPE };
}

const res = await this.validatePin(pin)

if(res.response == false || res.error) {
return { error: ERROR_MESSAGE.INCORRECT_PIN };
};

const { decryptedVault, error } = helper.validateEncryptionKey(vault, JSON.stringify(encryptionKey));

if (error) {
Expand Down Expand Up @@ -439,6 +459,12 @@ class Keyring {
return { error: ERROR_MESSAGE.INCORRECT_PIN };
};

const { error } = helper.validateEncryptionKey(this.vault, JSON.stringify(encryptionKey));

if (error) {
return { error }
}

let chain = (Chains.evmChains.hasOwnProperty(this.chain) || this.chain === 'ethereum') ? 'eth' : this.chain;

const importedChain = (chain === 'eth') ? 'evmChains' : chain;
Expand Down Expand Up @@ -481,16 +507,16 @@ class Keyring {

const response = await this.validatePin(pin);

if(response.response == false || response.error) {
return { error: ERROR_MESSAGE.INCORRECT_PIN };
};

const { error } = helper.validateEncryptionKey(this.vault, JSON.stringify(encryptionKey));

if (error) {
return { error }
}

if(response.response == false || response.error) {
return { error: ERROR_MESSAGE.INCORRECT_PIN };
};

if (privateKey.startsWith('0x')) {
privateKey = privateKey.slice(2)
}
Expand All @@ -502,7 +528,7 @@ class Keyring {
let isDuplicateAddress;
let numOfAcc;

accounts = await this.getAccounts(encryptionKey);
accounts = await this.getAccounts();

if (accounts.error) {
numOfAcc = 0;
Expand Down Expand Up @@ -627,6 +653,13 @@ class Keyring {
}

async getVaultDetails(encryptionKey) {

const { error } = helper.validateEncryptionKey(this.vault, JSON.stringify(encryptionKey));

if (error) {
return { error }
}

const decryptedVault = this.decryptedVault;

let accounts = { evm: { } };
Expand Down Expand Up @@ -736,6 +769,12 @@ class Keyring {

async updateLabel(address, encryptionKey, newLabel, chainName) {

const { error } = helper.validateEncryptionKey(this.vault, JSON.stringify(encryptionKey));

if (error) {
return { error }
}

if (newLabel === null || newLabel === undefined) {
return { error: ERROR_MESSAGE.INCORRECT_LABEL_TYPE };
}
Expand Down Expand Up @@ -785,6 +824,7 @@ class Keyring {
}

async changePin(currentPin, newPin, encryptionKey) {

if (!Number.isInteger(currentPin) || currentPin < 0) {
throw ERROR_MESSAGE.INCORRECT_PIN_TYPE
}
Expand All @@ -793,6 +833,18 @@ class Keyring {
throw ERROR_MESSAGE.INCORRECT_PIN_TYPE
}

const response = await this.validatePin(currentPin);

if (response.response == false || response.error) {
return { error: ERROR_MESSAGE.INCORRECT_PIN };
};

const err = helper.validateEncryptionKey(this.vault, JSON.stringify(encryptionKey));

if (err.error) {
return { error : err.error }
}

const { error, response: mnemonic }= await this.exportMnemonic(currentPin);

if (error) {
Expand Down
73 changes: 42 additions & 31 deletions src/lib/test/keyring.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ beforeAll(async() => {

result = await vault.generateVault(bufView,pin,phrase)
vaultAddress=result.response
await vault.getAccounts(bufView);
await vault.getAccounts();

});
describe('exportMnemonic' , ()=>{
Expand Down Expand Up @@ -115,14 +115,14 @@ describe('addAccount' , ()=>{

})

test('addAccount/empty encryption key' , async()=>{
test('addAccount/empty encryption key' , async()=>{
try{
let result = await vault.addAccount(null,pin)
}
catch(e){
expect(e.message).toBe("Cannot read properties of undefined (reading 'length')")
expect(e.message).toBe("Incorrect Encryption Key or vault string")
}

})

test('addAccount/empty pin' , async()=>{
Expand Down Expand Up @@ -892,6 +892,18 @@ describe('changePin',()=>{

})

test('changePin/wrong currentpin' , async()=>{
try{
let result = await vault.changePin(111111,pin,bufView)

}
catch(e){
expect(e).toBe('Wrong pin type, format or length')
}


})

test('changePin/invalid currentpin' , async()=>{
try{
let result = await vault.changePin('aefe',pin,bufView)
Expand Down Expand Up @@ -942,19 +954,23 @@ describe('changePin',()=>{
})

test('changePin/empty encryption key' , async()=>{

let result = await vault.changePin(pin,pin,null)
expect(result).toHaveProperty('response')



try{
let result = await vault.changePin(pin,pin,null)
}
catch(e){
expect(e.message).toBe("Incorrect Encryption Key or vault string")
}


})

test('changePin/invalid encryption key' , async()=>{

try{
let result = await vault.changePin(pin,pin,'efefe')
expect(result).toHaveProperty('response')
}
catch(e){
expect(e.message).toBe("Incorrect Encryption Key or vault string")
}



Expand Down Expand Up @@ -996,17 +1012,23 @@ describe('deleteAccount',()=>{

})
test('deleteAccount/empty encryption key' , async()=>{

try{
let result = await vault.deleteAccount(null,accAddress,pin)
expect(result.error).toBe('This address is not present in the vault')
}
catch(e){
expect(e.message).toBe("Incorrect Encryption Key or vault string")
}


})

test('deleteAccount/invalid encryption key' , async()=>{

try{
let result = await vault.deleteAccount(null,accAddress,pin)
expect(result.error).toBe('This address is not present in the vault')
}
catch(e){
expect(e.message).toBe("Incorrect Encryption Key or vault string")
}


})
Expand Down Expand Up @@ -1066,22 +1088,11 @@ describe('deleteAccount',()=>{
describe('getAccounts',()=>{
test('getAccounts/valid' , async()=>{
await vault.restoreKeyringState(vaultAddress,pin,bufView)
let result = await vault.getAccounts(bufView)
let result = await vault.getAccounts()
expect(result).toHaveProperty('response')

})
test('getAccounts/empty encryption key ' , async()=>{

let result = await vault.getAccounts(null)
expect(result.error).toBe('Incorrect Encryption Key or vault string')

})
test('getAccounts/invalid encryption key ' , async()=>{

let result = await vault.getAccounts("aefefe")
expect(result.error).toBe('Incorrect Encryption Key or vault string')

})

})


Expand Down Expand Up @@ -1142,7 +1153,7 @@ describe('signTransaction',()=>{

}
catch(e){
expect(e.message).toBe("No keyring found for the requested account.")
expect(e.message).toBe("Cannot read properties of undefined (reading 'toLowerCase')")

}

Expand Down Expand Up @@ -1173,7 +1184,7 @@ describe('signTransaction',()=>{

}
catch(e){
expect(e.message).toBe("No keyring found for the requested account.")
expect(e.message).toBe("Cannot read properties of undefined (reading 'toLowerCase')")

}

Expand Down
Loading

1 comment on commit 3b35cfc

@github-actions
Copy link

Choose a reason for hiding this comment

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

Coverage Report (69%)
File% Stmts% Branch% Funcs% LinesUncovered Line #s
All files68607069.21 
chains100100100100 
   index.js100100100100 
config100100100100 
   index.js100100100100 
constants100100100100 
   index.js100100100100 
constants/responses100100100100 
   index.js100100100100 
lib64.8660.2866.6666.23 
   keyring.js62.0859.6263.6363.5445, 131, 137–149, 169, 177–185, 194–196, 239–337, 356, 364–365, 373–383, 394, 414–425, 441–444, 476–480, 491–500, 534, 557–613, 645, 676, 684–700, 726–728, 759–767, 790–794, 813, 851
   vault.js84.7467.858084.7419, 26, 41–44, 56, 65–68
utils78.435681.2579.16 
   helper.js78.435681.2579.1610–17, 51–52, 68, 80, 92, 105–107, 123–125, 158–162, 203–205

Please sign in to comment.