Validate aliases, addresses and phone numbers in the send menu #176

Merged
kamikazechaser merged 14 commits from alias-address-validation into master 2024-11-26 07:24:57 +01:00
9 changed files with 186 additions and 41 deletions

View File

@ -20,6 +20,7 @@ import (
"git.defalsify.org/vise.git/logging" "git.defalsify.org/vise.git/logging"
"git.defalsify.org/vise.git/resource" "git.defalsify.org/vise.git/resource"
"git.grassecon.net/urdt/ussd/common"
"git.grassecon.net/urdt/ussd/config" "git.grassecon.net/urdt/ussd/config"
"git.grassecon.net/urdt/ussd/initializers" "git.grassecon.net/urdt/ussd/initializers"
"git.grassecon.net/urdt/ussd/internal/handlers" "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 "", 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) { func (arp *atRequestParser) GetInput(rq any) ([]byte, error) {

73
common/recipient.go Normal file
View File

@ -0,0 +1,73 @@
package common
import (
"errors"
"fmt"
"regexp"
"strings"
)
// Define the regex patterns as constants
Review

I think phone regex should be check formatting but not specifics; that is, country code should be filtered elsewhere.

I think phone regex should be check formatting but not specifics; that is, country code should be filtered elsewhere.
const (
phoneRegex = `^(?:\+254|254|0)?((?:7[0-9]{8})|(?:1[01][0-9]{7}))$`
kamikazechaser marked this conversation as resolved
Review

alias is same format as email? May want to have a look at https://emailregex.com/

alias is same format as email? May want to have a look at https://emailregex.com/
Review

No, it is not. This is the format that I picked from Sarafu's website

Your shortcode is a unique identifier that can be used by others to send you vouchers. It must be in the following format name@area

No, it is not. This is the format that I picked from Sarafu's website `Your shortcode is a unique identifier that can be used by others to send you vouchers. It must be in the following format name@area`
Review
@Alfred-mk See https://github.com/grassrootseconomics/sarafu.network/issues/174 for Alias requirements
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".
kamikazechaser marked this conversation as resolved Outdated

It should be + prefixed. Why? That is how we save it in our common db (graph) and also how most, if not all, API providers expect the phone number.

It should be + prefixed. Why? That is how we save it in our common db (graph) and also how most, if not all, API providers expect the phone number.

Thanks, this has been updated

Thanks, this has been updated
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
}

View File

@ -15,6 +15,7 @@ const (
voucherHoldingsPathPrefix = "/api/v1/holdings" voucherHoldingsPathPrefix = "/api/v1/holdings"
voucherTransfersPathPrefix = "/api/v1/transfers/last10" voucherTransfersPathPrefix = "/api/v1/transfers/last10"
voucherDataPathPrefix = "/api/v1/token" voucherDataPathPrefix = "/api/v1/token"
AliasPrefix = "api/v1/alias"
) )
var ( var (
@ -32,6 +33,7 @@ var (
VoucherHoldingsURL string VoucherHoldingsURL string
VoucherTransfersURL string VoucherTransfersURL string
VoucherDataURL string VoucherDataURL string
CheckAliasURL string
) )
func setBase() error { func setBase() error {
@ -66,6 +68,7 @@ func LoadConfig() error {
VoucherHoldingsURL, _ = url.JoinPath(dataURLBase, voucherHoldingsPathPrefix) VoucherHoldingsURL, _ = url.JoinPath(dataURLBase, voucherHoldingsPathPrefix)
VoucherTransfersURL, _ = url.JoinPath(dataURLBase, voucherTransfersPathPrefix) VoucherTransfersURL, _ = url.JoinPath(dataURLBase, voucherTransfersPathPrefix)
VoucherDataURL, _ = url.JoinPath(dataURLBase, voucherDataPathPrefix) VoucherDataURL, _ = url.JoinPath(dataURLBase, voucherDataPathPrefix)
CheckAliasURL, _ = url.JoinPath(dataURLBase, AliasPrefix)
return nil return nil
} }

View File

@ -35,12 +35,17 @@ var (
errResponse *api.ErrResponse errResponse *api.ErrResponse
) )
// Define the regex patterns as constants // Define the regex patterns as constants
const ( const (
phoneRegex = `(\(\d{3}\)\s?|\d{3}[-.\s]?)?\d{3}[-.\s]?\d{4}`
pinPattern = `^\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 // FlagManager handles centralized flag management
type FlagManager struct { type FlagManager struct {
parser *asm.FlagParser parser *asm.FlagParser
@ -95,17 +100,6 @@ func NewHandlers(appFlags *asm.FlagParser, userdataStore db.Db, adminstore *util
return h, nil 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 { func (h *Handlers) WithPersister(pe *persist.Persister) *Handlers {
if h.pe != nil { if h.pe != nil {
panic("persister already set") panic("persister already set")
@ -877,7 +871,7 @@ func (h *Handlers) ValidateBlockedNumber(ctx context.Context, sym string, input
} }
blockedNumber := string(input) blockedNumber := string(input)
_, err = store.ReadEntry(ctx, blockedNumber, common.DATA_PUBLIC_KEY) _, err = store.ReadEntry(ctx, blockedNumber, common.DATA_PUBLIC_KEY)
if !isValidPhoneNumber(blockedNumber) { if !common.IsValidPhoneNumber(blockedNumber) {
res.FlagSet = append(res.FlagSet, flag_unregistered_number) res.FlagSet = append(res.FlagSet, flag_unregistered_number)
return res, nil return res, nil
} }
@ -898,10 +892,9 @@ func (h *Handlers) ValidateBlockedNumber(ctx context.Context, sym string, input
return res, nil 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) { func (h *Handlers) ValidateRecipient(ctx context.Context, sym string, input []byte) (resource.Result, error) {
var res resource.Result var res resource.Result
var err error
store := h.userdataStore store := h.userdataStore
sessionId, ok := ctx.Value("SessionId").(string) 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") return res, fmt.Errorf("missing session")
} }
recipient := string(input)
flag_invalid_recipient, _ := h.flagManager.GetFlag("flag_invalid_recipient") flag_invalid_recipient, _ := h.flagManager.GetFlag("flag_invalid_recipient")
flag_invalid_recipient_with_invite, _ := h.flagManager.GetFlag("flag_invalid_recipient_with_invite") 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 recipient != "0" {
Review

when is recipient "0" ?

when is recipient "0" ?
Review

This is related to #126 (comment)

This is related to https://git.grassecon.net/urdt/ussd/pulls/126#issuecomment-2709
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.FlagSet = append(res.FlagSet, flag_invalid_recipient)
res.Content = recipient res.Content = recipient
@ -929,25 +925,61 @@ func (h *Handlers) ValidateRecipient(ctx context.Context, sym string, input []by
return res, err return res, err
} }
publicKey, err := store.ReadEntry(ctx, recipient, common.DATA_PUBLIC_KEY) switch recipientType {
if err != nil { case "phone number":
if db.IsNotFound(err) { // format the phone number
logg.InfoCtxf(ctx, "Unregistered number") formattedNumber, err := common.FormatPhoneNumber(recipient)
if err != nil {
res.FlagSet = append(res.FlagSet, flag_invalid_recipient_with_invite) logg.ErrorCtxf(ctx, "Failed to format the phone number: %s", recipient, "error", err)
res.Content = recipient return res, err
return res, nil
} }
logg.ErrorCtxf(ctx, "failed to read publicKey entry with", "key", common.DATA_PUBLIC_KEY, "error", err) // Check if the phone number is registered
return res, err 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) logg.ErrorCtxf(ctx, "failed to read publicKey entry with", "key", common.DATA_PUBLIC_KEY, "error", err)
if err != nil { return res, err
logg.ErrorCtxf(ctx, "failed to write recipient entry with", "key", common.DATA_RECIPIENT, "value", string(publicKey), "error", err) }
return res, nil
// 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
}
} }
} }

View File

@ -47,3 +47,8 @@ func (m *MockAccountService) TokenTransfer(ctx context.Context, amount, from, to
args := m.Called() args := m.Called()
return args.Get(0).(*models.TokenTransferResponse), args.Error(1) 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)
}

View File

@ -33,8 +33,8 @@ func (tas *TestAccountService) TrackAccountStatus(ctx context.Context, publicKey
} }
func (tas *TestAccountService) FetchVouchers(ctx context.Context, publicKey string) ([]dataserviceapi.TokenHoldings, error) { func (tas *TestAccountService) FetchVouchers(ctx context.Context, publicKey string) ([]dataserviceapi.TokenHoldings, error) {
return []dataserviceapi.TokenHoldings { return []dataserviceapi.TokenHoldings{
dataserviceapi.TokenHoldings { dataserviceapi.TokenHoldings{
ContractAddress: "0x6CC75A06ac72eB4Db2eE22F781F5D100d8ec03ee", ContractAddress: "0x6CC75A06ac72eB4Db2eE22F781F5D100d8ec03ee",
TokenSymbol: "SRF", TokenSymbol: "SRF",
TokenDecimals: "6", TokenDecimals: "6",
@ -56,3 +56,7 @@ func (tas *TestAccountService) TokenTransfer(ctx context.Context, amount, from,
TrackingId: "e034d147-747d-42ea-928d-b5a7cb3426af", TrackingId: "e034d147-747d-42ea-928d-b5a7cb3426af",
}, nil }, nil
} }
func (m TestAccountService) CheckAliasAddress(ctx context.Context, alias string) (*dataserviceapi.AliasAddress, error) {
return &dataserviceapi.AliasAddress{}, nil
}

View File

@ -61,7 +61,7 @@
}, },
{ {
"input": "1", "input": "1",
"expectedContent": "Enter recipient's phone number:\n0:Back" "expectedContent": "Enter recipient's phone number/address/alias:\n0:Back"
}, },
{ {
"input": "000", "input": "000",
@ -69,7 +69,7 @@
}, },
{ {
"input": "1", "input": "1",
"expectedContent": "Enter recipient's phone number:\n0:Back" "expectedContent": "Enter recipient's phone number/address/alias:\n0:Back"
}, },
{ {
"input": "0712345678", "input": "0712345678",

View File

@ -24,6 +24,7 @@ type AccountServiceInterface interface {
FetchTransactions(ctx context.Context, publicKey string) ([]dataserviceapi.Last10TxResponse, error) FetchTransactions(ctx context.Context, publicKey string) ([]dataserviceapi.Last10TxResponse, error)
VoucherData(ctx context.Context, address string) (*models.VoucherDataResult, error) VoucherData(ctx context.Context, address string) (*models.VoucherDataResult, error)
TokenTransfer(ctx context.Context, amount, from, to, tokenAddress string) (*models.TokenTransferResponse, 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 { type AccountService struct {
@ -209,6 +210,26 @@ func (as *AccountService) TokenTransfer(ctx context.Context, amount, from, to, t
return &r, nil 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) { func doRequest(ctx context.Context, req *http.Request, rcpt any) (*api.OKResponse, error) {
var okResponse api.OKResponse var okResponse api.OKResponse
var errResponse api.ErrResponse var errResponse api.ErrResponse

View File

@ -1 +1 @@
Enter recipient's phone number: Enter recipient's phone number/address/alias: