Repository: StellarCN/py-stellar-base

Issue:
Implement SEP-10 2.1.0 Support
## TL;DR

The SEP-10 protocol has been updated to relax the requirements of the single Manage Data operation included in SEP-10 challenge transactions, and **should be implemented by October 21st**. On this date the SDF will announce the SDK updates to Anchors and Wallets, urging them to update as soon as possible.

## Background 

To provide context, the [SEP-10 2.0 changes](https://github.com/stellar/stellar-protocol/pull/708) replaced SEP-10 challenges' Manage Data operation `anchorName` key with a `home_domain` key. However, we quickly realized there was confusion and misunderstanding about the value of the `home_domain` parameter introduced in most SDKs earlier. Specifically, some users of the SDK believe the `home_domain` value is the home domain used to request the SEP-10 challenge, while others believe it is the home domain that requires the JWT authentication token.

This is problematic because SDK's implemented equality checks on the `home_domain` value of the Manage Data operation, so mismatched domains would result in client-server (wallet-anchor) incompatibility.

## SEP-10 2.1.0 Changes

The following changes have been made to the SEP-10 protocol:

- Clients are no longer required to validate the `home_domain` value in a SEP-10 challenge's first Manage Data operation
- SEP-10 challenge transactions may now have additional Manage Data operations as long as the source account of the operation is the SEP-10 server's `SIGNING_KEY`
    - Note: additional Manage Data operations will be added in future changes, so any added Manage Data operations added by the issuer of the challenge will need to come *after* the Manage Data operations defined in this document
    
## References

[SEP-10 2.1.0 Changes](https://github.com/stellar/stellar-protocol/pull/740)
[Go SDK SEP-10 2.1.0 Support](https://github.com/stellar/go/pull/3108)
[JavaScript SDK SEP-10 2.1.0 Support](https://github.com/stellar/js-stellar-sdk/pull/580)

## Looking Forward 

Soon after SEP-10 2.1.0 is implmenented and announced, the SDF will ask SDK's to upgrade to SEP-10 3.0, but no action needs to be taken for this yet.


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/StellarCN/py-stellar-base/pull/380?src=pr&el=h1) Report
> Merging [#380](https://codecov.io/gh/StellarCN/py-stellar-base/pull/380?src=pr&el=desc) into [dev](https://codecov.io/gh/StellarCN/py-stellar-base/commit/9682f4420849842a1dc6eb46fb169fa28fefb1ef?el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/StellarCN/py-stellar-base/pull/380/graphs/tree.svg?width=650&height=150&src=pr&token=kHAKcotfqv)](https://codecov.io/gh/StellarCN/py-stellar-base/pull/380?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##              dev     #380      +/-   ##
==========================================
- Coverage   98.99%   98.98%   -0.01%     
==========================================
  Files         127      127              
  Lines        7578     7613      +35     
==========================================
+ Hits         7502     7536      +34     
- Misses         76       77       +1     
```

| Flag | Coverage Δ | |
|---|---|---|
| #unittests | `98.98% <100.00%> (-0.01%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/StellarCN/py-stellar-base/pull/380?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [stellar\_sdk/sep/stellar\_web\_authentication.py](https://codecov.io/gh/StellarCN/py-stellar-base/pull/380/diff?src=pr&el=tree#diff-c3RlbGxhcl9zZGsvc2VwL3N0ZWxsYXJfd2ViX2F1dGhlbnRpY2F0aW9uLnB5) | `91.89% <100.00%> (-0.53%)` | :arrow_down: |
| [tests/sep/test\_stellar\_web\_authentication.py](https://codecov.io/gh/StellarCN/py-stellar-base/pull/380/diff?src=pr&el=tree#diff-dGVzdHMvc2VwL3Rlc3Rfc3RlbGxhcl93ZWJfYXV0aGVudGljYXRpb24ucHk=) | `100.00% <100.00%> (ø)` | |


Patch:
diff --git a/stellar_sdk/sep/stellar_web_authentication.py b/stellar_sdk/sep/stellar_web_authentication.py
index 0d287b37..2ec477ac 100644
--- a/stellar_sdk/sep/stellar_web_authentication.py
+++ b/stellar_sdk/sep/stellar_web_authentication.py
@@ -49,7 +49,8 @@ def build_challenge_transaction(
     :param server_secret: secret key for server's stellar.toml `SIGNING_KEY`.
     :param client_account_id: The stellar account that the wallet wishes to authenticate with the server.
     :param domain_name: The `fully qualified domain name <https://en.wikipedia.org/wiki/Fully_qualified_domain_name>`_
-        of the service requiring authentication, for example: `example.com`.
+        of the service requiring authentication, for example: `example.com`. (The domain_name field is reserved for
+        future use and not used.)
     :param network_passphrase: The network to connect to for verifying and retrieving
         additional attributes from. (ex. 'Public Global Stellar Network ; September 2015')
     :param timeout: Challenge duration in seconds (default to 15 minutes).
@@ -96,7 +97,8 @@ def read_challenge_transaction(
     :param challenge_transaction: SEP0010 transaction challenge transaction in base64.
     :param server_account_id: public key for server's account.
     :param domain_name: The `fully qualified domain name <https://en.wikipedia.org/wiki/Fully_qualified_domain_name>`_
-        of the service requiring authentication, for example: `example.com`.
+        of the service requiring authentication, for example: `example.com`. (The domain_name field is reserved for
+        future use and not used.)
     :param network_passphrase: The network to connect to for verifying and retrieving
         additional attributes from. (ex. 'Public Global Stellar Network ; September 2015')
     :raises: :exc:`InvalidSep10ChallengeError <stellar_sdk.sep.exceptions.InvalidSep10ChallengeError>` - if the
@@ -157,9 +159,9 @@ def read_challenge_transaction(
         )
 
     # verify that transaction contains a single Manage Data operation and its source account is not null
-    if len(transaction.operations) != 1:
+    if len(transaction.operations) < 1:
         raise InvalidSep10ChallengeError(
-            "Transaction requires a single ManageData operation."
+            "Transaction should contain at least one operation."
         )
 
     manage_data_op = transaction.operations[0]
@@ -170,12 +172,6 @@ def read_challenge_transaction(
     if not client_account:
         raise InvalidSep10ChallengeError("Operation should have a source account.")
 
-    if manage_data_op.data_name != f"{domain_name} auth":
-        raise InvalidSep10ChallengeError(
-            "The transaction's operation key name does not "
-            "include the expected home domain."
-        )
-
     if len(manage_data_op.data_value) != 64:
         raise InvalidSep10ChallengeError(
             "Operation value encoded as base64 should be 64 bytes long."
@@ -187,6 +183,17 @@ def read_challenge_transaction(
             "Operation value before encoding as base64 should be 48 bytes long."
         )
 
+    # verify any subsequent operations are manage data ops and source account is the server
+    for op in transaction.operations[1:]:
+        if not isinstance(op, ManageData):
+            raise InvalidSep10ChallengeError("Operation type should be ManageData.")
+        if op.source is None:
+            raise InvalidSep10ChallengeError("Operation should have a source account.")
+        if op.source != server_account_id:
+            raise InvalidSep10ChallengeError(
+                "The transaction has operations that are unrecognized."
+            )
+
     # verify that transaction envelope has a correct signature by server's signing key
     if not _verify_te_signed_by_account_id(transaction_envelope, server_account_id):
         raise InvalidSep10ChallengeError(
@@ -215,7 +222,8 @@ def verify_challenge_transaction_signers(
     :param challenge_transaction: SEP0010 transaction challenge transaction in base64.
     :param server_account_id: public key for server's account.
     :param domain_name: The `fully qualified domain name <https://en.wikipedia.org/wiki/Fully_qualified_domain_name>`_
-        of the service requiring authentication, for example: `example.com`.
+        of the service requiring authentication, for example: `example.com`. (The domain_name field is reserved for
+        future use and not used.)
     :param network_passphrase: The network to connect to for verifying and retrieving
         additional attributes from. (ex. 'Public Global Stellar Network ; September 2015')
     :param signers: The signers of client account.
@@ -291,7 +299,8 @@ def verify_challenge_transaction_signed_by_client(
     :param challenge_transaction: SEP0010 transaction challenge transaction in base64.
     :param server_account_id: public key for server's account.
     :param domain_name: The `fully qualified domain name <https://en.wikipedia.org/wiki/Fully_qualified_domain_name>`_
-        of the service requiring authentication, for example: `example.com`.
+        of the service requiring authentication, for example: `example.com`. (The domain_name field is reserved for
+        future use and not used.)
     :param network_passphrase: The network to connect to for verifying and retrieving
         additional attributes from. (ex. 'Public Global Stellar Network ; September 2015')
 
@@ -320,7 +329,8 @@ def verify_challenge_transaction_signed_by_client_master_key(
     :param challenge_transaction: SEP0010 transaction challenge transaction in base64.
     :param server_account_id: public key for server's account.
     :param domain_name: The `fully qualified domain name <https://en.wikipedia.org/wiki/Fully_qualified_domain_name>`_
-        of the service requiring authentication, for example: `example.com`.
+        of the service requiring authentication, for example: `example.com`. (The domain_name field is reserved for
+        future use and not used.)
     :param network_passphrase: The network to connect to for verifying and retrieving
         additional attributes from. (ex. 'Public Global Stellar Network ; September 2015')
 
@@ -351,7 +361,8 @@ def verify_challenge_transaction_threshold(
     :param challenge_transaction: SEP0010 transaction challenge transaction in base64.
     :param server_account_id: public key for server's account.
     :param domain_name: The `fully qualified domain name <https://en.wikipedia.org/wiki/Fully_qualified_domain_name>`_
-        of the service requiring authentication, for example: `example.com`.
+        of the service requiring authentication, for example: `example.com`. (The domain_name field is reserved for
+        future use and not used.)
     :param network_passphrase: The network to connect to for verifying and retrieving
         additional attributes from. (ex. 'Public Global Stellar Network ; September 2015')
     :param threshold: The medThreshold on the client account.
@@ -401,7 +412,8 @@ def verify_challenge_transaction(
     :param challenge_transaction: SEP0010 transaction challenge transaction in base64.
     :param server_account_id: public key for server's account.
     :param domain_name: The `fully qualified domain name <https://en.wikipedia.org/wiki/Fully_qualified_domain_name>`_
-        of the service requiring authentication, for example: `example.com`.
+        of the service requiring authentication, for example: `example.com`. (The domain_name field is reserved for
+        future use and not used.)
     :param network_passphrase: The network to connect to for verifying and retrieving
         additional attributes from. (ex. 'Public Global Stellar Network ; September 2015')
     :raises: :exc:`InvalidSep10ChallengeError <stellar_sdk.sep.exceptions.InvalidSep10ChallengeError>` - if the


Test patch:
diff --git a/tests/sep/test_stellar_web_authentication.py b/tests/sep/test_stellar_web_authentication.py
index 2f0efe18..d16d261a 100644
--- a/tests/sep/test_stellar_web_authentication.py
+++ b/tests/sep/test_stellar_web_authentication.py
@@ -140,34 +140,6 @@ class TestStellarWebAuthentication:
                 network_passphrase,
             )
 
-    def test_verify_challenge_tx_donot_contain_any_operation(self):
-        server_kp = Keypair.random()
-        client_kp = Keypair.random()
-        network_passphrase = Network.PUBLIC_NETWORK_PASSPHRASE
-        domain_name = "example.com"
-        now = int(time.time())
-        server_account = Account(server_kp.public_key, -1)
-        challenge_te = (
-            TransactionBuilder(server_account, network_passphrase, 100)
-            .add_time_bounds(now, now + 900)
-            .build()
-        )
-
-        challenge_te.sign(server_kp)
-        challenge_te.sign(client_kp)
-        challenge_tx_signed = challenge_te.to_xdr()
-
-        with pytest.raises(
-            InvalidSep10ChallengeError,
-            match="Transaction requires a single ManageData operation.",
-        ):
-            verify_challenge_transaction(
-                challenge_tx_signed,
-                server_kp.public_key,
-                domain_name,
-                network_passphrase,
-            )
-
     def test_verify_challenge_tx_donot_contain_managedata_operation(self):
         server_kp = Keypair.random()
         client_kp = Keypair.random()
@@ -422,34 +394,211 @@ class TestStellarWebAuthentication:
                 network_passphrase,
             )
 
-    def test_verify_challenge_transaction_domain_name_mismatch_raise(self):
+    def test_verify_challenge_tx_allows_any_value_in_home_domain_field(self):
         server_kp = Keypair.random()
         client_kp = Keypair.random()
-        timeout = 600
         network_passphrase = Network.PUBLIC_NETWORK_PASSPHRASE
         domain_name = "example.com"
-        invalid_domain_name = "invalid_example.com"
+        now = int(time.time())
+        nonce = os.urandom(48)
+        nonce_encoded = base64.b64encode(nonce)
+        server_account = Account(server_kp.public_key, -1)
+        challenge_te = (
+            TransactionBuilder(server_account, network_passphrase, 100)
+            .append_manage_data_op(
+                data_name="{} auth".format(domain_name),
+                data_value=nonce_encoded,
+                source=client_kp.public_key,
+            )
+            .add_time_bounds(now, now + 900)
+            .build()
+        )
 
-        challenge = build_challenge_transaction(
-            server_secret=server_kp.secret,
-            client_account_id=client_kp.public_key,
-            domain_name=domain_name,
-            network_passphrase=network_passphrase,
-            timeout=timeout,
+        challenge_te.sign(server_kp)
+        challenge_te.sign(client_kp)
+        challenge_tx_signed = challenge_te.to_xdr()
+
+        verify_challenge_transaction(
+            challenge_tx_signed, server_kp.public_key, None, network_passphrase,
         )
 
-        transaction = TransactionEnvelope.from_xdr(challenge, network_passphrase)
-        transaction.sign(client_kp)
-        challenge_tx = transaction.to_xdr()
+    def test_verify_challenge_tx_contain_subsequent_manage_data_ops_with_server_account_as_source_account(
+        self,
+    ):
+        server_kp = Keypair.random()
+        client_kp = Keypair.random()
+        network_passphrase = Network.PUBLIC_NETWORK_PASSPHRASE
+        domain_name = "example.com"
+        now = int(time.time())
+        nonce = os.urandom(48)
+        nonce_encoded = base64.b64encode(nonce)
+        server_account = Account(server_kp.public_key, -1)
+        challenge_te = (
+            TransactionBuilder(server_account, network_passphrase, 100)
+            .append_manage_data_op(
+                data_name="{} auth".format(domain_name),
+                data_value=nonce_encoded,
+                source=client_kp.public_key,
+            )
+            .append_manage_data_op(
+                data_name="data key",
+                data_value="data value",
+                source=server_kp.public_key,
+            )
+            .add_time_bounds(now, now + 900)
+            .build()
+        )
+
+        challenge_te.sign(server_kp)
+        challenge_te.sign(client_kp)
+        challenge_tx_signed = challenge_te.to_xdr()
+
+        verify_challenge_transaction(
+            challenge_tx_signed, server_kp.public_key, domain_name, network_passphrase,
+        )
+
+    def test_verify_challenge_tx_contain_subsequent_manage_data_ops_without_the_server_account_as_the_source_account(
+        self,
+    ):
+        server_kp = Keypair.random()
+        client_kp = Keypair.random()
+        network_passphrase = Network.PUBLIC_NETWORK_PASSPHRASE
+        domain_name = "example.com"
+        now = int(time.time())
+        nonce = os.urandom(48)
+        nonce_encoded = base64.b64encode(nonce)
+        server_account = Account(server_kp.public_key, -1)
+        challenge_te = (
+            TransactionBuilder(server_account, network_passphrase, 100)
+            .append_manage_data_op(
+                data_name="{} auth".format(domain_name),
+                data_value=nonce_encoded,
+                source=client_kp.public_key,
+            )
+            .append_manage_data_op(
+                data_name="data key",
+                data_value="data value",
+                source=client_kp.public_key,
+            )
+            .add_time_bounds(now, now + 900)
+            .build()
+        )
+
+        challenge_te.sign(server_kp)
+        challenge_te.sign(client_kp)
+        challenge_tx_signed = challenge_te.to_xdr()
+
         with pytest.raises(
             InvalidSep10ChallengeError,
-            match="The transaction's operation key name "
-            "does not include the expected home domain.",
+            match="The transaction has operations that are unrecognized.",
         ):
             verify_challenge_transaction(
-                challenge_tx,
+                challenge_tx_signed,
                 server_kp.public_key,
-                invalid_domain_name,
+                domain_name,
+                network_passphrase,
+            )
+
+    def test_verify_challenge_tx_contain_subsequent_ops_that_are_not_manage_data_ops(
+        self,
+    ):
+        server_kp = Keypair.random()
+        client_kp = Keypair.random()
+        network_passphrase = Network.PUBLIC_NETWORK_PASSPHRASE
+        domain_name = "example.com"
+        now = int(time.time())
+        nonce = os.urandom(48)
+        nonce_encoded = base64.b64encode(nonce)
+        server_account = Account(server_kp.public_key, -1)
+        challenge_te = (
+            TransactionBuilder(server_account, network_passphrase, 100)
+            .append_manage_data_op(
+                data_name="{} auth".format(domain_name),
+                data_value=nonce_encoded,
+                source=client_kp.public_key,
+            )
+            .append_bump_sequence_op(bump_to=0, source=server_kp.public_key,)
+            .add_time_bounds(now, now + 900)
+            .build()
+        )
+
+        challenge_te.sign(server_kp)
+        challenge_te.sign(client_kp)
+        challenge_tx_signed = challenge_te.to_xdr()
+
+        with pytest.raises(
+            InvalidSep10ChallengeError, match="Operation type should be ManageData.",
+        ):
+            verify_challenge_transaction(
+                challenge_tx_signed,
+                server_kp.public_key,
+                domain_name,
+                network_passphrase,
+            )
+
+    def test_verify_challenge_tx_contain_subsequent_ops_that_secend_op_no_source_account(
+        self,
+    ):
+        server_kp = Keypair.random()
+        client_kp = Keypair.random()
+        network_passphrase = Network.PUBLIC_NETWORK_PASSPHRASE
+        domain_name = "example.com"
+        now = int(time.time())
+        nonce = os.urandom(48)
+        nonce_encoded = base64.b64encode(nonce)
+        server_account = Account(server_kp.public_key, -1)
+        challenge_te = (
+            TransactionBuilder(server_account, network_passphrase, 100)
+            .append_manage_data_op(
+                data_name="{} auth".format(domain_name),
+                data_value=nonce_encoded,
+                source=client_kp.public_key,
+            )
+            .append_manage_data_op(data_name="Hello", data_value="world")
+            .add_time_bounds(now, now + 900)
+            .build()
+        )
+
+        challenge_te.sign(server_kp)
+        challenge_te.sign(client_kp)
+        challenge_tx_signed = challenge_te.to_xdr()
+
+        with pytest.raises(
+            InvalidSep10ChallengeError, match="Operation should have a source account.",
+        ):
+            verify_challenge_transaction(
+                challenge_tx_signed,
+                server_kp.public_key,
+                domain_name,
+                network_passphrase,
+            )
+
+    def test_verify_challenge_tx_contain_zero_op(self,):
+        server_kp = Keypair.random()
+        client_kp = Keypair.random()
+        network_passphrase = Network.PUBLIC_NETWORK_PASSPHRASE
+        domain_name = "example.com"
+        now = int(time.time())
+        nonce = os.urandom(48)
+        server_account = Account(server_kp.public_key, -1)
+        challenge_te = (
+            TransactionBuilder(server_account, network_passphrase, 100)
+            .add_time_bounds(now, now + 900)
+            .build()
+        )
+
+        challenge_te.sign(server_kp)
+        challenge_te.sign(client_kp)
+        challenge_tx_signed = challenge_te.to_xdr()
+
+        with pytest.raises(
+            InvalidSep10ChallengeError,
+            match="Transaction should contain at least one operation.",
+        ):
+            verify_challenge_transaction(
+                challenge_tx_signed,
+                server_kp.public_key,
+                domain_name,
                 network_passphrase,
             )