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

(XS) Login.checkKeys not supporting faulty faucet created accounts #45

Closed
5 of 6 tasks
mseaward opened this issue Jan 27, 2020 · 1 comment
Closed
5 of 6 tasks
Assignees
Labels
bug Something isn't working

Comments

@mseaward
Copy link
Contributor

mseaward commented Jan 27, 2020

Overview

Some older accounts were registered on a faucet that had a bug associated with key allocation. The accounts that were created through this faucet in the time frame that the bug existed has their owner and active keys swapped. This causes login attempts to fail on any dapp that does not have support for this issue.
The peerplays-core-gui (Peerplays wallet) has some code within its login functions to handle this issue. Unfortunately, this fix has not been implemented into the peerplaysjs-lib.

Details

Within the Login.checkKeys function, additional code may be required. If a checkKeys attempt fails, the owner key is to be checked against the active key instead (swap) keys provided to the function are checked recursively against the active key on chain. Under ideal usage, a user will provide the Login class the correct array of roles (['owner', 'active', 'memo']) and the checkKeys function will iterate over all keys provided. This way, each key passed in will be checked against the active public key that exists on the Peerplays blockchain.

There is also a dependency of the prefix. For simple dapps, one cannot reliably use the peerplaysjs-lib without instantiating a websocket connection and using that for all blockchain interactions. There needs to be support added to the checkKeys function to pass in a custom prefix so that users are not forced to instantiate the ws chainstore and set the prefix through it.

image

Impact

There will be a bit more time required to re-process the key check as a result of this required fix but it is a low complexity solution and the algorithms in use are also low complexity so there ends up being no human noticeable increase in login time.

  • millisecond(s), or less, speed decrease
  • tests related to changes required must pass (change as needed)

Estimate

3-4 development hours (xxs)

  • requires cleanup of prior release (pre-release -> release) and then syncing with this branch
  • cut new version/release after this branch has merged (pre-release)

PBSA / Developer tasks

  • Evaluate / Prioritize Bug Report
  • Refine User Stories / Requirements
  • Define Test Cases
  • Design / Develop Solution
  • Perform QA/Testing
  • Update Documentation
@mseaward mseaward added the bug Something isn't working label Jan 27, 2020
@mseaward mseaward self-assigned this Jan 27, 2020
@mseaward mseaward changed the title Login.checkKeys not supported faulty faucet created accounts Login.checkKeys not supporting faulty faucet created accounts Jan 27, 2020
mseaward added a commit that referenced this issue Jan 28, 2020
Provide support for prefix assignment in checkKeys without requiring to instantiate the entire
chainStore and websocket connection. Document all function within the AccountLogin class.

#45
mseaward added a commit that referenced this issue Jan 28, 2020
resolve issue regarding deprecation in the test suite such that tests can be run. Repair issues with
tests related to AccountLogin. Replace instances of 'GPH' with 'PPY' where code references a public
key with a 'GPH' prefix (non-Peerplays indicator).

#46, #45
@belakon1975 belakon1975 changed the title Login.checkKeys not supporting faulty faucet created accounts SC-7 Login.checkKeys not supporting faulty faucet created accounts Jan 29, 2020
@belakon1975
Copy link

@mseaward please estimate! xxs=0.5 days, xs=1 day , s=2 days , m=3 days, L=5 days, XL=8 days etc.

mseaward added a commit that referenced this issue Jan 29, 2020
Provide support for prefix assignment in checkKeys without requiring to instantiate the entire
chainStore and websocket connection. Document all function within the AccountLogin class.

#45
mseaward added a commit that referenced this issue Jan 29, 2020
resolve issue regarding deprecation in the test suite such that tests can be run. Repair issues with
tests related to AccountLogin. Replace instances of 'GPH' with 'PPY' where code references a public
key with a 'GPH' prefix (non-Peerplays indicator).

#46, #45
mseaward added a commit that referenced this issue Jan 29, 2020
* fix: remove vesting_balance_type from vesting_balance_withdraw_operation

* chore: rebuild

* chore(release): 0.6.5

