diff --git a/API.md b/API.md index 97235ca..aacf5e8 100644 --- a/API.md +++ b/API.md @@ -25,7 +25,7 @@ everything the one below can: | role | can additionally | |---|---| | `staff` | see arrivals, search customers, edit a customer's profile, record a purchase | -| `manager` | manage cameras, invite and remove team members, issue shop-PC installation codes, erase a customer | +| `manager` | manage cameras, create / invite / reset / remove team members, issue shop-PC installation codes, erase a customer | | `owner` | promote somebody to owner | A platform admin has `role: "admin"` **and an empty `client_id`** — both @@ -51,7 +51,7 @@ user; the tenant is always taken from the session and never from the request. | `POST /api/sites/{site}/cameras` · `PATCH` / `DELETE /api/cameras/{id}` · `POST /api/cameras/{id}/check` | manager | | `POST /api/sites/{site}/enrolment-code` | manager | | `GET /api/team` | authed (tenant users only) | -| `PATCH /api/team/{id}` · `/api/team/invitations*` | manager | +| `POST /api/team/members` · `POST /api/team/{id}/password` · `PATCH /api/team/{id}` · `/api/team/invitations*` | manager | | `GET /api/reports/footfall` · `GET /api/reports/conversion` | authed | | `POST /api/assistant` | authed | | `GET` / `POST /api/admin/clients` | **platform admin** | @@ -69,12 +69,14 @@ Three tiers. Each one creates the login for the next, and nobody ever creates their own from nothing. ``` - ┌──────────────────┐ creates ┌──────────────────┐ invites ┌──────────────────┐ + ┌──────────────────┐ creates ┌──────────────────┐ creates ┌──────────────────┐ │ Platform admin │ ───────────▶ │ Merchant owner │ ───────────▶ │ Sales staff │ - │ (web) │ company + │ (web) │ code, │ (mobile) │ + │ (web) │ company + │ (web) │ login, pw │ (mobile) │ │ │ owner login │ │ shown once │ │ └──────────────────┘ └──────────────────┘ └──────────────────┘ - POST /api/admin/clients POST /api/team/invitations POST /api/auth/register + POST /api/admin/clients POST /api/team/members POST /api/auth/login + (or /api/team/invitations → (or /api/auth/register + a code they redeem themselves) with the code) ``` ### Tier 1 — the platform admin registers a merchant @@ -103,12 +105,33 @@ GET /api/admin/clients → every merchant, with site and user counts ### Tier 2 — the merchant owner registers sales staff -Signed in as the owner (or any manager). +Signed in as the owner (or any manager). Two ways to do it; use whichever fits +the moment. + +**Directly — create the login and hand it over.** For a salesperson being set +up before their first shift, without a phone in hand. Exactly how the admin +created the merchant in Tier 1. ``` POST /api/auth/login { "email": "suriya@tenext.in", "password": "xK9…", "device": "Head office" } +POST /api/team/members + { "email": "priya@tenext.in", "full_name": "Priya R", "role": "staff" } + → 201 { "id": "…", "email": "priya@tenext.in", "full_name": "Priya R", + "role": "staff", "active": true, "created_at": "…", + "password": "m4kq…" } ← shown ONCE +``` + +Leave `password` out and one is generated; give one and it is used (8 +characters minimum). Either way it is returned exactly once — write it on the +card now. The salesperson signs in on their phone with that email and +password, and the merchant login is done. + +**By invitation — the salesperson chooses their own password.** Better when +they have their phone: the merchant never sees or handles a staff password. + +``` POST /api/team/invitations { "email": "priya@tenext.in", "full_name": "Priya R", "role": "staff", "expires_in_days": 7 } @@ -117,9 +140,19 @@ POST /api/team/invitations ``` `code` is shown **once** and is what the merchant gives the salesperson — -read aloud, WhatsApp, printed on a card. It is single-use and expires. The -salesperson chooses their own password when they redeem it (Tier 3), so the -merchant never knows or handles a staff password. +read aloud, WhatsApp, printed on a card. Single-use, expires. They redeem it in +Tier 3 and pick a password there. + +**When they forget it** — which is the everyday case on a shop floor: + +``` +POST /api/team/{id}/password { } or { "password": "chosen" } + → 200 { "password": "n7xw…" } ← shown ONCE +``` + +Resets the password **and signs them out of every device** in one step, +because the other reason a manager resets a password is a lost phone, and a +reset that left that phone signed in would look complete while fixing nothing. Managing the team afterwards: @@ -137,7 +170,9 @@ The owner also sets the shop up from the same login — `POST ### Tier 3 — the salesperson gets their mobile login -Not signed in yet. They have the code. +If the merchant created the login directly, they already have an email and +password: skip straight to `POST /api/auth/login` below. Otherwise they have a +code. ``` GET /api/auth/invitation?code=LQOUHR-AYYTPE-7Q756N-PGAAN6 (no auth) @@ -180,13 +215,6 @@ somebody else's account. ### What does not exist, stated plainly -- **The merchant cannot create a staff login directly** with a password of - their choosing. The only path is an invitation code the staff member redeems - themselves. This is deliberate — it keeps staff passwords out of the - merchant's hands and off chat — but it means a salesperson without a phone in - hand cannot be set up *for* them. If that friction is real, a - `POST /api/team/members` that mirrors `POST /api/admin/clients` (generated - password, shown once) is a small addition. - **There is no mobile app in this repository.** Tier 3 is a complete API with no client yet. Everything above is what that app will call. - **The admin cannot reset a merchant owner's password over HTTP**, nor suspend @@ -443,6 +471,46 @@ deactivate them (§4); that revokes every session they hold. "last_login_at": "…", "created_at": "…" }] ``` +### `POST /api/team/members` — manager or owner + +Create a login directly and hand it over. The alternative to an invitation +(§2) for somebody without a phone in hand. + +```json +{ "email": "priya@tenext.in", "full_name": "Priya R", "role": "staff", + "password": "" } +``` + +```json +{ "id": "…", "email": "priya@tenext.in", "full_name": "Priya R", + "role": "staff", "active": true, "last_login_at": "", "created_at": "…", + "password": "m4kq…" } +``` + +- **`password` is returned once** and is not recoverable. Leave it empty in + the request and one is generated; supply one and it must be 8+ characters. +- `role` is `staff` (default), `manager` or `owner`. Only an owner may create + an owner; `admin` is refused. +- **409 `conflict`** if that email already has an account anywhere. + +### `POST /api/team/{id}/password` — manager or owner + +```json +{ } or { "password": "chosen-one" } +``` + +```json +{ "password": "n7xw…" } +``` + +Sets a new password (generated unless given) and **revokes every session the +member holds**, in one transaction. Returns the new password once. A member of +another company is **404**, never 403. + +There is deliberately no self-service reset and no reset-by-email: a shop-floor +account often has no mailbox anyone checks, and the person who can vouch for +the salesperson standing in front of them is their manager. + ### `PATCH /api/team/{id}` — manager or owner ```json diff --git a/server/internal/api/api.go b/server/internal/api/api.go index 7899d19..96d3ca9 100644 --- a/server/internal/api/api.go +++ b/server/internal/api/api.go @@ -63,6 +63,14 @@ type Store interface { RedeemInvitation(ctx context.Context, hash []byte, fullName, passwordHash string) (UserRecord, error) Team(ctx context.Context, clientID string) ([]TeamMember, error) UpdateTeamMember(ctx context.Context, clientID, userID string, up TeamUpdate) (TeamMember, error) + // CreateMember inserts an active account into the caller's tenant. The + // hash is computed by the handler, so the plaintext never reaches the + // store - same boundary invitations and sessions already keep. + CreateMember(ctx context.Context, clientID string, in NewMemberInput, hash string) (TeamMember, error) + // ResetMemberPassword replaces the hash and revokes every session the + // member holds, in one transaction. A reset is what happens after a lost + // phone; leaving that phone signed in would defeat it. + ResetMemberPassword(ctx context.Context, clientID, userID, hash string) (TeamMember, error) // --- public references --- // Resolving the names people actually use to the uuids the schema stores. @@ -246,6 +254,8 @@ func (s *Server) Routes() *http.ServeMux { // --- the people who work here --- mux.HandleFunc("GET /api/team", s.authed(s.handleTeam)) mux.HandleFunc("PATCH /api/team/{id}", s.authed(s.handleUpdateTeamMember)) + mux.HandleFunc("POST /api/team/members", s.authed(s.handleCreateMember)) + mux.HandleFunc("POST /api/team/{id}/password", s.authed(s.handleResetPassword)) mux.HandleFunc("GET /api/team/invitations", s.authed(s.handleInvitations)) mux.HandleFunc("POST /api/team/invitations", s.authed(s.handleInvite)) mux.HandleFunc("DELETE /api/team/invitations/{id}", diff --git a/server/internal/api/fake_test.go b/server/internal/api/fake_test.go index 62d5e81..e219528 100644 --- a/server/internal/api/fake_test.go +++ b/server/internal/api/fake_test.go @@ -995,3 +995,59 @@ func (f *fakeStore) VisitorIDByNumber(_ context.Context, clientID string, number } return "", nil } + +// CreateMember behaves like the real store on the two things the handler +// branches on: the account lands in the caller's tenant and nowhere else, and +// an address that already exists anywhere is a conflict named the way Postgres +// names it, so conflictMessage recognises it. +func (f *fakeStore) CreateMember(_ context.Context, clientID string, + in NewMemberInput, hash string) (TeamMember, error) { + + f.mu.Lock() + defer f.mu.Unlock() + if _, taken := f.users[in.Email]; taken { + return TeamMember{}, errors.New(`duplicate key value violates unique constraint "app_users_email_idx"`) + } + // The real UserByEmail joins clients for the name; this fake reads it off + // the record, so copy it from a tenant-mate or a login as the new member + // comes back with no company name and looks like it landed nowhere. + clientName := "" + for _, u := range f.users { + if u.ClientID == clientID && u.ClientName != "" { + clientName = u.ClientName + break + } + } + id := "member-" + itoa(len(f.users)+1) + f.users[in.Email] = UserRecord{ + ID: id, ClientID: clientID, ClientName: clientName, + Email: in.Email, FullName: in.FullName, + Role: in.Role, Active: true, PasswordHash: hash, Found: true, + } + return TeamMember{ID: id, Email: in.Email, FullName: in.FullName, + Role: in.Role, Active: true}, nil +} + +// ResetMemberPassword mirrors the real one: tenant-scoped, and every session +// the member holds is revoked with it. +func (f *fakeStore) ResetMemberPassword(_ context.Context, clientID, userID, + hash string) (TeamMember, error) { + + f.mu.Lock() + defer f.mu.Unlock() + for email, u := range f.users { + if u.ID != userID || u.ClientID != clientID { + continue + } + u.PasswordHash = hash + f.users[email] = u + for _, s := range f.sessions { + if s.p.UserID == userID { + s.revoked = true + } + } + return TeamMember{ID: u.ID, Email: u.Email, FullName: u.FullName, + Role: u.Role, Active: u.Active}, nil + } + return TeamMember{}, errors.New("no such team member") +} diff --git a/server/internal/api/handlers_team.go b/server/internal/api/handlers_team.go index 6965e8e..0341442 100644 --- a/server/internal/api/handlers_team.go +++ b/server/internal/api/handlers_team.go @@ -388,3 +388,134 @@ func (s *Server) lastOwner(r *http.Request, userID string) (bool, error) { } return isOwner && owners == 1, nil } + +// handleCreateMember is a manager creating a salesperson's login directly and +// handing it over - the path for somebody being set up before their first +// shift, without a phone in hand. +// +// Same rules as an invitation for who may create whom: manager and above, and +// only an owner mints an owner. Same rule as the platform admin creating a +// merchant for the password: generated unless given, returned exactly once. +func (s *Server) handleCreateMember(w http.ResponseWriter, r *http.Request) { + p := PrincipalFrom(r.Context()) + if !p.CanManageSites() || p.ClientID == "" { + writeErr(w, http.StatusForbidden, "forbidden", + "Only a manager or owner can add team members.") + return + } + + var in NewMemberInput + if err := decode(w, r, &in); err != nil { + badRequest(w, err.Error()) + return + } + in.Email = auth.NormalizeEmail(in.Email) + if in.Email == "" || !strings.Contains(in.Email, "@") { + badRequest(w, "an email address is required - it is what they will sign in with") + return + } + in.FullName = clip(trim(in.FullName), 200) + in.Role = strings.ToLower(trim(in.Role)) + if in.Role == "" { + in.Role = "staff" + } + switch in.Role { + case "owner", "manager", "staff": + default: + badRequest(w, "role must be owner, manager or staff") + return + } + if in.Role == "owner" && p.Role != "owner" && p.Role != "admin" { + writeErr(w, http.StatusForbidden, "forbidden", + "Only an owner can create another owner.") + return + } + + password := in.Password + if password == "" { + generated, err := auth.RandomPassword() + if err != nil { + s.serverError(w, "generate password", err) + return + } + password = generated + } + hash, err := auth.HashPassword(password) + if err != nil { + // The policy message ("at least 8 characters") is written for the + // person who typed it, so it goes out as-is. + badRequest(w, err.Error()) + return + } + + m, err := s.Store.CreateMember(r.Context(), p.ClientID, in, hash) + if err != nil { + if msg, ok := conflictMessage(err); ok { + writeErr(w, http.StatusConflict, "conflict", msg) + return + } + s.serverError(w, "create member", err) + return + } + + s.Store.Audit(r.Context(), AuditEntry{ + ClientID: p.ClientID, ActorID: p.UserID, ActorKind: "user", + Action: "team.create", Entity: "user", EntityID: m.ID, + Detail: map[string]any{"email": m.Email, "role": m.Role}, + }) + // The plaintext exists here and in this response, and nowhere else. + writeJSON(w, http.StatusCreated, NewMemberResult{TeamMember: m, Password: password}) +} + +// handleResetPassword is a manager resetting a member's password: the +// salesperson forgot it, or lost the phone it was on. Returns the new one +// once, and signs the member out everywhere - see the store for why those are +// one operation. +// +// Deliberately not self-service and not "send an email": a shop-floor account +// often has no mailbox anyone checks, and the person who can vouch for the +// salesperson standing in front of them is their manager. +func (s *Server) handleResetPassword(w http.ResponseWriter, r *http.Request) { + p := PrincipalFrom(r.Context()) + if !p.CanManageSites() || p.ClientID == "" { + writeErr(w, http.StatusForbidden, "forbidden", + "Only a manager or owner can reset a team member's password.") + return + } + userID := r.PathValue("id") + + var in PasswordReset + if err := decodeOptional(w, r, &in); err != nil { + badRequest(w, err.Error()) + return + } + password := in.Password + if password == "" { + generated, err := auth.RandomPassword() + if err != nil { + s.serverError(w, "generate password", err) + return + } + password = generated + } + hash, err := auth.HashPassword(password) + if err != nil { + badRequest(w, err.Error()) + return + } + + m, err := s.Store.ResetMemberPassword(r.Context(), p.ClientID, userID, hash) + if err != nil { + // A user id from another tenant matches nothing, so it reads as 404 - + // a tenant user has no business learning the id was real. + writeErr(w, http.StatusNotFound, "not_found", "No such team member.") + return + } + + s.Store.Audit(r.Context(), AuditEntry{ + ClientID: p.ClientID, ActorID: p.UserID, ActorKind: "user", + Action: "team.reset_password", Entity: "user", EntityID: m.ID, + Detail: map[string]any{"email": m.Email}, + }) + writeJSON(w, http.StatusOK, PasswordReset{Password: password}) +} diff --git a/server/internal/api/team_members_test.go b/server/internal/api/team_members_test.go new file mode 100644 index 0000000..730bd8c --- /dev/null +++ b/server/internal/api/team_members_test.go @@ -0,0 +1,201 @@ +package api + +import ( + "encoding/json" + "net/http" + "strings" + "testing" +) + +// The second way a salesperson gets a login: their manager creates it and hands +// it over. Everything here is a property of the one rule that path lives by - +// the password is shown once, to the manager, and to nobody afterwards. + +func createMember(t *testing.T, s *Server, token string, body map[string]any) (int, NewMemberResult, string) { + t.Helper() + rec := do(t, s, "POST", "/api/team/members", token, body) + var out NewMemberResult + if rec.Code == http.StatusCreated { + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatal(err) + } + } + return rec.Code, out, rec.Body.String() +} + +func TestAManagerCanCreateALoginAndHandItOver(t *testing.T) { + s, fs := newServer(t) + seedUser(fs) + mgr := login(t, s, "manager@acme.com", "correct horse battery") + + code, out, body := createMember(t, s, mgr.Token, map[string]any{ + "email": "Priya@Acme.com", "full_name": "Priya R", "role": "staff"}) + if code != http.StatusCreated { + t.Fatalf("create: got %d, body %s", code, body) + } + // Generated, not blank, and long enough to be a credential rather than a + // suggestion. The manager reads this off the screen onto a card. + if len(out.Password) < 12 { + t.Fatalf("password should be generated when not given, got %q", out.Password) + } + if out.Email != "priya@acme.com" || out.Role != "staff" || !out.Active { + t.Fatalf("member not as created: %+v", out.TeamMember) + } + + // The whole point: the salesperson can sign in with what the manager was + // shown, right now, on their own phone. + sess := login(t, s, "priya@acme.com", out.Password) + if sess.User.Client != "Acme Retail" || sess.User.Role != "staff" { + t.Fatalf("the new member landed somewhere odd: %+v", sess.User) + } +} + +// The password is returned by the request that set it and by nothing else. A +// credential a manager can look up later is one anybody at that screen can +// read off, and the team list is on screen all day. +func TestThePasswordIsShownOnceAndNeverListed(t *testing.T) { + s, fs := newServer(t) + seedUser(fs) + mgr := login(t, s, "manager@acme.com", "correct horse battery") + + _, out, _ := createMember(t, s, mgr.Token, map[string]any{"email": "sam@acme.com"}) + + rec := do(t, s, "GET", "/api/team", mgr.Token, nil) + if strings.Contains(rec.Body.String(), out.Password) { + t.Fatal("the team list carries a password") + } + if strings.Contains(rec.Body.String(), `"password"`) { + t.Fatal("the team list has a password field at all") + } +} + +// Same shape of permission as an invitation, on purpose: the two paths create +// the same thing, so a manager must not be able to do through one what they +// are refused through the other. +func TestStaffCannotCreateAndAManagerCannotCreateAnOwner(t *testing.T) { + s, fs := newServer(t) + seedUser(fs) + seedMember(fs, acmeStaffID, "staff@acme.com", "Sam", "staff") + seedMember(fs, acmeOwnerID, "owner@acme.com", "Olu", "owner") + + staff := login(t, s, "staff@acme.com", "correct horse battery") + if code, _, _ := createMember(t, s, staff.Token, map[string]any{"email": "x@acme.com"}); code != http.StatusForbidden { + t.Fatalf("staff creating a login: want 403, got %d", code) + } + + mgr := login(t, s, "manager@acme.com", "correct horse battery") + if code, _, _ := createMember(t, s, mgr.Token, map[string]any{"email": "boss@acme.com", "role": "owner"}); code != http.StatusForbidden { + t.Fatalf("manager minting an owner: want 403, got %d", code) + } + + owner := login(t, s, "owner@acme.com", "correct horse battery") + if code, _, body := createMember(t, s, owner.Token, map[string]any{"email": "boss@acme.com", "role": "owner"}); code != http.StatusCreated { + t.Fatalf("owner minting an owner: want 201, got %d %s", code, body) + } + + // Never admin. A platform admin is defined by having no company, so this + // could only ever mint the tenant-scoped role='admin' row that adminOnly + // exists to reject. + if code, _, _ := createMember(t, s, owner.Token, map[string]any{"email": "root@acme.com", "role": "admin"}); code != http.StatusBadRequest { + t.Fatalf("role=admin: want 400, got %d", code) + } +} + +func TestAnAddressThatAlreadyExistsIsAConflictNotAFault(t *testing.T) { + s, fs := newServer(t) + seedUser(fs) + mgr := login(t, s, "manager@acme.com", "correct horse battery") + + code, _, body := createMember(t, s, mgr.Token, map[string]any{"email": "manager@acme.com"}) + if code != http.StatusConflict { + t.Fatalf("want 409, got %d %s", code, body) + } + if !strings.Contains(body, "already has an account") { + t.Fatalf("the message should say what to do about it: %s", body) + } +} + +// A manager may choose the password, but not a bad one. The floor is the same +// as everywhere else, and the policy message goes to them unchanged. +func TestAChosenPasswordStillMeetsTheFloor(t *testing.T) { + s, fs := newServer(t) + seedUser(fs) + mgr := login(t, s, "manager@acme.com", "correct horse battery") + + code, _, body := createMember(t, s, mgr.Token, map[string]any{"email": "a@acme.com", "password": "short"}) + if code != http.StatusBadRequest { + t.Fatalf("want 400, got %d %s", code, body) + } + + code, out, _ := createMember(t, s, mgr.Token, map[string]any{"email": "b@acme.com", "password": "chosen-by-manager"}) + if code != http.StatusCreated || out.Password != "chosen-by-manager" { + t.Fatalf("a valid chosen password should be used and echoed once, got %d %q", code, out.Password) + } +} + +// Why a manager resets a password: the salesperson forgot it, or lost the +// phone it was saved on. In the second case the phone is the problem, so the +// reset that fixes the first must also fix the second. +func TestAResetSignsTheOldPhoneOutAndTheNewPasswordIn(t *testing.T) { + s, fs := newServer(t) + seedUser(fs) + seedMember(fs, acmeStaffID, "priya@acme.com", "Priya", "staff") + mgr := login(t, s, "manager@acme.com", "correct horse battery") + lostPhone := login(t, s, "priya@acme.com", "correct horse battery") + + rec := do(t, s, "POST", "/api/team/"+acmeStaffID+"/password", mgr.Token, nil) + if rec.Code != http.StatusOK { + t.Fatalf("reset: %d %s", rec.Code, rec.Body.String()) + } + var out PasswordReset + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatal(err) + } + if len(out.Password) < 12 { + t.Fatalf("reset should hand back a generated password, got %q", out.Password) + } + + // The lost phone is out. + if rec := do(t, s, "GET", "/api/auth/me", lostPhone.Token, nil); rec.Code != http.StatusUnauthorized { + t.Fatalf("the old session should be revoked by a reset, got %d", rec.Code) + } + // The old password is dead. + if rec := do(t, s, "POST", "/api/auth/login", "", map[string]any{ + "email": "priya@acme.com", "password": "correct horse battery"}); rec.Code != http.StatusUnauthorized { + t.Fatalf("the old password still works after a reset, got %d", rec.Code) + } + // The new one is alive. + login(t, s, "priya@acme.com", out.Password) +} + +// A user id is not a secret and this endpoint hands out a credential, so it +// must not be reachable across tenants - and it must read as "no such person", +// not as "that id is real but not yours". +func TestAResetCannotReachAnotherCompanysStaff(t *testing.T) { + s, fs := newServer(t) + seedUser(fs) + fs.addUser("theirs@other.com", "correct horse battery", UserRecord{ + ID: acmeOtherID, ClientID: "client-other", ClientName: "Other Ltd", + FullName: "Theo", Role: "staff", Active: true, + }) + mgr := login(t, s, "manager@acme.com", "correct horse battery") + + rec := do(t, s, "POST", "/api/team/"+acmeOtherID+"/password", mgr.Token, nil) + if rec.Code != http.StatusNotFound { + t.Fatalf("cross-tenant reset: want 404, got %d", rec.Code) + } + // And nothing happened to them. + login(t, s, "theirs@other.com", "correct horse battery") +} + +func TestStaffCannotResetAnyonesPassword(t *testing.T) { + s, fs := newServer(t) + seedUser(fs) + seedMember(fs, acmeStaffID, "staff@acme.com", "Sam", "staff") + staff := login(t, s, "staff@acme.com", "correct horse battery") + + rec := do(t, s, "POST", "/api/team/"+acmeStaffID+"/password", staff.Token, nil) + if rec.Code != http.StatusForbidden { + t.Fatalf("want 403, got %d", rec.Code) + } +} diff --git a/server/internal/api/types.go b/server/internal/api/types.go index 4922ec7..c5ed30e 100644 --- a/server/internal/api/types.go +++ b/server/internal/api/types.go @@ -695,6 +695,38 @@ type TeamUpdate struct { Active *bool `json:"active,omitempty"` } +// NewMemberInput is a staff account created directly by a manager, with a +// password the manager hands over. +// +// The other path - an invitation the salesperson redeems on their own phone - +// is better when it fits: the manager never touches the password. It does not +// fit a salesperson being set up before their first shift, without a phone in +// hand, by somebody who wants to write a login on a card and be done. This is +// that path, and it mirrors how the platform admin creates a merchant owner: +// same generated password, same shown-once rule. +type NewMemberInput struct { + Email string `json:"email"` + FullName string `json:"full_name"` + Role string `json:"role"` + // Password is optional. Empty means "generate one", which is the better + // default for the same reason it is on the admin side. + Password string `json:"password"` +} + +// NewMemberResult is the member plus the one moment their password is readable. +type NewMemberResult struct { + TeamMember + // Password is shown once. It is bcrypt-hashed on the way in and is not + // recoverable afterwards. + Password string `json:"password"` +} + +// PasswordReset is both the optional request ("use this one") and the response +// ("here is the one that was set") for a manager resetting a member's password. +type PasswordReset struct { + Password string `json:"password"` +} + // ==================================================== devices and sessions == // DeviceSession is one signed-in device, as its owner sees it. diff --git a/server/internal/auth/auth.go b/server/internal/auth/auth.go index ec54c76..5068094 100644 --- a/server/internal/auth/auth.go +++ b/server/internal/auth/auth.go @@ -71,6 +71,21 @@ func UseTestCost() func() { return func() { bcryptCost = previous; DummyHash = previousDummy } } +// RandomPassword mints a credential for somebody else - a merchant owner +// created by the platform admin, a salesperson created by their manager, a +// reset. 80 bits as 16 lowercase base32 characters: long enough that guessing +// it is not a plan, and a shape a person can read down a phone line without +// spelling out case. One generator rather than one per caller, so nobody +// later writes a shorter one for the "less important" account. +func RandomPassword() (string, error) { + b := make([]byte, 10) + if _, err := rand.Read(b); err != nil { + return "", err + } + return strings.ToLower(base32.StdEncoding. + WithPadding(base32.NoPadding).EncodeToString(b)), nil +} + func HashPassword(plain string) (string, error) { if err := CheckPasswordPolicy(plain); err != nil { return "", err diff --git a/server/internal/store/api_admin.go b/server/internal/store/api_admin.go index 77788e1..5dc1cec 100644 --- a/server/internal/store/api_admin.go +++ b/server/internal/store/api_admin.go @@ -2,10 +2,7 @@ package store import ( "context" - "crypto/rand" - "encoding/base32" "fmt" - "strings" "time" "github.com/loyaly/behavision-server/internal/api" @@ -28,7 +25,7 @@ func (s *Store) CreateClientWithOwner(ctx context.Context, in api.NewClientInput if password == "" { // Generated rather than defaulted. An operator inventing a password for // somebody else invents a weak one and then sends it over chat. - p, err := randomPassword() + p, err := auth.RandomPassword() if err != nil { return out, err } @@ -107,11 +104,3 @@ func (s *Store) ListClients(ctx context.Context) ([]api.ClientRow, error) { // base32 without padding, matching the rest of this system's generated // secrets: it gets read down a phone line and pasted into a form, and base64's // + / = survive neither. -func randomPassword() (string, error) { - b := make([]byte, 10) // 80 bits -> 16 characters - if _, err := rand.Read(b); err != nil { - return "", err - } - return strings.ToLower(base32.StdEncoding. - WithPadding(base32.NoPadding).EncodeToString(b)), nil -} diff --git a/server/internal/store/api_team.go b/server/internal/store/api_team.go index a055a7c..3b1b978 100644 --- a/server/internal/store/api_team.go +++ b/server/internal/store/api_team.go @@ -345,3 +345,72 @@ func (s *Store) RevokeOtherSessions(ctx context.Context, userID, keepSessionID s } return int(tag.RowsAffected()), nil } + +// CreateMember inserts an active account into a tenant. +// +// The email uniqueness constraint is global (migration 007), and a clash here +// is an ordinary typing mistake - somebody already has that address - so it +// surfaces as a conflict the manager can act on, not a 500. +func (s *Store) CreateMember(ctx context.Context, clientID string, + in api.NewMemberInput, hash string) (api.TeamMember, error) { + + var m api.TeamMember + err := s.pool.QueryRow(ctx, ` + INSERT INTO app_users (client_id, email, password_hash, full_name, role) + VALUES ($1::uuid, $2, $3, $4, $5) + RETURNING id::text, email, full_name, role, active, '', + to_char(created_at AT TIME ZONE 'UTC', 'YYYY-MM-DD"T"HH24:MI:SS"Z"')`, + clientID, in.Email, hash, in.FullName, in.Role, + ).Scan(&m.ID, &m.Email, &m.FullName, &m.Role, &m.Active, + &m.LastLoginAt, &m.CreatedAt) + if err != nil { + return api.TeamMember{}, fmt.Errorf("create member: %w", err) + } + return m, nil +} + +// ResetMemberPassword replaces a member's password and signs them out +// everywhere, in one transaction. +// +// The two go together because of why a manager resets a password at all: the +// salesperson forgot it, or lost the phone it was saved on. In the second case +// the old sessions are the problem, and a reset that left them valid would +// look complete while changing nothing that mattered. Scoped to the caller's +// tenant in the UPDATE itself, so a user id from another company matches no +// row rather than being reset. +func (s *Store) ResetMemberPassword(ctx context.Context, clientID, userID, + hash string) (api.TeamMember, error) { + + tx, err := s.pool.Begin(ctx) + if err != nil { + return api.TeamMember{}, err + } + defer tx.Rollback(ctx) //nolint:errcheck // no-op once committed + + var m api.TeamMember + err = tx.QueryRow(ctx, ` + UPDATE app_users SET password_hash = $3 + WHERE id = $2::uuid AND client_id = $1::uuid + RETURNING id::text, email, full_name, role, active, + COALESCE(to_char(last_login_at AT TIME ZONE 'UTC', + 'YYYY-MM-DD"T"HH24:MI:SS"Z"'), ''), + to_char(created_at AT TIME ZONE 'UTC', 'YYYY-MM-DD"T"HH24:MI:SS"Z"')`, + clientID, userID, hash, + ).Scan(&m.ID, &m.Email, &m.FullName, &m.Role, &m.Active, + &m.LastLoginAt, &m.CreatedAt) + if errors.Is(err, pgx.ErrNoRows) { + return api.TeamMember{}, errors.New("no such team member") + } + if err != nil { + return api.TeamMember{}, fmt.Errorf("reset password: %w", err) + } + if _, err := tx.Exec(ctx, ` + UPDATE sessions SET revoked_at = now() + WHERE user_id = $1::uuid AND revoked_at IS NULL`, userID); err != nil { + return api.TeamMember{}, fmt.Errorf("revoke sessions: %w", err) + } + if err := tx.Commit(ctx); err != nil { + return api.TeamMember{}, err + } + return m, nil +} diff --git a/server/internal/store/api_team_live_test.go b/server/internal/store/api_team_live_test.go new file mode 100644 index 0000000..f4c4c2b --- /dev/null +++ b/server/internal/store/api_team_live_test.go @@ -0,0 +1,96 @@ +package store + +import ( + "context" + "testing" + + "github.com/loyaly/behavision-server/internal/api" + "github.com/loyaly/behavision-server/internal/auth" +) + +// The in-memory fake agrees with whatever SQL I wrote. These run the two new +// statements against Postgres: the RETURNING list has to scan, the tenant +// scope has to hold, and a reset has to actually revoke the sessions row. + +func TestLiveAManagerCreatedLoginRoundTrips(t *testing.T) { + st := liveStore(t) + ctx := context.Background() + clientID, _ := seedTenant(t, st, "mem"+stamp(), 0, false) + + hash, err := auth.HashPassword("a-perfectly-good-password") + if err != nil { + t.Fatal(err) + } + m, err := st.CreateMember(ctx, clientID, api.NewMemberInput{ + Email: "priya@" + stamp() + ".test", FullName: "Priya R", Role: "staff", + }, hash) + if err != nil { + t.Fatalf("create: %v", err) + } + if m.ID == "" || !m.Active || m.Role != "staff" || m.CreatedAt == "" { + t.Fatalf("member not as created: %+v", m) + } + // LastLoginAt is RETURNED as '' for a brand-new row; it must scan into a + // string, not fail as an untyped literal. + if m.LastLoginAt != "" { + t.Fatalf("a new member has never logged in, got %q", m.LastLoginAt) + } + + // Findable by the login path, in the right tenant, with the hash intact. + rec, err := st.UserByEmail(ctx, m.Email) + if err != nil || !rec.Found { + t.Fatalf("new member not findable: %v found=%v", err, rec.Found) + } + if rec.ClientID != clientID || !auth.VerifyPassword(rec.PasswordHash, "a-perfectly-good-password") { + t.Fatalf("landed wrong: client=%s verify=%v", rec.ClientID, auth.VerifyPassword(rec.PasswordHash, "a-perfectly-good-password")) + } +} + +func TestLiveAResetIsTenantScopedAndRevokesSessions(t *testing.T) { + st := liveStore(t) + ctx := context.Background() + mine, _ := seedTenant(t, st, "rsa"+stamp(), 0, false) + theirs, _ := seedTenant(t, st, "rsb"+stamp(), 0, false) + + oldHash, _ := auth.HashPassword("old-password-here") + m, err := st.CreateMember(ctx, mine, api.NewMemberInput{ + Email: "sam@" + stamp() + ".test", FullName: "Sam", Role: "staff"}, oldHash) + if err != nil { + t.Fatalf("create: %v", err) + } + + // Give them a live session to lose. + if _, err := st.pool.Exec(ctx, ` + INSERT INTO sessions (user_id, client_id, access_hash, refresh_hash, + access_expires_at, refresh_expires_at, device) + VALUES ($1::uuid, $2::uuid, $3, $4, now() + interval '1 hour', + now() + interval '30 days', 'lost phone')`, + m.ID, mine, []byte("a"+stamp()), []byte("r"+stamp())); err != nil { + t.Fatalf("seed session: %v", err) + } + + // Another tenant's manager cannot reset them, and it reads as no such row. + newHash, _ := auth.HashPassword("new-password-here") + if _, err := st.ResetMemberPassword(ctx, theirs, m.ID, newHash); err == nil { + t.Fatal("a reset from another tenant should find nobody") + } + + // Their own tenant can, and it takes the session with it. + if _, err := st.ResetMemberPassword(ctx, mine, m.ID, newHash); err != nil { + t.Fatalf("reset: %v", err) + } + var live int + if err := st.pool.QueryRow(ctx, ` + SELECT count(*) FROM sessions WHERE user_id = $1::uuid AND revoked_at IS NULL`, + m.ID).Scan(&live); err != nil { + t.Fatal(err) + } + if live != 0 { + t.Fatalf("%d session(s) survived a password reset", live) + } + rec, _ := st.UserByEmail(ctx, m.Email) + if !auth.VerifyPassword(rec.PasswordHash, "new-password-here") || + auth.VerifyPassword(rec.PasswordHash, "old-password-here") { + t.Fatal("the hash did not change to the new password") + } +}