From e0823dbced0b499143c2efdd24bb0dc0d4e047c5 Mon Sep 17 00:00:00 2001 From: aureliusbtc <82057759+aureliusbtc@users.noreply.github.com> Date: Mon, 12 Aug 2024 12:31:32 +0100 Subject: [PATCH 1/3] add test for multiple incorrect auth submissions in a row --- services/rfq/api/rest/server.go | 16 ++++++---- services/rfq/api/rest/server_test.go | 47 +++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/services/rfq/api/rest/server.go b/services/rfq/api/rest/server.go index ccd3dc0c6f..e62c48b852 100644 --- a/services/rfq/api/rest/server.go +++ b/services/rfq/api/rest/server.go @@ -278,18 +278,22 @@ func (r *QuoterAPIServer) checkRole(c *gin.Context, destChainID uint32) (address if hasRole == nil || hasRole.IsExpired() { has, roleErr := bridge.HasRole(ops, relayerRole, addressRecovered) - if roleErr == nil { - r.roleCache[destChainID].Set(addressRecovered.Hex(), has, cacheInterval) - } - if roleErr != nil { err = fmt.Errorf("unable to check relayer role on-chain") return addressRecovered, err - } else if !has { - err = fmt.Errorf("q.Relayer not an on-chain relayer") + } + + r.roleCache[destChainID].Set(addressRecovered.Hex(), has, cacheInterval) + + if !has { + err = fmt.Errorf("relayer not an on-chain relayer") return addressRecovered, err } + } else if !hasRole.Value() { + err = fmt.Errorf("relayer not an on-chain relayer") + return addressRecovered, err } + return addressRecovered, nil } diff --git a/services/rfq/api/rest/server_test.go b/services/rfq/api/rest/server_test.go index dac06ae00e..63e1aaaeeb 100644 --- a/services/rfq/api/rest/server_test.go +++ b/services/rfq/api/rest/server_test.go @@ -4,12 +4,13 @@ import ( "bytes" "encoding/json" "fmt" - apiClient "github.com/synapsecns/sanguine/services/rfq/api/client" "io" "net/http" "strconv" "time" + apiClient "github.com/synapsecns/sanguine/services/rfq/api/client" + "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/crypto" "github.com/synapsecns/sanguine/ethergo/signer/wallet" @@ -205,6 +206,50 @@ func (c *ServerSuite) TestPutAndGetQuoteByRelayer() { c.Assert().True(found, "Newly added quote not found") } +func (c *ServerSuite) TestMultiplePutRequestsWithIncorrectAuth() { + // Start the API server in a separate goroutine and wait for it to initialize. + c.startQuoterAPIServer() + + // Create a random wallet for incorrect authorization + randomWallet, err := wallet.FromRandom() + c.Require().NoError(err) + + // Prepare the authorization header with a signed timestamp using the incorrect wallet + header, err := c.prepareAuthHeader(randomWallet) + c.Require().NoError(err) + + // Perform multiple PUT requests to the API server with the incorrect authorization header + for i := 0; i < 3; i++ { + resp, err := c.sendPutQuoteRequest(header) + c.Require().NoError(err) + defer func() { + err = resp.Body.Close() + c.Require().NoError(err) + }() + + // Read the response body + body, err := io.ReadAll(resp.Body) + c.Require().NoError(err) + + // Log the response body for debugging + fmt.Printf("Request %d response: Status: %d, Body: %s\n", i+1, resp.StatusCode, string(body)) + + switch resp.StatusCode { + case http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden: + // These are acceptable error status codes for failed authentication + c.Assert().True(true, "Request %d correctly failed with status %d", i+1, resp.StatusCode) + case http.StatusOK: + // The ModifyQuote method returns 200 OK with an empty body on success + c.Assert().Empty(string(body), "Request %d should return an empty body on success", i+1) + + // Since this shouldn't happen with incorrect auth, fail the test + c.Fail("Request %d unexpectedly succeeded, while submitting incorrect authentication", i+1) + default: + c.Fail("Unexpected status code %d for request %d", resp.StatusCode, i+1) + } + } +} + func (c *ServerSuite) TestPutAck() { c.startQuoterAPIServer() From a468459fa1a977927bc48e1ef090532b664a38a9 Mon Sep 17 00:00:00 2001 From: aureliusbtc <82057759+aureliusbtc@users.noreply.github.com> Date: Mon, 12 Aug 2024 13:00:29 +0100 Subject: [PATCH 2/3] - Separate cache check/population from role verification --- services/rfq/api/rest/server.go | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/services/rfq/api/rest/server.go b/services/rfq/api/rest/server.go index e62c48b852..caeb5a6d32 100644 --- a/services/rfq/api/rest/server.go +++ b/services/rfq/api/rest/server.go @@ -274,24 +274,26 @@ func (r *QuoterAPIServer) checkRole(c *gin.Context, destChainID uint32) (address return addressRecovered, err } - hasRole := r.roleCache[destChainID].Get(addressRecovered.Hex()) + // Check and update cache + cachedRoleItem := r.roleCache[destChainID].Get(addressRecovered.Hex()) + var hasRole bool - if hasRole == nil || hasRole.IsExpired() { - has, roleErr := bridge.HasRole(ops, relayerRole, addressRecovered) - if roleErr != nil { - err = fmt.Errorf("unable to check relayer role on-chain") - return addressRecovered, err + if cachedRoleItem == nil || cachedRoleItem.IsExpired() { + // Cache miss or expired, check on-chain + hasRole, err = bridge.HasRole(ops, relayerRole, addressRecovered) + if err != nil { + return addressRecovered, fmt.Errorf("unable to check relayer role on-chain: %w", err) } + // Update cache + r.roleCache[destChainID].Set(addressRecovered.Hex(), hasRole, cacheInterval) + } else { + // Use cached value + hasRole = cachedRoleItem.Value() + } - r.roleCache[destChainID].Set(addressRecovered.Hex(), has, cacheInterval) - - if !has { - err = fmt.Errorf("relayer not an on-chain relayer") - return addressRecovered, err - } - } else if !hasRole.Value() { - err = fmt.Errorf("relayer not an on-chain relayer") - return addressRecovered, err + // Verify role + if !hasRole { + return addressRecovered, fmt.Errorf("relayer not an on-chain relayer") } return addressRecovered, nil From 9422370254675eb045a8661c4bea10c2d11f6866 Mon Sep 17 00:00:00 2001 From: aureliusbtc <82057759+aureliusbtc@users.noreply.github.com> Date: Mon, 12 Aug 2024 13:02:31 +0100 Subject: [PATCH 3/3] rm duplicate import --- services/rfq/api/rest/server_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/rfq/api/rest/server_test.go b/services/rfq/api/rest/server_test.go index 8027a2f784..9fc48ddb51 100644 --- a/services/rfq/api/rest/server_test.go +++ b/services/rfq/api/rest/server_test.go @@ -13,8 +13,6 @@ import ( "github.com/synapsecns/sanguine/services/rfq/api/db" "github.com/synapsecns/sanguine/services/rfq/api/rest" - apiClient "github.com/synapsecns/sanguine/services/rfq/api/client" - "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/crypto" "github.com/synapsecns/sanguine/ethergo/signer/wallet"