* sync with master (#31)

* Pbsa develop (#28)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* PJL-24: security patches (#26)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* ci(packages): update packages to latest where possible

two audit reports cannot be resolved as we cannot update a packages dependencies ourself. They are
dev only issues so production is safe from these warnings

PJL-24-security-patches

* build: rebuild after npm package upgrades

* chore: rebuild after pbsa-develop merge sync and conflict resolution

* Patch npmignore file (#29)

* chore(release): 0.6.2

* chore(release): 0.6.3

* chore(.npmignore): patch npmignore file

ignore irrelevant files when publishing to NPM

* Update readme with sonar scan badges (#30)

* chore(release): 0.6.2

* chore(release): 0.6.3

* docs(readne): update readme to show status reports of repo

status reports are automatically synced/updated with sonarcloud

* Fix merge develop merge conflicts (#42)

* Pbsa develop (#28)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* PJL-24: security patches (#26)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* ci(packages): update packages to latest where possible

two audit reports cannot be resolved as we cannot update a packages dependencies ourself. They are
dev only issues so production is safe from these warnings

PJL-24-security-patches

* build: rebuild after npm package upgrades

* chore: rebuild after pbsa-develop merge sync and conflict resolution

* Patch npmignore file (#29)

* chore(release): 0.6.2

* chore(release): 0.6.3

* chore(.npmignore): patch npmignore file

ignore irrelevant files when publishing to NPM

* Update readme with sonar scan badges (#30)

* chore(release): 0.6.2

* chore(release): 0.6.3

* docs(readne): update readme to show status reports of repo

status reports are automatically synced/updated with sonarcloud

* sync with master (#31) (#32)

* Pbsa develop (#28)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* PJL-24: security patches (#26)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* ci(packages): update packages to latest where possible

two audit reports cannot be resolved as we cannot update a packages dependencies ourself. They are
dev only issues so production is safe from these warnings

PJL-24-security-patches

* build: rebuild after npm package upgrades

* chore: rebuild after pbsa-develop merge sync and conflict resolution

* Patch npmignore file (#29)

* chore(release): 0.6.2

* chore(release): 0.6.3

* chore(.npmignore): patch npmignore file

ignore irrelevant files when publishing to NPM

* Update readme with sonar scan badges (#30)

* chore(release): 0.6.2

* chore(release): 0.6.3

* docs(readne): update readme to show status reports of repo

status reports are automatically synced/updated with sonarcloud

* 0.6.4 release (#36)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* Sonar (#22)

* ci: sonarlint

* feat: add param to vesting_balance_create

vesting_balance_type new requirement due to gpos additions on mainnet

WAL-250

* build(packages): several packages updated

some potential breaking changes, test impact on updates

* docs: readme update indicating patch change reason

PJL-23

* build: rebuild

merged tag overlapping which adds sonarcloud files (no source code impact) and added new tag bump

PJL-23

* chore: rebuild and merge security PR in

* fix: use string instead of bool for balance type in vesting create

* build: rebuild

* improvement(vesting_balance_create): use enumerator for balance_type

vesting_balance_create balance_type parameter using enumerator now for serializing the transaction
to match the blockchain type for balance_type

* improvement(vesting_balance_create): use enumerator for balance_type

vesting_balance_create balance_type parameter using enumerator now for serializing the transaction
to match the blockchain type for balance_type

* improvement: vesting_balance_type enum

* improvement: re-add policy items

* improvement: re-add policy items

* fix: add export data for vesting balance type

* Revert "fix: add export data for vesting balance type"

This reverts commit 24138cc.

* fix: remove wrong export item

* fix: add vesting_policy_initializer to exports

* feat(account_update): added value.update_last_voting_time

added value.update_last_voting_time to account_update serializer extensions object

WAL-281

* feat: add balance_type to vesting balance withdraw

* chore: sync with master

some different versions existed, selected latest version from conflicts and reinstalled hence the
update to shrinkwrap file.

* chore: rebuild

* chore(release): 0.6.4

* fix: remove vesting_balance_type from vesting_balance_withdraw_operation (#37)

* Revert "fix: remove vesting_balance_type from vesting_balance_withdraw_operation (#37)" (#38)

This reverts commit 6d3293b.

Co-authored-by: Roshan Syed <[email protected]>
Co-authored-by: mseaward <[email protected]>
Co-authored-by: jotprabh1 <[email protected]>
Co-authored-by: Bobinson K B <[email protected]>

* Fix merge conflicts (#43)

* Pbsa develop (#28)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* PJL-24: security patches (#26)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* ci(packages): update packages to latest where possible

two audit reports cannot be resolved as we cannot update a packages dependencies ourself. They are
dev only issues so production is safe from these warnings

PJL-24-security-patches

* build: rebuild after npm package upgrades

* chore: rebuild after pbsa-develop merge sync and conflict resolution

* Patch npmignore file (#29)

* chore(release): 0.6.2

* chore(release): 0.6.3

* chore(.npmignore): patch npmignore file

ignore irrelevant files when publishing to NPM

* Update readme with sonar scan badges (#30)

* chore(release): 0.6.2

* chore(release): 0.6.3

* docs(readne): update readme to show status reports of repo

status reports are automatically synced/updated with sonarcloud

* sync with master (#31) (#32)

* Pbsa develop (#28)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* PJL-24: security patches (#26)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* ci(packages): update packages to latest where possible

two audit reports cannot be resolved as we cannot update a packages dependencies ourself. They are
dev only issues so production is safe from these warnings

PJL-24-security-patches

* build: rebuild after npm package upgrades

* chore: rebuild after pbsa-develop merge sync and conflict resolution

* Patch npmignore file (#29)

* chore(release): 0.6.2

* chore(release): 0.6.3

* chore(.npmignore): patch npmignore file

ignore irrelevant files when publishing to NPM

* Update readme with sonar scan badges (#30)

* chore(release): 0.6.2

* chore(release): 0.6.3

* docs(readne): update readme to show status reports of repo

status reports are automatically synced/updated with sonarcloud

* 0.6.4 release (#36)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* Sonar (#22)

* ci: sonarlint

* feat: add param to vesting_balance_create

vesting_balance_type new requirement due to gpos additions on mainnet

WAL-250

* build(packages): several packages updated

some potential breaking changes, test impact on updates

* docs: readme update indicating patch change reason

PJL-23

* build: rebuild

merged tag overlapping which adds sonarcloud files (no source code impact) and added new tag bump

PJL-23

* chore: rebuild and merge security PR in

* fix: use string instead of bool for balance type in vesting create

* build: rebuild

* improvement(vesting_balance_create): use enumerator for balance_type

vesting_balance_create balance_type parameter using enumerator now for serializing the transaction
to match the blockchain type for balance_type

* improvement(vesting_balance_create): use enumerator for balance_type

vesting_balance_create balance_type parameter using enumerator now for serializing the transaction
to match the blockchain type for balance_type

* improvement: vesting_balance_type enum

* improvement: re-add policy items

* improvement: re-add policy items

* fix: add export data for vesting balance type

* Revert "fix: add export data for vesting balance type"

This reverts commit 24138cc.

* fix: remove wrong export item

* fix: add vesting_policy_initializer to exports

* feat(account_update): added value.update_last_voting_time

added value.update_last_voting_time to account_update serializer extensions object

WAL-281

* feat: add balance_type to vesting balance withdraw

* chore: sync with master

some different versions existed, selected latest version from conflicts and reinstalled hence the
update to shrinkwrap file.

* chore: rebuild

* chore(release): 0.6.4

* fix: remove vesting_balance_type from vesting_balance_withdraw_operation (#37)

* Revert "fix: remove vesting_balance_type from vesting_balance_withdraw_operation (#37)" (#38)

This reverts commit 6d3293b.

* resolved merge conflict

Co-authored-by: Roshan Syed <[email protected]>
Co-authored-by: mseaward <[email protected]>
Co-authored-by: jotprabh1 <[email protected]>
Co-authored-by: Bobinson K B <[email protected]>

* improvement(accountlogin.js): improve the AccountLogin.js (Login) class

Provide support for prefix assignment in checkKeys without requiring to instantiate the entire
chainStore and websocket connection. Document all function within the AccountLogin class.

#45

* test: adjust scripts, packages, test files

resolve issue regarding deprecation in the test suite such that tests can be run. Repair issues with
tests related to AccountLogin. Replace instances of 'GPH' with 'PPY' where code references a public
key with a 'GPH' prefix (non-Peerplays indicator).

#46, #45

* build: rebuild files in lieu of recent changes

* refactor(accountlogin.js): resolve sonarcloud code smells

* sync with master (#31)

* Pbsa develop (#28)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* PJL-24: security patches (#26)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* ci(packages): update packages to latest where possible

two audit reports cannot be resolved as we cannot update a packages dependencies ourself. They are
dev only issues so production is safe from these warnings

PJL-24-security-patches

* build: rebuild after npm package upgrades

* chore: rebuild after pbsa-develop merge sync and conflict resolution

* Patch npmignore file (#29)

* chore(release): 0.6.2

* chore(release): 0.6.3

* chore(.npmignore): patch npmignore file

ignore irrelevant files when publishing to NPM

* Update readme with sonar scan badges (#30)

* chore(release): 0.6.2

* chore(release): 0.6.3

* docs(readne): update readme to show status reports of repo

status reports are automatically synced/updated with sonarcloud

* Fix merge develop merge conflicts (#42)

* Pbsa develop (#28)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* PJL-24: security patches (#26)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* ci(packages): update packages to latest where possible

two audit reports cannot be resolved as we cannot update a packages dependencies ourself. They are
dev only issues so production is safe from these warnings

PJL-24-security-patches

* build: rebuild after npm package upgrades

* chore: rebuild after pbsa-develop merge sync and conflict resolution

* Patch npmignore file (#29)

* chore(release): 0.6.2

* chore(release): 0.6.3

* chore(.npmignore): patch npmignore file

ignore irrelevant files when publishing to NPM

* Update readme with sonar scan badges (#30)

* chore(release): 0.6.2

* chore(release): 0.6.3

* docs(readne): update readme to show status reports of repo

status reports are automatically synced/updated with sonarcloud

* sync with master (#31) (#32)

* Pbsa develop (#28)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* PJL-24: security patches (#26)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* ci(packages): update packages to latest where possible

two audit reports cannot be resolved as we cannot update a packages dependencies ourself. They are
dev only issues so production is safe from these warnings

PJL-24-security-patches

* build: rebuild after npm package upgrades

* chore: rebuild after pbsa-develop merge sync and conflict resolution

* Patch npmignore file (#29)

* chore(release): 0.6.2

* chore(release): 0.6.3

* chore(.npmignore): patch npmignore file

ignore irrelevant files when publishing to NPM

* Update readme with sonar scan badges (#30)

* chore(release): 0.6.2

* chore(release): 0.6.3

* docs(readne): update readme to show status reports of repo

status reports are automatically synced/updated with sonarcloud

* 0.6.4 release (#36)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* Sonar (#22)

* ci: sonarlint

* feat: add param to vesting_balance_create

vesting_balance_type new requirement due to gpos additions on mainnet

WAL-250

* build(packages): several packages updated

some potential breaking changes, test impact on updates

* docs: readme update indicating patch change reason

PJL-23

* build: rebuild

merged tag overlapping which adds sonarcloud files (no source code impact) and added new tag bump

PJL-23

* chore: rebuild and merge security PR in

* fix: use string instead of bool for balance type in vesting create

* build: rebuild

* improvement(vesting_balance_create): use enumerator for balance_type

vesting_balance_create balance_type parameter using enumerator now for serializing the transaction
to match the blockchain type for balance_type

* improvement(vesting_balance_create): use enumerator for balance_type

vesting_balance_create balance_type parameter using enumerator now for serializing the transaction
to match the blockchain type for balance_type

* improvement: vesting_balance_type enum

* improvement: re-add policy items

* improvement: re-add policy items

* fix: add export data for vesting balance type

* Revert "fix: add export data for vesting balance type"

This reverts commit 24138cc.

* fix: remove wrong export item

* fix: add vesting_policy_initializer to exports

* feat(account_update): added value.update_last_voting_time

added value.update_last_voting_time to account_update serializer extensions object

WAL-281

* feat: add balance_type to vesting balance withdraw

* chore: sync with master

some different versions existed, selected latest version from conflicts and reinstalled hence the
update to shrinkwrap file.

* chore: rebuild

* chore(release): 0.6.4

* fix: remove vesting_balance_type from vesting_balance_withdraw_operation (#37)

* Revert "fix: remove vesting_balance_type from vesting_balance_withdraw_operation (#37)" (#38)

This reverts commit 6d3293b.

Co-authored-by: Roshan Syed <[email protected]>
Co-authored-by: mseaward <[email protected]>
Co-authored-by: jotprabh1 <[email protected]>
Co-authored-by: Bobinson K B <[email protected]>

* Fix merge conflicts (#43)

* Pbsa develop (#28)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* PJL-24: security patches (#26)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* ci(packages): update packages to latest where possible

two audit reports cannot be resolved as we cannot update a packages dependencies ourself. They are
dev only issues so production is safe from these warnings

PJL-24-security-patches

* build: rebuild after npm package upgrades

* chore: rebuild after pbsa-develop merge sync and conflict resolution

* Patch npmignore file (#29)

* chore(release): 0.6.2

* chore(release): 0.6.3

* chore(.npmignore): patch npmignore file

ignore irrelevant files when publishing to NPM

* Update readme with sonar scan badges (#30)

* chore(release): 0.6.2

* chore(release): 0.6.3

* docs(readne): update readme to show status reports of repo

status reports are automatically synced/updated with sonarcloud

* sync with master (#31) (#32)

* Pbsa develop (#28)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* PJL-24: security patches (#26)

* Sonar (#22)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* ci(packages): update packages to latest where possible

two audit reports cannot be resolved as we cannot update a packages dependencies ourself. They are
dev only issues so production is safe from these warnings

PJL-24-security-patches

* build: rebuild after npm package upgrades

* chore: rebuild after pbsa-develop merge sync and conflict resolution

* Patch npmignore file (#29)

* chore(release): 0.6.2

* chore(release): 0.6.3

* chore(.npmignore): patch npmignore file

ignore irrelevant files when publishing to NPM

* Update readme with sonar scan badges (#30)

* chore(release): 0.6.2

* chore(release): 0.6.3

* docs(readne): update readme to show status reports of repo

status reports are automatically synced/updated with sonarcloud

* 0.6.4 release (#36)

* ci: sonarlint

WAL-223

* chore: rebuild

* chore(release): 0.6.1

* Sonar (#22)

* ci: sonarlint

* feat: add param to vesting_balance_create

vesting_balance_type new requirement due to gpos additions on mainnet

WAL-250

* build(packages): several packages updated

some potential breaking changes, test impact on updates

* docs: readme update indicating patch change reason

PJL-23

* build: rebuild

merged tag overlapping which adds sonarcloud files (no source code impact) and added new tag bump

PJL-23

* chore: rebuild and merge security PR in

* fix: use string instead of bool for balance type in vesting create

* build: rebuild

* improvement(vesting_balance_create): use enumerator for balance_type

vesting_balance_create balance_type parameter using enumerator now for serializing the transaction
to match the blockchain type for balance_type

* improvement(vesting_balance_create): use enumerator for balance_type

vesting_balance_create balance_type parameter using enumerator now for serializing the transaction
to match the blockchain type for balance_type

* improvement: vesting_balance_type enum

* improvement: re-add policy items

* improvement: re-add policy items

* fix: add export data for vesting balance type

* Revert "fix: add export data for vesting balance type"

This reverts commit 24138cc.

* fix: remove wrong export item

* fix: add vesting_policy_initializer to exports

* feat(account_update): added value.update_last_voting_time

added value.update_last_voting_time to account_update serializer extensions object

WAL-281

* feat: add balance_type to vesting balance withdraw

* chore: sync with master

some different versions existed, selected latest version from conflicts and reinstalled hence the
update to shrinkwrap file.

* chore: rebuild

* chore(release): 0.6.4

* fix: remove vesting_balance_type from vesting_balance_withdraw_operation (#37)

* Revert "fix: remove vesting_balance_type from vesting_balance_withdraw_operation (#37)" (#38)

This reverts commit 6d3293b.

* resolved merge conflict

Co-authored-by: Roshan Syed <[email protected]>
Co-authored-by: mseaward <[email protected]>
Co-authored-by: jotprabh1 <[email protected]>
Co-authored-by: Bobinson K B <[email protected]>

* improvement(accountlogin.js): improve the AccountLogin.js (Login) class

Provide support for prefix assignment in checkKeys without requiring to instantiate the entire
chainStore and websocket connection. Document all function within the AccountLogin class.

#45

* test: adjust scripts, packages, test files

resolve issue regarding deprecation in the test suite such that tests can be run. Repair issues with
tests related to AccountLogin. Replace instances of 'GPH' with 'PPY' where code references a public
key with a 'GPH' prefix (non-Peerplays indicator).

#46, #45

* build: rebuild files in lieu of recent changes

* refactor(accountlogin.js): resolve sonarcloud code smells

* chore(npm): sync with pbsa-dev

new version was cut prior and changes needed on this branch

* build: rebuild after syncing with pbsa-develop

* build: rebuild after syncing with pbsa-develop

Co-authored-by: jotprabh1 <[email protected]>
Co-authored-by: Adrian Metzler <[email protected]>
Co-authored-by: Roshan Syed <[email protected]>
Co-authored-by: Bobinson K B <[email protected]>
@mseaward mseaward reopened this Feb 4, 2020
@mseaward mseaward closed this as completed Feb 4, 2020
@belakon1975 belakon1975 changed the title SC-7 Login.checkKeys not supporting faulty faucet created accounts (XS) SC-7 Login.checkKeys not supporting faulty faucet created accounts Mar 11, 2020
@belakon1975 belakon1975 changed the title (XS) SC-7 Login.checkKeys not supporting faulty faucet created accounts (XS) Login.checkKeys not supporting faulty faucet created accounts Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants