From 08ff1056d774433b6f72935751941aa993ba267a Mon Sep 17 00:00:00 2001 From: alfred-mk Date: Tue, 26 Nov 2024 07:24:57 +0100 Subject: [PATCH] Validate aliases, addresses and phone numbers in the send menu (#176) - Update the phone number regex - Check whether the recipient is a valid phone number, alias or address Reviewed-on: https://git.grassecon.net/urdt/ussd/pulls/176 Co-authored-by: alfred-mk Co-committed-by: alfred-mk --- cmd/africastalking/main.go | 9 +- common/recipient.go | 73 +++++++++++++ config/config.go | 3 + internal/handlers/ussd/menuhandler.go | 102 ++++++++++++------ internal/testutil/mocks/servicemock.go | 5 + .../testservice/TestAccountService.go | 8 +- menutraversal_test/test_setup.json | 4 +- remote/accountservice.go | 21 ++++ services/registration/send | 2 +- 9 files changed, 186 insertions(+), 41 deletions(-) create mode 100644 common/recipient.go diff --git a/cmd/africastalking/main.go b/cmd/africastalking/main.go index 7eb0497..7d1caa8 100644 --- a/cmd/africastalking/main.go +++ b/cmd/africastalking/main.go @@ -20,6 +20,7 @@ import ( "git.defalsify.org/vise.git/logging" "git.defalsify.org/vise.git/resource" + "git.grassecon.net/urdt/ussd/common" "git.grassecon.net/urdt/ussd/config" "git.grassecon.net/urdt/ussd/initializers" "git.grassecon.net/urdt/ussd/internal/handlers" @@ -76,7 +77,13 @@ func (arp *atRequestParser) GetSessionId(rq any) (string, error) { return "", fmt.Errorf("no phone number found") } - return phoneNumber, nil + formattedNumber, err := common.FormatPhoneNumber(phoneNumber) + if err != nil { + fmt.Printf("Error: %v\n", err) + return "", fmt.Errorf("failed to format number") + } + + return formattedNumber, nil } func (arp *atRequestParser) GetInput(rq any) ([]byte, error) { diff --git a/common/recipient.go b/common/recipient.go new file mode 100644 index 0000000..f463a32 --- /dev/null +++ b/common/recipient.go @@ -0,0 +1,73 @@ +package common + +import ( + "errors" + "fmt" + "regexp" + "strings" +) + +// Define the regex patterns as constants +const ( + phoneRegex = `^(?:\+254|254|0)?((?:7[0-9]{8})|(?:1[01][0-9]{7}))$` + addressRegex = `^0x[a-fA-F0-9]{40}$` + aliasRegex = `^[a-zA-Z0-9]+$` +) + +// IsValidPhoneNumber checks if the given number is a valid phone number +func IsValidPhoneNumber(phonenumber string) bool { + match, _ := regexp.MatchString(phoneRegex, phonenumber) + return match +} + +// IsValidAddress checks if the given address is a valid Ethereum address +func IsValidAddress(address string) bool { + match, _ := regexp.MatchString(addressRegex, address) + return match +} + +// IsValidAlias checks if the alias is a valid alias format +func IsValidAlias(alias string) bool { + match, _ := regexp.MatchString(aliasRegex, alias) + return match +} + +// CheckRecipient validates the recipient format based on the criteria +func CheckRecipient(recipient string) (string, error) { + if IsValidPhoneNumber(recipient) { + return "phone number", nil + } + + if IsValidAddress(recipient) { + return "address", nil + } + + if IsValidAlias(recipient) { + return "alias", nil + } + + return "", fmt.Errorf("invalid recipient: must be a phone number, address or alias") +} + +// FormatPhoneNumber formats a Kenyan phone number to "+254xxxxxxxx". +func FormatPhoneNumber(phone string) (string, error) { + if !IsValidPhoneNumber(phone) { + return "", errors.New("invalid phone number") + } + + // Remove any leading "+" and spaces + phone = strings.TrimPrefix(phone, "+") + phone = strings.ReplaceAll(phone, " ", "") + + // Replace leading "0" with "254" if present + if strings.HasPrefix(phone, "0") { + phone = "254" + phone[1:] + } + + // Add "+" if not already present + if !strings.HasPrefix(phone, "254") { + return "", errors.New("unexpected format") + } + + return "+" + phone, nil +} diff --git a/config/config.go b/config/config.go index edb33fe..3a8e8ed 100644 --- a/config/config.go +++ b/config/config.go @@ -15,6 +15,7 @@ const ( voucherHoldingsPathPrefix = "/api/v1/holdings" voucherTransfersPathPrefix = "/api/v1/transfers/last10" voucherDataPathPrefix = "/api/v1/token" + AliasPrefix = "api/v1/alias" ) var ( @@ -32,6 +33,7 @@ var ( VoucherHoldingsURL string VoucherTransfersURL string VoucherDataURL string + CheckAliasURL string ) func setBase() error { @@ -66,6 +68,7 @@ func LoadConfig() error { VoucherHoldingsURL, _ = url.JoinPath(dataURLBase, voucherHoldingsPathPrefix) VoucherTransfersURL, _ = url.JoinPath(dataURLBase, voucherTransfersPathPrefix) VoucherDataURL, _ = url.JoinPath(dataURLBase, voucherDataPathPrefix) + CheckAliasURL, _ = url.JoinPath(dataURLBase, AliasPrefix) return nil } diff --git a/internal/handlers/ussd/menuhandler.go b/internal/handlers/ussd/menuhandler.go index 065da85..8d3b928 100644 --- a/internal/handlers/ussd/menuhandler.go +++ b/internal/handlers/ussd/menuhandler.go @@ -35,12 +35,17 @@ var ( errResponse *api.ErrResponse ) -// Define the regex patterns as constants +// Define the regex patterns as constants const ( - phoneRegex = `(\(\d{3}\)\s?|\d{3}[-.\s]?)?\d{3}[-.\s]?\d{4}` pinPattern = `^\d{4}$` ) +// isValidPIN checks whether the given input is a 4 digit number +func isValidPIN(pin string) bool { + match, _ := regexp.MatchString(pinPattern, pin) + return match +} + // FlagManager handles centralized flag management type FlagManager struct { parser *asm.FlagParser @@ -95,17 +100,6 @@ func NewHandlers(appFlags *asm.FlagParser, userdataStore db.Db, adminstore *util return h, nil } -// isValidPIN checks whether the given input is a 4 digit number -func isValidPIN(pin string) bool { - match, _ := regexp.MatchString(pinPattern, pin) - return match -} - -func isValidPhoneNumber(phonenumber string) bool { - match, _ := regexp.MatchString(phoneRegex, phonenumber) - return match -} - func (h *Handlers) WithPersister(pe *persist.Persister) *Handlers { if h.pe != nil { panic("persister already set") @@ -877,7 +871,7 @@ func (h *Handlers) ValidateBlockedNumber(ctx context.Context, sym string, input } blockedNumber := string(input) _, err = store.ReadEntry(ctx, blockedNumber, common.DATA_PUBLIC_KEY) - if !isValidPhoneNumber(blockedNumber) { + if !common.IsValidPhoneNumber(blockedNumber) { res.FlagSet = append(res.FlagSet, flag_unregistered_number) return res, nil } @@ -898,10 +892,9 @@ func (h *Handlers) ValidateBlockedNumber(ctx context.Context, sym string, input return res, nil } -// ValidateRecipient validates that the given input is a valid phone number. +// ValidateRecipient validates that the given input is valid. func (h *Handlers) ValidateRecipient(ctx context.Context, sym string, input []byte) (resource.Result, error) { var res resource.Result - var err error store := h.userdataStore sessionId, ok := ctx.Value("SessionId").(string) @@ -909,13 +902,16 @@ func (h *Handlers) ValidateRecipient(ctx context.Context, sym string, input []by return res, fmt.Errorf("missing session") } - recipient := string(input) - flag_invalid_recipient, _ := h.flagManager.GetFlag("flag_invalid_recipient") flag_invalid_recipient_with_invite, _ := h.flagManager.GetFlag("flag_invalid_recipient_with_invite") + flag_api_error, _ := h.flagManager.GetFlag("flag_api_call_error") + + recipient := string(input) if recipient != "0" { - if !isValidPhoneNumber(recipient) { + recipientType, err := common.CheckRecipient(recipient) + if err != nil { + // Invalid recipient format (not a phone number, address, or valid alias format) res.FlagSet = append(res.FlagSet, flag_invalid_recipient) res.Content = recipient @@ -929,25 +925,61 @@ func (h *Handlers) ValidateRecipient(ctx context.Context, sym string, input []by return res, err } - publicKey, err := store.ReadEntry(ctx, recipient, common.DATA_PUBLIC_KEY) - if err != nil { - if db.IsNotFound(err) { - logg.InfoCtxf(ctx, "Unregistered number") - - res.FlagSet = append(res.FlagSet, flag_invalid_recipient_with_invite) - res.Content = recipient - - return res, nil + switch recipientType { + case "phone number": + // format the phone number + formattedNumber, err := common.FormatPhoneNumber(recipient) + if err != nil { + logg.ErrorCtxf(ctx, "Failed to format the phone number: %s", recipient, "error", err) + return res, err } - logg.ErrorCtxf(ctx, "failed to read publicKey entry with", "key", common.DATA_PUBLIC_KEY, "error", err) - return res, err - } + // Check if the phone number is registered + publicKey, err := store.ReadEntry(ctx, formattedNumber, common.DATA_PUBLIC_KEY) + if err != nil { + if db.IsNotFound(err) { + logg.InfoCtxf(ctx, "Unregistered phone number: %s", recipient) + res.FlagSet = append(res.FlagSet, flag_invalid_recipient_with_invite) + res.Content = recipient + return res, nil + } - err = store.WriteEntry(ctx, sessionId, common.DATA_RECIPIENT, publicKey) - if err != nil { - logg.ErrorCtxf(ctx, "failed to write recipient entry with", "key", common.DATA_RECIPIENT, "value", string(publicKey), "error", err) - return res, nil + logg.ErrorCtxf(ctx, "failed to read publicKey entry with", "key", common.DATA_PUBLIC_KEY, "error", err) + return res, err + } + + // Save the publicKey as the recipient + err = store.WriteEntry(ctx, sessionId, common.DATA_RECIPIENT, publicKey) + if err != nil { + logg.ErrorCtxf(ctx, "failed to write recipient entry with", "key", common.DATA_RECIPIENT, "value", string(publicKey), "error", err) + return res, err + } + + case "address": + // Save the valid Ethereum address as the recipient + err = store.WriteEntry(ctx, sessionId, common.DATA_RECIPIENT, []byte(recipient)) + if err != nil { + logg.ErrorCtxf(ctx, "failed to write recipient entry with", "key", common.DATA_RECIPIENT, "value", recipient, "error", err) + return res, err + } + + case "alias": + // Call the API to validate and retrieve the address for the alias + r, aliasErr := h.accountService.CheckAliasAddress(ctx, recipient) + if aliasErr != nil { + res.FlagSet = append(res.FlagSet, flag_api_error) + res.Content = recipient + + logg.ErrorCtxf(ctx, "failed on CheckAliasAddress", "error", aliasErr) + return res, err + } + + // Alias validation succeeded, save the Ethereum address + err = store.WriteEntry(ctx, sessionId, common.DATA_RECIPIENT, []byte(r.Address)) + if err != nil { + logg.ErrorCtxf(ctx, "failed to write recipient entry with", "key", common.DATA_RECIPIENT, "value", r.Address, "error", err) + return res, err + } } } diff --git a/internal/testutil/mocks/servicemock.go b/internal/testutil/mocks/servicemock.go index a25f1ae..521cfa0 100644 --- a/internal/testutil/mocks/servicemock.go +++ b/internal/testutil/mocks/servicemock.go @@ -47,3 +47,8 @@ func (m *MockAccountService) TokenTransfer(ctx context.Context, amount, from, to args := m.Called() return args.Get(0).(*models.TokenTransferResponse), args.Error(1) } + +func (m *MockAccountService) CheckAliasAddress(ctx context.Context, alias string) (*dataserviceapi.AliasAddress, error) { + args := m.Called() + return args.Get(0).(*dataserviceapi.AliasAddress), args.Error(1) +} diff --git a/internal/testutil/testservice/TestAccountService.go b/internal/testutil/testservice/TestAccountService.go index 7c486f3..96dacbc 100644 --- a/internal/testutil/testservice/TestAccountService.go +++ b/internal/testutil/testservice/TestAccountService.go @@ -33,8 +33,8 @@ func (tas *TestAccountService) TrackAccountStatus(ctx context.Context, publicKey } func (tas *TestAccountService) FetchVouchers(ctx context.Context, publicKey string) ([]dataserviceapi.TokenHoldings, error) { - return []dataserviceapi.TokenHoldings { - dataserviceapi.TokenHoldings { + return []dataserviceapi.TokenHoldings{ + dataserviceapi.TokenHoldings{ ContractAddress: "0x6CC75A06ac72eB4Db2eE22F781F5D100d8ec03ee", TokenSymbol: "SRF", TokenDecimals: "6", @@ -56,3 +56,7 @@ func (tas *TestAccountService) TokenTransfer(ctx context.Context, amount, from, TrackingId: "e034d147-747d-42ea-928d-b5a7cb3426af", }, nil } + +func (m TestAccountService) CheckAliasAddress(ctx context.Context, alias string) (*dataserviceapi.AliasAddress, error) { + return &dataserviceapi.AliasAddress{}, nil +} diff --git a/menutraversal_test/test_setup.json b/menutraversal_test/test_setup.json index a554939..4d8414f 100644 --- a/menutraversal_test/test_setup.json +++ b/menutraversal_test/test_setup.json @@ -61,7 +61,7 @@ }, { "input": "1", - "expectedContent": "Enter recipient's phone number:\n0:Back" + "expectedContent": "Enter recipient's phone number/address/alias:\n0:Back" }, { "input": "000", @@ -69,7 +69,7 @@ }, { "input": "1", - "expectedContent": "Enter recipient's phone number:\n0:Back" + "expectedContent": "Enter recipient's phone number/address/alias:\n0:Back" }, { "input": "0712345678", diff --git a/remote/accountservice.go b/remote/accountservice.go index 23b62ca..b0e9eb4 100644 --- a/remote/accountservice.go +++ b/remote/accountservice.go @@ -24,6 +24,7 @@ type AccountServiceInterface interface { FetchTransactions(ctx context.Context, publicKey string) ([]dataserviceapi.Last10TxResponse, error) VoucherData(ctx context.Context, address string) (*models.VoucherDataResult, error) TokenTransfer(ctx context.Context, amount, from, to, tokenAddress string) (*models.TokenTransferResponse, error) + CheckAliasAddress(ctx context.Context, alias string) (*dataserviceapi.AliasAddress, error) } type AccountService struct { @@ -209,6 +210,26 @@ func (as *AccountService) TokenTransfer(ctx context.Context, amount, from, to, t return &r, nil } +// CheckAliasAddress retrieves the address of an alias from the API endpoint. +// Parameters: +// - alias: The alias of the user. +func (as *AccountService) CheckAliasAddress(ctx context.Context, alias string) (*dataserviceapi.AliasAddress, error) { + var r dataserviceapi.AliasAddress + + ep, err := url.JoinPath(config.CheckAliasURL, alias) + if err != nil { + return nil, err + } + + req, err := http.NewRequest("GET", ep, nil) + if err != nil { + return nil, err + } + + _, err = doRequest(ctx, req, &r) + return &r, err +} + func doRequest(ctx context.Context, req *http.Request, rcpt any) (*api.OKResponse, error) { var okResponse api.OKResponse var errResponse api.ErrResponse diff --git a/services/registration/send b/services/registration/send index d124026..306466c 100644 --- a/services/registration/send +++ b/services/registration/send @@ -1 +1 @@ -Enter recipient's phone number: \ No newline at end of file +Enter recipient's phone number/address/alias: \ No newline at end of file