From 87f653b33b126a40e22df302d12a07ca8b977f93 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Tue, 24 Jan 2023 09:10:15 -0800 Subject: [PATCH 01/22] add memo to BuildChallengeTx params and ReadChallengeTx return value --- txnbuild/transaction.go | 64 +++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index bd3e784f55..1ba2831438 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -999,7 +999,7 @@ func NewFeeBumpTransaction(params FeeBumpTransactionParams) (*FeeBumpTransaction // BuildChallengeTx is a factory method that creates a valid SEP 10 challenge, for use in web authentication. // "timebound" is the time duration the transaction should be valid for, and must be greater than 1s (300s is recommended). // More details on SEP 10: https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0010.md -func BuildChallengeTx(serverSignerSecret, clientAccountID, webAuthDomain, homeDomain, network string, timebound time.Duration) (*Transaction, error) { +func BuildChallengeTx(serverSignerSecret, clientAccountID, webAuthDomain, homeDomain, network string, timebound time.Duration, memo Memo) (*Transaction, error) { if timebound < time.Second { return nil, errors.New("provided timebound must be at least 1s (300s is recommended)") } @@ -1021,7 +1021,15 @@ func BuildChallengeTx(serverSignerSecret, clientAccountID, webAuthDomain, homeDo } if _, err = xdr.AddressToAccountId(clientAccountID); err != nil { - return nil, errors.Wrapf(err, "%s is not a valid account id", clientAccountID) + if _, err = xdr.AddressToMuxedAccount(clientAccountID); err != nil { + return nil, errors.Wrapf(err, "%s is not a valid account id or muxed account", clientAccountID) + } else if memo != nil { + return nil, errors.New("memos are not valid for challenge transactions with a muxed client account") + } + } else if memo != nil { + if _, err := memo.ToXDR(); err != nil { + return nil, errors.New("memo must be of type MemoID") + } } // represent server signing account as SimpleAccount @@ -1052,7 +1060,7 @@ func BuildChallengeTx(serverSignerSecret, clientAccountID, webAuthDomain, homeDo }, }, BaseFee: MinBaseFee, - Memo: nil, + Memo: memo, Preconditions: Preconditions{ TimeBounds: NewTimebounds(currentTime.Unix(), maxTime.Unix()), }, @@ -1105,57 +1113,57 @@ func generateRandomNonce(n int) ([]byte, error) { // one of the following functions to completely verify the transaction: // - VerifyChallengeTxThreshold // - VerifyChallengeTxSigners -func ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain string, homeDomains []string) (tx *Transaction, clientAccountID string, matchedHomeDomain string, err error) { +func ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain string, homeDomains []string) (tx *Transaction, clientAccountID string, matchedHomeDomain string, memo MemoID, err error) { parsed, err := TransactionFromXDR(challengeTx) if err != nil { - return tx, clientAccountID, matchedHomeDomain, errors.Wrap(err, "could not parse challenge") + return tx, clientAccountID, matchedHomeDomain, memo, errors.Wrap(err, "could not parse challenge") } var isSimple bool tx, isSimple = parsed.Transaction() if !isSimple { - return tx, clientAccountID, matchedHomeDomain, errors.New("challenge cannot be a fee bump transaction") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("challenge cannot be a fee bump transaction") } // Enforce no muxed accounts (at least until we understand their impact) if tx.envelope.SourceAccount().Type == xdr.CryptoKeyTypeKeyTypeMuxedEd25519 { err = errors.New("invalid source account: only valid Ed25519 accounts are allowed in challenge transactions") - return tx, clientAccountID, matchedHomeDomain, err + return tx, clientAccountID, matchedHomeDomain, memo, err } // verify transaction source if tx.SourceAccount().AccountID != serverAccountID { - return tx, clientAccountID, matchedHomeDomain, errors.New("transaction source account is not equal to server's account") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("transaction source account is not equal to server's account") } // verify sequence number if tx.SourceAccount().Sequence != 0 { - return tx, clientAccountID, matchedHomeDomain, errors.New("transaction sequence number must be 0") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("transaction sequence number must be 0") } // verify timebounds if tx.Timebounds().MaxTime == TimeoutInfinite { - return tx, clientAccountID, matchedHomeDomain, errors.New("transaction requires non-infinite timebounds") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("transaction requires non-infinite timebounds") } // Apply a grace period to the challenge MinTime to account for clock drift between the server and client var gracePeriod int64 = 5 * 60 // seconds currentTime := time.Now().UTC().Unix() if currentTime+gracePeriod < tx.Timebounds().MinTime || currentTime > tx.Timebounds().MaxTime { - return tx, clientAccountID, matchedHomeDomain, errors.Errorf("transaction is not within range of the specified timebounds (currentTime=%d, MinTime=%d, MaxTime=%d)", + return tx, clientAccountID, matchedHomeDomain, memo, errors.Errorf("transaction is not within range of the specified timebounds (currentTime=%d, MinTime=%d, MaxTime=%d)", currentTime, tx.Timebounds().MinTime, tx.Timebounds().MaxTime) } // verify operation operations := tx.Operations() if len(operations) < 1 { - return tx, clientAccountID, matchedHomeDomain, errors.New("transaction requires at least one manage_data operation") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("transaction requires at least one manage_data operation") } op, ok := operations[0].(*ManageData) if !ok { - return tx, clientAccountID, matchedHomeDomain, errors.New("operation type should be manage_data") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("operation type should be manage_data") } if op.SourceAccount == "" { - return tx, clientAccountID, matchedHomeDomain, errors.New("operation should have a source account") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("operation should have a source account") } for _, homeDomain := range homeDomains { if op.Name == homeDomain+" auth" { @@ -1164,60 +1172,60 @@ func ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain string } } if matchedHomeDomain == "" { - return tx, clientAccountID, matchedHomeDomain, errors.Errorf("operation key does not match any homeDomains passed (key=%q, homeDomains=%v)", op.Name, homeDomains) + return tx, clientAccountID, matchedHomeDomain, memo, errors.Errorf("operation key does not match any homeDomains passed (key=%q, homeDomains=%v)", op.Name, homeDomains) } clientAccountID = op.SourceAccount rawOperations := tx.envelope.Operations() - if len(rawOperations) > 0 && rawOperations[0].SourceAccount.Type == xdr.CryptoKeyTypeKeyTypeMuxedEd25519 { - err = errors.New("invalid operation source account: only valid Ed25519 accounts are allowed in challenge transactions") - return tx, clientAccountID, matchedHomeDomain, err + if len(rawOperations) > 0 && rawOperations[0].SourceAccount.Type == xdr.CryptoKeyTypeKeyTypeMuxedEd25519 && tx.Memo() != nil { + err = errors.New("memos are not valid for challenge transactions with a muxed client account") + return tx, clientAccountID, matchedHomeDomain, memo, err } // verify manage data value nonceB64 := string(op.Value) if len(nonceB64) != 64 { - return tx, clientAccountID, matchedHomeDomain, errors.New("random nonce encoded as base64 should be 64 bytes long") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("random nonce encoded as base64 should be 64 bytes long") } nonceBytes, err := base64.StdEncoding.DecodeString(nonceB64) if err != nil { - return tx, clientAccountID, matchedHomeDomain, errors.Wrap(err, "failed to decode random nonce provided in manage_data operation") + return tx, clientAccountID, matchedHomeDomain, memo, errors.Wrap(err, "failed to decode random nonce provided in manage_data operation") } if len(nonceBytes) != 48 { - return tx, clientAccountID, matchedHomeDomain, errors.New("random nonce before encoding as base64 should be 48 bytes long") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("random nonce before encoding as base64 should be 48 bytes long") } // verify subsequent operations are manage data ops and known, or unknown with source account set to server account for _, op := range operations[1:] { op, ok := op.(*ManageData) if !ok { - return tx, clientAccountID, matchedHomeDomain, errors.New("operation type should be manage_data") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("operation type should be manage_data") } if op.SourceAccount == "" { - return tx, clientAccountID, matchedHomeDomain, errors.New("operation should have a source account") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("operation should have a source account") } switch op.Name { case "web_auth_domain": if op.SourceAccount != serverAccountID { - return tx, clientAccountID, matchedHomeDomain, errors.New("web auth domain operation must have server source account") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("web auth domain operation must have server source account") } if !bytes.Equal(op.Value, []byte(webAuthDomain)) { - return tx, clientAccountID, matchedHomeDomain, errors.Errorf("web auth domain operation value is %q but expect %q", string(op.Value), webAuthDomain) + return tx, clientAccountID, matchedHomeDomain, memo, errors.Errorf("web auth domain operation value is %q but expect %q", string(op.Value), webAuthDomain) } default: // verify unknown subsequent operations are manage data ops with source account set to server account if op.SourceAccount != serverAccountID { - return tx, clientAccountID, matchedHomeDomain, errors.New("subsequent operations are unrecognized") + return tx, clientAccountID, matchedHomeDomain, memo, errors.New("subsequent operations are unrecognized") } } } err = verifyTxSignature(tx, network, serverAccountID) if err != nil { - return tx, clientAccountID, matchedHomeDomain, err + return tx, clientAccountID, matchedHomeDomain, memo, err } - return tx, clientAccountID, matchedHomeDomain, nil + return tx, clientAccountID, matchedHomeDomain, memo, nil } // VerifyChallengeTxThreshold verifies that for a SEP 10 challenge transaction From 11d5a949b9362827f5ea305c3bbd6139b0898ea8 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Tue, 24 Jan 2023 09:17:35 -0800 Subject: [PATCH 02/22] require memos of type ID in BuildChallengeTx --- txnbuild/transaction.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index 1ba2831438..f7a874056f 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -1027,7 +1027,7 @@ func BuildChallengeTx(serverSignerSecret, clientAccountID, webAuthDomain, homeDo return nil, errors.New("memos are not valid for challenge transactions with a muxed client account") } } else if memo != nil { - if _, err := memo.ToXDR(); err != nil { + if xdrMemo, err := memo.ToXDR(); err != nil || xdrMemo.Type != xdr.MemoTypeMemoId { return nil, errors.New("memo must be of type MemoID") } } From 58bafb115cd924e20e13aa7a3c2e6abcc1bd91fc Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Tue, 24 Jan 2023 12:54:18 -0800 Subject: [PATCH 03/22] fix tests, update webauth server --- .../webauth/internal/serve/challenge.go | 12 +++ exp/services/webauth/internal/serve/token.go | 14 ++- .../webauth/internal/serve/token_test.go | 12 +++ txnbuild/example_test.go | 2 +- txnbuild/transaction.go | 12 ++- .../transaction_challenge_example_test.go | 6 +- txnbuild/transaction_test.go | 95 ++++++++++--------- 7 files changed, 96 insertions(+), 57 deletions(-) diff --git a/exp/services/webauth/internal/serve/challenge.go b/exp/services/webauth/internal/serve/challenge.go index a01458215d..20e1635c61 100644 --- a/exp/services/webauth/internal/serve/challenge.go +++ b/exp/services/webauth/internal/serve/challenge.go @@ -2,6 +2,7 @@ package serve import ( "net/http" + "strconv" "strings" "time" @@ -57,6 +58,16 @@ func (h challengeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { homeDomain = h.HomeDomains[0] } + var memo txnbuild.MemoID + if queryValues.Get("memo") != "" { + memoInt, err := strconv.ParseUint(queryValues.Get("memo"), 10, 64) + if err != nil { + badRequest.Render(w) + return + } + memo = txnbuild.MemoID(memoInt) + } + tx, err := txnbuild.BuildChallengeTx( h.SigningKey.Seed(), account, @@ -64,6 +75,7 @@ func (h challengeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { homeDomain, h.NetworkPassphrase, h.ChallengeExpiresIn, + memo, ) if err != nil { h.Logger.Ctx(ctx).WithStack(err).Error(err) diff --git a/exp/services/webauth/internal/serve/token.go b/exp/services/webauth/internal/serve/token.go index 56db5f269f..c69ce6586a 100644 --- a/exp/services/webauth/internal/serve/token.go +++ b/exp/services/webauth/internal/serve/token.go @@ -11,6 +11,7 @@ import ( supportlog "github.com/stellar/go/support/log" "github.com/stellar/go/support/render/httpjson" "github.com/stellar/go/txnbuild" + "github.com/stellar/go/xdr" "gopkg.in/square/go-jose.v2" "gopkg.in/square/go-jose.v2/jwt" ) @@ -52,9 +53,10 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { clientAccountID string signingAddress *keypair.FromAddress homeDomain string + memo txnbuild.Memo ) for _, s := range h.SigningAddresses { - tx, clientAccountID, homeDomain, err = txnbuild.ReadChallengeTx(req.Transaction, s.Address(), h.NetworkPassphrase, h.Domain, h.HomeDomains) + tx, clientAccountID, homeDomain, memo, err = txnbuild.ReadChallengeTx(req.Transaction, s.Address(), h.NetworkPassphrase, h.Domain, h.HomeDomains) if err == nil { signingAddress = s break @@ -80,10 +82,16 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { WithField("tx", hash). WithField("account", clientAccountID). WithField("serversigner", signingAddress.Address()). - WithField("homedomain", homeDomain) + WithField("homedomain", homeDomain). + WithField("memo", memo) l.Info("Start verifying challenge transaction.") + var muxedAccount xdr.MuxedAccount + if muxedAccount, err = xdr.AddressToMuxedAccount(clientAccountID); err == nil { + clientAccountID = muxedAccount.ToAccountId().Address() + } + var clientAccountExists bool clientAccount, err := h.HorizonClient.AccountDetail(horizonclient.AccountRequest{AccountID: clientAccountID}) switch { @@ -143,7 +151,7 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { issuedAt := time.Unix(tx.Timebounds().MinTime, 0) claims := jwt.Claims{ Issuer: h.JWTIssuer, - Subject: clientAccountID, + Subject: muxedAccount.Address(), IssuedAt: jwt.NewNumericDate(issuedAt), Expiry: jwt.NewNumericDate(issuedAt.Add(h.JWTExpiresIn)), } diff --git a/exp/services/webauth/internal/serve/token_test.go b/exp/services/webauth/internal/serve/token_test.go index 7aa41d4b78..1c2745fa5a 100644 --- a/exp/services/webauth/internal/serve/token_test.go +++ b/exp/services/webauth/internal/serve/token_test.go @@ -46,6 +46,7 @@ func TestToken_formInputSuccess(t *testing.T) { homeDomain, network.TestNetworkPassphrase, time.Minute, + nil, ) require.NoError(t, err) @@ -146,6 +147,7 @@ func TestToken_formInputSuccess_jwtHeaderAndPayloadAreDeterministic(t *testing.T homeDomain, network.TestNetworkPassphrase, time.Minute, + nil, ) require.NoError(t, err) @@ -255,6 +257,7 @@ func TestToken_jsonInputSuccess(t *testing.T) { homeDomain, network.TestNetworkPassphrase, time.Minute, + nil, ) require.NoError(t, err) @@ -412,6 +415,7 @@ func TestToken_jsonInputValidRotatingServerSigners(t *testing.T) { homeDomain, network.TestNetworkPassphrase, time.Minute, + nil, ) require.NoError(t, err) @@ -497,6 +501,7 @@ func TestToken_jsonInputValidMultipleSigners(t *testing.T) { homeDomain, network.TestNetworkPassphrase, time.Minute, + nil, ) require.NoError(t, err) @@ -605,6 +610,7 @@ func TestToken_jsonInputNotEnoughWeight(t *testing.T) { homeDomain, network.TestNetworkPassphrase, time.Minute, + nil, ) require.NoError(t, err) @@ -691,6 +697,7 @@ func TestToken_jsonInputUnrecognizedSigner(t *testing.T) { homeDomain, network.TestNetworkPassphrase, time.Minute, + nil, ) require.NoError(t, err) @@ -777,6 +784,7 @@ func TestToken_jsonInputAccountNotExistSuccess(t *testing.T) { homeDomain, network.TestNetworkPassphrase, time.Minute, + nil, ) require.NoError(t, err) @@ -881,6 +889,7 @@ func TestToken_jsonInputAccountNotExistFail(t *testing.T) { homeDomain, network.TestNetworkPassphrase, time.Minute, + nil, ) require.NoError(t, err) @@ -963,6 +972,7 @@ func TestToken_jsonInputAccountNotExistNotAllowed(t *testing.T) { homeDomain, network.TestNetworkPassphrase, time.Minute, + nil, ) require.NoError(t, err) @@ -1047,6 +1057,7 @@ func TestToken_jsonInputUnrecognizedServerSigner(t *testing.T) { homeDomain, network.TestNetworkPassphrase, time.Minute, + nil, ) require.NoError(t, err) @@ -1248,6 +1259,7 @@ func TestToken_jsonInputInvalidWebAuthDomainFail(t *testing.T) { homeDomain, network.TestNetworkPassphrase, time.Minute, + nil, ) require.NoError(t, err) diff --git a/txnbuild/example_test.go b/txnbuild/example_test.go index 3aab258ab9..26b5d95187 100644 --- a/txnbuild/example_test.go +++ b/txnbuild/example_test.go @@ -760,7 +760,7 @@ func ExampleBuildChallengeTx() { webAuthDomain := "webauthdomain.example.org" timebound := time.Duration(5 * time.Minute) - tx, err := BuildChallengeTx(serverSignerSeed, clientAccountID, webAuthDomain, anchorName, network.TestNetworkPassphrase, timebound) + tx, err := BuildChallengeTx(serverSignerSeed, clientAccountID, webAuthDomain, anchorName, network.TestNetworkPassphrase, timebound, nil) check(err) txeBase64, err := tx.Base64() diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index f7a874056f..af23b48831 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -1113,7 +1113,7 @@ func generateRandomNonce(n int) ([]byte, error) { // one of the following functions to completely verify the transaction: // - VerifyChallengeTxThreshold // - VerifyChallengeTxSigners -func ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain string, homeDomains []string) (tx *Transaction, clientAccountID string, matchedHomeDomain string, memo MemoID, err error) { +func ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain string, homeDomains []string) (tx *Transaction, clientAccountID string, matchedHomeDomain string, memo Memo, err error) { parsed, err := TransactionFromXDR(challengeTx) if err != nil { return tx, clientAccountID, matchedHomeDomain, memo, errors.Wrap(err, "could not parse challenge") @@ -1176,10 +1176,16 @@ func ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain string } clientAccountID = op.SourceAccount + memo = tx.Memo() rawOperations := tx.envelope.Operations() - if len(rawOperations) > 0 && rawOperations[0].SourceAccount.Type == xdr.CryptoKeyTypeKeyTypeMuxedEd25519 && tx.Memo() != nil { + if rawOperations[0].SourceAccount.Type == xdr.CryptoKeyTypeKeyTypeMuxedEd25519 && memo != nil { err = errors.New("memos are not valid for challenge transactions with a muxed client account") return tx, clientAccountID, matchedHomeDomain, memo, err + } else if rawOperations[0].SourceAccount.Type == xdr.CryptoKeyTypeKeyTypeEd25519 && memo != nil { + if rawMemo, err := memo.ToXDR(); err != nil || rawMemo.Type != xdr.MemoTypeMemoId { + err = errors.New("invalid memo, only ID memos are permitted") + return tx, clientAccountID, matchedHomeDomain, memo, err + } } // verify manage data value @@ -1297,7 +1303,7 @@ func VerifyChallengeTxThreshold(challengeTx, serverAccountID, network, webAuthDo // server account or one of the signers provided in the arguments. func VerifyChallengeTxSigners(challengeTx, serverAccountID, network, webAuthDomain string, homeDomains []string, signers ...string) ([]string, error) { // Read the transaction which validates its structure. - tx, _, _, err := ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain, homeDomains) + tx, _, _, _, err := ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain, homeDomains) if err != nil { return nil, err } diff --git a/txnbuild/transaction_challenge_example_test.go b/txnbuild/transaction_challenge_example_test.go index a183aa44f4..220782579e 100644 --- a/txnbuild/transaction_challenge_example_test.go +++ b/txnbuild/transaction_challenge_example_test.go @@ -37,7 +37,7 @@ func ExampleVerifyChallengeTxThreshold() { // Server builds challenge transaction var challengeTx string { - tx, err := txnbuild.BuildChallengeTx(serverAccount.Seed(), clientAccount.Address(), "webauthdomain.stellar.org", "test", network.TestNetworkPassphrase, time.Minute) + tx, err := txnbuild.BuildChallengeTx(serverAccount.Seed(), clientAccount.Address(), "webauthdomain.stellar.org", "test", network.TestNetworkPassphrase, time.Minute, nil) if err != nil { fmt.Println("Error:", err) return @@ -52,7 +52,7 @@ func ExampleVerifyChallengeTxThreshold() { // Client reads and signs challenge transaction var signedChallengeTx string { - tx, txClientAccountID, _, err := txnbuild.ReadChallengeTx(challengeTx, serverAccount.Address(), network.TestNetworkPassphrase, "webauthdomain.stellar.org", []string{"test"}) + tx, txClientAccountID, _, _, err := txnbuild.ReadChallengeTx(challengeTx, serverAccount.Address(), network.TestNetworkPassphrase, "webauthdomain.stellar.org", []string{"test"}) if err != nil { fmt.Println("Error:", err) return @@ -75,7 +75,7 @@ func ExampleVerifyChallengeTxThreshold() { // Server verifies signed challenge transaction { - _, txClientAccountID, _, err := txnbuild.ReadChallengeTx(challengeTx, serverAccount.Address(), network.TestNetworkPassphrase, "webauthdomain.stellar.org", []string{"test"}) + _, txClientAccountID, _, _, err := txnbuild.ReadChallengeTx(challengeTx, serverAccount.Address(), network.TestNetworkPassphrase, "webauthdomain.stellar.org", []string{"test"}) if err != nil { fmt.Println("Error:", err) return diff --git a/txnbuild/transaction_test.go b/txnbuild/transaction_test.go index b813dd8b95..b9452f2929 100644 --- a/txnbuild/transaction_test.go +++ b/txnbuild/transaction_test.go @@ -1073,7 +1073,7 @@ func TestBuildChallengeTx(t *testing.T) { { // 1 minute timebound - tx, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Minute) + tx, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Minute, nil) assert.NoError(t, err) txeBase64, err := tx.Base64() assert.NoError(t, err) @@ -1097,7 +1097,7 @@ func TestBuildChallengeTx(t *testing.T) { { // 5 minutes timebound - tx, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Duration(5*time.Minute)) + tx, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Duration(5*time.Minute), nil) assert.NoError(t, err) txeBase64, err := tx.Base64() assert.NoError(t, err) @@ -1121,7 +1121,7 @@ func TestBuildChallengeTx(t *testing.T) { } //transaction with infinite timebound - _, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "webauthdomain", "sdf", network.TestNetworkPassphrase, 0) + _, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "webauthdomain", "sdf", network.TestNetworkPassphrase, 0, nil) if assert.Error(t, err) { assert.Contains(t, err.Error(), "provided timebound must be at least 1s (300s is recommended)") } @@ -1878,7 +1878,7 @@ func TestReadChallengeTx_validSignedByServerAndClient(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, clientKP.Address(), readClientAccountID) assert.NoError(t, err) @@ -1913,7 +1913,7 @@ func TestReadChallengeTx_validSignedByServer(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, clientKP.Address(), readClientAccountID) assert.NoError(t, err) @@ -1946,7 +1946,7 @@ func TestReadChallengeTx_invalidNotSignedByServer(t *testing.T) { tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, clientKP.Address(), readClientAccountID) assert.EqualError(t, err, "transaction not signed by "+serverKP.Address()) @@ -1982,7 +1982,7 @@ func TestReadChallengeTx_invalidCorrupted(t *testing.T) { tx64, err := tx.Base64() require.NoError(t, err) tx64 = strings.ReplaceAll(tx64, "A", "B") - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Nil(t, readTx) assert.Equal(t, "", readClientAccountID) assert.EqualError( @@ -2022,7 +2022,7 @@ func TestReadChallengeTx_invalidServerAccountIDMismatch(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, "", readClientAccountID) assert.EqualError(t, err, "transaction source account is not equal to server's account") @@ -2057,7 +2057,7 @@ func TestReadChallengeTx_invalidSeqNoNotZero(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, "", readClientAccountID) assert.EqualError(t, err, "transaction sequence number must be 0") @@ -2092,7 +2092,7 @@ func TestReadChallengeTx_invalidTimeboundsInfinite(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, "", readClientAccountID) assert.EqualError(t, err, "transaction requires non-infinite timebounds") @@ -2127,7 +2127,7 @@ func TestReadChallengeTx_invalidTimeboundsOutsideRange(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, "", readClientAccountID) assert.Error(t, err) @@ -2164,7 +2164,7 @@ func TestReadChallengeTx_validTimeboundsWithGracePeriod(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, clientKP.Address(), readClientAccountID) assert.NoError(t, err) @@ -2200,7 +2200,7 @@ func TestReadChallengeTx_invalidTimeboundsWithGracePeriod(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, "", readClientAccountID) assert.Error(t, err) @@ -2230,7 +2230,7 @@ func TestReadChallengeTx_invalidOperationWrongType(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, "", readClientAccountID) assert.EqualError(t, err, "operation type should be manage_data") @@ -2258,7 +2258,7 @@ func TestReadChallengeTx_invalidOperationNoSourceAccount(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + _, _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.EqualError(t, err, "operation should have a source account") } @@ -2291,7 +2291,7 @@ func TestReadChallengeTx_invalidDataValueWrongEncodedLength(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, clientKP.Address(), readClientAccountID) assert.EqualError(t, err, "random nonce encoded as base64 should be 64 bytes long") @@ -2326,7 +2326,7 @@ func TestReadChallengeTx_invalidDataValueCorruptBase64(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, clientKP.Address(), readClientAccountID) assert.EqualError(t, err, "failed to decode random nonce provided in manage_data operation: illegal base64 data at input byte 37") @@ -2362,7 +2362,7 @@ func TestReadChallengeTx_invalidDataValueWrongByteLength(t *testing.T) { tx64, err := tx.Base64() assert.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, clientKP.Address(), readClientAccountID) assert.EqualError(t, err, "random nonce before encoding as base64 should be 48 bytes long") @@ -2377,6 +2377,7 @@ func TestReadChallengeTx_acceptsV0AndV1Transactions(t *testing.T) { "testanchor.stellar.org", network.TestNetworkPassphrase, time.Hour, + nil, ) assert.NoError(t, err) @@ -2391,7 +2392,7 @@ func TestReadChallengeTx_acceptsV0AndV1Transactions(t *testing.T) { assert.NoError(t, err) for _, challenge := range []string{v1Challenge, v0Challenge} { - parsedTx, clientAccountID, _, err := ReadChallengeTx( + parsedTx, clientAccountID, _, _, err := ReadChallengeTx( challenge, kp0.Address(), network.TestNetworkPassphrase, @@ -2417,6 +2418,7 @@ func TestReadChallengeTx_forbidsFeeBumpTransactions(t *testing.T) { "testanchor.stellar.org", network.TestNetworkPassphrase, time.Hour, + nil, ) assert.NoError(t, err) @@ -2435,7 +2437,7 @@ func TestReadChallengeTx_forbidsFeeBumpTransactions(t *testing.T) { challenge, err := feeBumpTx.Base64() assert.NoError(t, err) - _, _, _, err = ReadChallengeTx( + _, _, _, _, err = ReadChallengeTx( challenge, kp0.Address(), network.TestNetworkPassphrase, @@ -2445,41 +2447,40 @@ func TestReadChallengeTx_forbidsFeeBumpTransactions(t *testing.T) { assert.EqualError(t, err, "challenge cannot be a fee bump transaction") } -func TestReadChallengeTx_forbidsMuxedAccounts(t *testing.T) { +func TestReadChallengeTx_allowsMuxedAccounts(t *testing.T) { kp0 := newKeypair0() + kp1 := newKeypair1() + aid := xdr.MustAddress(kp1.Address()) + muxedAccount := xdr.MuxedAccount{ + Type: xdr.CryptoKeyTypeKeyTypeMuxedEd25519, + Med25519: &xdr.MuxedAccountMed25519{ + Id: 0xcafebabe, + Ed25519: *aid.Ed25519, + }, + } tx, err := BuildChallengeTx( kp0.Seed(), - kp0.Address(), + muxedAccount.Address(), "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Hour, + nil, ) - - env := tx.ToXDR() assert.NoError(t, err) - aid := xdr.MustAddress(kp0.Address()) - muxedAccount := xdr.MuxedAccount{ - Type: xdr.CryptoKeyTypeKeyTypeMuxedEd25519, - Med25519: &xdr.MuxedAccountMed25519{ - Id: 0xcafebabe, - Ed25519: *aid.Ed25519, - }, - } - *env.V1.Tx.Operations[0].SourceAccount = muxedAccount - challenge, err := marshallBase64(env, env.Signatures()) + challenge, err := marshallBase64(tx.ToXDR(), tx.Signatures()) assert.NoError(t, err) - _, _, _, err = ReadChallengeTx( + tx, _, _, _, err = ReadChallengeTx( challenge, kp0.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}, ) - errorMessage := "only valid Ed25519 accounts are allowed in challenge transactions" - assert.Contains(t, err.Error(), errorMessage) + assert.NoError(t, err) + assert.Equal(t, tx.envelope.Operations()[0].SourceAccount, &muxedAccount) } func TestReadChallengeTx_doesVerifyHomeDomainFailure(t *testing.T) { @@ -2511,7 +2512,7 @@ func TestReadChallengeTx_doesVerifyHomeDomainFailure(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"willfail"}) + _, _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"willfail"}) assert.EqualError(t, err, "operation key does not match any homeDomains passed (key=\"testanchor.stellar.org auth\", homeDomains=[willfail])") } @@ -2544,7 +2545,7 @@ func TestReadChallengeTx_doesVerifyHomeDomainSuccess(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + _, _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, nil, err) } @@ -2582,7 +2583,7 @@ func TestReadChallengeTx_allowsAdditionalManageDataOpsWithSourceAccountSetToServ assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, clientKP.Address(), readClientAccountID) assert.NoError(t, err) @@ -2622,7 +2623,7 @@ func TestReadChallengeTx_disallowsAdditionalManageDataOpsWithoutSourceAccountSet assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + _, _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.EqualError(t, err, "subsequent operations are unrecognized") } @@ -2659,7 +2660,7 @@ func TestReadChallengeTx_disallowsAdditionalOpsOfOtherTypes(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - readTx, readClientAccountID, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + readTx, readClientAccountID, _, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.Equal(t, tx, readTx) assert.Equal(t, clientKP.Address(), readClientAccountID) assert.EqualError(t, err, "operation type should be manage_data") @@ -2693,7 +2694,7 @@ func TestReadChallengeTx_matchesHomeDomain(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - _, _, matchedHomeDomain, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + _, _, matchedHomeDomain, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) require.NoError(t, err) assert.Equal(t, matchedHomeDomain, "testanchor.stellar.org") } @@ -2726,7 +2727,7 @@ func TestReadChallengeTx_doesNotMatchHomeDomain(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - _, _, matchedHomeDomain, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"not", "going", "to", "match"}) + _, _, matchedHomeDomain, _, err := ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"not", "going", "to", "match"}) assert.Equal(t, matchedHomeDomain, "") assert.EqualError(t, err, "operation key does not match any homeDomains passed (key=\"testanchor.stellar.org auth\", homeDomains=[not going to match])") } @@ -2754,7 +2755,7 @@ func TestReadChallengeTx_validWhenWebAuthDomainMissing(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + _, _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.NoError(t, err) } @@ -2786,7 +2787,7 @@ func TestReadChallengeTx_invalidWebAuthDomainSourceAccount(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + _, _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.EqualError(t, err, `web auth domain operation must have server source account`) } @@ -2818,7 +2819,7 @@ func TestReadChallengeTx_invalidWebAuthDomain(t *testing.T) { assert.NoError(t, err) tx64, err := tx.Base64() require.NoError(t, err) - _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) + _, _, _, _, err = ReadChallengeTx(tx64, serverKP.Address(), network.TestNetworkPassphrase, "testwebauth.stellar.org", []string{"testanchor.stellar.org"}) assert.EqualError(t, err, `web auth domain operation value is "testwebauth.example.org" but expect "testwebauth.stellar.org"`) } From d8ce2a5b1656b42ac01e088819689cf1e6e58f54 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Tue, 24 Jan 2023 12:59:29 -0800 Subject: [PATCH 04/22] reject requests with memo & muxed account --- exp/services/webauth/internal/serve/challenge.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/exp/services/webauth/internal/serve/challenge.go b/exp/services/webauth/internal/serve/challenge.go index 20e1635c61..b7d1a22def 100644 --- a/exp/services/webauth/internal/serve/challenge.go +++ b/exp/services/webauth/internal/serve/challenge.go @@ -34,7 +34,9 @@ func (h challengeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { queryValues := r.URL.Query() account := queryValues.Get("account") - if !strkey.IsValidEd25519PublicKey(account) { + isStellarAccount := strkey.IsValidEd25519PublicKey(account) + isMuxedAccount := strkey.IsValidMuxedAccountEd25519PublicKey(account) + if !isStellarAccount && !isMuxedAccount { badRequest.Render(w) return } @@ -60,6 +62,10 @@ func (h challengeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var memo txnbuild.MemoID if queryValues.Get("memo") != "" { + if isMuxedAccount { + badRequest.Render(w) + return + } memoInt, err := strconv.ParseUint(queryValues.Get("memo"), 10, 64) if err != nil { badRequest.Render(w) From 8ccbe3de7a63278f92b86f44a4029dc308ff57a8 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Tue, 24 Jan 2023 13:40:00 -0800 Subject: [PATCH 05/22] use correct 'sub' value for JWT --- exp/services/webauth/internal/serve/token.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/exp/services/webauth/internal/serve/token.go b/exp/services/webauth/internal/serve/token.go index c69ce6586a..ba9df83bef 100644 --- a/exp/services/webauth/internal/serve/token.go +++ b/exp/services/webauth/internal/serve/token.go @@ -2,6 +2,7 @@ package serve import ( "net/http" + "strconv" "strings" "time" @@ -148,10 +149,21 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + var sub string + if muxedAccount == (xdr.MuxedAccount{}) { + sub = clientAccountID + if memo != nil { + xdrMemo, _ := memo.ToXDR() + sub += ":" + strconv.FormatUint(uint64(xdrMemo.MustId()), 10) + } + } else { + sub = muxedAccount.Address() + } + issuedAt := time.Unix(tx.Timebounds().MinTime, 0) claims := jwt.Claims{ Issuer: h.JWTIssuer, - Subject: muxedAccount.Address(), + Subject: sub, IssuedAt: jwt.NewNumericDate(issuedAt), Expiry: jwt.NewNumericDate(issuedAt.Add(h.JWTExpiresIn)), } From 2ee1c582839a4d3c435eb4c2f2c719806cf5d435 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Tue, 24 Jan 2023 13:55:07 -0800 Subject: [PATCH 06/22] minor adjustments for go check --- exp/services/webauth/internal/serve/token.go | 4 ++-- txnbuild/transaction.go | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/exp/services/webauth/internal/serve/token.go b/exp/services/webauth/internal/serve/token.go index ba9df83bef..22796d21c6 100644 --- a/exp/services/webauth/internal/serve/token.go +++ b/exp/services/webauth/internal/serve/token.go @@ -88,8 +88,8 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { l.Info("Start verifying challenge transaction.") - var muxedAccount xdr.MuxedAccount - if muxedAccount, err = xdr.AddressToMuxedAccount(clientAccountID); err == nil { + muxedAccount, err := xdr.AddressToMuxedAccount(clientAccountID) + if err == nil { clientAccountID = muxedAccount.ToAccountId().Address() } diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index af23b48831..a43fa72ea2 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -1027,7 +1027,8 @@ func BuildChallengeTx(serverSignerSecret, clientAccountID, webAuthDomain, homeDo return nil, errors.New("memos are not valid for challenge transactions with a muxed client account") } } else if memo != nil { - if xdrMemo, err := memo.ToXDR(); err != nil || xdrMemo.Type != xdr.MemoTypeMemoId { + var xdrMemo xdr.Memo + if xdrMemo, err = memo.ToXDR(); err != nil || xdrMemo.Type != xdr.MemoTypeMemoId { return nil, errors.New("memo must be of type MemoID") } } @@ -1182,7 +1183,8 @@ func ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain string err = errors.New("memos are not valid for challenge transactions with a muxed client account") return tx, clientAccountID, matchedHomeDomain, memo, err } else if rawOperations[0].SourceAccount.Type == xdr.CryptoKeyTypeKeyTypeEd25519 && memo != nil { - if rawMemo, err := memo.ToXDR(); err != nil || rawMemo.Type != xdr.MemoTypeMemoId { + var rawMemo xdr.Memo + if rawMemo, err = memo.ToXDR(); err != nil || rawMemo.Type != xdr.MemoTypeMemoId { err = errors.New("invalid memo, only ID memos are permitted") return tx, clientAccountID, matchedHomeDomain, memo, err } From 73372426e91c82dcaba6b3a0c25fade72500af3c Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Tue, 24 Jan 2023 14:42:13 -0800 Subject: [PATCH 07/22] added first test --- txnbuild/transaction_test.go | 64 ++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/txnbuild/transaction_test.go b/txnbuild/transaction_test.go index b9452f2929..d86678f4e6 100644 --- a/txnbuild/transaction_test.go +++ b/txnbuild/transaction_test.go @@ -2483,6 +2483,70 @@ func TestReadChallengeTx_allowsMuxedAccounts(t *testing.T) { assert.Equal(t, tx.envelope.Operations()[0].SourceAccount, &muxedAccount) } +func TestReadChallengeTransaction_forbidsMemoWithMuxedClientAccount(t *testing.T) { + kp0 := newKeypair0() + kp1 := newKeypair1() + homeDomain := "testanchor.stellar.org" + webAuthDomain := "testwebauth.stellar.org" + + serverAccount := SimpleAccount{ + AccountID: kp0.Address(), + Sequence: 0, + } + aid := xdr.MustAddress(kp1.Address()) + muxedAccount := xdr.MuxedAccount{ + Type: xdr.CryptoKeyTypeKeyTypeMuxedEd25519, + Med25519: &xdr.MuxedAccountMed25519{ + Id: 0xcafebabe, + Ed25519: *aid.Ed25519, + }, + } + randomNonce, _ := generateRandomNonce(48) + randomNonceToString := base64.StdEncoding.EncodeToString(randomNonce) + currentTime := time.Now().UTC() + maxTime := currentTime.Add(300) + + tx, err := NewTransaction( + TransactionParams{ + SourceAccount: &serverAccount, + IncrementSequenceNum: false, + Operations: []Operation{ + &ManageData{ + SourceAccount: muxedAccount.Address(), + Name: homeDomain + " auth", + Value: []byte(randomNonceToString), + }, + &ManageData{ + SourceAccount: serverAccount.GetAccountID(), + Name: "web_auth_domain", + Value: []byte(webAuthDomain), + }, + }, + BaseFee: MinBaseFee, + Memo: MemoID(1), + Preconditions: Preconditions{ + TimeBounds: NewTimebounds(currentTime.Unix(), maxTime.Unix()), + }, + }, + ) + assert.NoError(t, err) + + tx, err = tx.Sign(network.TestNetworkPassphrase, kp0) + assert.NoError(t, err) + + challenge, err := marshallBase64(tx.ToXDR(), tx.Signatures()) + assert.NoError(t, err) + + _, _, _, _, err = ReadChallengeTx( + challenge, + kp0.Address(), + network.TestNetworkPassphrase, + webAuthDomain, + []string{homeDomain}, + ) + assert.EqualError(t, err, "memos are not valid for challenge transactions with a muxed client account") +} + func TestReadChallengeTx_doesVerifyHomeDomainFailure(t *testing.T) { serverKP := newKeypair0() clientKP := newKeypair1() From af134305d3434fbb34168ebd564ed182fc8cea41 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Tue, 24 Jan 2023 17:10:03 -0800 Subject: [PATCH 08/22] add more tests --- txnbuild/transaction_test.go | 83 ++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/txnbuild/transaction_test.go b/txnbuild/transaction_test.go index d86678f4e6..b4dae1a743 100644 --- a/txnbuild/transaction_test.go +++ b/txnbuild/transaction_test.go @@ -1120,6 +1120,33 @@ func TestBuildChallengeTx(t *testing.T) { assert.Equal(t, "testwebauth.stellar.org", string(*webAuthOp.Body.ManageDataOp.DataValue), "DataValue should be 'testwebauth.stellar.org'") } + // transaction with memo + { + tx, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Minute, MemoID(1)) + assert.NoError(t, err) + assert.Equal(t, tx.Memo(), MemoID(1)) + } + + // transaction with bad memo + { + _, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Minute, MemoText("test")) + assert.EqualError(t, err, "memo must be of type MemoID") + } + + // transaction with muxed account + { + muxedAccount := "MA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVAAAAAAAAAAAAAJLK" + tx, err := BuildChallengeTx(kp0.Seed(), muxedAccount, "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Minute, nil) + assert.NoError(t, err) + assert.Equal(t, tx.operations[0].GetSourceAccount(), muxedAccount) + } + + // transaction with memo and muxed account + { + _, err := BuildChallengeTx(kp0.Seed(), "MA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVAAAAAAAAAAAAAJLK", "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Minute, MemoID(1)) + assert.EqualError(t, err, "memos are not valid for challenge transactions with a muxed client account") + } + //transaction with infinite timebound _, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "webauthdomain", "sdf", network.TestNetworkPassphrase, 0, nil) if assert.Error(t, err) { @@ -2547,6 +2574,62 @@ func TestReadChallengeTransaction_forbidsMemoWithMuxedClientAccount(t *testing.T assert.EqualError(t, err, "memos are not valid for challenge transactions with a muxed client account") } +func TestReadChallengeTransaction_forbidsNonIdMemo(t *testing.T) { + kp0 := newKeypair0() + kp1 := newKeypair1() + homeDomain := "testanchor.stellar.org" + webAuthDomain := "testwebauth.stellar.org" + + serverAccount := SimpleAccount{ + AccountID: kp0.Address(), + Sequence: 0, + } + randomNonce, _ := generateRandomNonce(48) + randomNonceToString := base64.StdEncoding.EncodeToString(randomNonce) + currentTime := time.Now().UTC() + maxTime := currentTime.Add(300) + + tx, err := NewTransaction( + TransactionParams{ + SourceAccount: &serverAccount, + IncrementSequenceNum: false, + Operations: []Operation{ + &ManageData{ + SourceAccount: kp1.Address(), + Name: homeDomain + " auth", + Value: []byte(randomNonceToString), + }, + &ManageData{ + SourceAccount: serverAccount.GetAccountID(), + Name: "web_auth_domain", + Value: []byte(webAuthDomain), + }, + }, + BaseFee: MinBaseFee, + Memo: MemoText("test"), + Preconditions: Preconditions{ + TimeBounds: NewTimebounds(currentTime.Unix(), maxTime.Unix()), + }, + }, + ) + assert.NoError(t, err) + + tx, err = tx.Sign(network.TestNetworkPassphrase, kp0) + assert.NoError(t, err) + + challenge, err := marshallBase64(tx.ToXDR(), tx.Signatures()) + assert.NoError(t, err) + + _, _, _, _, err = ReadChallengeTx( + challenge, + kp0.Address(), + network.TestNetworkPassphrase, + webAuthDomain, + []string{homeDomain}, + ) + assert.EqualError(t, err, "invalid memo, only ID memos are permitted") +} + func TestReadChallengeTx_doesVerifyHomeDomainFailure(t *testing.T) { serverKP := newKeypair0() clientKP := newKeypair1() From d318150d7ecc93bd3255b804967909eccbd0685e Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Tue, 24 Jan 2023 17:22:04 -0800 Subject: [PATCH 09/22] use Memo not MemoID type in challenge.go --- exp/services/webauth/internal/serve/challenge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exp/services/webauth/internal/serve/challenge.go b/exp/services/webauth/internal/serve/challenge.go index b7d1a22def..d777d8c2cc 100644 --- a/exp/services/webauth/internal/serve/challenge.go +++ b/exp/services/webauth/internal/serve/challenge.go @@ -60,7 +60,7 @@ func (h challengeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { homeDomain = h.HomeDomains[0] } - var memo txnbuild.MemoID + var memo txnbuild.Memo if queryValues.Get("memo") != "" { if isMuxedAccount { badRequest.Render(w) From 98382441b33ab2e8dfc17f7a3a5864e71dc2a872 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Wed, 25 Jan 2023 08:45:46 -0800 Subject: [PATCH 10/22] add to function documentation --- txnbuild/transaction.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index a43fa72ea2..6c5e07ec21 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -998,6 +998,7 @@ func NewFeeBumpTransaction(params FeeBumpTransactionParams) (*FeeBumpTransaction // BuildChallengeTx is a factory method that creates a valid SEP 10 challenge, for use in web authentication. // "timebound" is the time duration the transaction should be valid for, and must be greater than 1s (300s is recommended). +// Muxed accounts or ID memos can be provided to identity a user of a shared Stellar account. // More details on SEP 10: https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0010.md func BuildChallengeTx(serverSignerSecret, clientAccountID, webAuthDomain, homeDomain, network string, timebound time.Duration, memo Memo) (*Transaction, error) { if timebound < time.Second { @@ -1114,6 +1115,10 @@ func generateRandomNonce(n int) ([]byte, error) { // one of the following functions to completely verify the transaction: // - VerifyChallengeTxThreshold // - VerifyChallengeTxSigners +// +// The returned clientAccountID may be a Stellar account or Muxed account address. If +// the address is muxed, or if the memo returned is non-nil, the challenge transaction +// is being used to authenticate a user of a shared Stellar account. func ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain string, homeDomains []string) (tx *Transaction, clientAccountID string, matchedHomeDomain string, memo Memo, err error) { parsed, err := TransactionFromXDR(challengeTx) if err != nil { From ebe0f2aeac433fff9c3f56188fe2cf4239aa65e2 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Wed, 25 Jan 2023 08:54:29 -0800 Subject: [PATCH 11/22] add to txnbuild CHANGELOG.md --- txnbuild/CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/txnbuild/CHANGELOG.md b/txnbuild/CHANGELOG.md index d7ed9c8cd1..82839d1711 100644 --- a/txnbuild/CHANGELOG.md +++ b/txnbuild/CHANGELOG.md @@ -6,6 +6,16 @@ file. This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased +### Breaking changes + +* Muxed accounts and ID memos can be used in the `BuildChallengeTx()` and `ReadChallengeTx()` SEP-10 utilitiy functions to identify users of shared Stellar accounts. ([#4746](https://github.com/stellar/go/pull/4746)) + * `BuildChallengeTx()`: + * Muxed account addresses can be passed as the `clientAccountID`. + * Adds an additional parameter of type `txnbuild.Memo`. Memos must be of type ID and cannot be specified if the `clientAccoutID` id a muxed account address. + * `ReadChallengeTx()`: + * Muxed account addresses may be returned as the `clientAccountID`. + * Adds an additional return value of type `txnbuild.Memo`, which if non-nil, will always be a memo of type ID. + ## [10.0.0](https://github.com/stellar/go/releases/tag/horizonclient-v9.0.0) - 2022-04-18 From 9562534557a971ef05e190514532ce72821b6f98 Mon Sep 17 00:00:00 2001 From: Jake Urban <10968980+JakeUrban@users.noreply.github.com> Date: Wed, 25 Jan 2023 11:01:51 -0800 Subject: [PATCH 12/22] Update txnbuild/transaction.go Co-authored-by: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> --- txnbuild/transaction.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index 6c5e07ec21..d4e5c0fd6f 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -1116,7 +1116,7 @@ func generateRandomNonce(n int) ([]byte, error) { // - VerifyChallengeTxThreshold // - VerifyChallengeTxSigners // -// The returned clientAccountID may be a Stellar account or Muxed account address. If +// The returned clientAccountID may be a Stellar account (G...) or Muxed account (M...) address. If // the address is muxed, or if the memo returned is non-nil, the challenge transaction // is being used to authenticate a user of a shared Stellar account. func ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain string, homeDomains []string) (tx *Transaction, clientAccountID string, matchedHomeDomain string, memo Memo, err error) { From aeff367eaf14630024f503a950d461d676114e78 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Wed, 25 Jan 2023 13:55:04 -0800 Subject: [PATCH 13/22] use *MemoID instead of Memo for param and return type --- .../webauth/internal/serve/challenge.go | 9 +-- exp/services/webauth/internal/serve/token.go | 5 +- txnbuild/transaction.go | 71 ++++++++++--------- txnbuild/transaction_test.go | 14 ++-- 4 files changed, 49 insertions(+), 50 deletions(-) diff --git a/exp/services/webauth/internal/serve/challenge.go b/exp/services/webauth/internal/serve/challenge.go index d777d8c2cc..ba48e987e8 100644 --- a/exp/services/webauth/internal/serve/challenge.go +++ b/exp/services/webauth/internal/serve/challenge.go @@ -60,13 +60,14 @@ func (h challengeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { homeDomain = h.HomeDomains[0] } - var memo txnbuild.Memo - if queryValues.Get("memo") != "" { + var memo txnbuild.MemoID + memoParam := queryValues.Get("memo") + if memoParam != "" { if isMuxedAccount { badRequest.Render(w) return } - memoInt, err := strconv.ParseUint(queryValues.Get("memo"), 10, 64) + memoInt, err := strconv.ParseUint(memoParam, 10, 64) if err != nil { badRequest.Render(w) return @@ -81,7 +82,7 @@ func (h challengeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { homeDomain, h.NetworkPassphrase, h.ChallengeExpiresIn, - memo, + &memo, ) if err != nil { h.Logger.Ctx(ctx).WithStack(err).Error(err) diff --git a/exp/services/webauth/internal/serve/token.go b/exp/services/webauth/internal/serve/token.go index 22796d21c6..9b32e9384d 100644 --- a/exp/services/webauth/internal/serve/token.go +++ b/exp/services/webauth/internal/serve/token.go @@ -54,7 +54,7 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { clientAccountID string signingAddress *keypair.FromAddress homeDomain string - memo txnbuild.Memo + memo *txnbuild.MemoID ) for _, s := range h.SigningAddresses { tx, clientAccountID, homeDomain, memo, err = txnbuild.ReadChallengeTx(req.Transaction, s.Address(), h.NetworkPassphrase, h.Domain, h.HomeDomains) @@ -153,8 +153,7 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if muxedAccount == (xdr.MuxedAccount{}) { sub = clientAccountID if memo != nil { - xdrMemo, _ := memo.ToXDR() - sub += ":" + strconv.FormatUint(uint64(xdrMemo.MustId()), 10) + sub += ":" + strconv.FormatUint(uint64(*memo), 10) } } else { sub = muxedAccount.Address() diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index d4e5c0fd6f..0b8bf7a4bc 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -1000,7 +1000,7 @@ func NewFeeBumpTransaction(params FeeBumpTransactionParams) (*FeeBumpTransaction // "timebound" is the time duration the transaction should be valid for, and must be greater than 1s (300s is recommended). // Muxed accounts or ID memos can be provided to identity a user of a shared Stellar account. // More details on SEP 10: https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0010.md -func BuildChallengeTx(serverSignerSecret, clientAccountID, webAuthDomain, homeDomain, network string, timebound time.Duration, memo Memo) (*Transaction, error) { +func BuildChallengeTx(serverSignerSecret, clientAccountID, webAuthDomain, homeDomain, network string, timebound time.Duration, memo *MemoID) (*Transaction, error) { if timebound < time.Second { return nil, errors.New("provided timebound must be at least 1s (300s is recommended)") } @@ -1027,11 +1027,6 @@ func BuildChallengeTx(serverSignerSecret, clientAccountID, webAuthDomain, homeDo } else if memo != nil { return nil, errors.New("memos are not valid for challenge transactions with a muxed client account") } - } else if memo != nil { - var xdrMemo xdr.Memo - if xdrMemo, err = memo.ToXDR(); err != nil || xdrMemo.Type != xdr.MemoTypeMemoId { - return nil, errors.New("memo must be of type MemoID") - } } // represent server signing account as SimpleAccount @@ -1045,29 +1040,32 @@ func BuildChallengeTx(serverSignerSecret, clientAccountID, webAuthDomain, homeDo // Create a SEP 10 compatible response. See // https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0010.md#response - tx, err := NewTransaction( - TransactionParams{ - SourceAccount: &sa, - IncrementSequenceNum: false, - Operations: []Operation{ - &ManageData{ - SourceAccount: clientAccountID, - Name: homeDomain + " auth", - Value: []byte(randomNonceToString), - }, - &ManageData{ - SourceAccount: serverKP.Address(), - Name: "web_auth_domain", - Value: []byte(webAuthDomain), - }, + txParams := TransactionParams{ + SourceAccount: &sa, + IncrementSequenceNum: false, + Operations: []Operation{ + &ManageData{ + SourceAccount: clientAccountID, + Name: homeDomain + " auth", + Value: []byte(randomNonceToString), }, - BaseFee: MinBaseFee, - Memo: memo, - Preconditions: Preconditions{ - TimeBounds: NewTimebounds(currentTime.Unix(), maxTime.Unix()), + &ManageData{ + SourceAccount: serverKP.Address(), + Name: "web_auth_domain", + Value: []byte(webAuthDomain), }, }, - ) + BaseFee: MinBaseFee, + Preconditions: Preconditions{ + TimeBounds: NewTimebounds(currentTime.Unix(), maxTime.Unix()), + }, + } + // Do not replace this if-then-assign block by assigning `memo` within the `TransactionParams` + // struct above. Doing so will cause errors as described here: https://go.dev/doc/faq#nil_error + if memo != nil { + txParams.Memo = memo + } + tx, err := NewTransaction(txParams) if err != nil { return nil, err } @@ -1119,7 +1117,7 @@ func generateRandomNonce(n int) ([]byte, error) { // The returned clientAccountID may be a Stellar account (G...) or Muxed account (M...) address. If // the address is muxed, or if the memo returned is non-nil, the challenge transaction // is being used to authenticate a user of a shared Stellar account. -func ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain string, homeDomains []string) (tx *Transaction, clientAccountID string, matchedHomeDomain string, memo Memo, err error) { +func ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain string, homeDomains []string) (tx *Transaction, clientAccountID string, matchedHomeDomain string, memo *MemoID, err error) { parsed, err := TransactionFromXDR(challengeTx) if err != nil { return tx, clientAccountID, matchedHomeDomain, memo, errors.Wrap(err, "could not parse challenge") @@ -1182,17 +1180,22 @@ func ReadChallengeTx(challengeTx, serverAccountID, network, webAuthDomain string } clientAccountID = op.SourceAccount - memo = tx.Memo() - rawOperations := tx.envelope.Operations() - if rawOperations[0].SourceAccount.Type == xdr.CryptoKeyTypeKeyTypeMuxedEd25519 && memo != nil { - err = errors.New("memos are not valid for challenge transactions with a muxed client account") + firstOpSourceAccountType := tx.envelope.Operations()[0].SourceAccount.Type + if firstOpSourceAccountType != xdr.CryptoKeyTypeKeyTypeMuxedEd25519 && firstOpSourceAccountType != xdr.CryptoKeyTypeKeyTypeEd25519 { + err = errors.New("invalid source account for first operation: only valid Ed25519 or muxed accounts are valid") return tx, clientAccountID, matchedHomeDomain, memo, err - } else if rawOperations[0].SourceAccount.Type == xdr.CryptoKeyTypeKeyTypeEd25519 && memo != nil { - var rawMemo xdr.Memo - if rawMemo, err = memo.ToXDR(); err != nil || rawMemo.Type != xdr.MemoTypeMemoId { + } + if tx.Memo() != nil { + if firstOpSourceAccountType == xdr.CryptoKeyTypeKeyTypeMuxedEd25519 { + err = errors.New("memos are not valid for challenge transactions with a muxed client account") + return tx, clientAccountID, matchedHomeDomain, memo, err + } + var txMemo MemoID + if txMemo, ok = (tx.Memo()).(MemoID); !ok { err = errors.New("invalid memo, only ID memos are permitted") return tx, clientAccountID, matchedHomeDomain, memo, err } + memo = &txMemo } // verify manage data value diff --git a/txnbuild/transaction_test.go b/txnbuild/transaction_test.go index b4dae1a743..69c5008b71 100644 --- a/txnbuild/transaction_test.go +++ b/txnbuild/transaction_test.go @@ -1122,15 +1122,10 @@ func TestBuildChallengeTx(t *testing.T) { // transaction with memo { - tx, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Minute, MemoID(1)) + var memo MemoID = MemoID(1) + tx, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Minute, &memo) assert.NoError(t, err) - assert.Equal(t, tx.Memo(), MemoID(1)) - } - - // transaction with bad memo - { - _, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Minute, MemoText("test")) - assert.EqualError(t, err, "memo must be of type MemoID") + assert.Equal(t, tx.Memo(), &memo) } // transaction with muxed account @@ -1143,7 +1138,8 @@ func TestBuildChallengeTx(t *testing.T) { // transaction with memo and muxed account { - _, err := BuildChallengeTx(kp0.Seed(), "MA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVAAAAAAAAAAAAAJLK", "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Minute, MemoID(1)) + var memo MemoID = MemoID(1) + _, err := BuildChallengeTx(kp0.Seed(), "MA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVAAAAAAAAAAAAAJLK", "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Minute, &memo) assert.EqualError(t, err, "memos are not valid for challenge transactions with a muxed client account") } From 4678d6b625f0f8b2ec7d0a9b0a860575ac60b3a1 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Wed, 25 Jan 2023 14:10:00 -0800 Subject: [PATCH 14/22] ensure memo in challenge.go can be nil --- exp/services/webauth/internal/serve/challenge.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/exp/services/webauth/internal/serve/challenge.go b/exp/services/webauth/internal/serve/challenge.go index ba48e987e8..ae5b486a10 100644 --- a/exp/services/webauth/internal/serve/challenge.go +++ b/exp/services/webauth/internal/serve/challenge.go @@ -60,7 +60,7 @@ func (h challengeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { homeDomain = h.HomeDomains[0] } - var memo txnbuild.MemoID + var memo *txnbuild.MemoID memoParam := queryValues.Get("memo") if memoParam != "" { if isMuxedAccount { @@ -72,7 +72,8 @@ func (h challengeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { badRequest.Render(w) return } - memo = txnbuild.MemoID(memoInt) + memoId := txnbuild.MemoID(memoInt) + memo = &memoId } tx, err := txnbuild.BuildChallengeTx( @@ -82,7 +83,7 @@ func (h challengeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { homeDomain, h.NetworkPassphrase, h.ChallengeExpiresIn, - &memo, + memo, ) if err != nil { h.Logger.Ctx(ctx).WithStack(err).Error(err) From 3fdf4d3a8b68e0a5348034e4476428c1574dd337 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Wed, 25 Jan 2023 14:15:01 -0800 Subject: [PATCH 15/22] update CHANGELOG.md --- txnbuild/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/txnbuild/CHANGELOG.md b/txnbuild/CHANGELOG.md index 82839d1711..0e3f776a49 100644 --- a/txnbuild/CHANGELOG.md +++ b/txnbuild/CHANGELOG.md @@ -11,10 +11,10 @@ file. This project adheres to [Semantic Versioning](http://semver.org/). * Muxed accounts and ID memos can be used in the `BuildChallengeTx()` and `ReadChallengeTx()` SEP-10 utilitiy functions to identify users of shared Stellar accounts. ([#4746](https://github.com/stellar/go/pull/4746)) * `BuildChallengeTx()`: * Muxed account addresses can be passed as the `clientAccountID`. - * Adds an additional parameter of type `txnbuild.Memo`. Memos must be of type ID and cannot be specified if the `clientAccoutID` id a muxed account address. + * Adds an additional parameter of type `*txnbuild.MemoID`. Memos cannot be specified if the `clientAccoutID` id a muxed account address. * `ReadChallengeTx()`: * Muxed account addresses may be returned as the `clientAccountID`. - * Adds an additional return value of type `txnbuild.Memo`, which if non-nil, will always be a memo of type ID. + * Adds an additional return value of type `*txnbuild.MemoID`. ## [10.0.0](https://github.com/stellar/go/releases/tag/horizonclient-v9.0.0) - 2022-04-18 From 229efa317c82437e0b170d0e449f712c120d9e1b Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Wed, 25 Jan 2023 14:24:41 -0800 Subject: [PATCH 16/22] adjust test --- txnbuild/transaction_challenge_example_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/txnbuild/transaction_challenge_example_test.go b/txnbuild/transaction_challenge_example_test.go index 220782579e..a0db63064a 100644 --- a/txnbuild/transaction_challenge_example_test.go +++ b/txnbuild/transaction_challenge_example_test.go @@ -75,10 +75,13 @@ func ExampleVerifyChallengeTxThreshold() { // Server verifies signed challenge transaction { - _, txClientAccountID, _, _, err := txnbuild.ReadChallengeTx(challengeTx, serverAccount.Address(), network.TestNetworkPassphrase, "webauthdomain.stellar.org", []string{"test"}) + _, txClientAccountID, _, memo, err := txnbuild.ReadChallengeTx(challengeTx, serverAccount.Address(), network.TestNetworkPassphrase, "webauthdomain.stellar.org", []string{"test"}) if err != nil { fmt.Println("Error:", err) return + } else if memo != nil { + fmt.Println("Expected memo to be nil, got: ", memo) + return } // Server gets account From e4288dbc66f5a6f225dbb03d1b5e74ed37bc85c7 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Wed, 25 Jan 2023 14:29:33 -0800 Subject: [PATCH 17/22] use badRequest instead of serverError when BuildChallengeTx() fails --- exp/services/webauth/internal/serve/challenge.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/exp/services/webauth/internal/serve/challenge.go b/exp/services/webauth/internal/serve/challenge.go index ae5b486a10..dccfd06494 100644 --- a/exp/services/webauth/internal/serve/challenge.go +++ b/exp/services/webauth/internal/serve/challenge.go @@ -63,10 +63,6 @@ func (h challengeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var memo *txnbuild.MemoID memoParam := queryValues.Get("memo") if memoParam != "" { - if isMuxedAccount { - badRequest.Render(w) - return - } memoInt, err := strconv.ParseUint(memoParam, 10, 64) if err != nil { badRequest.Render(w) @@ -87,7 +83,7 @@ func (h challengeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { ) if err != nil { h.Logger.Ctx(ctx).WithStack(err).Error(err) - serverError.Render(w) + badRequest.Render(w) return } From b3800014c5a9f808c1e95d8415bcf3130b64bf61 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Wed, 25 Jan 2023 17:39:24 -0800 Subject: [PATCH 18/22] add token tests --- exp/services/webauth/internal/serve/token.go | 8 +- .../webauth/internal/serve/token_test.go | 216 ++++++++++++++++++ 2 files changed, 222 insertions(+), 2 deletions(-) diff --git a/exp/services/webauth/internal/serve/token.go b/exp/services/webauth/internal/serve/token.go index 9b32e9384d..e5212503a4 100644 --- a/exp/services/webauth/internal/serve/token.go +++ b/exp/services/webauth/internal/serve/token.go @@ -89,7 +89,11 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { l.Info("Start verifying challenge transaction.") muxedAccount, err := xdr.AddressToMuxedAccount(clientAccountID) - if err == nil { + if err != nil { + serverError.Render(w) + return + } + if muxedAccount.Type == xdr.CryptoKeyTypeKeyTypeMuxedEd25519 { clientAccountID = muxedAccount.ToAccountId().Address() } @@ -150,7 +154,7 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } var sub string - if muxedAccount == (xdr.MuxedAccount{}) { + if muxedAccount.Type == xdr.CryptoKeyTypeKeyTypeEd25519 { sub = clientAccountID if memo != nil { sub += ":" + strconv.FormatUint(uint64(*memo), 10) diff --git a/exp/services/webauth/internal/serve/token_test.go b/exp/services/webauth/internal/serve/token_test.go index 1c2745fa5a..1f120ed7c1 100644 --- a/exp/services/webauth/internal/serve/token_test.go +++ b/exp/services/webauth/internal/serve/token_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strconv" "strings" "testing" "time" @@ -18,6 +19,7 @@ import ( "github.com/stellar/go/keypair" "github.com/stellar/go/network" "github.com/stellar/go/protocols/horizon" + "github.com/stellar/go/strkey" supportlog "github.com/stellar/go/support/log" "github.com/stellar/go/support/render/problem" "github.com/stellar/go/txnbuild" @@ -1321,3 +1323,217 @@ func TestToken_jsonInputInvalidWebAuthDomainFail(t *testing.T) { assert.JSONEq(t, `{"error":"The request was invalid in some way."}`, string(respBodyBytes)) } + +func TestToken_successWithIdMemo(t *testing.T) { + serverKey := keypair.MustRandom() + t.Logf("Server signing key: %s", serverKey.Address()) + + jwtPrivateKey, err := jwtkey.GenerateKey() + require.NoError(t, err) + jwk := jose.JSONWebKey{Key: jwtPrivateKey, Algorithm: string(jose.ES256)} + + account := keypair.MustRandom() + t.Logf("Client account: %s", account.Address()) + + domain := "webauth.example.com" + homeDomain := "example.com" + + memo := txnbuild.MemoID(1) + tx, err := txnbuild.BuildChallengeTx( + serverKey.Seed(), + account.Address(), + domain, + homeDomain, + network.TestNetworkPassphrase, + time.Minute, + &memo, + ) + require.NoError(t, err) + + chTx, err := tx.Base64() + require.NoError(t, err) + t.Logf("Tx: %s", chTx) + + tx, err = tx.Sign(network.TestNetworkPassphrase, account) + require.NoError(t, err) + txSigned, err := tx.Base64() + require.NoError(t, err) + t.Logf("Signed: %s", txSigned) + + horizonClient := &horizonclient.MockClient{} + horizonClient. + On("AccountDetail", horizonclient.AccountRequest{AccountID: account.Address()}). + Return( + horizon.Account{ + Thresholds: horizon.AccountThresholds{ + LowThreshold: 1, + MedThreshold: 10, + HighThreshold: 100, + }, + Signers: []horizon.Signer{ + { + Key: account.Address(), + Weight: 100, + }, + }}, + nil, + ) + + h := tokenHandler{ + Logger: supportlog.DefaultLogger, + HorizonClient: horizonClient, + NetworkPassphrase: network.TestNetworkPassphrase, + SigningAddresses: []*keypair.FromAddress{serverKey.FromAddress()}, + JWK: jwk, + JWTIssuer: "https://example.com", + JWTExpiresIn: time.Minute, + Domain: domain, + HomeDomains: []string{homeDomain}, + } + + body := struct { + Transaction string `json:"transaction"` + }{ + Transaction: txSigned, + } + bodyBytes, err := json.Marshal(body) + require.NoError(t, err) + r := httptest.NewRequest("POST", "/", bytes.NewReader(bodyBytes)) + r.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + resp := w.Result() + + require.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "application/json; charset=utf-8", resp.Header.Get("Content-Type")) + + res := struct { + Token string `json:"token"` + }{} + err = json.NewDecoder(resp.Body).Decode(&res) + require.NoError(t, err) + + t.Logf("JWT: %s", res.Token) + + token, err := jwt.Parse(res.Token, func(token *jwt.Token) (interface{}, error) { + if _, ok := token.Method.(*jwt.SigningMethodECDSA); !ok { + return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) + } + return &jwtPrivateKey.PublicKey, nil + }) + require.NoError(t, err) + + claims := token.Claims.(jwt.MapClaims) + + require.Equal(t, account.Address()+":"+strconv.FormatUint(uint64(memo), 10), claims["sub"]) +} + +func TestToken_successWithMuxedAccount(t *testing.T) { + serverKey := keypair.MustRandom() + t.Logf("Server signing key: %s", serverKey.Address()) + + jwtPrivateKey, err := jwtkey.GenerateKey() + require.NoError(t, err) + jwk := jose.JSONWebKey{Key: jwtPrivateKey, Algorithm: string(jose.ES256)} + + account := keypair.MustRandom() + t.Logf("Stellar account: %s", account.Address()) + + muxedAccount := strkey.MuxedAccount{} + muxedAccount.SetAccountID(account.Address()) + muxedAccount.SetID(1) + muxedAccountAddress, err := muxedAccount.Address() + require.NoError(t, err) + t.Logf("Muxed account: %s", muxedAccountAddress) + + domain := "webauth.example.com" + homeDomain := "example.com" + + tx, err := txnbuild.BuildChallengeTx( + serverKey.Seed(), + muxedAccountAddress, + domain, + homeDomain, + network.TestNetworkPassphrase, + time.Minute, + nil, + ) + require.NoError(t, err) + + chTx, err := tx.Base64() + require.NoError(t, err) + t.Logf("Tx: %s", chTx) + + tx, err = tx.Sign(network.TestNetworkPassphrase, account) + require.NoError(t, err) + txSigned, err := tx.Base64() + require.NoError(t, err) + t.Logf("Signed: %s", txSigned) + + horizonClient := &horizonclient.MockClient{} + horizonClient. + On("AccountDetail", horizonclient.AccountRequest{AccountID: account.Address()}). + Return( + horizon.Account{ + Thresholds: horizon.AccountThresholds{ + LowThreshold: 1, + MedThreshold: 10, + HighThreshold: 100, + }, + Signers: []horizon.Signer{ + { + Key: account.Address(), + Weight: 100, + }, + }}, + nil, + ) + + h := tokenHandler{ + Logger: supportlog.DefaultLogger, + HorizonClient: horizonClient, + NetworkPassphrase: network.TestNetworkPassphrase, + SigningAddresses: []*keypair.FromAddress{serverKey.FromAddress()}, + JWK: jwk, + JWTIssuer: "https://example.com", + JWTExpiresIn: time.Minute, + Domain: domain, + HomeDomains: []string{homeDomain}, + } + + body := struct { + Transaction string `json:"transaction"` + }{ + Transaction: txSigned, + } + bodyBytes, err := json.Marshal(body) + require.NoError(t, err) + r := httptest.NewRequest("POST", "/", bytes.NewReader(bodyBytes)) + r.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + resp := w.Result() + + require.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "application/json; charset=utf-8", resp.Header.Get("Content-Type")) + + res := struct { + Token string `json:"token"` + }{} + err = json.NewDecoder(resp.Body).Decode(&res) + require.NoError(t, err) + + t.Logf("JWT: %s", res.Token) + + token, err := jwt.Parse(res.Token, func(token *jwt.Token) (interface{}, error) { + if _, ok := token.Method.(*jwt.SigningMethodECDSA); !ok { + return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) + } + return &jwtPrivateKey.PublicKey, nil + }) + require.NoError(t, err) + + claims := token.Claims.(jwt.MapClaims) + + require.Equal(t, muxedAccountAddress, claims["sub"]) +} From 396c454f0520f5223d80883dd9ebc16117b6ddc8 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Wed, 25 Jan 2023 17:47:07 -0800 Subject: [PATCH 19/22] add challenge tests --- .../webauth/internal/serve/challenge_test.go | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/exp/services/webauth/internal/serve/challenge_test.go b/exp/services/webauth/internal/serve/challenge_test.go index 9391224763..af406f0750 100644 --- a/exp/services/webauth/internal/serve/challenge_test.go +++ b/exp/services/webauth/internal/serve/challenge_test.go @@ -11,7 +11,9 @@ import ( "github.com/stellar/go/keypair" "github.com/stellar/go/network" + "github.com/stellar/go/strkey" supportlog "github.com/stellar/go/support/log" + "github.com/stellar/go/txnbuild" "github.com/stellar/go/xdr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -185,3 +187,81 @@ func TestChallenge_invalidHomeDomain(t *testing.T) { require.NoError(t, err) assert.JSONEq(t, `{"error":"The request was invalid in some way."}`, string(body)) } + +func TestChallengeWithMemo(t *testing.T) { + serverKey := keypair.MustRandom() + account := keypair.MustRandom() + + h := challengeHandler{ + Logger: supportlog.DefaultLogger, + NetworkPassphrase: network.TestNetworkPassphrase, + SigningKey: serverKey, + ChallengeExpiresIn: time.Minute, + Domain: "webauthdomain", + HomeDomains: []string{"testdomain"}, + } + + r := httptest.NewRequest("GET", "/?account="+account.Address()+"&memo=1", nil) + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + resp := w.Result() + + require.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "application/json; charset=utf-8", resp.Header.Get("Content-Type")) + + res := struct { + Transaction string `json:"transaction"` + NetworkPassphrase string `json:"network_passphrase"` + }{} + err := json.NewDecoder(resp.Body).Decode(&res) + require.NoError(t, err) + + var tx xdr.TransactionEnvelope + err = xdr.SafeUnmarshalBase64(res.Transaction, &tx) + require.NoError(t, err) + + memo, err := txnbuild.MemoID(1).ToXDR() + require.NoError(t, err) + require.Equal(t, tx.Memo(), memo) +} + +func TestChallengeWithMuxedAccount(t *testing.T) { + serverKey := keypair.MustRandom() + account := keypair.MustRandom() + + muxedAccount := strkey.MuxedAccount{} + muxedAccount.SetAccountID(account.Address()) + muxedAccount.SetID(1) + muxedAccountAddress, err := muxedAccount.Address() + require.NoError(t, err) + + h := challengeHandler{ + Logger: supportlog.DefaultLogger, + NetworkPassphrase: network.TestNetworkPassphrase, + SigningKey: serverKey, + ChallengeExpiresIn: time.Minute, + Domain: "webauthdomain", + HomeDomains: []string{"testdomain"}, + } + + r := httptest.NewRequest("GET", "/?account="+muxedAccountAddress, nil) + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + resp := w.Result() + + require.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "application/json; charset=utf-8", resp.Header.Get("Content-Type")) + + res := struct { + Transaction string `json:"transaction"` + NetworkPassphrase string `json:"network_passphrase"` + }{} + err = json.NewDecoder(resp.Body).Decode(&res) + require.NoError(t, err) + + var tx xdr.TransactionEnvelope + err = xdr.SafeUnmarshalBase64(res.Transaction, &tx) + require.NoError(t, err) + + require.Equal(t, tx.Operations()[0].SourceAccount.Address(), muxedAccountAddress) +} From 59b6ae254069bc7c7c134783c276da9632f46d2e Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Thu, 26 Jan 2023 09:54:48 -0800 Subject: [PATCH 20/22] add bad memo test for challenge endpoint --- .../webauth/internal/serve/challenge_test.go | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/exp/services/webauth/internal/serve/challenge_test.go b/exp/services/webauth/internal/serve/challenge_test.go index af406f0750..cd5deb2451 100644 --- a/exp/services/webauth/internal/serve/challenge_test.go +++ b/exp/services/webauth/internal/serve/challenge_test.go @@ -225,6 +225,32 @@ func TestChallengeWithMemo(t *testing.T) { require.Equal(t, tx.Memo(), memo) } +func TestChallengeWithBadMemo(t *testing.T) { + serverKey := keypair.MustRandom() + account := keypair.MustRandom() + + h := challengeHandler{ + Logger: supportlog.DefaultLogger, + NetworkPassphrase: network.TestNetworkPassphrase, + SigningKey: serverKey, + ChallengeExpiresIn: time.Minute, + Domain: "webauthdomain", + HomeDomains: []string{"testdomain"}, + } + + r := httptest.NewRequest("GET", "/?account="+account.Address()+"&memo=test", nil) + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + resp := w.Result() + + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + assert.Equal(t, "application/json; charset=utf-8", resp.Header.Get("Content-Type")) + + body, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + assert.JSONEq(t, `{"error":"The request was invalid in some way."}`, string(body)) +} + func TestChallengeWithMuxedAccount(t *testing.T) { serverKey := keypair.MustRandom() account := keypair.MustRandom() From 6699055909cdf4ecb065092e6d3ffa8396fccd9e Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Thu, 26 Jan 2023 12:25:37 -0800 Subject: [PATCH 21/22] add tests for uncovered conditions --- txnbuild/transaction_test.go | 72 +++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/txnbuild/transaction_test.go b/txnbuild/transaction_test.go index 69c5008b71..f8fce12e9c 100644 --- a/txnbuild/transaction_test.go +++ b/txnbuild/transaction_test.go @@ -1120,6 +1120,12 @@ func TestBuildChallengeTx(t *testing.T) { assert.Equal(t, "testwebauth.stellar.org", string(*webAuthOp.Body.ManageDataOp.DataValue), "DataValue should be 'testwebauth.stellar.org'") } + // transaction with invalid clientAccountID + { + _, err := BuildChallengeTx(kp0.Seed(), "test", "testwebauth.stellar.org", "testanchor.stellar.org", network.TestNetworkPassphrase, time.Minute, nil) + require.EqualError(t, err, "test is not a valid account id or muxed account: invalid address length") + } + // transaction with memo { var memo MemoID = MemoID(1) @@ -2470,7 +2476,7 @@ func TestReadChallengeTx_forbidsFeeBumpTransactions(t *testing.T) { assert.EqualError(t, err, "challenge cannot be a fee bump transaction") } -func TestReadChallengeTx_allowsMuxedAccounts(t *testing.T) { +func TestReadChallengeTx_allowsMuxedAccountsForClientAccountId(t *testing.T) { kp0 := newKeypair0() kp1 := newKeypair1() aid := xdr.MustAddress(kp1.Address()) @@ -2506,6 +2512,70 @@ func TestReadChallengeTx_allowsMuxedAccounts(t *testing.T) { assert.Equal(t, tx.envelope.Operations()[0].SourceAccount, &muxedAccount) } +func TestReadChallengeTransaction_forbidsMuxedTxSourceAccount(t *testing.T) { + kp0 := newKeypair0() + kp1 := newKeypair1() + homeDomain := "testanchor.stellar.org" + webAuthDomain := "testwebauth.stellar.org" + + aid := xdr.MustAddress(kp0.Address()) + muxedAccount := xdr.MuxedAccount{ + Type: xdr.CryptoKeyTypeKeyTypeMuxedEd25519, + Med25519: &xdr.MuxedAccountMed25519{ + Id: 0xcafebabe, + Ed25519: *aid.Ed25519, + }, + } + serverAccount := SimpleAccount{ + AccountID: muxedAccount.Address(), + Sequence: 0, + } + randomNonce, _ := generateRandomNonce(48) + randomNonceToString := base64.StdEncoding.EncodeToString(randomNonce) + currentTime := time.Now().UTC() + maxTime := currentTime.Add(300) + + tx, err := NewTransaction( + TransactionParams{ + SourceAccount: &serverAccount, + IncrementSequenceNum: false, + Operations: []Operation{ + &ManageData{ + SourceAccount: kp1.Address(), + Name: homeDomain + " auth", + Value: []byte(randomNonceToString), + }, + &ManageData{ + SourceAccount: serverAccount.GetAccountID(), + Name: "web_auth_domain", + Value: []byte(webAuthDomain), + }, + }, + BaseFee: MinBaseFee, + Memo: MemoID(1), + Preconditions: Preconditions{ + TimeBounds: NewTimebounds(currentTime.Unix(), maxTime.Unix()), + }, + }, + ) + assert.NoError(t, err) + + tx, err = tx.Sign(network.TestNetworkPassphrase, kp0) + assert.NoError(t, err) + + challenge, err := marshallBase64(tx.ToXDR(), tx.Signatures()) + assert.NoError(t, err) + + _, _, _, _, err = ReadChallengeTx( + challenge, + kp0.Address(), + network.TestNetworkPassphrase, + webAuthDomain, + []string{homeDomain}, + ) + assert.EqualError(t, err, "invalid source account: only valid Ed25519 accounts are allowed in challenge transactions") +} + func TestReadChallengeTransaction_forbidsMemoWithMuxedClientAccount(t *testing.T) { kp0 := newKeypair0() kp1 := newKeypair1() From 0f31bdd2c5f31127d0af2ec3bec9440b2e777445 Mon Sep 17 00:00:00 2001 From: Jake Urban Date: Mon, 30 Jan 2023 09:11:55 -0800 Subject: [PATCH 22/22] change server error to bad request in token endpoint --- exp/services/webauth/internal/serve/token.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exp/services/webauth/internal/serve/token.go b/exp/services/webauth/internal/serve/token.go index e5212503a4..8b8a722526 100644 --- a/exp/services/webauth/internal/serve/token.go +++ b/exp/services/webauth/internal/serve/token.go @@ -90,7 +90,7 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { muxedAccount, err := xdr.AddressToMuxedAccount(clientAccountID) if err != nil { - serverError.Render(w) + badRequest.Render(w) return } if muxedAccount.Type == xdr.CryptoKeyTypeKeyTypeMuxedEd25519 {