Skip to content
This repository has been archived by the owner on Dec 15, 2021. It is now read-only.

Add tests for SEP10 Signer Support #9

Merged
merged 7 commits into from
Jan 26, 2020
Merged

Add tests for SEP10 Signer Support #9

merged 7 commits into from
Jan 26, 2020

Conversation

msfeldstein
Copy link
Contributor

The new SEP10 addendums require token providers to check that the account signing for the JWT actually has threshold capabilities to sign for the account in question. This adds the tests:

  • An account with a 0 masterWeight can no longer get a token for itself
  • An account that can act as a signer for another account can request a token for that account
  • An account can't sign twice to double its signing weight
  • Multiple accounts that together reach a signing threshold can get the token

Copy link

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

🎉 I love the tests being included. You could also add tests for:

  1. The server is a signer on the account, and shouldn't be allowed to sign for the account to gain weight. The server shouldn't help a client authenticate.

  2. There's a signer on the transaction that is not recognized as belonging to the account or the server, it should decline in that case. There's no big vulnerability here, I just think it's good practice to keep that behavior predicable and that's how txnbuild: update challenge tx helpers for SEP-10 v1.3.0 stellar/go#2071 is implemented.

  3. The account has 5 signers and 2 signers sign the transaction which meet the threshold.

  4. The challenge transaction has no signatures.

  5. The challenge transaction has only the server signature.

  6. The challenge transaction has no signatures and the account also has no signers.

  7. The challenge transaction has only the server signature and the account also has no signers.

  8. The weights of the signers add to a number greater than 255.

  9. A non-existent account can be auth'd with its master key.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants