Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SDP-962] Change SEP-24 Flow to display different verifications based on Disbursement's verification type #116

Merged
merged 9 commits into from
Dec 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/data/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func NewModels(dbConnectionPool db.DBConnectionPool) (*Models, error) {
Payment: &PaymentModel{dbConnectionPool: dbConnectionPool},
Receiver: &ReceiverModel{},
DisbursementInstructions: NewDisbursementInstructionModel(dbConnectionPool),
ReceiverVerification: &ReceiverVerificationModel{},
ReceiverVerification: &ReceiverVerificationModel{dbConnectionPool: dbConnectionPool},
ReceiverWallet: &ReceiverWalletModel{dbConnectionPool: dbConnectionPool},
DisbursementReceivers: &DisbursementReceiverModel{dbConnectionPool: dbConnectionPool},
Message: &MessageModel{dbConnectionPool: dbConnectionPool},
Expand Down
44 changes: 37 additions & 7 deletions internal/data/receiver_verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package data

import (
"context"
"database/sql"
"errors"
"fmt"
"strings"
"time"
Expand All @@ -24,7 +26,9 @@ type ReceiverVerification struct {
FailedAt *time.Time `db:"failed_at"`
}

type ReceiverVerificationModel struct{}
type ReceiverVerificationModel struct {
dbConnectionPool db.DBConnectionPool
}

type ReceiverVerificationInsert struct {
ReceiverID string `db:"receiver_id"`
Expand All @@ -48,7 +52,7 @@ func (rvi *ReceiverVerificationInsert) Validate() error {
}

// GetByReceiverIDsAndVerificationField returns receiver verifications by receiver IDs and verification type.
func (m ReceiverVerificationModel) GetByReceiverIDsAndVerificationField(ctx context.Context, sqlExec db.SQLExecuter, receiverIds []string, verificationField VerificationField) ([]*ReceiverVerification, error) {
func (m *ReceiverVerificationModel) GetByReceiverIDsAndVerificationField(ctx context.Context, sqlExec db.SQLExecuter, receiverIds []string, verificationField VerificationField) ([]*ReceiverVerification, error) {
receiverVerifications := []*ReceiverVerification{}
query := `
SELECT
Expand All @@ -74,7 +78,7 @@ func (m ReceiverVerificationModel) GetByReceiverIDsAndVerificationField(ctx cont
}

// GetAllByReceiverId returns all receiver verifications by receiver id.
func (m ReceiverVerificationModel) GetAllByReceiverId(ctx context.Context, sqlExec db.SQLExecuter, receiverId string) ([]ReceiverVerification, error) {
func (m *ReceiverVerificationModel) GetAllByReceiverId(ctx context.Context, sqlExec db.SQLExecuter, receiverId string) ([]ReceiverVerification, error) {
receiverVerifications := []ReceiverVerification{}
query := `
SELECT
Expand All @@ -91,8 +95,35 @@ func (m ReceiverVerificationModel) GetAllByReceiverId(ctx context.Context, sqlEx
return receiverVerifications, nil
}

// GetLatestByPhoneNumber returns the latest updated receiver verification for some receiver that is associated with a phone number.
func (m *ReceiverVerificationModel) GetLatestByPhoneNumber(ctx context.Context, phoneNumber string) (*ReceiverVerification, error) {
receiverVerification := ReceiverVerification{}
query := `
SELECT
rv.*
FROM
receiver_verifications rv
JOIN receivers r ON rv.receiver_id = r.id
WHERE
r.phone_number = $1
ORDER BY
rv.updated_at DESC
LIMIT 1
`

err := m.dbConnectionPool.GetContext(ctx, &receiverVerification, query, phoneNumber)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, ErrRecordNotFound
}
return nil, fmt.Errorf("fetching receiver verifications for phone number %s: %w", phoneNumber, err)
}

return &receiverVerification, nil
}

// Insert inserts a new receiver verification
func (m ReceiverVerificationModel) Insert(ctx context.Context, sqlExec db.SQLExecuter, verificationInsert ReceiverVerificationInsert) (string, error) {
func (m *ReceiverVerificationModel) Insert(ctx context.Context, sqlExec db.SQLExecuter, verificationInsert ReceiverVerificationInsert) (string, error) {
err := verificationInsert.Validate()
if err != nil {
return "", fmt.Errorf("error validating receiver verification insert: %w", err)
Expand All @@ -111,7 +142,6 @@ func (m ReceiverVerificationModel) Insert(ctx context.Context, sqlExec db.SQLExe
`

_, err = sqlExec.ExecContext(ctx, query, verificationInsert.ReceiverID, verificationInsert.VerificationField, hashedValue)

if err != nil {
return "", fmt.Errorf("error inserting receiver verification: %w", err)
}
Expand All @@ -120,7 +150,7 @@ func (m ReceiverVerificationModel) Insert(ctx context.Context, sqlExec db.SQLExe
}

// UpdateVerificationValue updates the hashed value of a receiver verification.
func (m ReceiverVerificationModel) UpdateVerificationValue(ctx context.Context,
func (m *ReceiverVerificationModel) UpdateVerificationValue(ctx context.Context,
sqlExec db.SQLExecuter,
receiverID string,
verificationField VerificationField,
Expand Down Expand Up @@ -148,7 +178,7 @@ func (m ReceiverVerificationModel) UpdateVerificationValue(ctx context.Context,
}

// UpdateVerificationValue updates the hashed value of a receiver verification.
func (m ReceiverVerificationModel) UpdateReceiverVerification(ctx context.Context, receiverVerification ReceiverVerification, sqlExec db.SQLExecuter) error {
func (m *ReceiverVerificationModel) UpdateReceiverVerification(ctx context.Context, receiverVerification ReceiverVerification, sqlExec db.SQLExecuter) error {
query := `
UPDATE
receiver_verifications
Expand Down
58 changes: 58 additions & 0 deletions internal/data/receiver_verification_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package data

import (
"context"
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -125,6 +126,63 @@ func Test_ReceiverVerificationModel_GetAllByReceiverId(t *testing.T) {
})
}

func Test_ReceiverVerificationModel_GetReceiverVerificationByReceiverId(t *testing.T) {
dbt := dbtest.Open(t)
defer dbt.Close()

dbConnectionPool, err := db.OpenDBConnectionPool(dbt.DSN)
require.NoError(t, err)
defer dbConnectionPool.Close()

ctx := context.Background()

receiver := CreateReceiverFixture(t, ctx, dbConnectionPool, &Receiver{
PhoneNumber: "+13334445555",
})

t.Run("returns error when the receiver has no verifications registered", func(t *testing.T) {
receiverVerificationModel := ReceiverVerificationModel{dbConnectionPool: dbConnectionPool}
_, err := receiverVerificationModel.GetLatestByPhoneNumber(ctx, receiver.PhoneNumber)
require.Error(t, err, fmt.Errorf("cannot query any receiver verifications for phone number %s", receiver.PhoneNumber))
})

t.Run("returns the latest receiver verification for a list of receiver verifications", func(t *testing.T) {
earlierTime := time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)
verification1 := CreateReceiverVerificationFixture(t, ctx, dbConnectionPool, ReceiverVerificationInsert{
ReceiverID: receiver.ID,
VerificationField: VerificationFieldDateOfBirth,
VerificationValue: "1990-01-01",
})
verification1.UpdatedAt = earlierTime

verification2 := CreateReceiverVerificationFixture(t, ctx, dbConnectionPool, ReceiverVerificationInsert{
ReceiverID: receiver.ID,
VerificationField: VerificationFieldPin,
VerificationValue: "1234",
})
verification2.UpdatedAt = earlierTime

verification3 := CreateReceiverVerificationFixture(t, ctx, dbConnectionPool, ReceiverVerificationInsert{
ReceiverID: receiver.ID,
VerificationField: VerificationFieldNationalID,
VerificationValue: "5678",
})

receiverVerificationModel := ReceiverVerificationModel{dbConnectionPool: dbConnectionPool}
actualVerification, err := receiverVerificationModel.GetLatestByPhoneNumber(ctx, receiver.PhoneNumber)
require.NoError(t, err)

assert.Equal(t,
ReceiverVerification{
ReceiverID: receiver.ID,
VerificationField: VerificationFieldNationalID,
HashedValue: verification3.HashedValue,
CreatedAt: verification3.CreatedAt,
UpdatedAt: verification3.UpdatedAt,
}, *actualVerification)
})
}

func Test_ReceiverVerificationModel_Insert(t *testing.T) {
dbt := dbtest.Open(t)
defer dbt.Close()
Expand Down
7 changes: 2 additions & 5 deletions internal/htmltemplate/tmpl/receiver_register.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
</div>
</section>

<!-- Enter passcode and DOB page -->
<!-- Enter passcode and verification field page -->
<section data-section="passcode" style="display: none">
<div class="WalletRegistration__MainContent">
<h2>Enter passcode</h2>
Expand Down Expand Up @@ -119,14 +119,11 @@
/>
</div>
<div class="Form__item">
<label for="verification">Date of birth</label>
<label for="verification"></label>
<input
type="date"
id="verification"
name="verification"
required
/>
</div>

<div class="g-recaptcha" data-sitekey="{{.ReCAPTCHASiteKey}}"></div>

Expand Down
12 changes: 10 additions & 2 deletions internal/serve/httphandler/receiver_send_otp_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ type ReceiverSendOTPRequest struct {
}

type ReceiverSendOTPResponseBody struct {
Message string `json:"message"`
Message string `json:"message"`
VerificationField data.VerificationField `json:"verification_field"`
}

func (h ReceiverSendOTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -90,6 +91,12 @@ func (h ReceiverSendOTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request
return
}

receiverVerification, err := h.Models.ReceiverVerification.GetLatestByPhoneNumber(ctx, receiverSendOTPRequest.PhoneNumber)
if err != nil {
httperror.InternalError(ctx, "Cannot find latest receiver verification for receiver", err, nil).Render(w)
return
}

// Generate a new 6 digits OTP
newOTP, err := utils.RandomString(6, utils.NumberBytes)
if err != nil {
Expand Down Expand Up @@ -149,7 +156,8 @@ func (h ReceiverSendOTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request
}

response := ReceiverSendOTPResponseBody{
Message: "if your phone number is registered, you'll receive an OTP",
Message: "if your phone number is registered, you'll receive an OTP",
VerificationField: receiverVerification.VerificationField,
}
httpjson.RenderStatus(w, http.StatusOK, response, httpjson.JSON)
}
53 changes: 49 additions & 4 deletions internal/serve/httphandler/receiver_send_otp_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,14 @@ func Test_ReceiverSendOTPHandler_ServeHTTP(t *testing.T) {

ctx := context.Background()

receiver1 := data.CreateReceiverFixture(t, ctx, dbConnectionPool, &data.Receiver{PhoneNumber: "+380443973607"})
phoneNumber := "+380443973607"
receiver1 := data.CreateReceiverFixture(t, ctx, dbConnectionPool, &data.Receiver{PhoneNumber: phoneNumber})
receiver2 := data.CreateReceiverFixture(t, ctx, dbConnectionPool, &data.Receiver{})
wallet1 := data.CreateWalletFixture(t, ctx, dbConnectionPool, "testWallet", "https://home.page", "home.page", "wallet123://")
data.CreateReceiverVerificationFixture(t, ctx, dbConnectionPool, data.ReceiverVerificationInsert{
ReceiverID: receiver1.ID,
VerificationField: data.VerificationFieldDateOfBirth,
})

_ = data.CreateReceiverWalletFixture(t, ctx, dbConnectionPool, receiver1.ID, wallet1.ID, data.RegisteredReceiversWalletStatus)
_ = data.CreateReceiverWalletFixture(t, ctx, dbConnectionPool, receiver2.ID, wallet1.ID, data.RegisteredReceiversWalletStatus)
Expand Down Expand Up @@ -157,7 +162,7 @@ func Test_ReceiverSendOTPHandler_ServeHTTP(t *testing.T) {
assert.JSONEq(t, `{"error": "request invalid", "extras": {"phone_number": "invalid phone number provided"}}`, string(respBody))
})

t.Run("returns 200 - Ok if the token is in the request context and body it's valid", func(t *testing.T) {
t.Run("returns 200 - Ok if the token is in the request context and body is valid", func(t *testing.T) {
reCAPTCHAValidator.
On("IsTokenValid", mock.Anything, "XyZ").
Return(true, nil).
Expand Down Expand Up @@ -193,7 +198,7 @@ func Test_ReceiverSendOTPHandler_ServeHTTP(t *testing.T) {

assert.Equal(t, http.StatusOK, resp.StatusCode)
assert.Contains(t, resp.Header.Get("Content-Type"), "/json; charset=utf-8")
assert.JSONEq(t, string(respBody), `{"message":"if your phone number is registered, you'll receive an OTP"}`)
assert.JSONEq(t, string(respBody), `{"message":"if your phone number is registered, you'll receive an OTP", "verification_field":"DATE_OF_BIRTH"}`)
})

t.Run("returns 200 - parses a custom OTP message template successfully", func(t *testing.T) {
Expand Down Expand Up @@ -237,7 +242,7 @@ func Test_ReceiverSendOTPHandler_ServeHTTP(t *testing.T) {

assert.Equal(t, http.StatusOK, resp.StatusCode)
assert.Contains(t, resp.Header.Get("Content-Type"), "/json; charset=utf-8")
assert.JSONEq(t, string(respBody), `{"message":"if your phone number is registered, you'll receive an OTP"}`)
assert.JSONEq(t, string(respBody), `{"message":"if your phone number is registered, you'll receive an OTP", "verification_field":"DATE_OF_BIRTH"}`)
})

t.Run("returns 500 - InternalServerError when something goes wrong when sending the SMS", func(t *testing.T) {
Expand Down Expand Up @@ -309,6 +314,46 @@ func Test_ReceiverSendOTPHandler_ServeHTTP(t *testing.T) {
assert.JSONEq(t, wantsBody, string(respBody))
})

t.Run("returns 500 - InternalServerError if phone number is not associated with receiver verification", func(t *testing.T) {
requestSendOTP := ReceiverSendOTPRequest{
PhoneNumber: "+14152223333",
ReCAPTCHAToken: "XyZ",
}
reqBody, _ = json.Marshal(requestSendOTP)

reCAPTCHAValidator.
On("IsTokenValid", mock.Anything, "XyZ").
Return(true, nil).
Once()
req, err := http.NewRequest(http.MethodPost, "/wallet-registration/otp", strings.NewReader(string(reqBody)))
require.NoError(t, err)

validClaims := &anchorplatform.SEP24JWTClaims{
ClientDomainClaim: wallet1.SEP10ClientDomain,
RegisteredClaims: jwt.RegisteredClaims{
ID: "test-transaction-id",
Subject: "GBLTXF46JTCGMWFJASQLVXMMA36IPYTDCN4EN73HRXCGDCGYBZM3A444",
ExpiresAt: jwt.NewNumericDate(time.Now().Add(5 * time.Minute)),
},
}
req = req.WithContext(context.WithValue(req.Context(), anchorplatform.SEP24ClaimsContextKey, validClaims))

rr := httptest.NewRecorder()
r.ServeHTTP(rr, req)

resp := rr.Result()
respBody, err := io.ReadAll(resp.Body)
require.NoError(t, err)

wantsBody := `
{
"error": "Cannot find latest receiver verification for receiver"
}
`
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
assert.JSONEq(t, wantsBody, string(respBody))
})

t.Run("returns 400 - BadRequest when recaptcha token is invalid", func(t *testing.T) {
reCAPTCHAValidator.
On("IsTokenValid", mock.Anything, "XyZ").
Expand Down
26 changes: 22 additions & 4 deletions internal/serve/publicfiles/js/receiver_registration.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ async function sendSms(phoneNumber, reCAPTCHAToken, onSuccess, onError) {
recaptcha_token: reCAPTCHAToken,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@quietbits or @ceciliaromao can you help review this part please ? I can't help much with JS.

@ziyliu did we check with Tori about validation of these fields ? I think there should be some sort of front-end validation for this, especially the PIN.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These updates look good to me. 👍

Copy link
Contributor Author

@ziyliu ziyliu Dec 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to add the backend validation too, but I did just now! I confirmed with Tori when I worked on the disbursement file validations about the requirements. (I also performed the manual validations again)

}),
});
await request.json();
const resp = await request.json();

onSuccess();
onSuccess(resp.verification_field);
} catch (error) {
onError(error);
}
Expand Down Expand Up @@ -96,6 +96,8 @@ async function submitPhoneNumber(event) {
"#g-recaptcha-response"
);
const buttonEls = phoneNumberSectionEl.querySelectorAll("[data-button]");
const verificationFieldTitle = document.querySelector("label[for='verification']");
const verificationFieldInput = document.querySelector("#verification");

if (!reCAPTCHATokenEl || !reCAPTCHATokenEl.value) {
toggleErrorNotification(
Expand Down Expand Up @@ -133,7 +135,22 @@ async function submitPhoneNumber(event) {
return;
}

function showNextPage() {
function showNextPage(verificationField) {
verificationFieldInput.type = "text";
if(verificationField === "DATE_OF_BIRTH") {
verificationFieldTitle.textContent = "Date of birth";
verificationFieldInput.name = "date_of_birth";
verificationFieldInput.type = "date";
}
else if(verificationField === "NATIONAL_ID_NUMBER") {
verificationFieldTitle.textContent = "National ID number";
verificationFieldInput.name = "national_id_number";
}
else if(verificationField === "PIN") {
verificationFieldTitle.textContent = "Pin";
verificationFieldInput.name = "pin";
}

phoneNumberSectionEl.style.display = "none";
reCAPTCHATokenEl.style.display = "none";
passcodeSectionEl.style.display = "flex";
Expand Down Expand Up @@ -169,6 +186,7 @@ async function submitOtp(event) {
);
const otpEl = document.getElementById("otp");
const verificationEl = document.getElementById("verification");
const verificationField = verificationEl.getAttribute("name");

const buttonEls = passcodeSectionEl.querySelectorAll("[data-button]");

Expand Down Expand Up @@ -213,7 +231,7 @@ async function submitOtp(event) {
phone_number: phoneNumber,
otp: otp,
verification: verification,
verification_type: "date_of_birth",
verification_type: verificationField,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should think about making the vocabulary around this more consistent

recaptcha_token: reCAPTCHATokenEl.value,
}),
});
Expand Down
Loading
Loading