Skip to content

Commit

Permalink
feat: send over user in SendSMS Hook instead of UserID (#1551)
Browse files Browse the repository at this point in the history
## What kind of change does this PR introduce?

We align convention with `SendEmail` and send over a user to avoid
having the user make an additional `getUser` call. Also allows access to
`app_metadata` and `user_metadata` which would be useful for
internationalization where you may want the locale of the user to
determine which template to send.

We also introduce a `PhoneData` struct through which we can introduce
any potential phone related fields. This struct currently lives under
the `hooks` package as there is no `phone` package currently and
introducing one might require a significant refactor. Importing it as as
is under `api` package would cause a circular dependency between `hooks`
and `api` packages.

---------

Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com>
  • Loading branch information
J0 and hf committed Apr 22, 2024
1 parent 91e9eca commit d4d743c
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 18 deletions.
36 changes: 24 additions & 12 deletions internal/api/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import (
"testing"

"errors"
"github.com/gofrs/uuid"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/supabase/auth/internal/conf"
"github.com/supabase/auth/internal/hooks"
"github.com/supabase/auth/internal/models"
"github.com/supabase/auth/internal/storage"
"net/http/httptest"

Expand All @@ -22,8 +22,9 @@ var handleApiRequest func(*http.Request) (*http.Response, error)

type HooksTestSuite struct {
suite.Suite
API *API
Config *conf.GlobalConfiguration
API *API
Config *conf.GlobalConfiguration
TestUser *models.User
}

type MockHttpClient struct {
Expand All @@ -47,13 +48,22 @@ func TestHooks(t *testing.T) {
suite.Run(t, ts)
}

func (ts *HooksTestSuite) SetupTest() {
models.TruncateAll(ts.API.db)
u, err := models.NewUser("123456789", "testemail@gmail.com", "securetestpassword", ts.Config.JWT.Aud, nil)
require.NoError(ts.T(), err, "Error creating test user model")
require.NoError(ts.T(), ts.API.db.Create(u), "Error saving new test user")
ts.TestUser = u
}

func (ts *HooksTestSuite) TestRunHTTPHook() {
defer gock.OffAll()

input := hooks.SendSMSInput{
UserID: uuid.Must(uuid.NewV4()),
Phone: "1234567890",
OTP: "123456",
User: ts.TestUser,
SMS: hooks.SMS{
OTP: "123456",
},
}
successOutput := hooks.SendSMSOutput{Success: true}
testURL := "http://localhost:54321/functions/v1/custom-sms-sender"
Expand Down Expand Up @@ -117,9 +127,10 @@ func (ts *HooksTestSuite) TestShouldRetryWithRetryAfterHeader() {
defer gock.OffAll()

input := hooks.SendSMSInput{
UserID: uuid.Must(uuid.NewV4()),
Phone: "1234567890",
OTP: "123456",
User: ts.TestUser,
SMS: hooks.SMS{
OTP: "123456",
},
}
successOutput := hooks.SendSMSOutput{Success: true}
testURL := "http://localhost:54321/functions/v1/custom-sms-sender"
Expand Down Expand Up @@ -159,9 +170,10 @@ func (ts *HooksTestSuite) TestShouldReturnErrorForNonJSONContentType() {
defer gock.OffAll()

input := hooks.SendSMSInput{
UserID: uuid.Must(uuid.NewV4()),
Phone: "1234567890",
OTP: "123456",
User: ts.TestUser,
SMS: hooks.SMS{
OTP: "123456",
},
}
testURL := "http://localhost:54321/functions/v1/custom-sms-sender"
ts.Config.Hook.SendSMS.URI = testURL
Expand Down
7 changes: 4 additions & 3 deletions internal/api/phone.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,10 @@ func (a *API) sendPhoneConfirmation(ctx context.Context, r *http.Request, tx *st
}
if config.Hook.SendSMS.Enabled {
input := hooks.SendSMSInput{
UserID: user.ID,
Phone: user.Phone.String(),
OTP: otp,
User: user,
SMS: hooks.SMS{
OTP: otp,
},
}
output := hooks.SendSMSOutput{}
err := a.invokeHook(tx, r, &input, &output, a.config.Hook.SendSMS.URI)
Expand Down
10 changes: 7 additions & 3 deletions internal/hooks/auth_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ type HookOutput interface {
Error() string
}

// TODO(joel): Move this to phone package
type SMS struct {
OTP string `json:"otp,omitempty"`
}

// #nosec
const MinimumViableTokenSchema = `{
"$schema": "http://json-schema.org/draft-07/schema#",
Expand Down Expand Up @@ -141,9 +146,8 @@ type CustomAccessTokenOutput struct {
}

type SendSMSInput struct {
UserID uuid.UUID `json:"user_id"`
Phone string `json:"phone"`
OTP string `json:"otp"`
User *models.User `json:"user,omitempty"`
SMS SMS `json:"sms,omitempty"`
}

type SendSMSOutput struct {
Expand Down

0 comments on commit d4d743c

Please sign in to comment.