diff --git a/.env b/.env index 82a4e5e..ca148d6 100644 --- a/.env +++ b/.env @@ -66,3 +66,24 @@ REDIS_DB=0 POS_TOKEN_SECRET=local-dev-signing-secret-not-real JWT_SECRET_KEY= USER_CONTEXT_KEY= + +# ── Email ─────────────────────────────────────────────────────────────────── +# +# The first-password invitation. See docs/MAIL_SETUP.md. +# +# MAIL_HOST IS DELIBERATELY BLANK HERE. This file is tracked and shared, and a +# host set here would mean any local run could email a real merchant a real +# password link. Blank is the documented off state: the server boots, onboarding +# works, and every create answers `invited: false` with the reason. +# +# Turn it on by putting the Postal host and its per-application credentials in +# `.env.secrets`, which is read first and is the only one of these git ignores. +MAIL_HOST= +MAIL_PORT=587 +MAIL_USERNAME= +MAIL_PASSWORD= +# On nearledaily.com because the link points at app.nearledaily.com — a password +# mail whose sender and destination are different domains reads as phishing. +MAIL_FROM=care@nearledaily.com +MAIL_FROM_NAME=Nearle +MAIL_CONSOLE_URL=https://app.nearledaily.com diff --git a/.env.example b/.env.example index 7c6dd33..e78750b 100644 --- a/.env.example +++ b/.env.example @@ -120,6 +120,55 @@ EMBEDDING_DIMENSIONS=0 # ── Geocoding ─────────────────────────────────────────────────────────────── # Google Geocoding when set; OpenStreetMap's Nominatim otherwise. GEOCODER_API_KEY= +# ── Email ─────────────────────────────────────────────────────────────────── +# +# Sending the first-password invitation a newly onboarded merchant receives. +# Without MAIL_HOST the server still boots and still onboards tenants — the +# create response comes back `invited: false` with the reason — but nobody is +# emailed, and the only way into a new account is a Nearle staff member using +# Resend invite. +# +# SMTP, because every provider speaks it. Any transactional service is the same +# five variables: its host, 587, the API key as MAIL_PASSWORD, and whatever +# username it documents. +# +# WE RUN POSTAL — a self-hosted transactional mail server. Its SMTP endpoint is +# the host below, and the credentials are a per-application pair generated in +# Postal's UI, NOT a mailbox login. See docs/MAIL_SETUP.md for standing it up +# and for the DNS records, which are what actually decide whether the invitation +# reaches an inbox. +# +# Postal (ours): postal.nearledaily.com 587 credentials per-app +# Gmail / Workspace: smtp.gmail.com 587 an app password, and only +# ever for a smoke test +# Amazon SES: email-smtp..amazonaws.com 587 +# SendGrid: smtp.sendgrid.net 587 username literally "apikey" +# Resend: smtp.resend.com 587 username literally "resend" +MAIL_HOST= +MAIL_PORT=587 +# Optional. Leave both empty for a relay that authenticates by network rather +# than by credentials. +MAIL_USERNAME= +MAIL_PASSWORD= +# Who the invitation appears to come from. Separate from MAIL_USERNAME because +# most providers authenticate as one identity and send as another, and using +# the login as the From address is how mail lands in spam. +# +# ON NEARLEDAILY.COM, DELIBERATELY. The link in the mail points at +# app.nearledaily.com, and a password link arriving from a DIFFERENT domain than +# the one it sends you to is the exact shape of a phishing mail — to a filter +# and to the merchant reading it. Sender and link stay on one domain. +# +# `care@` rather than `no-reply@`, also deliberately: somebody who replies "I +# never got this" is the single most useful reply this system can receive, and +# it should reach a person. +MAIL_FROM=care@nearledaily.com +MAIL_FROM_NAME=Nearle +# Where the invitation link points — the MERCHANT console, always. A merchant +# sets their password there and nowhere else, so this is never the platform +# console's address. +MAIL_CONSOLE_URL=https://app.nearledaily.com + # ── Nearle Buddy ──────────────────────────────────────────────────────────── # diff --git a/config/config.go b/config/config.go index 4d3b535..56e036c 100644 --- a/config/config.go +++ b/config/config.go @@ -74,6 +74,9 @@ type Config struct { // Assistant is the model behind Nearle Buddy. Empty provider = no typed // questions; the tools still work. Assistant AssistantConfig + // Mail. Optional: a deployment without it still onboards tenants and reports + // the invitation as unsent. + Mail MailConfig // POSTokenSecret signs terminal sessions. Falls back to JWTSecret when // unset, matching utils/postoken.go. @@ -357,6 +360,7 @@ func Load() (*Config, error) { }, Assistant: AssistantFromEnv(), + Mail: MailFromEnv(), POSTokenSecret: env("POS_TOKEN_SECRET", ""), JWTSecret: env("JWT_SECRET_KEY", ""), diff --git a/config/mail.go b/config/mail.go new file mode 100644 index 0000000..b3a7a1d --- /dev/null +++ b/config/mail.go @@ -0,0 +1,106 @@ +package config + +import ( + "fmt" + "strconv" + "strings" +) + +// Sending email. +// +// ── Why this exists at all ────────────────────────────────────────────────── +// +// A newly onboarded merchant's admin account arrives with no password, and the +// only safe way to let them set one is a signed invitation sent to the primary +// email they gave us. Until this, the server could not send email: no library, +// no configuration, and `NotifyUser` is Firebase push rather than mail. +// +// ── Shaped like AssistantConfig, for the same reasons ─────────────────────── +// +// Unconfigured is a deployment choice and not a fault, so `Enabled` reports it +// and `Why` says which variable is missing. A server with no mail still boots +// and still onboards tenants — the invitation is recorded as unsent rather than +// failing the creation, because a tenant that exists and cannot be reached is +// recoverable and a tenant that was rolled back by a mail outage is confusing. +type MailConfig struct { + // SMTP, because it is the one protocol every provider speaks. A transactional + // service (SES, SendGrid, Resend) is reached the same way, with its own host + // and an API key as the password — so choosing one later is configuration + // rather than code. + Host string + Port int + Username string + Password string + // Who the invitation appears to come from. Separate from the username + // because most providers authenticate as one identity and send as another, + // and using the login as the From address is how mail ends up in spam. + FromAddress string + FromName string + // Where the invitation link points. The merchant console, always — a + // merchant sets their password there and nowhere else — and a build + // variable rather than a constant because the site can move. + ConsoleURL string +} + +func (m MailConfig) Enabled() bool { return m.Why() == "" } + +// Why says what is missing, or "" when mail can be sent. +// +// A sentence rather than a bool. "Off" is the same answer for five different +// mistakes, and the difference between "we have not set this up" and "somebody +// misspelled a variable" is invisible from outside — which is exactly how the +// assistant sat switched off for two days. +func (m MailConfig) Why() string { + if strings.TrimSpace(m.Host) == "" { + return "MAIL_HOST is not set, so no invitation can be sent" + } + if m.Port <= 0 { + return "MAIL_PORT is not a usable port number" + } + if strings.TrimSpace(m.FromAddress) == "" { + return "MAIL_FROM is not set; an invitation needs a sender address" + } + // Username and password are deliberately NOT required. An internal relay + // that authenticates by network is a real deployment, and demanding + // credentials would refuse it. + if strings.TrimSpace(m.ConsoleURL) == "" { + return "MAIL_CONSOLE_URL is not set; the invitation would have nowhere to point" + } + return "" +} + +// Address is host:port, as the SMTP client wants it. +func (m MailConfig) Address() string { return fmt.Sprintf("%s:%d", m.Host, m.Port) } + +// InviteLink is where an invitation sends somebody. +// +// Built here rather than in the mailer so the shape is decided once, beside the +// console URL it depends on. The token is the whole credential, so it is the +// only thing in the query string — never an email address or a userid, which +// would put both halves of an account into a URL that lands in server logs, +// browser history and whatever proxy sits between. +func (m MailConfig) InviteLink(token string) string { + base := strings.TrimRight(strings.TrimSpace(m.ConsoleURL), "/") + return base + "/set-password?t=" + token +} + +// MailFromEnv reads the mail settings. +func MailFromEnv() MailConfig { + port, err := strconv.Atoi(strings.TrimSpace(env("MAIL_PORT", "587"))) + if err != nil { + // Zero rather than the default, so `Why` reports it instead of the + // server quietly dialling a port nobody asked for. + port = 0 + } + + return MailConfig{ + Host: env("MAIL_HOST", ""), + Port: port, + Username: env("MAIL_USERNAME", ""), + Password: env("MAIL_PASSWORD", ""), + // A name is optional; an address is not. + FromAddress: env("MAIL_FROM", ""), + FromName: env("MAIL_FROM_NAME", "Nearle"), + ConsoleURL: env("MAIL_CONSOLE_URL", "https://app.nearledaily.com"), + } +} diff --git a/config/mail_test.go b/config/mail_test.go new file mode 100644 index 0000000..60da73d --- /dev/null +++ b/config/mail_test.go @@ -0,0 +1,36 @@ +package config + +import "testing" + +// Confirms docs/MAIL_SETUP.md is telling the truth about the committed `.env`: +// a sender is set, a host is not, and the server therefore reports mail OFF with +// a reason naming the variable — rather than trying and failing to send. +func TestCommittedEnvLeavesMailOffWithAReason(t *testing.T) { + t.Setenv("MAIL_HOST", "") + t.Setenv("MAIL_PORT", "587") + t.Setenv("MAIL_FROM", "care@nearledaily.com") + t.Setenv("MAIL_FROM_NAME", "Nearle") + t.Setenv("MAIL_CONSOLE_URL", "https://app.nearledaily.com") + + cfg := MailFromEnv() + if cfg.Enabled() { + t.Fatal("mail reported as enabled with no host") + } + if cfg.Why() == "" || cfg.Why()[:9] != "MAIL_HOST" { + t.Fatalf("the reason does not name the missing variable: %q", cfg.Why()) + } + + // And with the Postal host supplied from .env.secrets, it comes on and the + // link points at the MERCHANT console. + t.Setenv("MAIL_HOST", "postal.nearledaily.com") + on := MailFromEnv() + if !on.Enabled() { + t.Fatalf("still off with a host set: %s", on.Why()) + } + if got := on.InviteLink("i1.abc.def"); got != "https://app.nearledaily.com/set-password?t=i1.abc.def" { + t.Fatalf("the invitation would point at %q", got) + } + if on.Address() != "postal.nearledaily.com:587" { + t.Fatalf("wrong SMTP address: %q", on.Address()) + } +} diff --git a/controllers/resendInvite_test.go b/controllers/resendInvite_test.go new file mode 100644 index 0000000..ee8571f --- /dev/null +++ b/controllers/resendInvite_test.go @@ -0,0 +1,210 @@ +package controllers + +import ( + "io" + "net/http/httptest" + "strings" + "testing" + "time" + + "nearle/middleware" + "nearle/services" + "nearle/utils" + + "github.com/gofiber/fiber/v2" +) + +/* +Who may re-issue a first-password link, and for whom. + +This endpoint mints a credential, so most of what matters is what it refuses. +The service layer refuses the business cases — an account that already has a +password, a tenant whose primary email matches no login — and those are covered +in `services/resendInvite_test.go`. This file is about the door: who gets +through it, and which account a request actually names. +*/ + +// resendService answers both resends and records which was called. Only the two +// methods under test are real; the rest of TenantService is embedded nil, which +// panics if anything else is reached — exactly the signal wanted. +type resendService struct { + services.TenantService + byTenant int + byUser int + outcome services.InviteOutcome + err error +} + +func (s *resendService) ResendInvite(tenantID int) (services.InviteOutcome, error) { + s.byTenant = tenantID + return s.outcome, s.err +} + +func (s *resendService) ResendInviteToUser(userID int) (services.InviteOutcome, error) { + s.byUser = userID + return s.outcome, s.err +} + +func resendApp(t *testing.T, service *resendService) *fiber.App { + t.Helper() + t.Setenv("POS_TOKEN_SECRET", testSecret) + + app := fiber.New() + // The real guard, mounted as routes.go mounts it: this endpoint sits behind + // the session, and the handler then requires a platform account on top. + app.Use("/live/api/v1/web", middleware.WebAuth(nil)) + app.Post("/live/api/v1/web/tenants/resendinvite", NewTenantController(service).ResendInvite) + + return app +} + +// staffToken is a signed session for a Nearle staff account. +// +// `Superadmin` is the signal, and it is minted from `app_users.issuperadmin` — +// not from the tenant being zero and not from a role id. Both of those look +// equivalent and are not: `app_roles` calls roleid 1 "Super admin" and +// onboarding wrote 1 for every shop owner, and a zero tenant is what an +// unfilled column looks like. See `utils.WebClaims`. +func staffToken(t *testing.T) string { + t.Helper() + token, _, err := utils.MintWebToken(utils.WebClaims{ + Userid: 12, Roleid: 1, Configid: 1, Superadmin: true, + }, time.Now()) + if err != nil { + t.Fatalf("mint: %v", err) + } + return token +} + +// merchantToken is a signed session for a shop's own admin. +func merchantToken(t *testing.T) string { + t.Helper() + token, _, err := utils.MintWebToken(utils.WebClaims{ + Userid: 904, Tenantid: 1147, Roleid: 3, Configid: 1, + }, time.Now()) + if err != nil { + t.Fatalf("mint: %v", err) + } + return token +} + +func postAs(t *testing.T, app *fiber.App, token, body string) (int, string) { + t.Helper() + + req := httptest.NewRequest("POST", "/live/api/v1/web/tenants/resendinvite", + strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+token) + + resp, err := app.Test(req, -1) + if err != nil { + t.Fatalf("resendinvite: %v", err) + } + raw, _ := io.ReadAll(resp.Body) + return resp.StatusCode, string(raw) +} + +func TestAMerchantCannotResendAnything(t *testing.T) { + // A merchant's session is pinned to their own tenant, so the worst they could + // do is re-invite themselves — and the service refuses that, because an + // account signing in to ask already has a password. Refusing here as well + // means the endpoint does not rely on two other checks to make the wrong case + // impossible. + service := &resendService{outcome: services.InviteOutcome{Sent: true}} + app := resendApp(t, service) + + status, body := postAs(t, app, merchantToken(t), `{"tenantid":1147}`) + + if status != 403 { + t.Fatalf("a merchant was let through: %d %s", status, body) + } + if service.byTenant != 0 || service.byUser != 0 { + t.Fatal("the service was reached by a caller who should have been refused") + } +} + +func TestNearleStaffCanResendToATenantsOwner(t *testing.T) { + service := &resendService{outcome: services.InviteOutcome{Sent: true}} + app := resendApp(t, service) + + status, body := postAs(t, app, staffToken(t), `{"tenantid":1147}`) + + if status != 200 { + t.Fatalf("refused Nearle staff: %d %s", status, body) + } + if service.byTenant != 1147 { + t.Fatalf("resent for tenant %d, want 1147", service.byTenant) + } +} + +func TestAUseridNamesOnePersonRatherThanTheOwner(t *testing.T) { + // The reason this parameter exists. Staff added after onboarding, and the + // login every branch spawns, are created with no password too — and a + // business has many of them, so "the tenant's invitation" cannot reach them. + service := &resendService{outcome: services.InviteOutcome{Sent: true}} + app := resendApp(t, service) + + status, body := postAs(t, app, staffToken(t), `{"userid":7781}`) + + if status != 200 { + t.Fatalf("refused: %d %s", status, body) + } + if service.byUser != 7781 { + t.Fatalf("resent for user %d, want 7781", service.byUser) + } + if service.byTenant != 0 { + t.Fatal("emailed the owner when a person was named") + } +} + +func TestAUseridWinsOverATenantid(t *testing.T) { + // A caller that sent a person's id meant that person. Falling back to the + // owner would be the wrong mailbox with nothing on the response to say so. + service := &resendService{outcome: services.InviteOutcome{Sent: true}} + app := resendApp(t, service) + + if status, body := postAs(t, app, staffToken(t), `{"tenantid":1147,"userid":7781}`); status != 200 { + t.Fatalf("refused: %d %s", status, body) + } + if service.byUser != 7781 || service.byTenant != 0 { + t.Fatalf("resolved to the wrong account: user=%d tenant=%d", service.byUser, service.byTenant) + } +} + +func TestAnEmptyBodyIsRefusedRatherThanSentToTenantZero(t *testing.T) { + // `{}` parses cleanly into two zeroes. Without this check it would reach the + // service as tenant 0 and come back "tenant 0 has no account matching its + // primary email address", which describes nothing the caller did. + service := &resendService{outcome: services.InviteOutcome{Sent: true}} + app := resendApp(t, service) + + status, body := postAs(t, app, staffToken(t), `{}`) + + if status != 400 { + t.Fatalf("an empty request was accepted: %d %s", status, body) + } + if service.byTenant != 0 || service.byUser != 0 { + t.Fatal("the service was called with nothing to act on") + } + if !strings.Contains(body, "tenantid") || !strings.Contains(body, "userid") { + t.Errorf("the refusal does not say what to send: %s", body) + } +} + +func TestMailThatDidNotLeaveIsReportedAsAFailure(t *testing.T) { + // The operator pressed a button expecting an email to go. "Success" with no + // mail sent is the one answer they cannot act on. + service := &resendService{outcome: services.InviteOutcome{ + Sent: false, Reason: "MAIL_HOST is not set", + }} + app := resendApp(t, service) + + status, body := postAs(t, app, staffToken(t), `{"tenantid":1147}`) + + if status != 409 { + t.Fatalf("an unsent invitation was reported as sent: %d %s", status, body) + } + if !strings.Contains(body, "MAIL_HOST") { + t.Errorf("the reason was lost: %s", body) + } +} diff --git a/controllers/setPassword_test.go b/controllers/setPassword_test.go index 025464c..47b01f0 100644 --- a/controllers/setPassword_test.go +++ b/controllers/setPassword_test.go @@ -6,9 +6,12 @@ import ( "net/http/httptest" "strings" "testing" + "time" "nearle/middleware" "nearle/models" + "nearle/services" + "nearle/utils" fiberv1 "github.com/gofiber/fiber" "github.com/gofiber/fiber/v2" @@ -38,11 +41,13 @@ session, and it must refuse everything except the one case it exists for. type fakePasswords struct { // set records what reached the write, so a refusal can be shown to have // refused rather than merely reported. - set []string - refuseIt error + set []string + lastUserid int + refuseIt error } func (f *fakePasswords) SetInitialPassword(userid int, password string) error { + f.lastUserid = userid if f.refuseIt != nil { return f.refuseIt } @@ -65,8 +70,8 @@ func (f *fakePasswords) UpdateStaff(models.User) error { return nil } func (f *fakePasswords) AppLogin(models.User) (models.TenantUserInfo, fiberv1.Map, error) { return models.TenantUserInfo{}, fiberv1.Map{}, nil } -func (f *fakePasswords) CreateUser(models.User) (models.UserInfo, error) { - return models.UserInfo{}, nil +func (f *fakePasswords) CreateUser(models.User) (models.UserInfo, services.InviteOutcome, error) { + return models.UserInfo{}, services.InviteOutcome{}, nil } func passwordApp(t *testing.T, service *fakePasswords) *fiber.App { @@ -103,7 +108,7 @@ func TestAFirstPasswordCanBeSetWithoutASession(t *testing.T) { app := passwordApp(t, service) status, body := send(t, app, "POST", "/live/api/v1/web/users/setpassword", - `{"userid":904,"password":"opensesame"}`) + `{"token":"`+invite(t, 904)+`","password":"opensesame"}`) if status == fiber.StatusUnauthorized { t.Fatalf("the guard blocked the one call that cannot present a token: %s", body) @@ -139,7 +144,7 @@ func TestAnAccountThatAlreadyHasOneIsRefusedAsAConflict(t *testing.T) { app := passwordApp(t, service) status, body := send(t, app, "POST", "/live/api/v1/web/users/setpassword", - `{"userid":904,"password":"opensesame"}`) + `{"token":"`+invite(t, 904)+`","password":"opensesame"}`) if status != fiber.StatusConflict { t.Fatalf("expected 409, got %d: %s", status, body) @@ -157,7 +162,7 @@ func TestTheRefusalDoesNotSayWhichAccountsExist(t *testing.T) { app := passwordApp(t, service) _, body := send(t, app, "POST", "/live/api/v1/web/users/setpassword", - `{"userid":904,"password":"opensesame"}`) + `{"token":"`+invite(t, 904)+`","password":"opensesame"}`) for _, leak := range []string{"not found", "no such", "does not exist"} { if strings.Contains(strings.ToLower(body), leak) { @@ -179,7 +184,7 @@ func TestTheAnswerIsTheEnvelopeTheConsoleUnwraps(t *testing.T) { // A handler answering at the top level passes a service test and hands the // console `undefined`. _, body := send(t, passwordApp(t, &fakePasswords{}), "POST", - "/live/api/v1/web/users/setpassword", `{"userid":904,"password":"opensesame"}`) + "/live/api/v1/web/users/setpassword", `{"token":"`+invite(t, 904)+`","password":"opensesame"}`) var envelope struct { Status bool `json:"status"` @@ -204,3 +209,94 @@ func (f *fakePasswords) TenantWebLogin(models.User) (models.TenantUserInfo, map[ return models.TenantUserInfo{}, map[string]interface{}{} } func (f *fakePasswords) DeleteUser(int) error { return nil } + +// invite mints a real invitation for the test's account. +// +// A helper rather than a literal, because the token is signed: a hand-written +// string would test the refusal path and nothing else, and the point of these +// is what happens when a genuine invitation arrives. +func invite(t *testing.T, userid int) string { + t.Helper() + token, _, err := utils.MintInviteToken(utils.InviteClaims{Userid: userid, Tenantid: 1147}, time.Now()) + if err != nil { + t.Fatalf("minting an invitation: %v", err) + } + return token +} + +/* +The invitation replaced a userid, and that was a security fix rather than a +tidy-up. + +`applogin` answers a POST carrying an email and no password with 409 and the +userid, for any account that has not set one. So the recipe was: know a +merchant's primary email — usually printed on their shopfront — POST it, receive +their userid, set their password, own the business's admin account. No guessing +at any step, and the empty-password check was no defence because an un-set-up +account is exactly what such an attacker wants. +*/ + +func TestAUseridIsNoLongerEnoughToSetAPassword(t *testing.T) { + // The hole, asserted closed. A body carrying a userid and no invitation + // must not set anything, whatever the userid is. + service := &fakePasswords{} + app := passwordApp(t, service) + + status, body := send(t, app, "POST", "/live/api/v1/web/users/setpassword", + `{"userid":904,"password":"opensesame"}`) + + if status == fiber.StatusOK { + t.Fatalf("a bare userid still set a password: %s", body) + } + if len(service.set) != 0 { + t.Fatalf("a bare userid reached the service: %v", service.set) + } +} + +func TestAnInvitationSetsThePasswordForTheAccountItNames(t *testing.T) { + service := &fakePasswords{} + app := passwordApp(t, service) + + status, body := send(t, app, "POST", "/live/api/v1/web/users/setpassword", + `{"token":"`+invite(t, 904)+`","password":"opensesame"}`) + + if status != fiber.StatusOK { + t.Fatalf("HTTP %d: %s", status, body) + } + if len(service.set) != 1 || service.set[0] != "opensesame" { + t.Fatalf("the password did not reach the service: %v", service.set) + } +} + +func TestTheUseridComesFromTheSignatureNotTheRequest(t *testing.T) { + // An invitation for 904 with a `userid` field claiming 999 must set 904's + // password. If the body could override it, the token would be decoration. + service := &fakePasswords{} + app := passwordApp(t, service) + + status, _ := send(t, app, "POST", "/live/api/v1/web/users/setpassword", + `{"token":"`+invite(t, 904)+`","userid":999,"password":"opensesame"}`) + + if status != fiber.StatusOK { + t.Fatalf("a valid invitation was refused: %d", status) + } + if service.lastUserid != 904 { + t.Fatalf("the request's userid won: set the password for %d", service.lastUserid) + } +} + +func TestAForgedInvitationIsRefused(t *testing.T) { + service := &fakePasswords{} + app := passwordApp(t, service) + + for _, token := range []string{"", "i1.forged.signature", "not-a-token", "w1.a.b"} { + status, _ := send(t, app, "POST", "/live/api/v1/web/users/setpassword", + `{"token":"`+token+`","password":"opensesame"}`) + if status == fiber.StatusOK { + t.Fatalf("%q was accepted as an invitation", token) + } + } + if len(service.set) != 0 { + t.Fatalf("a forged invitation wrote: %v", service.set) + } +} diff --git a/controllers/tenantController.go b/controllers/tenantController.go index cfbbf66..e8a69b3 100644 --- a/controllers/tenantController.go +++ b/controllers/tenantController.go @@ -3,6 +3,7 @@ package controllers import ( "fmt" "log" + "nearle/middleware" "nearle/models" "nearle/services" "net/http" @@ -346,7 +347,8 @@ func (ctl *TenantController) CreateStaff(c *fiber.Ctx) error { }) } - if err := ctl.tenantService.CreateStaff(data); err != nil { + invite, err := ctl.tenantService.CreateStaff(data) + if err != nil { // A rejected PIN, a missing name, a role nobody set — these are things // the person filling in the form can fix, so they come back as 400 with // the reason. This answered 500 with a body claiming 409, which told a @@ -358,10 +360,17 @@ func (ctl *TenantController) CreateStaff(c *fiber.Ctx) error { }) } + // The person was hired either way. Whether they were emailed their + // first-password link is reported beside that rather than folded into + // `status`: this account is created with no password and the link is the only + // way in, so an operator who is not told cannot know they have added somebody + // who cannot sign in. return c.JSON(fiber.Map{ - "code": http.StatusCreated, - "message": "Staff created successfully", - "status": true, + "code": http.StatusCreated, + "message": "Staff created successfully", + "status": true, + "invited": invite.Sent, + "invitereason": invite.Reason, }) } @@ -436,7 +445,7 @@ func (ctl *TenantController) CreateTenantUser(c *fiber.Ctx) error { }) } - result, err := ctl.tenantService.CreateTenantUser(data) + result, invite, err := ctl.tenantService.CreateTenantUser(data) if err != nil { if err.Error() == "Tenant Already Exists" { return c.Status(http.StatusConflict).JSON(fiber.Map{ @@ -453,11 +462,21 @@ func (ctl *TenantController) CreateTenantUser(c *fiber.Ctx) error { }) } + // The tenant was created either way. The invitation is reported beside it + // rather than folded into `status`, because a merchant who exists and has + // not been emailed is a task for the operator — resend, or correct the + // address — and not a failed onboarding to be retried. + // + // `invited: false` with a reason is the state the platform console shows on + // the tenant, so it never has to guess whether the email went. return c.Status(http.StatusCreated).JSON(fiber.Map{ "code": 201, "status": true, "message": "Successfully Created", "details": result, + "invited": invite.Sent, + // Omitted when it sent, so a successful onboarding carries no apology. + "invitereason": invite.Reason, }) } @@ -776,3 +795,79 @@ func (ctl *TenantController) AssignPartner(c *fiber.Ctx) error { "code": http.StatusOK, "status": true, "message": "Successfully Updated", }) } + +// ResendInvite re-issues a merchant's first-password link. +// +// ── Why this is platform staff only ───────────────────────────────────────── +// +// It mints a credential. `middleware.WebAuth` already pins a merchant's session +// to their own tenant, so a shop could at most re-invite itself — but the +// account it would be inviting is the one signing in to ask, which can only +// happen if that account already has a password, and the service refuses that +// case outright. +// +// So the only caller this is for is Nearle's own staff, chasing a merchant who +// never received the mail. Saying so explicitly is better than relying on two +// other checks to make the wrong case impossible. +func (ctl *TenantController) ResendInvite(c *fiber.Ctx) error { + claims, ok := middleware.WebClaimsFrom(c) + if !ok || !claims.IsPlatformAccount() { + return c.Status(http.StatusForbidden).JSON(fiber.Map{ + "code": http.StatusForbidden, "status": false, + "message": "Only Nearle staff can resend an invitation.", + }) + } + + // Either a tenant — meaning its owner, the one account onboarding created — + // or one named person. Staff added later and the login every branch spawns + // are created with no password too, and a business has many of them, so + // "the tenant's invitation" cannot reach them. + var req struct { + Tenantid int `json:"tenantid"` + Userid int `json:"userid"` + } + if err := c.BodyParser(&req); err != nil { + return c.Status(http.StatusBadRequest).JSON(fiber.Map{ + "code": http.StatusBadRequest, "status": false, "message": "Invalid request body", + }) + } + if req.Tenantid <= 0 && req.Userid <= 0 { + return c.Status(http.StatusBadRequest).JSON(fiber.Map{ + "code": http.StatusBadRequest, "status": false, + "message": "Send a tenantid to re-invite the owner, or a userid to re-invite one person.", + }) + } + + // `userid` wins when both arrive. It is the more specific of the two, and a + // caller that sent a person's id meant that person — silently emailing the + // owner instead would be the wrong mailbox with no sign anything was off. + var ( + outcome services.InviteOutcome + err error + ) + if req.Userid > 0 { + outcome, err = ctl.tenantService.ResendInviteToUser(req.Userid) + } else { + outcome, err = ctl.tenantService.ResendInvite(req.Tenantid) + } + if err != nil { + // 409, not 500. Every failure here is a business fact the operator can + // act on — no such tenant, an address that matches no login, a merchant + // already set up — rather than a fault in the server. + return c.Status(http.StatusConflict).JSON(fiber.Map{ + "code": http.StatusConflict, "status": false, "message": err.Error(), + }) + } + if !outcome.Sent { + // The tenant is fine and the mail did not go. Reported as a failure + // because the operator pressed a button expecting an email to leave, + // and the reason names what to fix. + return c.Status(http.StatusConflict).JSON(fiber.Map{ + "code": http.StatusConflict, "status": false, "message": outcome.Reason, + }) + } + + return c.JSON(fiber.Map{ + "code": http.StatusOK, "status": true, "message": "Invitation sent.", + }) +} diff --git a/controllers/userController.go b/controllers/userController.go index afbf868..099b692 100644 --- a/controllers/userController.go +++ b/controllers/userController.go @@ -255,7 +255,7 @@ func (ctl *UserController) CreateUser(c *fiber.Ctx) error { } // Call service - info, err := ctl.userService.CreateUser(user) + info, invite, err := ctl.userService.CreateUser(user) if err != nil { return c.Status(http.StatusConflict).JSON(fiber.Map{ "code": http.StatusConflict, @@ -264,11 +264,17 @@ func (ctl *UserController) CreateUser(c *fiber.Ctx) error { }) } + // The account was created either way. Whether its first-password invitation + // was emailed is reported beside it rather than folded into `status`: the + // account has no password and the link is the only way to set one, so an + // operator who is not told has hired somebody who cannot sign in. return c.Status(http.StatusCreated).JSON(fiber.Map{ - "code": http.StatusCreated, - "status": true, - "message": "Success", - "details": info, + "code": http.StatusCreated, + "status": true, + "message": "Success", + "details": info, + "invited": invite.Sent, + "invitereason": invite.Reason, }) } @@ -347,7 +353,9 @@ func (ctl *UserController) DeleteUser(c *fiber.Ctx) error { // what makes it safe to leave open. See the repository for the rest. func (ctl *UserController) SetPassword(c *fiber.Ctx) error { var req struct { - Userid int `json:"userid"` + // The invitation, exactly as it arrived in the emailed link. The userid + // is read out of the signature and never out of the request — see below. + Token string `json:"token"` Password string `json:"password"` } if err := c.BodyParser(&req); err != nil { @@ -356,7 +364,27 @@ func (ctl *UserController) SetPassword(c *fiber.Ctx) error { }) } - if err := ctl.userService.SetInitialPassword(req.Userid, req.Password); err != nil { + // ── Why this takes a token and no longer takes a userid ───────────────── + // + // It used to accept `{userid, password}`, and that was an account takeover + // waiting to be noticed. `applogin` answers a POST carrying an email and NO + // password with 409 and the userid, for any account that has not set one — + // which is how the console's own setup step learned it. So the whole recipe + // was: know a merchant's primary email, which is usually printed on their + // shopfront, POST it here, receive their userid, then set their password + // and own the business's admin account. No guessing at any step. + // + // The invitation closes it. It is signed with the deployment's key, names + // the account in a payload the server produced, and expires. Knowing an + // email is no longer enough, and neither is knowing a userid. + claims, err := utils.ParseInviteToken(req.Token, time.Now()) + if err != nil { + return c.Status(http.StatusConflict).JSON(fiber.Map{ + "status": false, "code": http.StatusConflict, "message": err.Error(), + }) + } + + if err := ctl.userService.SetInitialPassword(claims.Userid, req.Password); err != nil { // 409, not 401. Nothing about this is an authentication failure — the // caller is not supposed to have a session — and answering 401 would // send the console into its sign-out-and-reload path on the one screen diff --git a/docs/MAIL_SETUP.md b/docs/MAIL_SETUP.md new file mode 100644 index 0000000..db0fd40 --- /dev/null +++ b/docs/MAIL_SETUP.md @@ -0,0 +1,168 @@ +# Mail setup — Postal, sending as care@nearledaily.com + +What this is for: the first-password invitation. Every back-office account on +Fiesta is created with an empty password, and the link in this email is the only +way to set one — the sign-in screen no longer offers a form, because a public one +meant that knowing a merchant's email address was enough to claim their account. + +So this is not newsletter plumbing. **If the mail lands in spam, a business that +was just onboarded cannot sign in**, and the first anyone hears of it is a phone +call. The DNS section below matters more than the install. + +--- + +## The decisions already made + +| | | why | +|---|---|---| +| Server | Postal, self-hosted | open source, purpose-built for transactional mail, speaks plain SMTP so nothing in Go changes | +| Sender | `care@nearledaily.com` | the link points at `app.nearledaily.com`; a password mail whose sender and destination are different domains is the shape of a phishing mail | +| `care@` not `no-reply@` | | someone replying "I never got this" is the most useful reply this system can get, and it should reach a person | +| Outbound | relay through a smarthost at first | see [Delivery](#delivery-the-hard-half) | + +--- + +## 1. Stand Postal up + +Postal needs a host of its own — it wants ports 25, 80 and 443, plus MariaDB and +RabbitMQ. A 2 vCPU / 4 GB box is ample for our volume. + +```sh +# on the mail host +git clone https://github.com/postalserver/install /opt/postal/install +ln -s /opt/postal/install/bin/postal /usr/bin/postal +postal bootstrap postal.nearledaily.com +postal initialize +postal make-user # your admin login +postal start +``` + +Then in Postal's web UI: + +1. **Create an organisation** — `Nearle`. +2. **Add a mail server** under it — call it `transactional`. Keep marketing mail + out of this one forever; shared reputation is the whole point. +3. **Add the domain** `nearledaily.com`. Postal prints the DNS records it wants. + Section 2 is those records. +4. **Create a credential** of type *SMTP*, scoped to that server. Postal gives + you a username and password pair. **This is not a mailbox login** — it exists + only for Fiesta to authenticate with, and it can be revoked on its own. + +--- + +## 2. DNS on nearledaily.com + +This is the part that decides whether the invitation is read or binned. Postal's +domain page shows the exact values; the shapes are: + +| Record | Name | Value | +|---|---|---| +| TXT (SPF) | `nearledaily.com` | `v=spf1 a mx include:spf.postal.nearledaily.com ~all` | +| TXT (DKIM) | `postal._domainkey.nearledaily.com` | the public key Postal generates | +| CNAME (Return-Path) | `psrp.nearledaily.com` | `rp.postal.nearledaily.com` | +| TXT (DMARC) | `_dmarc.nearledaily.com` | `v=DMARC1; p=none; rua=mailto:care@nearledaily.com` | +| PTR (rDNS) | the mail host's IP | `postal.nearledaily.com` — set at your VPS provider, not in DNS | + +Notes that save an afternoon: + +- **One SPF record per domain.** If `nearledaily.com` already has one, merge the + `include:` into it rather than adding a second — two SPF records is a permerror + and fails every check. +- **Start DMARC at `p=none`.** It reports without rejecting. Read the reports for + a couple of weeks, confirm everything legitimate is aligned, then move to + `p=quarantine`. Going straight to `p=reject` is how you discover a misaligned + sender by losing its mail. +- **The Return-Path CNAME is not optional.** Without it, bounces go nowhere and + Postal cannot tell you a merchant's address is dead — which, for this mail, is + exactly the fact you most need. +- Verify with `dig TXT nearledaily.com`, and send a test to a + [mail-tester.com](https://mail-tester.com) address. Aim for 9/10 or better + before the first real merchant. + +--- + +## 3. Point Fiesta at it + +Credentials go in `.env.secrets`, which is read first and is the only env file +git ignores. Never in `.env` — that one is tracked and shared. + +```sh +# .env.secrets on the Fiesta host +MAIL_HOST=postal.nearledaily.com +MAIL_USERNAME= +MAIL_PASSWORD= +``` + +Everything else is already set in `.env`: + +```sh +MAIL_PORT=587 +MAIL_FROM=care@nearledaily.com +MAIL_FROM_NAME=Nearle +MAIL_CONSOLE_URL=https://app.nearledaily.com +``` + +`MAIL_CONSOLE_URL` is the **merchant** console and never the platform one — a +merchant sets their password at `app.nearledaily.com/set-password` and nowhere +else. + +Restart and read the first log line: + +``` +mail: sending as care@nearledaily.com via postal.nearledaily.com:587 +``` + +If it instead says `mail: OFF — `, the reason names the missing variable. +Nothing else breaks: the server boots, onboarding works, and every create answers +`invited: false` with that same reason on screen. + +--- + +## 4. Prove it end to end + +Not "the config looks right" — actually watch one arrive. + +1. Onboard a test merchant in the platform console with an address you can read. +2. The success screen should say the invitation is on its way. If it says + **No invitation was sent**, the reason on screen is the server's own. +3. Open the mail. Check it is **not** in spam — that is the whole test. +4. Follow the link, set a password, sign in at `app.nearledaily.com`. +5. Press **Resend invite** on that tenant. It must refuse, naming the business: + *"… has already set a password — send them to the sign-in page instead."* + That refusal is what stops this becoming a password reset. + +--- + +## Delivery, the hard half + +Postal is the easy part. Getting mail *accepted* from your own IP is not: + +- Most clouds block outbound port 25 by default. AWS, GCP, Azure, DigitalOcean, + Oracle and Hetzner all require an exception request; some decline. +- A fresh IP has no sending reputation. Gmail and Outlook throttle or spam-folder + it until it is warmed over weeks. A recycled VPS IP is frequently already on + Spamhaus — check before you commit to one. +- rDNS must match the HELO hostname. Not every provider lets you set it. + +**So configure Postal to relay outbound through a smarthost to begin with.** You +keep the open-source stack, your own queue, your own logs and the freedom to +move — and you borrow established IP reputation for the last hop only. Postal +supports this per mail server under *Settings → SMTP relays*. Once you have +volume and a warm dedicated IP, cut over to sending directly; nothing on the +Fiesta side changes, because it only ever talks to Postal. + +At our volume — a few dozen invitations a month — carrying full deliverability +operations to save roughly ₹1,000 a year is a bad trade against one merchant +locked out of their own business. + +--- + +## What the merchant actually receives + +Plain text, deliberately. A password link arriving as an image-heavy HTML +template is the shape of a phishing mail, and plain text renders identically +everywhere. The body names the business, puts the link on its own line, and says +it expires in seven days — because an invitation found three weeks later needs to +explain itself rather than look broken. + +The wording lives in `inviteMessage` in `services/inviteService.go`. diff --git a/facade/container.go b/facade/container.go index 1560036..156863d 100644 --- a/facade/container.go +++ b/facade/container.go @@ -3,6 +3,7 @@ package facade import ( "log" + "nearle/config" "nearle/controllers" "nearle/repositories" "nearle/services" @@ -48,11 +49,29 @@ type Facade struct { // it may be nil if catalogue env vars are not configured, in which case // catalogue endpoints will error at query time rather than at startup. // embedder may be nil too: scan-to-order then matches on words alone. -func NewFacade(db *gorm.DB, catalogueDB *gorm.DB, embedder utils.Embedder, chat utils.Chat, agentsDir, assistantWhy string) *Facade { +func NewFacade(db *gorm.DB, catalogueDB *gorm.DB, embedder utils.Embedder, chat utils.Chat, agentsDir, assistantWhy string, mailer utils.Mailer, mailCfg config.MailConfig) *Facade { + + // The invitation, built first because two modules need it. + // + // Every back-office account on this platform is created with NO password — + // the onboarded merchant, every person added to the directory, and the login + // each branch spawns — and since the sign-in screen stopped offering to set + // one, the emailed link is the only way in. So whichever module creates an + // account has to be able to send it. + // + // `mailer` may be nil: a deployment with no mail configured still creates + // everything, and each response says the invitation was not sent and names + // the variable, rather than failing the create. + // + // The tenant repository supplies the business name for the mail's first line + // (`services.TenantNamer`), which is why it is built here rather than in the + // tenant module below. + tenantRepo := repositories.NewTenantRepository(db) + inviteService := services.NewInviteService(mailer, mailCfg, tenantRepo) // User Module userRepo := repositories.NewUserRepository(db) - userService := services.NewUserService(userRepo) + userService := services.NewUserService(userRepo, inviteService) userController := controllers.NewUserController(userService) // Catalogue Module (separate pgvector DB — never the main `db`). Built @@ -83,8 +102,11 @@ func NewFacade(db *gorm.DB, catalogueDB *gorm.DB, embedder utils.Embedder, chat utilsController := controllers.NewUtilsController(utilsService) //Tenant Module - tenantRepo := repositories.NewTenantRepository(db) - tenantService := services.NewTenantService(tenantRepo) + // + // Onboarding, adding a person and commissioning a branch all create an + // account with no password, so all three send an invitation. `tenantRepo` and + // `inviteService` are built above, where the reasoning is. + tenantService := services.NewTenantService(tenantRepo, inviteService) tenantController := controllers.NewTenantController(tenantService) //Partner Module diff --git a/main.go b/main.go index 4183672..4080f92 100644 --- a/main.go +++ b/main.go @@ -442,7 +442,28 @@ func main() { // ASSISTANT_AGENTS_DIR replaces the compiled-in agent definitions wholesale. // Empty uses the embedded ones, so a deployment cannot be broken by a missing // directory. - f := facade.NewFacade(db.DB, db.CatalogueDB, embedder, chat, os.Getenv("ASSISTANT_AGENTS_DIR"), cfg.Assistant.Why()) + // Mail, for the invitation a newly onboarded merchant is sent. + // + // Optional in the same way as the model and the embedder: without it the + // server still boots and still onboards tenants, and the create response + // says the invitation was not sent and which variable is missing. Refusing + // to start would make a mail relay a hard dependency of creating a shop, + // which it is not. + mailer, err := utils.NewMailer(cfg.Mail) + if err != nil { + // A configured-but-invalid sender, as opposed to no mail at all. That + // fails every message, so it is worth stopping for rather than + // discovering one silent invitation at a time. + log.Fatal("mail:", err) + } + if mailer == nil { + log.Printf("mail: OFF — %s", cfg.Mail.Why()) + } else { + log.Printf("mail: sending as %s via %s", cfg.Mail.FromAddress, cfg.Mail.Address()) + } + + f := facade.NewFacade(db.DB, db.CatalogueDB, embedder, chat, + os.Getenv("ASSISTANT_AGENTS_DIR"), cfg.Assistant.Why(), mailer, cfg.Mail) routes.RegisterRoutes(app, f) diff --git a/models/tenant.go b/models/tenant.go index 44cb310..6e99367 100644 --- a/models/tenant.go +++ b/models/tenant.go @@ -202,6 +202,18 @@ type StaffInfo struct { // Without it every row on the console's Users & access screen read // "Unknown", because the field was never selected or sent. Status string `json:"status"` + // Whether they have ever chosen a password. + // + // Every back-office account is created with an empty one and emailed a link + // to set it. Until that link is used the person is in this list, in every + // branch picker, and cannot sign in — and `Status` does not say so: an + // Active account with no password is refused at the login screen like any + // other. `false` is the row that needs an action, which is why the directory + // reads it and offers a resend there and nowhere else. + // + // Computed in the query. `Password` is also on this struct, which is its own + // problem, but nothing should have to look at it to answer this. + IsSetUp bool `json:"issetup"` } type Tenantuser struct { diff --git a/repositories/tenantRepository.go b/repositories/tenantRepository.go index 9e029f7..0c37652 100644 --- a/repositories/tenantRepository.go +++ b/repositories/tenantRepository.go @@ -25,14 +25,20 @@ type TenantRepository interface { UpdateTenantProfile(tenantID int, fields map[string]any) error UpdateOwnProfile(userID, tenantID int, fields map[string]any) error GetStaffs(tid int) ([]models.StaffInfo, error) - CreateStaff(user models.User) error + // Returns the new userid: the account has no password and has to be invited. + CreateStaff(user models.User) (int, error) AssignStaffToBranch(tenantID, userID, locationID int) error UpdateStaff(user models.User) error - CreateTenantLocation(data models.Tenantlocations) (models.Tenantlocations, error) + // Second return is the userid of the login this spawned for the branch, or 0 + // when an existing person was named and no account was created. + CreateTenantLocation(data models.Tenantlocations) (models.Tenantlocations, int, error) UpdateTenantLocation(data models.Tenantlocations) error CheckTenantByNo(cno string) int CreateTenantUser(data models.Tenants) (bool, error) GetUserByNo(cno string) models.UserInfo + PrimaryAdminForTenant(tenantID int) (InviteTarget, error) + InviteTargetForUser(userID int) (InviteTarget, error) + TenantNameByID(tenantID int) (string, error) GetTenantByID(tid int, locationid int, userid int) (models.Tenantinfo, error) AssignPartner(tenantID, partnerID int) error GetTenantByKeyword(keyword string) ([]models.TenantSearch, error) @@ -357,7 +363,20 @@ func (r *tenantRepository) GetStaffs(tid int) ([]models.StaffInfo, error) { -- until now, so Users & access had nothing to read and showed -- every person on the platform as "Unknown" — an admin could not -- tell a working login from one that had been switched off. - COALESCE(a.status,'') AS status + COALESCE(a.status,'') AS status, + -- Whether they have ever signed in — or can. + -- + -- Every back-office account is created with an empty password and + -- is emailed a link to choose one. Until they use it they are in + -- this list, in every branch picker, and cannot sign in at all. + -- Without this column the directory cannot tell that person from + -- anybody else, so a lost invitation is invisible until they say + -- so — and the screen has no way to offer them a new one. + -- + -- Computed here rather than by returning the password: there is no + -- reason for a cleartext password to travel up through a service + -- and a controller to answer a yes/no question. + (COALESCE(TRIM(a.password), '') <> '') AS issetup FROM app_users a LEFT JOIN tenantlocations b ON a.locationid = b.locationid LEFT JOIN app_roles c ON c.roleid = a.roleid @@ -383,27 +402,32 @@ func (r *tenantRepository) GetStaffs(tid int) ([]models.StaffInfo, error) { // `userid` is deliberately not set: it is a `GENERATED BY DEFAULT AS IDENTITY` // column and Postgres allocates it. Computing one here would leave the sequence // unadvanced and two allocators racing each other. -func (r *tenantRepository) CreateStaff(user models.User) error { +// The userid is returned because the account is created with NO password and the +// caller has to invite it. Postgres allocates the id and GORM writes it back +// onto `user`, so this costs nothing — and without it the service would have to +// look the row up again by authname, which is the one field a concurrent create +// could collide on. +func (r *tenantRepository) CreateStaff(user models.User) (int, error) { pin, err := ValidateStaffUser(&user) if err != nil { - return err + return 0, err } user.Pin = int(pin) if pin > 0 && user.Tenantid > 0 && user.Locationid > 0 { taken, err := posPinTaken(r.db, user.Tenantid, user.Locationid, pin, user.Userid) if err != nil { - return err + return 0, err } if taken { - return fmt.Errorf("another person at this outlet already uses that PIN") + return 0, fmt.Errorf("another person at this outlet already uses that PIN") } } if err := r.db.Table("app_users").Create(&user).Error; err != nil { - return err + return 0, err } - return nil + return user.Userid, nil } func (r *tenantRepository) UpdateStaff(user models.User) error { @@ -413,7 +437,7 @@ func (r *tenantRepository) UpdateStaff(user models.User) error { return nil } -func (r *tenantRepository) CreateTenantLocation(data models.Tenantlocations) (models.Tenantlocations, error) { +func (r *tenantRepository) CreateTenantLocation(data models.Tenantlocations) (models.Tenantlocations, int, error) { var user models.Tenantuser tx := r.db.Begin() @@ -438,7 +462,7 @@ func (r *tenantRepository) CreateTenantLocation(data models.Tenantlocations) (mo // authenticate. if data.Operatorid <= 0 && strings.TrimSpace(data.Email) == "" { tx.Rollback() - return models.Tenantlocations{}, errors.New( + return models.Tenantlocations{}, 0, errors.New( "a branch needs somebody to run it: name an existing user in operatorid, or give an email to create a login from") } @@ -447,7 +471,7 @@ func (r *tenantRepository) CreateTenantLocation(data models.Tenantlocations) (mo // QR code (payload is just {tenantid, locationid}) right after onboarding. if err := tx.Create(&data).Error; err != nil { tx.Rollback() - return models.Tenantlocations{}, err + return models.Tenantlocations{}, 0, err } // Step 2a: bind an existing person, when one was named. @@ -464,21 +488,25 @@ func (r *tenantRepository) CreateTenantLocation(data models.Tenantlocations) (mo Updates(map[string]any{"locationid": data.Locationid}) if res.Error != nil { tx.Rollback() - return models.Tenantlocations{}, res.Error + return models.Tenantlocations{}, 0, res.Error } if res.RowsAffected == 0 { // Either the person does not exist, belongs to another merchant, or // is a till account. All three are the same answer to the caller, // and none of them should leave a branch standing. tx.Rollback() - return models.Tenantlocations{}, fmt.Errorf( + return models.Tenantlocations{}, 0, fmt.Errorf( "user %d cannot run this branch — they belong to another business, do not exist, or are a till account", data.Operatorid) } if err := tx.Commit().Error; err != nil { - return models.Tenantlocations{}, err + return models.Tenantlocations{}, 0, err } - return data, nil + // No userid: nothing was created. The named person already had an account + // before this branch existed, so there is nothing here to invite — if + // THEY have never set a password, it is their own creation that owes them + // an invitation, not this one. + return data, 0, nil } // Step 2b: no person named — spawn a login, as before. @@ -508,15 +536,19 @@ func (r *tenantRepository) CreateTenantLocation(data models.Tenantlocations) (mo if err := tx.Table("app_users").Create(&user).Error; err != nil { tx.Rollback() - return models.Tenantlocations{}, err + return models.Tenantlocations{}, 0, err } // Commit if err := tx.Commit().Error; err != nil { - return models.Tenantlocations{}, err + return models.Tenantlocations{}, 0, err } - return data, nil + // The spawned login's userid, so the service can invite it. This account is + // created with `Password = ""` a few lines above, and the invitation is now + // the only way to fill that in — the sign-in screen no longer offers a form. + // Without this the branch would be commissioned with a login nobody can use. + return data, user.Userid, nil } func (r *tenantRepository) UpdateTenantLocation(input models.Tenantlocations) error { @@ -1062,3 +1094,119 @@ func (r *tenantRepository) AssignPartner(tenantID, partnerID int) error { } return nil } + +// InviteTarget is the account a tenant's invitation is addressed to. +type InviteTarget struct { + Userid int + Email string + Tenantname string + // True when the account already has a password, which means the merchant is + // set up and there is nothing to invite them to. + IsSetUp bool +} + +// PrimaryAdminForTenant finds the account an invitation should go to. +// +// ── Which of a tenant's users is "the" admin ──────────────────────────────── +// +// A business can have several roleid-3 accounts — staff added later are the +// same role. The one onboarding created is identified by its authname matching +// the tenant's own `primaryemail`, which is how `CreateTenantUser` writes it, +// and that is the account the invitation belongs to. Picking any roleid-3 row +// would email whichever staff member happened to sort first. +// +// `IsSetUp` is computed in the query rather than by returning the password. +// There is no reason for a hash — or on this backend, a cleartext password — to +// travel up through a service and a controller to answer a yes/no question. +func (r *tenantRepository) PrimaryAdminForTenant(tenantID int) (InviteTarget, error) { + if tenantID <= 0 { + return InviteTarget{}, errors.New("tenantid is required") + } + + var row InviteTarget + query := ` + SELECT u.userid AS userid, + COALESCE(NULLIF(TRIM(u.email), ''), t.primaryemail) AS email, + t.tenantname AS tenantname, + (COALESCE(TRIM(u.password), '') <> '') AS issetup + FROM tenants t + JOIN app_users u + ON u.tenantid = t.tenantid + AND LOWER(TRIM(u.authname)) = LOWER(TRIM(t.primaryemail)) + WHERE t.tenantid = ? + LIMIT 1` + + if err := r.db.Raw(query, tenantID).Scan(&row).Error; err != nil { + return InviteTarget{}, err + } + if row.Userid == 0 { + // Either no such tenant, or one whose primary email matches no account. + // The second happens when the address was changed on the tenant after + // onboarding without the login being changed with it — worth saying, + // because the fix is to correct one of the two rather than to resend. + return InviteTarget{}, fmt.Errorf( + "tenant %d has no account matching its primary email address", tenantID) + } + return row, nil +} + +// InviteTargetForUser finds one account by its userid. +// +// The other half of resend. `PrimaryAdminForTenant` answers "the owner of this +// business", which is the only account a tenant HAS at onboarding — but staff +// added later and the login every branch spawns are created with no password +// too, and there is exactly one of the owner, so they cannot be reached that +// way. An operator chasing a branch manager who never got their mail needs to +// name the person. +// +// The tenant is joined for its name only, and joined LEFT: a back-office account +// with no tenant is a Nearle staff row, and one exists — the platform agent's. +// Failing the lookup on that would be refusing to answer a question that has a +// perfectly good answer. +func (r *tenantRepository) InviteTargetForUser(userID int) (InviteTarget, error) { + if userID <= 0 { + return InviteTarget{}, errors.New("userid is required") + } + + var row InviteTarget + query := ` + SELECT u.userid AS userid, + COALESCE(NULLIF(TRIM(u.email), ''), TRIM(u.authname)) AS email, + COALESCE(t.tenantname, '') AS tenantname, + (COALESCE(TRIM(u.password), '') <> '') AS issetup + FROM app_users u + LEFT JOIN tenants t ON t.tenantid = u.tenantid + WHERE u.userid = ? + AND COALESCE(u.roleid, 0) NOT IN (7, 8) + LIMIT 1` + + if err := r.db.Raw(query, userID).Scan(&row).Error; err != nil { + return InviteTarget{}, err + } + if row.Userid == 0 { + // No such account, or a till one. Roles 7 and 8 are excluded because a + // cashier does not sign in to the console at all — they authenticate at + // the terminal with a PIN, and an invitation would send them to a screen + // that cannot help them. + return InviteTarget{}, fmt.Errorf( + "user %d is not a back-office account on this platform", userID) + } + return row, nil +} + +// TenantNameByID is the business's name, for an invitation's first line. +// +// Its own tiny read rather than a field threaded through the create paths: a +// staff account arrives carrying a tenantid and nothing else about the business, +// and the alternative was every caller passing a name it would have had to look +// up anyway. An empty name is not an error — `inviteMessage` says "your +// business" instead, which is worse copy and a working email. +func (r *tenantRepository) TenantNameByID(tenantID int) (string, error) { + if tenantID <= 0 { + return "", nil + } + var name string + err := r.db.Raw(`SELECT COALESCE(tenantname, '') FROM tenants WHERE tenantid = ? LIMIT 1`, + tenantID).Scan(&name).Error + return name, err +} diff --git a/routes/startup_test.go b/routes/startup_test.go index fd385f1..6c14a6b 100644 --- a/routes/startup_test.go +++ b/routes/startup_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "nearle/config" "nearle/facade" "github.com/gofiber/fiber/v2" @@ -42,7 +43,7 @@ func testFacade(t *testing.T) *facade.Facade { // No database, no catalogue, no embedder, no model. A deployment with none // of those must still boot and say what it is missing, rather than failing // somewhere the operator cannot see. - return facade.NewFacade(nil, nil, nil, nil, "", "no model in tests") + return facade.NewFacade(nil, nil, nil, nil, "", "no model in tests", nil, config.MailConfig{}) } func TestTheServerCanBeBuilt(t *testing.T) { diff --git a/routes/tenantroutes.go b/routes/tenantroutes.go index ccfafa6..d3be1ce 100644 --- a/routes/tenantroutes.go +++ b/routes/tenantroutes.go @@ -22,6 +22,14 @@ func RegisterTenantRoutes(api fiber.Router, f *facade.Facade) { tenant.Put("/updatetenantlocation", f.TenantController.UpdateTenantLocation) tenant.Post("/createtenantuser", f.TenantController.CreateTenantUser) + // Re-issuing a merchant's first-password link, for the one who never got + // the mail or whose invitation expired. + // + // Web group only, and the handler additionally requires a platform account: + // this mints a credential, and the merchant it would invite is by + // definition somebody who cannot sign in to ask for it themselves. + tenant.Post("/resendinvite", f.TenantController.ResendInvite) + // One business, by id. // // Also /mob-only until now, so the console's only way to read its own diff --git a/services/inviteEveryAccount_test.go b/services/inviteEveryAccount_test.go new file mode 100644 index 0000000..0cd39d9 --- /dev/null +++ b/services/inviteEveryAccount_test.go @@ -0,0 +1,337 @@ +package services + +import ( + "errors" + "strings" + "testing" + + "nearle/models" + "nearle/repositories" +) + +/* +Every account that is created with no password gets invited. + +── Why this file exists ───────────────────────────────────────────────────── + +There are three ways a back-office login comes into being on this platform, and +all three write `password = ''`: + + - `CreateTenantUser` — the merchant, at onboarding + - `CreateUser` / `CreateStaff` — a person added to the directory afterwards + - `CreateTenantLocation` — the login a branch spawns when no operator is named + +Only the first was ever invited. That was survivable while the sign-in screen +carried a "set your password" form, because the other two could use it. That form +is gone — it was an account takeover, since the probe behind it answered any +email with the userid needed to claim the account — so an uninvited account is now +one nobody can ever sign in to. It is listed, it appears in every branch picker, +and the first person to find out is whoever is standing in the shop. + +So these tests are about coverage of the three paths, not about the mail. What +the message says and when sending fails is `inviteService_test.go`. +*/ + +// countingInviter records who was invited. `countingInvites` in +// resendInvite_test.go counts calls and nothing else; this one keeps the +// arguments, because the whole question here is who got the mail. +type countingInviter struct { + calls int + userid int + tenantid int + email string + business string + sent bool + reason string + failEvery bool +} + +func (c *countingInviter) Invite(userid, tenantid int, email, businessName string) (bool, string) { + c.calls++ + c.userid, c.tenantid, c.email, c.business = userid, tenantid, email, businessName + if c.failEvery { + return false, c.reason + } + return c.sent, c.reason +} + +/* ── A person added to the directory ─────────────────────────────────────── */ + +type stubUserRepo struct { + repositories.UserRepository + newid int + err error +} + +func (r *stubUserRepo) CreateUser(models.User) (int, error) { + if r.err != nil { + return 0, r.err + } + return r.newid, nil +} + +func (r *stubUserRepo) GetUserById(uid int) (models.UserInfo, error) { + return models.UserInfo{Userid: uid}, nil +} + +func TestANewStaffAccountIsInvited(t *testing.T) { + invites := &countingInviter{sent: true} + service := NewUserService(&stubUserRepo{newid: 7781}, invites) + + _, outcome, err := service.CreateUser(models.User{ + Email: "meena@rmart.example", Tenantid: 1147, + }) + if err != nil { + t.Fatalf("CreateUser: %v", err) + } + + if !outcome.Sent { + t.Fatalf("hired and not invited: %+v", outcome) + } + if invites.userid != 7781 { + t.Errorf("invited user %d, want the one that was just created (7781)", invites.userid) + } + if invites.email != "meena@rmart.example" { + t.Errorf("invitation addressed to %q", invites.email) + } + if invites.tenantid != 1147 { + // The token carries the tenant, and the mail names the business. A zero + // here is an invitation from nobody, to an account belonging to nobody. + t.Errorf("invited against tenant %d, want 1147", invites.tenantid) + } +} + +func TestAStaffAccountWithOnlyAnAuthnameIsStillInvited(t *testing.T) { + // `PrepareNewAccount` copies email → authname, not the other way round, so a + // caller that filled in only the sign-in name leaves `Email` empty. That is + // the same mailbox, and refusing to write to it would strand the person over + // which of two identical fields was filled in. + invites := &countingInviter{sent: true} + service := NewUserService(&stubUserRepo{newid: 7782}, invites) + + if _, outcome, err := service.CreateUser(models.User{ + Authname: "arun@rmart.example", Tenantid: 1147, + }); err != nil || !outcome.Sent { + t.Fatalf("not invited: outcome=%+v err=%v", outcome, err) + } + if invites.email != "arun@rmart.example" { + t.Errorf("invitation addressed to %q, want the authname", invites.email) + } +} + +func TestHiringSucceedsWhenTheInvitationDoesNot(t *testing.T) { + // The person is hired either way. A mail relay that refuses the address must + // not undo a hire — the operator resends, or corrects the address. + invites := &countingInviter{failEvery: true, reason: "mailbox full"} + service := NewUserService(&stubUserRepo{newid: 7783}, invites) + + info, outcome, err := service.CreateUser(models.User{Email: "raj@rmart.example"}) + if err != nil { + t.Fatalf("a failed invitation failed the hire: %v", err) + } + if info.Userid != 7783 { + t.Fatalf("the account was not created: %+v", info) + } + if outcome.Sent || outcome.Reason != "mailbox full" { + t.Fatalf("the reason was lost: %+v", outcome) + } +} + +func TestAFailedCreateIsNotInvited(t *testing.T) { + invites := &countingInviter{sent: true} + service := NewUserService(&stubUserRepo{err: errors.New("duplicate authname")}, invites) + + if _, _, err := service.CreateUser(models.User{Email: "raj@rmart.example"}); err == nil { + t.Fatal("a failed create was reported as success") + } + if invites.calls != 0 { + t.Fatal("invited an account that was never created") + } +} + +func TestStaffCreatedThroughTheTenantPathIsAlsoInvited(t *testing.T) { + // Two endpoints make back-office accounts — `users/create` and + // `tenants/createstaff` — and the console has used both. An invitation on one + // only would be a gap nobody could see from the screen they were using. + invites := &countingInviter{sent: true} + service := NewTenantService(&recordingTenantRepo{}, invites) + + outcome, err := service.CreateStaff(models.User{ + Email: "kavi@rmart.example", Tenantid: 1147, + }) + if err != nil { + t.Fatalf("CreateStaff: %v", err) + } + if !outcome.Sent || invites.userid != 5150 { + t.Fatalf("not invited, or the wrong account: outcome=%+v userid=%d", outcome, invites.userid) + } +} + +/* ── The login a branch spawns ───────────────────────────────────────────── */ + +type branchRepo struct { + repositories.TenantRepository + spawned int + err error +} + +func (r *branchRepo) CreateTenantLocation(data models.Tenantlocations) (models.Tenantlocations, int, error) { + if r.err != nil { + return models.Tenantlocations{}, 0, r.err + } + data.Locationid = 4420 + return data, r.spawned, nil +} + +func TestABranchesOwnLoginIsInvited(t *testing.T) { + invites := &countingInviter{sent: true} + service := NewTenantService(&branchRepo{spawned: 9001}, invites) + + resp := service.CreateTenantLocation(models.Tenantlocations{ + Locationname: "R Mart Peelamedu", Email: "peelamedu@rmart.example", + Tenantid: 1147, Address: "100 Feet Road", City: "Coimbatore", + }) + + if resp["status"] != true { + t.Fatalf("branch not created: %+v", resp) + } + if resp["invited"] != true { + t.Fatalf("the branch's own login was not invited: %+v", resp) + } + if invites.userid != 9001 { + t.Errorf("invited user %d, want the spawned login (9001)", invites.userid) + } + if invites.email != "peelamedu@rmart.example" { + t.Errorf("invitation addressed to %q, want the branch's email", invites.email) + } +} + +func TestABranchHandedToAnExistingPersonSendsNothing(t *testing.T) { + // `spawned: 0` is the repository saying it created no account, because an + // `operatorid` was named. That person had a login before this branch existed, + // and re-inviting them would be a password reset in a branch's clothing — + // the one thing this whole mechanism is built to not become. + invites := &countingInviter{sent: true} + service := NewTenantService(&branchRepo{spawned: 0}, invites) + + resp := service.CreateTenantLocation(models.Tenantlocations{ + Locationname: "R Mart Gandhipuram", Operatorid: 7781, Tenantid: 1147, + }) + + if resp["status"] != true { + t.Fatalf("branch not created: %+v", resp) + } + if invites.calls != 0 { + t.Fatal("re-invited an existing person because a branch was commissioned") + } + if resp["invited"] != false { + t.Errorf("invited should be false when there was nobody to invite: %+v", resp) + } + if reason, _ := resp["invitereason"].(string); reason != "" { + // Nothing went wrong, so nothing is explained. A reason here reads as a + // failure on a screen that is reporting a success. + t.Errorf("apologised for an invitation that was never owed: %q", reason) + } +} + +func TestAFailedBranchIsNotInvited(t *testing.T) { + invites := &countingInviter{sent: true} + service := NewTenantService(&branchRepo{err: errors.New("no such tenant")}, invites) + + resp := service.CreateTenantLocation(models.Tenantlocations{ + Locationname: "R Mart Nowhere", Email: "nowhere@rmart.example", + }) + + if resp["status"] != false { + t.Fatalf("a failed create was reported as success: %+v", resp) + } + if invites.calls != 0 { + t.Fatal("invited a login for a branch that does not exist") + } +} + +/* ── Resending to one named person ───────────────────────────────────────── */ + +type userTargetRepo struct { + repositories.TenantRepository + target repositories.InviteTarget + err error + asked int +} + +func (r *userTargetRepo) InviteTargetForUser(userID int) (repositories.InviteTarget, error) { + r.asked = userID + if r.err != nil { + return repositories.InviteTarget{}, r.err + } + return r.target, nil +} + +func TestResendReachesAStaffMemberByUserid(t *testing.T) { + // The owner is reachable by tenantid because there is one of them. Everybody + // else has to be named, and before this there was no way to reach them at + // all — the only repair was editing the database. + repo := &userTargetRepo{target: repositories.InviteTarget{ + Userid: 7781, Email: "meena@rmart.example", Tenantname: "R Mart", + }} + invites := &countingInviter{sent: true} + + outcome, err := NewTenantService(repo, invites).ResendInviteToUser(7781) + if err != nil { + t.Fatalf("ResendInviteToUser: %v", err) + } + if !outcome.Sent || repo.asked != 7781 || invites.userid != 7781 { + t.Fatalf("wrong person: outcome=%+v asked=%d invited=%d", outcome, repo.asked, invites.userid) + } +} + +func TestResendToAUserRefusesOneWhoAlreadyHasAPassword(t *testing.T) { + // Same line as the tenant resend draws, and it has to be drawn here too: + // this endpoint takes any userid, so without the check it would re-issue a + // working password link for every account on the platform. + repo := &userTargetRepo{target: repositories.InviteTarget{ + Userid: 7781, Email: "meena@rmart.example", Tenantname: "R Mart", IsSetUp: true, + }} + invites := &countingInviter{sent: true} + + _, err := NewTenantService(repo, invites).ResendInviteToUser(7781) + if err == nil { + t.Fatal("re-invited an account that already has a password") + } + if invites.calls != 0 { + t.Fatal("a link was minted for an account that is already set up") + } + if !strings.Contains(err.Error(), "sign-in") { + t.Errorf("the refusal does not say what to do instead: %v", err) + } +} + +func TestResendToAUserWithNoBusinessStillReadsSensibly(t *testing.T) { + // A back-office account with no tenant is a Nearle staff row, and one exists. + // The refusal interpolates the business name, so an empty one would read + // " has already set a password". + repo := &userTargetRepo{target: repositories.InviteTarget{ + Userid: 12, Email: "ops@nearle.in", Tenantname: "", IsSetUp: true, + }} + + _, err := NewTenantService(repo, &countingInviter{}).ResendInviteToUser(12) + if err == nil { + t.Fatal("re-invited an account that already has a password") + } + if strings.HasPrefix(err.Error(), " ") || strings.Contains(err.Error(), " ") { + t.Errorf("the refusal has a hole where the business name should be: %q", err) + } +} + +func TestResendToAUserPassesThroughALookupFailure(t *testing.T) { + repo := &userTargetRepo{err: errors.New("user 99 is not a back-office account on this platform")} + invites := &countingInviter{} + + _, err := NewTenantService(repo, invites).ResendInviteToUser(99) + if err == nil || !strings.Contains(err.Error(), "back-office") { + t.Fatalf("the lookup's reason was lost: %v", err) + } + if invites.calls != 0 { + t.Fatal("a link was minted for an account that could not be found") + } +} diff --git a/services/inviteService.go b/services/inviteService.go new file mode 100644 index 0000000..ec92947 --- /dev/null +++ b/services/inviteService.go @@ -0,0 +1,151 @@ +package services + +import ( + "fmt" + "log" + "strings" + "time" + + "nearle/config" + "nearle/utils" +) + +// The invitation a newly onboarded merchant receives. +// +// ── Why onboarding does not fail when this does ───────────────────────────── +// +// `Invite` never returns an error to the onboarding path. A tenant that exists +// and has not been emailed is recoverable — somebody presses resend — while a +// tenant rolled back because a mail relay was slow is a business that was +// onboarded, told it was onboarded, and is not in the system. The first is a +// task; the second is a phone call nobody can explain. +// +// So a failure is logged loudly and reported as `false`, and the caller decides +// what to tell the operator. The platform console shows "invitation not sent" +// beside the tenant, which is the state somebody can act on. +type InviteService interface { + // Invite emails a first-password link. Reports whether it was sent, and + // why not when it was not — a sentence for the operator, not an error. + // + // `businessName` may be empty: the name is then looked up from `tenantid`, + // because most callers hold an account and a tenantid and nothing else about + // the business. A caller that already has the name — onboarding, which was + // handed it in the form — passes it and saves the query. + Invite(userid, tenantid int, email, businessName string) (bool, string) +} + +// TenantNamer reads a business's name for the invitation's first line. +// +// A one-method interface rather than the whole tenant repository, because that +// is all this needs and because it keeps `inviteService` testable without a +// database. `repositories.TenantRepository` satisfies it. +type TenantNamer interface { + TenantNameByID(tenantID int) (string, error) +} + +type inviteService struct { + mailer utils.Mailer + cfg config.MailConfig + // May be nil. The invitation then says "your business", which is worse copy + // and a working link — never a reason not to send. + names TenantNamer +} + +func NewInviteService(mailer utils.Mailer, cfg config.MailConfig, names TenantNamer) InviteService { + return &inviteService{mailer: mailer, cfg: cfg, names: names} +} + +func (s *inviteService) Invite(userid, tenantid int, email, businessName string) (bool, string) { + address := strings.TrimSpace(email) + if address == "" { + return false, "no email address on the account" + } + if s.mailer == nil { + // Not a fault. A deployment with no mail configured still onboards; the + // reason names the variable so it is fixable rather than mysterious. + return false, s.cfg.Why() + } + + token, _, err := utils.MintInviteToken( + utils.InviteClaims{Userid: userid, Tenantid: tenantid}, time.Now()) + if err != nil { + // Only happens with no signing secret, which is already fatal at boot + // in production — but an invitation with no token would be a link that + // cannot work, and sending it would be worse than not sending. + log.Printf("invite: could not sign an invitation for user %d: %v", userid, err) + return false, "this server cannot sign an invitation" + } + + subject, body := inviteMessage(s.businessName(tenantid, businessName), s.cfg.InviteLink(token)) + + if err := s.mailer.Send(address, subject, body); err != nil { + log.Printf("invite: could not email user %d at %s: %v", userid, address, err) + return false, err.Error() + } + + log.Printf("invite: sent to user %d for tenant %d", userid, tenantid) + return true, "" +} + +// businessName is the name for the mail's first line. +// +// Looked up only when the caller did not have one. A failure is logged and +// swallowed: the alternative is refusing to send somebody their only way into +// their account because a name could not be read, and "your business has been +// set up on Nearle" is a perfectly usable sentence. +func (s *inviteService) businessName(tenantid int, given string) string { + if name := strings.TrimSpace(given); name != "" { + return name + } + if s.names == nil || tenantid <= 0 { + return "" + } + name, err := s.names.TenantNameByID(tenantid) + if err != nil { + log.Printf("invite: could not read tenant %d's name: %v", tenantid, err) + return "" + } + return strings.TrimSpace(name) +} + +// inviteMessage is what the merchant reads. +// +// ── Why it says so little ─────────────────────────────────────────────────── +// +// This is the first thing a new merchant receives from us and the only way into +// their account, so it has one job: make the link obvious and make it credible. +// Every extra paragraph is somewhere for the link to hide, and a mail full of +// features reads like marketing — which is the thing people delete. +// +// It states who it is for and what it does, gives the link on its own line, and +// says how long it lasts. The expiry is there because an invitation found three +// weeks later needs to explain itself rather than look broken. +// +// Plain text, not HTML. A password link that arrives as an image-heavy template +// is the shape of a phishing mail, and plain text renders identically +// everywhere. +func inviteMessage(businessName, link string) (subject, body string) { + name := strings.TrimSpace(businessName) + if name == "" { + name = "your business" + } + + subject = "Set your Nearle password" + + body = fmt.Sprintf(`%s has been set up on Nearle. + +To finish, choose a password for your account: + +%s + +This link is for you alone and works once. It expires in 7 days — if it has, +ask whoever set you up to send another. + +If you were not expecting this, you can ignore it. Nothing happens until +somebody uses the link. + +— Nearle +`, name, link) + + return subject, body +} diff --git a/services/inviteService_test.go b/services/inviteService_test.go new file mode 100644 index 0000000..d16d423 --- /dev/null +++ b/services/inviteService_test.go @@ -0,0 +1,241 @@ +package services + +import ( + "errors" + "strings" + "testing" + + "nearle/config" +) + +/* +The invitation a newly onboarded merchant receives. + +It is the only way into their account, so the tests are about two things: that a +failure to send never costs them the tenant, and that the message itself is one +a person will act on rather than delete. +*/ + +type recordingMailer struct { + to, subject, body string + refuse error + sent int +} + +func (m *recordingMailer) Send(to, subject, body string) error { + if m.refuse != nil { + return m.refuse + } + m.to, m.subject, m.body = to, subject, body + m.sent++ + return nil +} + +func workingMail() config.MailConfig { + return config.MailConfig{ + Host: "smtp.example.com", Port: 587, + FromAddress: "noreply@nearledaily.com", FromName: "Nearle", + ConsoleURL: "https://app.nearledaily.com", + } +} + +func TestAnInvitationCarriesALinkAndNothingElseIdentifying(t *testing.T) { + t.Setenv("POS_TOKEN_SECRET", "a-signing-secret-of-ample-length") + + mailer := &recordingMailer{} + sent, reason := NewInviteService(mailer, workingMail(), nil). + Invite(904, 1147, "owner@rmart.example", "R Mart") + + if !sent { + t.Fatalf("not sent: %s", reason) + } + if mailer.to != "owner@rmart.example" { + t.Fatalf("addressed to %q", mailer.to) + } + if !strings.Contains(mailer.body, "https://app.nearledaily.com/set-password?t=i1.") { + t.Fatalf("no invitation link in the body:\n%s", mailer.body) + } + + // The token is the whole credential, so it is the only thing in the URL. + // An email address or a userid in a query string ends up in server logs, + // browser history and whatever proxy sits between — which would put both + // halves of an account somewhere neither belongs. + link := mailer.body[strings.Index(mailer.body, "https://"):] + link = strings.Fields(link)[0] + if strings.Contains(link, "@") || strings.Contains(link, "904") { + t.Fatalf("the link identifies the account beyond the token: %s", link) + } +} + +func TestTheInvitationNamesTheBusinessAndTheExpiry(t *testing.T) { + // Named, because this arrives unannounced at an address the merchant gave + // during a sales conversation weeks earlier. "Your business has been set + // up" reads like a phishing template; their own name does not. + // + // The expiry is there so an invitation found three weeks later explains + // itself rather than looking broken. + t.Setenv("POS_TOKEN_SECRET", "a-signing-secret-of-ample-length") + + mailer := &recordingMailer{} + NewInviteService(mailer, workingMail(), nil).Invite(904, 1147, "owner@rmart.example", "R Mart") + + if !strings.Contains(mailer.body, "R Mart") { + t.Fatalf("the business is not named:\n%s", mailer.body) + } + if !strings.Contains(mailer.body, "7 days") { + t.Fatalf("the expiry is not stated:\n%s", mailer.body) + } + if strings.TrimSpace(mailer.subject) == "" { + t.Fatal("no subject") + } +} + +func TestAnUnnamedBusinessStillReadsAsASentence(t *testing.T) { + // `tenantname` is not enforced anywhere upstream, and " has been set up on + // Nearle" is the kind of thing that ships. + t.Setenv("POS_TOKEN_SECRET", "a-signing-secret-of-ample-length") + + mailer := &recordingMailer{} + NewInviteService(mailer, workingMail(), nil).Invite(904, 1147, "owner@rmart.example", " ") + + if strings.Contains(mailer.body, " has been set up") { + t.Fatalf("the blank name left a gap:\n%s", mailer.body) + } + if !strings.Contains(mailer.body, "your business") { + t.Fatalf("no fallback for an unnamed business:\n%s", mailer.body) + } +} + +func TestNoMailerMeansNotSentRatherThanAPanic(t *testing.T) { + // The ordinary state of a deployment that has not configured mail. It must + // report, not crash and not pretend. + sent, reason := NewInviteService(nil, config.MailConfig{}, nil). + Invite(904, 1147, "owner@rmart.example", "R Mart") + + if sent { + t.Fatal("reported as sent with no mailer") + } + if !strings.Contains(reason, "MAIL_HOST") { + t.Fatalf("the reason does not name what is missing: %q", reason) + } +} + +func TestARefusedSendIsReportedNotSwallowed(t *testing.T) { + // The operator has to learn that the merchant was not emailed, or the + // merchant waits for a link that never comes and nobody knows. + t.Setenv("POS_TOKEN_SECRET", "a-signing-secret-of-ample-length") + + mailer := &recordingMailer{refuse: errors.New("mailbox full")} + sent, reason := NewInviteService(mailer, workingMail(), nil). + Invite(904, 1147, "owner@rmart.example", "R Mart") + + if sent { + t.Fatal("a refused send reported as sent") + } + if !strings.Contains(reason, "mailbox full") { + t.Fatalf("the provider's reason was lost: %q", reason) + } +} + +func TestAnAccountWithNoEmailIsNotInvited(t *testing.T) { + // `primaryemail` is not enforced at creation. Worth reporting rather than + // handing an empty address to the relay and reading its refusal instead. + t.Setenv("POS_TOKEN_SECRET", "a-signing-secret-of-ample-length") + + mailer := &recordingMailer{} + sent, reason := NewInviteService(mailer, workingMail(), nil).Invite(904, 1147, " ", "R Mart") + + if sent || mailer.sent != 0 { + t.Fatal("an invitation was sent to nobody") + } + if !strings.Contains(reason, "no email") { + t.Fatalf("unhelpful reason: %q", reason) + } +} + +func TestNoSigningSecretMeansNoInvitationRatherThanADeadLink(t *testing.T) { + // Sending a link that cannot work is worse than not sending: the merchant + // tries it, it fails, and the failure looks like the product. + t.Setenv("POS_TOKEN_SECRET", "") + t.Setenv("JWT_SECRET_KEY", "") + + mailer := &recordingMailer{} + sent, _ := NewInviteService(mailer, workingMail(), nil). + Invite(904, 1147, "owner@rmart.example", "R Mart") + + if sent || mailer.sent != 0 { + t.Fatal("an invitation went out with no signing secret") + } +} + +/* ── Whose name is on the mail ───────────────────────────────────────────── */ + +type stubNamer struct { + name string + err error + asked int +} + +func (s *stubNamer) TenantNameByID(tenantID int) (string, error) { + s.asked = tenantID + return s.name, s.err +} + +func TestTheBusinessNameIsLookedUpWhenTheCallerHasNone(t *testing.T) { + // A staff row arrives with a tenantid and nothing else about the business, so + // the caller cannot name it. Without the lookup every invitation but the + // merchant's own would open "your business has been set up on Nearle". + t.Setenv("POS_TOKEN_SECRET", "a-signing-secret-of-ample-length") + + mailer := &recordingMailer{} + namer := &stubNamer{name: "R Mart"} + + if sent, reason := NewInviteService(mailer, workingMail(), namer). + Invite(7781, 1147, "meena@rmart.example", ""); !sent { + t.Fatalf("not sent: %s", reason) + } + + if namer.asked != 1147 { + t.Errorf("looked up tenant %d, want 1147", namer.asked) + } + if !strings.Contains(mailer.body, "R Mart") { + t.Errorf("the mail does not name the business:\n%s", mailer.body) + } +} + +func TestACallerThatKnowsTheNameIsNotMadeToLookItUp(t *testing.T) { + // Onboarding was handed the name in the form. A query to learn something the + // caller already holds is a round trip inside a request somebody is waiting + // on, for a guaranteed identical answer. + t.Setenv("POS_TOKEN_SECRET", "a-signing-secret-of-ample-length") + + namer := &stubNamer{name: "Should Not Be Read"} + NewInviteService(&recordingMailer{}, workingMail(), namer). + Invite(904, 1147, "owner@rmart.example", "R Mart") + + if namer.asked != 0 { + t.Error("looked the name up although the caller passed one") + } +} + +func TestAnUnreadableBusinessNameStillSendsTheInvitation(t *testing.T) { + // The name is one word in the first line. The link is the person's only way + // into their account, and refusing to send it over a failed lookup would + // trade a worse sentence for somebody locked out. + t.Setenv("POS_TOKEN_SECRET", "a-signing-secret-of-ample-length") + + mailer := &recordingMailer{} + namer := &stubNamer{err: errors.New("connection reset")} + + sent, reason := NewInviteService(mailer, workingMail(), namer). + Invite(7781, 1147, "meena@rmart.example", "") + if !sent { + t.Fatalf("a failed name lookup stopped the invitation: %s", reason) + } + if !strings.Contains(mailer.body, "your business") { + t.Errorf("no fallback for the missing name:\n%s", mailer.body) + } + if !strings.Contains(mailer.body, "set-password?t=") { + t.Errorf("the link is missing:\n%s", mailer.body) + } +} diff --git a/services/newAccount_test.go b/services/newAccount_test.go index cee720d..7656a5e 100644 --- a/services/newAccount_test.go +++ b/services/newAccount_test.go @@ -93,7 +93,7 @@ func (r *recordingUserRepo) GetUserById(uid int) (models.UserInfo, error) { func TestCreateUserActuallyPreparesTheAccount(t *testing.T) { repo := &recordingUserRepo{} - if _, err := NewUserService(repo).CreateUser(models.User{ + if _, _, err := NewUserService(repo, nil).CreateUser(models.User{ Email: "thiruomart@gmail.com", }); err != nil { t.Fatalf("CreateUser: %v", err) @@ -114,16 +114,16 @@ type recordingTenantRepo struct { created models.User } -func (r *recordingTenantRepo) CreateStaff(user models.User) error { +func (r *recordingTenantRepo) CreateStaff(user models.User) (int, error) { r.created = user - return nil + return 5150, nil } // The other creation path. Both make back-office accounts, so both have to // prepare them — and only one of them did. func TestCreateStaffActuallyPreparesTheAccount(t *testing.T) { repo := &recordingTenantRepo{} - if err := NewTenantService(repo).CreateStaff(models.User{Email: "suriya@example.com"}); err != nil { + if _, err := NewTenantService(repo, nil).CreateStaff(models.User{Email: "suriya@example.com"}); err != nil { t.Fatalf("CreateStaff: %v", err) } if repo.created.Authname != "suriya@example.com" || repo.created.Configid != ConsoleConfigID { diff --git a/services/resendInvite_test.go b/services/resendInvite_test.go new file mode 100644 index 0000000..c58c4fb --- /dev/null +++ b/services/resendInvite_test.go @@ -0,0 +1,168 @@ +package services + +import ( + "errors" + "strings" + "testing" + + "nearle/config" + "nearle/models" + "nearle/repositories" +) + +/* +Re-issuing a merchant's first-password link. + +Invitations get lost — spam folders, typo'd addresses, a seven-day expiry that +runs out over a holiday. Without a resend the only recovery is a database edit, +so this exists. + +It is also the endpoint most at risk of quietly becoming something else. An +endpoint that re-issues a working password link for any account IS a password +reset, whatever it is called, and nothing on this backend verifies identity well +enough to support one. So most of what follows is about what it refuses. +*/ + +type inviteRepo struct { + repositories.TenantRepository + target repositories.InviteTarget + err error +} + +func (r *inviteRepo) PrimaryAdminForTenant(int) (repositories.InviteTarget, error) { + if r.err != nil { + return repositories.InviteTarget{}, r.err + } + return r.target, nil +} + +type countingInvites struct { + calls int + sent bool + reason string +} + +func (c *countingInvites) Invite(_, _ int, _, _ string) (bool, string) { + c.calls++ + return c.sent, c.reason +} + +func waiting() repositories.InviteTarget { + return repositories.InviteTarget{ + Userid: 904, Email: "owner@rmart.example", Tenantname: "R Mart", IsSetUp: false, + } +} + +func TestResendEmailsAMerchantWhoNeverGotOne(t *testing.T) { + invites := &countingInvites{sent: true} + service := NewTenantService(&inviteRepo{target: waiting()}, invites) + + outcome, err := service.ResendInvite(1147) + if err != nil { + t.Fatalf("resend: %v", err) + } + if !outcome.Sent || invites.calls != 1 { + t.Fatalf("not sent: %+v, calls=%d", outcome, invites.calls) + } +} + +func TestResendRefusesAMerchantWhoAlreadyHasAPassword(t *testing.T) { + // The line between a resend and a password reset. + // + // `SetInitialPassword` would refuse such a link anyway, so the merchant + // could come to no harm — but the operator would be told mail was sent, the + // merchant would follow a link that does nothing, and neither would know + // why. Refusing here names the real situation. + target := waiting() + target.IsSetUp = true + + invites := &countingInvites{sent: true} + service := NewTenantService(&inviteRepo{target: target}, invites) + + _, err := service.ResendInvite(1147) + if err == nil { + t.Fatal("re-invited an account that already has a password") + } + if invites.calls != 0 { + t.Fatal("a link was minted for an account that is already set up") + } + // Says what to do instead, because the merchant's actual problem is signing + // in rather than setting up. + if !strings.Contains(err.Error(), "sign-in") { + t.Fatalf("the refusal does not say what to do instead: %v", err) + } +} + +func TestResendNamesTheBusinessItRefused(t *testing.T) { + // An operator working through a list needs to know which one, not that + // "an account" was already set up. + target := waiting() + target.IsSetUp = true + + _, err := NewTenantService(&inviteRepo{target: target}, &countingInvites{}).ResendInvite(1147) + if err == nil || !strings.Contains(err.Error(), "R Mart") { + t.Fatalf("the refusal does not name the business: %v", err) + } +} + +func TestResendPassesThroughALookupFailure(t *testing.T) { + // No such tenant, or one whose primary email matches no login — which + // happens when the address is changed on the tenant without the account + // being changed with it. The fix is to correct one of the two, so the + // message has to survive rather than become "could not resend". + repo := &inviteRepo{err: errors.New("tenant 1147 has no account matching its primary email address")} + invites := &countingInvites{} + + _, err := NewTenantService(repo, invites).ResendInvite(1147) + if err == nil || !strings.Contains(err.Error(), "primary email") { + t.Fatalf("the lookup's reason was lost: %v", err) + } + if invites.calls != 0 { + t.Fatal("a link was minted for an account that could not be found") + } +} + +func TestResendWithNoMailConfiguredSaysSoRatherThanFailing(t *testing.T) { + // Not an error: the tenant is fine and the server simply cannot send. The + // controller turns this into a refusal for the operator, with the variable + // named. + service := NewTenantService(&inviteRepo{target: waiting()}, nil) + + outcome, err := service.ResendInvite(1147) + if err != nil { + t.Fatalf("unconfigured mail reported as an error: %v", err) + } + if outcome.Sent || !strings.Contains(outcome.Reason, "not configured") { + t.Fatalf("unhelpful outcome: %+v", outcome) + } +} + +func TestResendReportsWhyTheMailWasRefused(t *testing.T) { + invites := &countingInvites{sent: false, reason: "mailbox full"} + service := NewTenantService(&inviteRepo{target: waiting()}, invites) + + outcome, err := service.ResendInvite(1147) + if err != nil { + t.Fatalf("resend: %v", err) + } + if outcome.Sent || outcome.Reason != "mailbox full" { + t.Fatalf("the provider's reason was lost: %+v", outcome) + } +} + +// Onboarding and resend share the invite path, so a nil mailer must be safe on +// both. This is the create side. +func TestOnboardingWithNoMailStillCreatesTheTenant(t *testing.T) { + _ = models.Tenants{} + _ = config.MailConfig{} + + service := NewTenantService(&inviteRepo{target: waiting()}, nil) + outcome := service.(*tenantService).inviteFor(models.UserInfo{Userid: 904}, models.Tenants{}) + + if outcome.Sent { + t.Fatal("reported as sent with no invite service") + } + if outcome.Reason == "" { + t.Fatal("not sent, and no reason for the operator") + } +} diff --git a/services/tenantService.go b/services/tenantService.go index cff6aec..c78fa98 100644 --- a/services/tenantService.go +++ b/services/tenantService.go @@ -2,6 +2,7 @@ package services import ( "errors" + "fmt" "net/http" "strings" @@ -25,22 +26,43 @@ type TenantService interface { UpdateTenantProfile(tenantID int, fields map[string]any) error UpdateOwnProfile(userID, tenantID int, fields map[string]any) error GetStaffs(tid int) ([]models.StaffInfo, error) - CreateStaff(user models.User) error + // Adds a back-office person and emails their first-password invitation. + // + // Same shape as `CreateTenantUser`, and for the same reason: the account is + // created with no password, and whether the mail left is a separate fact the + // console has to show. A failure to send is not a failure to hire. + CreateStaff(user models.User) (InviteOutcome, error) AssignStaffToBranch(tenantID, userID, locationID int) error UpdateStaff(user models.User) error CreateTenantLocation(data models.Tenantlocations) map[string]interface{} UpdateTenantLocation(data models.Tenantlocations) map[string]interface{} - CreateTenantUser(data models.Tenants) (models.UserInfo, error) + // Onboards a merchant and emails their first-password invitation. + // + // The outcome is returned rather than stashed on the service: it belongs to + // one call, and a field would race between two operators onboarding at the + // same moment. + CreateTenantUser(data models.Tenants) (models.UserInfo, InviteOutcome, error) + // ResendInvite re-issues a first-password link for a tenant that never got + // one, or whose invitation expired. + ResendInvite(tenantID int) (InviteOutcome, error) + // ResendInviteToUser does the same for one named account — a staff member or + // a branch's own login, neither of which is reachable by tenantid because a + // business has many of them. + ResendInviteToUser(userID int) (InviteOutcome, error) GetTenantByID(tid int, locationid int, userid int) (models.Tenantinfo, error) GetTenantByKeyword(keyword string) ([]models.TenantSearch, error) } type tenantService struct { repo repositories.TenantRepository + // May be nil. A deployment with no mail configured still onboards tenants + // — the merchant is told by whoever set them up — and the outcome says so + // rather than the creation failing. + invites InviteService } -func NewTenantService(repo repositories.TenantRepository) TenantService { - return &tenantService{repo: repo} +func NewTenantService(repo repositories.TenantRepository, invites InviteService) TenantService { + return &tenantService{repo: repo, invites: invites} } func (s *tenantService) SearchTenant(status, keyword string) ([]models.Tenantinfo, error) { @@ -105,11 +127,53 @@ func (s *tenantService) GetStaffs(tid int) ([]models.StaffInfo, error) { return s.repo.GetStaffs(tid) } -func (s *tenantService) CreateStaff(user models.User) error { +func (s *tenantService) CreateStaff(user models.User) (InviteOutcome, error) { // Same two fields, same reason: without an authname and a console configid // the account is created, listed, and refused at the login screen. This path // and users/create both make back-office accounts, so both need it. - return s.repo.CreateStaff(PrepareNewAccount(user)) + ready := PrepareNewAccount(user) + + userid, err := s.repo.CreateStaff(ready) + if err != nil { + return InviteOutcome{}, err + } + + // The invitation, for the same reason onboarding sends one: this account is + // created with no password, and the sign-in screen no longer offers to set + // one. Without the mail the person is added to the directory, appears in + // every branch picker, and cannot sign in — and nothing anywhere would say + // so. That was true for a while, and this is the fix. + // + // After the write and outside it, like the tenant's. A person who exists and + // was not emailed is a resend; a person rolled back by a slow mail relay is + // somebody the manager was told they had hired. + return s.inviteAccount(userid, ready.Tenantid, ready.Email, ready.Authname), nil +} + +// inviteAccount emails one newly created back-office account. +// +// The business name is left to the invite service, which looks it up from the +// tenantid: a staff row arrives with an id and nothing else about the business, +// and every caller doing that lookup itself would be the same query written four +// times. +func (s *tenantService) inviteAccount(userid, tenantid int, email, authname string) InviteOutcome { + if s.invites == nil { + return InviteOutcome{Reason: "invitations are not configured on this server"} + } + if userid <= 0 { + return InviteOutcome{Reason: "the new account could not be read back to invite it"} + } + + address := strings.TrimSpace(email) + if address == "" { + // The authname IS the email on every back-office account — `users/create` + // and `createstaff` both copy one to the other — so this is a fallback + // for a caller that filled in only one of the two, not a second address. + address = strings.TrimSpace(authname) + } + + sent, reason := s.invites.Invite(userid, tenantid, address, "") + return InviteOutcome{Sent: sent, Reason: reason} } func (s *tenantService) UpdateStaff(user models.User) error { @@ -125,7 +189,7 @@ func (s *tenantService) CreateTenantLocation(data models.Tenantlocations) map[st data.Address, data.Suburb, data.City, data.State, data.Postcode) } - created, err := s.repo.CreateTenantLocation(data) + created, spawnedUserid, err := s.repo.CreateTenantLocation(data) if err != nil { return map[string]interface{}{ "code": http.StatusConflict, @@ -134,6 +198,17 @@ func (s *tenantService) CreateTenantLocation(data models.Tenantlocations) map[st } } + // A branch that spawned its own login needs that login invited — it is + // created with no password, and the invitation is the only way to set one. + // `spawnedUserid` is 0 when an existing person was named instead, and there + // is deliberately nothing to send then: they had an account before this + // branch existed, and re-inviting somebody who may already have a password + // would be a password reset wearing a branch's clothes. + invite := InviteOutcome{} + if spawnedUserid > 0 { + invite = s.inviteAccount(spawnedUserid, data.Tenantid, data.Email, data.Email) + } + // "details" carries back the DB-assigned locationid so the frontend can // build the store's QR code (tenantid+locationid) immediately after // onboarding, instead of having to look the new location up separately. @@ -142,6 +217,21 @@ func (s *tenantService) CreateTenantLocation(data models.Tenantlocations) map[st "message": "Tenant Location Successfully Created", "status": true, "details": created, + // Beside "details" for the same reason it is on the tenant create: the + // branch exists either way, and whether its operator was emailed is a + // separate fact the console has to be able to show. + "invited": invite.Sent, + // Omitted when it sent, and when there was nobody to send to — a branch + // handed to an existing person has no invitation to report, and an + // apology there would read as a failure. + "invitereason": invite.Reason, + // Who to resend to, when it did not go. 0 when no login was spawned. + // + // The console cannot work this out: `details` is the tenantlocations row, + // and the account lives in `app_users`. Without this the only route to a + // resend is finding the right row in the people list by eye, on a screen + // that has just told somebody the mail failed. + "inviteuserid": spawnedUserid, } } @@ -162,11 +252,11 @@ func (s *tenantService) UpdateTenantLocation(data models.Tenantlocations) map[st } } -func (s *tenantService) CreateTenantUser(data models.Tenants) (models.UserInfo, error) { +func (s *tenantService) CreateTenantUser(data models.Tenants) (models.UserInfo, InviteOutcome, error) { // ✅ Check if tenant already exists exists := s.repo.CheckTenantByNo(data.Primarycontact) if exists != 0 { - return models.UserInfo{}, errors.New("Tenant Already Exists") + return models.UserInfo{}, InviteOutcome{}, errors.New("Tenant Already Exists") } // Coordinates from the address, for the tenant and its primary outlet. @@ -184,12 +274,61 @@ func (s *tenantService) CreateTenantUser(data models.Tenants) (models.UserInfo, // ✅ Create Tenant User status, err := s.repo.CreateTenantUser(data) if err != nil || !status { - return models.UserInfo{}, err + return models.UserInfo{}, InviteOutcome{}, err } // ✅ Get user details by contact number result := s.repo.GetUserByNo(data.Primarycontact) - return result, nil + + // The invitation, sent after everything above is committed and never inside + // it. `CreateTenantUser` in the repository runs a transaction; this does not + // join it. + // + // A tenant that exists and has not been emailed is recoverable — somebody + // presses resend. A tenant rolled back because a mail relay was slow is a + // business that was onboarded, told it was onboarded, and is not in the + // system. The first is a task; the second is a phone call nobody can + // explain. + // + // The account being invited is the one the repository just wrote: primary + // email as the authname, roleid 3, and no password. That empty password is + // what makes the invitation the only way in, and what `SetInitialPassword` + // re-checks before it writes. + return result, s.inviteFor(result, data), nil +} + +// InviteOutcome is what the operator is told about the invitation. +// +// Its own type rather than a bool, because "not sent" is only useful with the +// reason attached: somebody who sees a tenant created and no mail sent needs to +// know whether to correct an address or set a variable. +// +// Returned rather than stashed on the service. The first version of this kept +// it in a field for the controller to read afterwards, which races — the +// service is one shared instance, and two operators onboarding at the same +// moment would each read the other's result. A value belonging to one call +// travels with that call. +type InviteOutcome struct { + Sent bool + Reason string +} + +// inviteFor emails the new merchant, and says what happened. +// +// Never returns an error: the outcome is for the operator who onboarded them, +// not something for the caller to fail on. +func (s *tenantService) inviteFor(user models.UserInfo, data models.Tenants) InviteOutcome { + if s.invites == nil { + return InviteOutcome{Reason: "invitations are not configured on this server"} + } + if user.Userid <= 0 { + // The account was written but could not be read back, so there is + // nobody to address. Worth saying rather than silently not sending. + return InviteOutcome{Reason: "the new account could not be read back to invite it"} + } + + sent, reason := s.invites.Invite(user.Userid, user.Tenantid, data.Primaryemail, data.Tenantname) + return InviteOutcome{Sent: sent, Reason: reason} } func (s *tenantService) GetTenantByID(tid int, locationid int, userid int) (models.Tenantinfo, error) { @@ -268,3 +407,63 @@ func (s *tenantService) UpdateOwnProfile(userID, tenantID int, fields map[string func (s *tenantService) AssignPartner(tenantID, partnerID int) error { return s.repo.AssignPartner(tenantID, partnerID) } + +// ResendInvite emails a fresh first-password link to a tenant's admin. +// +// ── Why it refuses an account that is already set up ──────────────────────── +// +// `SetInitialPassword` would refuse such a link anyway, so the merchant could +// come to no harm — but the operator would be told the invitation was sent, the +// merchant would follow a link that does not work, and nobody would understand +// why. Refusing here names the real situation: they already have a password, +// and what they need is help signing in. +// +// It also keeps this from quietly becoming a password reset. Nothing on this +// backend verifies identity well enough to support one, and an endpoint that +// re-issues a working link for any account is that, whatever it is called. +func (s *tenantService) ResendInvite(tenantID int) (InviteOutcome, error) { + target, err := s.repo.PrimaryAdminForTenant(tenantID) + if err != nil { + return InviteOutcome{}, err + } + return s.resendTo(target, tenantID) +} + +// ResendInviteToUser re-invites one named account. +// +// The owner is reachable by tenantid because there is exactly one of them. Staff +// added after onboarding, and the login every branch spawns, are not — a business +// has many, and all of them are created with no password. So an operator chasing +// a branch manager who never received their mail names the person. +// +// Same refusals as above, for the same reason: this must not become a password +// reset for anybody whose userid can be found. +func (s *tenantService) ResendInviteToUser(userID int) (InviteOutcome, error) { + target, err := s.repo.InviteTargetForUser(userID) + if err != nil { + return InviteOutcome{}, err + } + return s.resendTo(target, 0) +} + +// resendTo is the half the two resends share. +// +// `tenantID` is passed in rather than read off the target because the token's +// claim should carry the tenant the CALLER asked about; a staff resend has no +// tenant in hand and 0 is honest about that. +func (s *tenantService) resendTo(target repositories.InviteTarget, tenantID int) (InviteOutcome, error) { + if target.IsSetUp { + who := strings.TrimSpace(target.Tenantname) + if who == "" { + who = "That account" + } + return InviteOutcome{}, fmt.Errorf( + "%s has already set a password — send them to the sign-in page instead", who) + } + if s.invites == nil { + return InviteOutcome{Reason: "invitations are not configured on this server"}, nil + } + + sent, reason := s.invites.Invite(target.Userid, tenantID, target.Email, target.Tenantname) + return InviteOutcome{Sent: sent, Reason: reason}, nil +} diff --git a/services/userLogin_test.go b/services/userLogin_test.go index 83cecad..ef2b8e5 100644 --- a/services/userLogin_test.go +++ b/services/userLogin_test.go @@ -50,7 +50,7 @@ func (r *loginRepo) GetTenantUserById(uid int) models.TenantUserInfo { func TestADatabaseFailureIsNotAnInvalidEmail(t *testing.T) { repo := &loginRepo{err: errors.New("dial tcp 10.0.0.5:5433: connection refused")} - svc := NewUserService(repo) + svc := NewUserService(repo, nil) user := models.User{Authname: "owner@shop.example", Password: "pw", Configid: 1} t.Run("app login", func(t *testing.T) { @@ -81,7 +81,7 @@ func TestADatabaseFailureIsNotAnInvalidEmail(t *testing.T) { // registered" from every other failure, so it must survive the refactor. func TestNobodyMatchingIsStillInvalidEmail(t *testing.T) { repo := &loginRepo{} // uid 0, no error: the query ran and found no row - svc := NewUserService(repo) + svc := NewUserService(repo, nil) user := models.User{Authname: "nobody@shop.example", Password: "pw", Configid: 1} _, resp, err := svc.AppLogin(user) @@ -97,7 +97,7 @@ func TestNobodyMatchingIsStillInvalidEmail(t *testing.T) { func TestLookupPrefersAuthnameAndFallsBackToContactNo(t *testing.T) { repo := &loginRepo{uid: 7, password: "pw", status: "Active"} - svc := NewUserService(repo) + svc := NewUserService(repo, nil) svc.AppLogin(models.User{Authname: "owner@shop.example", Contactno: "9999999999", Password: "pw", Configid: 1}) if repo.field != "authname" || repo.value != "owner@shop.example" || repo.configid != 1 { @@ -112,7 +112,7 @@ func TestLookupPrefersAuthnameAndFallsBackToContactNo(t *testing.T) { func TestNeitherIdentifierIsRefusedBeforeTheLookup(t *testing.T) { repo := &loginRepo{err: errors.New("must not be called")} - svc := NewUserService(repo) + svc := NewUserService(repo, nil) _, resp, err := svc.AppLogin(models.User{Password: "pw", Configid: 1}) if err == nil || resp["code"] != 400 { @@ -125,7 +125,7 @@ func TestNeitherIdentifierIsRefusedBeforeTheLookup(t *testing.T) { func TestAMatchedAccountStillSignsIn(t *testing.T) { repo := &loginRepo{uid: 42, password: "secret", status: "Active", roleid: 2} - svc := NewUserService(repo) + svc := NewUserService(repo, nil) info, resp, err := svc.AppLogin(models.User{Authname: "owner@shop.example", Password: "secret", Configid: 1}) if err != nil || resp["code"] != 200 || info.Userid != 42 { diff --git a/services/userService.go b/services/userService.go index ad5d884..38cbdbd 100644 --- a/services/userService.go +++ b/services/userService.go @@ -63,17 +63,25 @@ type UserService interface { // the repository for what makes that safe. SetInitialPassword(userid int, password string) error AppLogin(user models.User) (models.TenantUserInfo, fiber.Map, error) - CreateUser(user models.User) (models.UserInfo, error) + // Creates a back-office account and emails its first-password invitation. + // + // The outcome travels beside the user rather than as an error: the person is + // hired either way, and whether the mail left is something the console shows + // so somebody can resend it. + CreateUser(user models.User) (models.UserInfo, InviteOutcome, error) TenantWebLogin(user models.User) (models.TenantUserInfo, map[string]interface{}) DeleteUser(userid int) error } type userService struct { repo repositories.UserRepository + // May be nil, like the tenant service's. A deployment with no mail still + // creates accounts; the outcome names the missing variable. + invites InviteService } -func NewUserService(repo repositories.UserRepository) UserService { - return &userService{repo: repo} +func NewUserService(repo repositories.UserRepository, invites InviteService) UserService { + return &userService{repo: repo, invites: invites} } func (s *userService) GetAllUsers(roleID, tenantID, pageno, pagesize int, keyword string) ([]models.UserInfo, error) { @@ -162,16 +170,35 @@ func (s *userService) AppLogin(user models.User) (models.TenantUserInfo, fiber.M return models.TenantUserInfo{}, resp, errors.New("inactive account") } - // No password set + // No password set. + // + // ── The userid used to be in here, and that was the whole exploit ─────── + // + // This branch is reached by a POST carrying an email and NO password, so + // anyone could ask it about any account. It answered with the userid, and + // `setpassword` then took a bare userid — so the recipe was: read a + // merchant's primary email off their shopfront, POST it here, receive their + // userid, set their password, own the business's admin account. No guessing + // at any step. + // + // `setpassword` now requires a signed invitation, so the userid alone is no + // longer a way in. It is still removed, because handing it out told an + // unauthenticated caller which businesses exist and which have never been + // set up — a list worth having if you are the one sending the phishing + // email that arrives before the real invitation does. + // + // The message is kept deliberately vague for the same reason. "Please set + // up a password" invited the caller to do exactly that; this says where the + // link comes from instead, which is true for the person who belongs here + // and useless to anyone else. if strings.TrimSpace(dbPassword) == "" { resp := fiber.Map{ "status": true, "code": 409, - "message": "Please setup a password.", + "message": "This account has not been set up yet. Use the invitation link that was emailed to you.", "tenantform": true, "details": fiber.Map{ - "userid": uid, - "setup": true, + "setup": true, }, } return models.TenantUserInfo{}, resp, nil @@ -231,7 +258,7 @@ func (s *userService) AppLogin(user models.User) (models.TenantUserInfo, fiber.M return info, resp, nil } -func (s *userService) CreateUser(user models.User) (models.UserInfo, error) { +func (s *userService) CreateUser(user models.User) (models.UserInfo, InviteOutcome, error) { // Without an authname and a console configid the account is created, // listed, and then refused at the login screen: `weblogin` matches // `WHERE authname = ? AND configid = ?` and never looks at the email @@ -241,16 +268,53 @@ func (s *userService) CreateUser(user models.User) (models.UserInfo, error) { // Call repository to create user userid, err := s.repo.CreateUser(user) if err != nil { - return models.UserInfo{}, err + return models.UserInfo{}, InviteOutcome{}, err } // Get user info by id info, err := s.repo.GetUserById(userid) if err != nil { - return models.UserInfo{}, err + return models.UserInfo{}, InviteOutcome{}, err } - return info, nil + // The invitation, after the write and outside it. + // + // This account is created with NO password — nothing on this path sets one — + // and since the sign-in screen stopped offering to set a first password, the + // emailed link is the only way in. Without this the person is added to the + // directory, appears in every branch picker, and cannot sign in, with nothing + // anywhere to say why. + // + // `info.Userid` rather than `userid`: identical, but this is the row that was + // actually read back, so an invitation is never addressed to an id the + // database did not confirm. + return info, s.inviteNewAccount(info, user), nil +} + +// inviteNewAccount emails the person who was just hired. +// +// Never an error. A failure is the operator's task — resend, or fix the address — +// and not a reason to unwind a hire that has already happened. +func (s *userService) inviteNewAccount(info models.UserInfo, user models.User) InviteOutcome { + if s.invites == nil { + return InviteOutcome{Reason: "invitations are not configured on this server"} + } + if info.Userid <= 0 { + return InviteOutcome{Reason: "the new account could not be read back to invite it"} + } + + // The authname IS the email on a back-office account — `PrepareNewAccount` + // copies one to the other — so this is a fallback for a caller that filled in + // only one of the two, never a second address. + address := strings.TrimSpace(user.Email) + if address == "" { + address = strings.TrimSpace(user.Authname) + } + + // Empty business name: the invite service reads it from the tenantid. A staff + // row carries the id and nothing else about the business. + sent, reason := s.invites.Invite(info.Userid, user.Tenantid, address, "") + return InviteOutcome{Sent: sent, Reason: reason} } func (s *userService) TenantWebLogin(user models.User) (models.TenantUserInfo, map[string]interface{}) { @@ -297,16 +361,19 @@ func (s *userService) TenantWebLogin(user models.User) (models.TenantUserInfo, m } } - // Step 3: Password checks + // Step 3: Password checks. + // + // The userid is withheld here for the same reason as in `AppLogin` above: + // this branch answers an unauthenticated caller asking about an email, and + // the userid was half of an account takeover. See the long note there. if strings.TrimSpace(dbPassword) == "" { return models.TenantUserInfo{}, map[string]interface{}{ "status": true, "code": 409, - "message": "Please setup a password.", + "message": "This account has not been set up yet. Use the invitation link that was emailed to you.", "tenantform": tenantFormExists, "details": map[string]interface{}{ - "userid": uid, - "setup": true, + "setup": true, }, } } diff --git a/utils/invitetoken.go b/utils/invitetoken.go new file mode 100644 index 0000000..f63d5af --- /dev/null +++ b/utils/invitetoken.go @@ -0,0 +1,152 @@ +package utils + +import ( + "crypto/hmac" + "encoding/base64" + "encoding/json" + "fmt" + "strings" + "time" +) + +// The invitation a newly onboarded merchant receives by email. +// +// ── Why this is a token and not a userid ──────────────────────────────────── +// +// `setpassword` used to take a bare `userid`, which was safe only because the +// caller had to reach it through a sign-in: `applogin` answers 409 with the +// userid for an account that has no password, and nothing else hands one out. +// +// Putting that userid in a link and mailing it changes the threat entirely. +// Userids are sequential, so a link is a guessable capability: walk low numbers +// and claim any merchant that has been onboarded and not yet set up. The +// attacker would own a real business's admin account — the empty-password check +// does not help, because an un-set-up account is exactly what they are hunting. +// +// So the invitation carries a signature instead. The userid is read out of the +// payload the server signed, never out of the request, which makes a forged or +// edited link fail before anything is looked up. +// +// ── Why the same secret ───────────────────────────────────────────────────── +// +// One signing key for the deployment, one place it can be missing, one error +// when it is — the same argument `MintWebToken` makes for sharing with the POS +// token. The prefix is what keeps the three kinds apart, and it is checked +// before the signature so an invitation can never be presented as a session. + +// InviteClaims is who an invitation is for. +// +// Deliberately thin. A session carries a role, a branch and a config because +// requests are authorised against them; an invitation authorises exactly one +// act — setting a first password — and the account it names already holds +// everything else. Claims it does not need are claims that cannot be wrong. +type InviteClaims struct { + Userid int `json:"uid"` + // Carried for the audit line, not for the decision. `SetInitialPassword` + // re-derives everything it enforces from the account itself. + Tenantid int `json:"tid,omitempty"` + Issuedat int64 `json:"iat"` + Expiresat int64 `json:"exp"` +} + +// InviteTokenTTL is how long an invitation stays usable. +// +// Seven days: long enough to survive a weekend, a holiday and an email that +// went to spam, short enough that a forwarded invitation found in a mailbox +// months later is no longer a way into the account. Merchants who miss it get +// a fresh one — a resend is cheap and an eternal link is not. +const InviteTokenTTL = 7 * 24 * time.Hour + +// inviteTokenPrefix keeps an invitation from being mistaken for a session. +// +// Without it the two are the same shape signed with the same key, so an +// invitation would verify as a console session — and it names a userid with no +// role, no tenant check and a seven-day life. That is a far weaker credential +// than a session, and it must not be usable as one. +const inviteTokenPrefix = "i1." + +// MintInviteToken issues the link a new merchant is emailed. +func MintInviteToken(claims InviteClaims, now time.Time) (string, time.Time, error) { + secret, err := posTokenSecret() + if err != nil { + return "", time.Time{}, err + } + if claims.Userid <= 0 { + return "", time.Time{}, fmt.Errorf("an invitation must name a user") + } + + expires := now.Add(InviteTokenTTL) + claims.Issuedat = now.Unix() + claims.Expiresat = expires.Unix() + + payload, err := json.Marshal(claims) + if err != nil { + return "", time.Time{}, err + } + + encoded := base64.RawURLEncoding.EncodeToString(payload) + return inviteTokenPrefix + encoded + "." + sign(encoded, secret), expires, nil +} + +// ParseInviteToken verifies an invitation and returns who it is for. +// +// Same order as the session parser, for the same reason: nothing in the payload +// is trusted — not the expiry, not the user — until the signature has been +// checked. Reading `exp` from an unverified payload is taking the caller's word +// for when their own link runs out. +// Every refusal below is written as a sentence, capital letter and full stop, +// against Go's convention for error strings — because these are not read by a +// developer. `SetPassword` puts them straight into the `message` a merchant sees +// on `/set-password`, and they are the only explanation that screen has. A +// lowercase fragment in a red banner reads as something that leaked out of the +// machine rather than something anybody meant to say. +// +// They also all say what to do next, because every one of them is a dead end +// otherwise: the person is holding a link that does not work and has no password +// to sign in with instead. +func ParseInviteToken(token string, now time.Time) (InviteClaims, error) { + secret, err := posTokenSecret() + if err != nil { + return InviteClaims{}, err + } + + raw := strings.TrimSpace(token) + after, found := strings.CutPrefix(raw, inviteTokenPrefix) + if !found { + return InviteClaims{}, fmt.Errorf("This is not an invitation link.") + } + + encoded, signature, found := strings.Cut(after, ".") + if !found || encoded == "" || signature == "" { + return InviteClaims{}, fmt.Errorf("This invitation link is incomplete. Use the whole link from the email.") + } + + // Constant time, so the right signature cannot be learned a byte at a time + // from how long the comparison took. + if !hmac.Equal([]byte(signature), []byte(sign(encoded, secret))) { + return InviteClaims{}, fmt.Errorf("This invitation link is not valid. Ask whoever set you up to send another.") + } + + payload, err := base64.RawURLEncoding.DecodeString(encoded) + if err != nil { + return InviteClaims{}, fmt.Errorf("This invitation link is incomplete. Use the whole link from the email.") + } + + var claims InviteClaims + if err := json.Unmarshal(payload, &claims); err != nil { + return InviteClaims{}, fmt.Errorf("This invitation link is incomplete. Use the whole link from the email.") + } + + if claims.Expiresat > 0 && now.Unix() >= claims.Expiresat { + // Says what to do about it. An expired invitation is the one failure + // here somebody can resolve themselves, and "invalid" would send them + // to support instead of to whoever onboarded them. + return InviteClaims{}, fmt.Errorf("This invitation has expired. Ask whoever set you up to send another.") + } + + if claims.Userid <= 0 { + return InviteClaims{}, fmt.Errorf("This invitation names no account. Ask whoever set you up to send another.") + } + + return claims, nil +} diff --git a/utils/invitetoken_test.go b/utils/invitetoken_test.go new file mode 100644 index 0000000..ecaa3a6 --- /dev/null +++ b/utils/invitetoken_test.go @@ -0,0 +1,191 @@ +package utils + +import ( + "strings" + "testing" + "time" +) + +/* +The invitation a newly onboarded merchant is emailed. + +`setpassword` took a bare userid, which was safe only because the caller had to +reach it through a sign-in — `applogin` answers 409 with the userid for an +account that has no password, and nothing else hands one out. + +Mailing that userid as a link changes the threat completely: userids are +sequential, so the link becomes a guessable capability. Walk low numbers and +claim any merchant onboarded but not yet set up, and you own a real business's +admin account. The empty-password check is no defence — an un-set-up account is +precisely what such an attacker is looking for. + +So these tests are mostly about what the token REFUSES. +*/ + +const inviteSecret = "an-invitation-signing-secret-long-enough" + +func inviteEnv(t *testing.T) { + t.Helper() + t.Setenv("POS_TOKEN_SECRET", inviteSecret) +} + +func TestAnInvitationNamesTheAccountItWasIssuedFor(t *testing.T) { + inviteEnv(t) + now := time.Now() + + token, expires, err := MintInviteToken(InviteClaims{Userid: 904, Tenantid: 1147}, now) + if err != nil { + t.Fatalf("minting: %v", err) + } + + claims, err := ParseInviteToken(token, now) + if err != nil { + t.Fatalf("parsing: %v", err) + } + if claims.Userid != 904 || claims.Tenantid != 1147 { + t.Fatalf("claims came back as %+v", claims) + } + if expires.Sub(now) != InviteTokenTTL { + t.Fatalf("expiry is %v, want %v", expires.Sub(now), InviteTokenTTL) + } +} + +func TestAnEditedInvitationIsRefused(t *testing.T) { + // The whole point. If the payload could be edited, the link would be a + // userid in a longer coat and every account would be one base64 edit away. + inviteEnv(t) + now := time.Now() + + token, _, err := MintInviteToken(InviteClaims{Userid: 904}, now) + if err != nil { + t.Fatalf("minting: %v", err) + } + + // Re-sign nothing; just change the payload, which is what an attacker who + // decoded the link and wanted a different userid would do. + forged, _, err := MintInviteToken(InviteClaims{Userid: 905}, now) + if err != nil { + t.Fatalf("minting: %v", err) + } + parts := strings.Split(token, ".") + other := strings.Split(forged, ".") + swapped := parts[0] + "." + other[1] + "." + parts[2] + + if _, err := ParseInviteToken(swapped, now); err == nil { + t.Fatal("a payload swapped under an old signature was accepted") + } +} + +func TestAnInvitationSignedWithAnotherSecretIsRefused(t *testing.T) { + now := time.Now() + + t.Setenv("POS_TOKEN_SECRET", "one-secret-that-is-long-enough-here") + token, _, err := MintInviteToken(InviteClaims{Userid: 904}, now) + if err != nil { + t.Fatalf("minting: %v", err) + } + + t.Setenv("POS_TOKEN_SECRET", "a-different-secret-also-long-enough") + if _, err := ParseInviteToken(token, now); err == nil { + t.Fatal("an invitation from another deployment was accepted") + } +} + +func TestAnExpiredInvitationSaysWhatToDo(t *testing.T) { + // The one failure here somebody can resolve themselves. "Not valid" would + // send them to support; naming a resend sends them to whoever onboarded + // them, which is where the fix actually is. + inviteEnv(t) + now := time.Now() + + token, _, err := MintInviteToken(InviteClaims{Userid: 904}, now) + if err != nil { + t.Fatalf("minting: %v", err) + } + + _, err = ParseInviteToken(token, now.Add(InviteTokenTTL+time.Second)) + if err == nil { + t.Fatal("an expired invitation was accepted") + } + // An expired invitation is the one failure here somebody can resolve + // themselves, so it has to say how. "Invalid" would send them to support + // instead of to whoever onboarded them. + if !strings.Contains(err.Error(), "send another") { + t.Fatalf("the refusal does not say what to do: %v", err) + } + // Read by a merchant on `/set-password`, not by a developer in a log. A + // lowercase fragment in a red banner reads as something that leaked out. + if !strings.HasPrefix(err.Error(), "This") || !strings.HasSuffix(err.Error(), ".") { + t.Errorf("the refusal is not written as a sentence: %q", err) + } +} + +func TestAnInvitationIsNotASession(t *testing.T) { + // They are the same shape signed with the same key. Without the prefix + // check an invitation would verify as a console session — and it names a + // userid with no role, no tenant check and a seven-day life, which is a far + // weaker credential than a session and must never be usable as one. + inviteEnv(t) + now := time.Now() + + invite, _, err := MintInviteToken(InviteClaims{Userid: 904, Tenantid: 1147}, now) + if err != nil { + t.Fatalf("minting: %v", err) + } + + if _, err := ParseWebToken(invite, now); err == nil { + t.Fatal("an invitation was accepted as a console session") + } +} + +func TestASessionIsNotAnInvitation(t *testing.T) { + // The other direction, which matters less but costs nothing to close: a + // stolen session should not double as a password-reset link. + inviteEnv(t) + now := time.Now() + + session, _, err := MintWebToken(WebClaims{Userid: 904, Tenantid: 1147}, now) + if err != nil { + t.Fatalf("minting: %v", err) + } + + if _, err := ParseInviteToken(session, now); err == nil { + t.Fatal("a console session was accepted as an invitation") + } +} + +func TestRubbishIsRefusedWithoutPanicking(t *testing.T) { + inviteEnv(t) + now := time.Now() + + for _, bad := range []string{ + "", " ", "i1.", "i1..", "i1.onlyonepart", "not-a-token", + "i1.!!!not-base64!!!.signature", "w1.something.else", + } { + if _, err := ParseInviteToken(bad, now); err == nil { + t.Fatalf("%q was accepted as an invitation", bad) + } + } +} + +func TestAnInvitationMustNameSomebody(t *testing.T) { + // A token naming nobody authorises nothing, and must not be mistaken for + // one authorising everything — the same rule the session parser applies. + inviteEnv(t) + + if _, _, err := MintInviteToken(InviteClaims{Userid: 0}, time.Now()); err == nil { + t.Fatal("an invitation was minted for user 0") + } +} + +func TestNoSigningSecretMeansNoInvitations(t *testing.T) { + // Rather than issuing something unverifiable. A deployment that cannot sign + // cannot invite, and saying so at the point of minting is better than an + // email whose link never works. + t.Setenv("POS_TOKEN_SECRET", "") + t.Setenv("JWT_SECRET_KEY", "") + + if _, _, err := MintInviteToken(InviteClaims{Userid: 904}, time.Now()); err == nil { + t.Fatal("an invitation was minted with no signing secret") + } +} diff --git a/utils/mail.go b/utils/mail.go new file mode 100644 index 0000000..27a52d5 --- /dev/null +++ b/utils/mail.go @@ -0,0 +1,154 @@ +package utils + +import ( + "crypto/tls" + "fmt" + "net/mail" + "net/smtp" + "strings" + "time" + + "nearle/config" +) + +// Sending mail. +// +// ── Why the standard library and not a client ─────────────────────────────── +// +// `net/smtp` is enough for what this sends: a handful of invitations a day, one +// at a time, to addresses a person typed. Every transactional provider speaks +// SMTP — SES, SendGrid, Resend, a company relay — so the choice of provider is +// a host and a password rather than a dependency and a rewrite. Adding an SDK +// would buy templating and analytics that nothing here wants yet, in exchange +// for a supply chain. +// +// ── Nil is a configuration, not a failure ─────────────────────────────────── +// +// `NewMailer` returns nil when no host is set, matching `NewChat` and +// `NewEmbedder`. A deployment without mail still boots, still onboards tenants, +// and records the invitation as unsent. The alternative — refusing to start — +// would make mail a hard dependency of creating a merchant, which it is not. + +// Mailer sends one message. +// +// One method, on purpose. Everything this server sends is a short transactional +// note to one recipient; a richer interface would be describing a mail product +// nobody asked for. +type Mailer interface { + Send(to, subject, body string) error +} + +// mailTimeout bounds a send. +// +// Onboarding waits on this, so it cannot be generous. A relay that has not +// answered in ten seconds is not going to, and the invitation is better +// recorded as unsent — and resent — than holding a tenant creation open. +const mailTimeout = 10 * time.Second + +type smtpMailer struct { + cfg config.MailConfig +} + +// NewMailer builds a sender, or nil when none is configured. +func NewMailer(cfg config.MailConfig) (Mailer, error) { + if !cfg.Enabled() { + return nil, nil + } + if _, err := mail.ParseAddress(cfg.FromAddress); err != nil { + // Caught here rather than at the first send, because a malformed + // sender fails every message and should stop the deployment being + // described as able to send. + return nil, fmt.Errorf("MAIL_FROM is not a valid address: %w", err) + } + return &smtpMailer{cfg: cfg}, nil +} + +func (m *smtpMailer) Send(to, subject, body string) error { + recipient, err := mail.ParseAddress(strings.TrimSpace(to)) + if err != nil { + // A merchant's primary email is typed by whoever onboarded them, so a + // typo here is ordinary. Named clearly, because the fix is to correct + // the tenant's record and resend. + return fmt.Errorf("%q is not a valid email address", to) + } + + from := mail.Address{Name: m.cfg.FromName, Address: m.cfg.FromAddress} + message := buildMessage(from, *recipient, subject, body) + + client, err := m.dial() + if err != nil { + return err + } + defer client.Close() + + if m.cfg.Username != "" { + auth := smtp.PlainAuth("", m.cfg.Username, m.cfg.Password, m.cfg.Host) + if err := client.Auth(auth); err != nil { + return fmt.Errorf("the mail server refused our credentials: %w", err) + } + } + + if err := client.Mail(m.cfg.FromAddress); err != nil { + return fmt.Errorf("the mail server refused the sender: %w", err) + } + if err := client.Rcpt(recipient.Address); err != nil { + return fmt.Errorf("the mail server refused %s: %w", recipient.Address, err) + } + + writer, err := client.Data() + if err != nil { + return err + } + if _, err := writer.Write([]byte(message)); err != nil { + return err + } + if err := writer.Close(); err != nil { + return err + } + return client.Quit() +} + +// dial opens a connection, upgrading to TLS where the server offers it. +// +// STARTTLS rather than implicit TLS, because 587 is the submission port every +// provider documents and it begins in the clear. The upgrade is attempted +// whenever the server advertises it and the connection is abandoned if it fails +// — an invitation is a credential, and sending one over plaintext to a server +// that offered encryption would be choosing not to use it. +func (m *smtpMailer) dial() (*smtp.Client, error) { + client, err := smtp.Dial(m.cfg.Address()) + if err != nil { + return nil, fmt.Errorf("could not reach the mail server at %s: %w", m.cfg.Address(), err) + } + + if ok, _ := client.Extension("STARTTLS"); ok { + if err := client.StartTLS(&tls.Config{ServerName: m.cfg.Host}); err != nil { + client.Close() + return nil, fmt.Errorf("the mail server offered TLS and then refused it: %w", err) + } + } + return client, nil +} + +// buildMessage assembles the wire format. +// +// Headers then a blank line then the body, with CRLF line endings — SMTP is +// specified in terms of CRLF and some servers reject bare newlines, which +// presents as mail that works locally and vanishes in production. +func buildMessage(from, to mail.Address, subject, body string) string { + var b strings.Builder + + b.WriteString("From: " + from.String() + "\r\n") + b.WriteString("To: " + to.String() + "\r\n") + // Folded and encoded by `mail.Address`'s rules for the addresses; the + // subject is plain ASCII by construction in this codebase, so it needs no + // MIME word encoding. If a subject ever carries a merchant's name, that + // changes and this needs `mime.QEncoding`. + b.WriteString("Subject: " + strings.ReplaceAll(subject, "\n", " ") + "\r\n") + b.WriteString("MIME-Version: 1.0\r\n") + b.WriteString("Content-Type: text/plain; charset=UTF-8\r\n") + b.WriteString("\r\n") + b.WriteString(strings.ReplaceAll(body, "\n", "\r\n")) + + return b.String() +}