login fix
This commit is contained in:
@@ -6,6 +6,7 @@ import (
|
||||
"strings"
|
||||
|
||||
"nearle/services"
|
||||
"nearle/utils"
|
||||
|
||||
"github.com/gofiber/fiber/v2"
|
||||
)
|
||||
@@ -93,6 +94,15 @@ func (ctl *HealthController) Health(c *fiber.Ctx) error {
|
||||
"code": http.StatusOK, "status": true, "message": "Success",
|
||||
"details": fiber.Map{
|
||||
"version": buildVersion(),
|
||||
// Can this server issue console sessions at all?
|
||||
//
|
||||
// `attachWebSession` logs a minting failure and lets the login
|
||||
// succeed without a token, so a server with no signing secret hands
|
||||
// out sessions that cannot authenticate: the console renders, and
|
||||
// every request after it comes back 401 with no `authorization`
|
||||
// header on it. False here is that, stated once, instead of found
|
||||
// by reading request headers on a Friday morning.
|
||||
"sessions": utils.WebTokenConfigured(),
|
||||
// True when a model is configured and the assistant can answer. False
|
||||
// is the answer to "I set the key and redeployed, did it take?" —
|
||||
// which took a day to establish without it.
|
||||
|
||||
@@ -163,3 +163,32 @@ func (s stubAssistant) Ask(_ context.Context, _, _ string, _ tools.Caller) (serv
|
||||
func (s stubAssistant) Approve(_ context.Context, _, _ string, _ tools.Caller) (services.AssistantAnswer, error) {
|
||||
return services.AssistantAnswer{}, nil
|
||||
}
|
||||
|
||||
func TestHealthSaysWhetherSessionsCanBeIssued(t *testing.T) {
|
||||
// The failure this exists for: `attachWebSession` logs a minting failure and
|
||||
// lets the login succeed anyway, so a server with no signing secret issues
|
||||
// sessions that cannot authenticate. The console renders, every request
|
||||
// after it 401s with no `authorization` header, and nothing says why.
|
||||
t.Setenv("POS_TOKEN_SECRET", "")
|
||||
t.Setenv("JWT_SECRET_KEY", "")
|
||||
_, broken, body := readHealth(t, healthApp(t, true, true))
|
||||
if broken["sessions"] != false {
|
||||
t.Fatalf("a server that cannot sign a session claimed it could: %s", body)
|
||||
}
|
||||
|
||||
t.Setenv("POS_TOKEN_SECRET", "a-secret-of-quite-sufficient-length")
|
||||
_, working, _ := readHealth(t, healthApp(t, true, true))
|
||||
if working["sessions"] != true {
|
||||
t.Fatal("a server with a signing secret reported it could not issue sessions")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHealthDoesNotLeakTheSigningSecret(t *testing.T) {
|
||||
// A boolean about the secret, never the secret.
|
||||
t.Setenv("POS_TOKEN_SECRET", "correct-horse-battery-staple")
|
||||
_, _, body := readHealth(t, healthApp(t, true, true))
|
||||
|
||||
if strings.Contains(body, "correct-horse") {
|
||||
t.Fatalf("the signing secret is on an unauthenticated endpoint: %s", body)
|
||||
}
|
||||
}
|
||||
|
||||
206
controllers/setPassword_test.go
Normal file
206
controllers/setPassword_test.go
Normal file
@@ -0,0 +1,206 @@
|
||||
package controllers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"nearle/middleware"
|
||||
"nearle/models"
|
||||
|
||||
fiberv1 "github.com/gofiber/fiber"
|
||||
"github.com/gofiber/fiber/v2"
|
||||
)
|
||||
|
||||
/*
|
||||
Setting a first password, with no session and no way to get one.
|
||||
|
||||
A branch login created by `createtenantlocation` arrives with an empty password.
|
||||
The console signs in, is told to set one, and does — and until now it did that
|
||||
through `PUT /users/update`, which is behind the session guard. Once
|
||||
WEB_AUTH_REQUIRED began defaulting on, that answered
|
||||
|
||||
401 "a session token is required; sign in again"
|
||||
|
||||
to somebody who could not sign in, because signing in needs the password they
|
||||
were trying to set. Every such account was unusable, and the 401 read as an
|
||||
authentication bug rather than a deadlock.
|
||||
|
||||
`publicWebPaths` had named `/users/setpassword` since the guard was written. The
|
||||
path was reserved; the handler never existed, so it answered 404.
|
||||
|
||||
The tests that matter are about the two halves: it must be reachable WITHOUT a
|
||||
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
|
||||
}
|
||||
|
||||
func (f *fakePasswords) SetInitialPassword(userid int, password string) error {
|
||||
if f.refuseIt != nil {
|
||||
return f.refuseIt
|
||||
}
|
||||
f.set = append(f.set, password)
|
||||
return nil
|
||||
}
|
||||
|
||||
// The rest of UserService, unused here.
|
||||
func (f *fakePasswords) GetAllUsers(int, int, int, int, string) ([]models.UserInfo, error) {
|
||||
return nil, nil
|
||||
}
|
||||
func (f *fakePasswords) GetUserByID(int) (models.UserInfo, error) { return models.UserInfo{}, nil }
|
||||
func (f *fakePasswords) Login(models.User) (models.UserInfo, error) {
|
||||
return models.UserInfo{}, nil
|
||||
}
|
||||
func (f *fakePasswords) TenantLogin(models.User) (models.TenantUserInfo, error) {
|
||||
return models.TenantUserInfo{}, nil
|
||||
}
|
||||
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 passwordApp(t *testing.T, service *fakePasswords) *fiber.App {
|
||||
t.Helper()
|
||||
t.Setenv("POS_TOKEN_SECRET", testSecret)
|
||||
|
||||
app := fiber.New()
|
||||
// The real guard, mounted exactly as routes.go mounts it. The point of this
|
||||
// file is which side of it this endpoint lands on.
|
||||
app.Use("/live/api/v1/web", middleware.WebAuth(nil))
|
||||
app.Post("/live/api/v1/web/users/setpassword", NewUserController(service).SetPassword)
|
||||
app.Put("/live/api/v1/web/users/update", NewUserController(service).UpdateStaff)
|
||||
|
||||
return app
|
||||
}
|
||||
|
||||
func send(t *testing.T, app *fiber.App, method, path, body string) (int, string) {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequest(method, path, strings.NewReader(body))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
|
||||
resp, err := app.Test(req, -1)
|
||||
if err != nil {
|
||||
t.Fatalf("%s %s: %v", method, path, err)
|
||||
}
|
||||
raw, _ := io.ReadAll(resp.Body)
|
||||
return resp.StatusCode, string(raw)
|
||||
}
|
||||
|
||||
func TestAFirstPasswordCanBeSetWithoutASession(t *testing.T) {
|
||||
// The whole point. There is no session to present and no way to obtain one.
|
||||
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.StatusUnauthorized {
|
||||
t.Fatalf("the guard blocked the one call that cannot present a token: %s", body)
|
||||
}
|
||||
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 TestTheGeneralUpdateStaysBehindTheGuard(t *testing.T) {
|
||||
// The reason this is a new endpoint rather than `/users/update` being
|
||||
// opened up: that one writes whatever struct it is handed, so unauthenticated
|
||||
// it would let anybody change any field of any user.
|
||||
service := &fakePasswords{}
|
||||
app := passwordApp(t, service)
|
||||
|
||||
status, body := send(t, app, "PUT", "/live/api/v1/web/users/update",
|
||||
`{"userid":904,"roleid":1,"tenantid":9}`)
|
||||
|
||||
if status != fiber.StatusUnauthorized {
|
||||
t.Fatalf("an untokened user update was not refused: %d %s", status, body)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAnAccountThatAlreadyHasOneIsRefusedAsAConflict(t *testing.T) {
|
||||
// 409, never 401. Nothing here is an authentication failure — the caller is
|
||||
// not supposed to have a session — and a 401 would send the console into its
|
||||
// sign-out-and-reload path on the one screen with nothing to sign out of.
|
||||
service := &fakePasswords{refuseIt: errAlreadySet{}}
|
||||
app := passwordApp(t, service)
|
||||
|
||||
status, body := send(t, app, "POST", "/live/api/v1/web/users/setpassword",
|
||||
`{"userid":904,"password":"opensesame"}`)
|
||||
|
||||
if status != fiber.StatusConflict {
|
||||
t.Fatalf("expected 409, got %d: %s", status, body)
|
||||
}
|
||||
if len(service.set) != 0 {
|
||||
t.Fatalf("a refused call still wrote: %v", service.set)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTheRefusalDoesNotSayWhichAccountsExist(t *testing.T) {
|
||||
// "No such user" and "already has a password" must read identically, or
|
||||
// this becomes a way to ask whether a userid exists and whether it has been
|
||||
// set up — unauthenticated, one request at a time.
|
||||
service := &fakePasswords{refuseIt: errAlreadySet{}}
|
||||
app := passwordApp(t, service)
|
||||
|
||||
_, body := send(t, app, "POST", "/live/api/v1/web/users/setpassword",
|
||||
`{"userid":904,"password":"opensesame"}`)
|
||||
|
||||
for _, leak := range []string{"not found", "no such", "does not exist"} {
|
||||
if strings.Contains(strings.ToLower(body), leak) {
|
||||
t.Fatalf("the refusal distinguishes a missing account: %s", body)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestAMalformedBodyIsRefusedWithoutPanicking(t *testing.T) {
|
||||
app := passwordApp(t, &fakePasswords{})
|
||||
|
||||
status, _ := send(t, app, "POST", "/live/api/v1/web/users/setpassword", `{"userid":`)
|
||||
if status != fiber.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d", status)
|
||||
}
|
||||
}
|
||||
|
||||
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"}`)
|
||||
|
||||
var envelope struct {
|
||||
Status bool `json:"status"`
|
||||
Code int `json:"code"`
|
||||
Message string `json:"message"`
|
||||
}
|
||||
if err := json.Unmarshal([]byte(body), &envelope); err != nil {
|
||||
t.Fatalf("not an envelope: %s", body)
|
||||
}
|
||||
if !envelope.Status || envelope.Code != fiber.StatusOK {
|
||||
t.Fatalf("success did not read as success: %s", body)
|
||||
}
|
||||
}
|
||||
|
||||
type errAlreadySet struct{}
|
||||
|
||||
func (errAlreadySet) Error() string {
|
||||
return "that account cannot have its password set here — it may already have one"
|
||||
}
|
||||
|
||||
func (f *fakePasswords) TenantWebLogin(models.User) (models.TenantUserInfo, map[string]interface{}) {
|
||||
return models.TenantUserInfo{}, map[string]interface{}{}
|
||||
}
|
||||
func (f *fakePasswords) DeleteUser(int) error { return nil }
|
||||
@@ -324,3 +324,50 @@ func (ctl *UserController) DeleteUser(c *fiber.Ctx) error {
|
||||
})
|
||||
}
|
||||
|
||||
// SetPassword gives a never-used account its first password.
|
||||
//
|
||||
// ── Why this endpoint exists ────────────────────────────────────────────────
|
||||
//
|
||||
// Because the flow was impossible without it. A branch login created by
|
||||
// `createtenantlocation` arrives with an empty password; the console signs in,
|
||||
// is told to set one, and does so — through `PUT /users/update`, which sits
|
||||
// behind the session guard. So the call answered "a session token is required;
|
||||
// sign in again" to a person who could not sign in, because they had no
|
||||
// password yet. Every such account was unusable.
|
||||
//
|
||||
// `publicWebPaths` has named `/users/setpassword` since the guard was written.
|
||||
// The path was reserved and the handler never built, so it answered 404 and the
|
||||
// console went on using the guarded one.
|
||||
//
|
||||
// ── Why not simply open up `/users/update` ──────────────────────────────────
|
||||
//
|
||||
// It writes whatever struct it is handed. Unauthenticated, it would let anybody
|
||||
// change any field of any user — their email, their role, their tenant. This
|
||||
// takes two fields and can only act on an account with no password, which is
|
||||
// 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"`
|
||||
Password string `json:"password"`
|
||||
}
|
||||
if err := c.BodyParser(&req); err != nil {
|
||||
return c.Status(http.StatusBadRequest).JSON(fiber.Map{
|
||||
"status": false, "code": http.StatusBadRequest, "message": "Invalid request body",
|
||||
})
|
||||
}
|
||||
|
||||
if err := ctl.userService.SetInitialPassword(req.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
|
||||
// where there is nothing to sign out of.
|
||||
return c.Status(http.StatusConflict).JSON(fiber.Map{
|
||||
"status": false, "code": http.StatusConflict, "message": err.Error(),
|
||||
})
|
||||
}
|
||||
|
||||
return c.JSON(fiber.Map{
|
||||
"status": true, "code": http.StatusOK,
|
||||
"message": "Password set. Sign in with it.",
|
||||
})
|
||||
}
|
||||
|
||||
@@ -14,6 +14,7 @@ import (
|
||||
type UserRepository interface {
|
||||
GetAllUsers(roleID, tenantID, pageno, pagesize int, keyword string) ([]models.UserInfo, error)
|
||||
GetUserByID(uid int) (models.UserInfo, error)
|
||||
SetInitialPassword(userid int, password string) error
|
||||
Login(user models.User) (models.UserInfo, error)
|
||||
FindUserID(authname, contactno string, configid int) (int, error)
|
||||
UpdateStaff(user models.User) error
|
||||
@@ -391,3 +392,46 @@ func (r *userRepository) GetLocationStatus(locationid int) string {
|
||||
func (r *userRepository) DeleteUser(userid int) error {
|
||||
return r.db.Table("app_users").Where("userid = ?", userid).Delete(&models.User{}).Error
|
||||
}
|
||||
|
||||
// SetInitialPassword writes the first password on an account that has none.
|
||||
//
|
||||
// ── Why this is a separate call and not `UpdateStaff` ───────────────────────
|
||||
//
|
||||
// It is the one write that MUST work without a session, and that is the whole
|
||||
// difficulty. A brand-new account — `createtenantlocation` spawns branch logins
|
||||
// with an empty password — signs in, is told to set one, and at that moment has
|
||||
// no token and no way to get one. The console was doing this through
|
||||
// `PUT /users/update`, which sits behind the session guard, so the call came
|
||||
// back "a session token is required; sign in again" and the account could never
|
||||
// be used. Sign-in needs a password; setting the password needed a sign-in.
|
||||
//
|
||||
// `/users/update` could not simply be opened up: it writes whatever struct it
|
||||
// is handed, so an unauthenticated caller could edit any field of any user.
|
||||
// This can do exactly one thing, to exactly one kind of account.
|
||||
//
|
||||
// ── What makes it safe to expose ────────────────────────────────────────────
|
||||
//
|
||||
// The empty-password check IS the authorisation. An account with a password set
|
||||
// is refused, so this can never overwrite a credential — it is a setup call,
|
||||
// never a reset. There is no "forgot password" flow on this backend and this
|
||||
// must not become one by accident: a reset needs proof of identity, and nothing
|
||||
// here has any.
|
||||
//
|
||||
// The check and the write are one statement, so two callers racing cannot both
|
||||
// see an empty password and both set one. Postgres decides, not this process.
|
||||
func (r *userRepository) SetInitialPassword(userid int, password string) error {
|
||||
result := r.db.Table("app_users").
|
||||
Where("userid = ? AND (password IS NULL OR TRIM(password) = '')", userid).
|
||||
Update("password", password)
|
||||
|
||||
if result.Error != nil {
|
||||
return result.Error
|
||||
}
|
||||
if result.RowsAffected == 0 {
|
||||
// One message for "no such user" and "already has a password". They
|
||||
// must not be distinguishable, or this becomes a way to ask whether a
|
||||
// given userid exists and whether it has been set up.
|
||||
return errors.New("that account cannot have its password set here — it may already have one")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -14,6 +14,10 @@ func RegisterUserRoutes(api fiber.Router, f *facade.Facade) {
|
||||
users.Post("/applogin", f.UserController.AppLogin)
|
||||
users.Post("/create", f.UserController.CreateUser)
|
||||
users.Post("/tenant/weblogin", f.UserController.TenantWebLogin)
|
||||
|
||||
// First password, before a session can exist. Public by necessity and safe
|
||||
// because of what it refuses — see userController.SetPassword.
|
||||
users.Post("/setpassword", f.UserController.SetPassword)
|
||||
users.Put("/update", f.UserController.UpdateStaff)
|
||||
users.Delete("/delete", f.UserController.DeleteUser)
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ package services
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"log"
|
||||
"nearle/models"
|
||||
"nearle/repositories"
|
||||
@@ -58,6 +59,9 @@ type UserService interface {
|
||||
Login(user models.User) (models.UserInfo, error)
|
||||
TenantLogin(user models.User) (models.TenantUserInfo, error)
|
||||
UpdateStaff(user models.User) error
|
||||
// SetInitialPassword is the one write reachable without a session — see
|
||||
// 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)
|
||||
TenantWebLogin(user models.User) (models.TenantUserInfo, map[string]interface{})
|
||||
@@ -355,3 +359,25 @@ func (s *userService) TenantWebLogin(user models.User) (models.TenantUserInfo, m
|
||||
func (s *userService) DeleteUser(userid int) error {
|
||||
return s.repo.DeleteUser(userid)
|
||||
}
|
||||
|
||||
// SetInitialPassword gives a never-used account its first password.
|
||||
//
|
||||
// The minimum length is enforced here as well as at the edge: this is the only
|
||||
// write in the product reachable without a session, so the rule cannot live
|
||||
// only in a handler that a second caller might not go through.
|
||||
func (s *userService) SetInitialPassword(userid int, password string) error {
|
||||
if userid <= 0 {
|
||||
return errors.New("which account?")
|
||||
}
|
||||
password = strings.TrimSpace(password)
|
||||
if len(password) < MinPasswordLength {
|
||||
return fmt.Errorf("the password must be at least %d characters", MinPasswordLength)
|
||||
}
|
||||
return s.repo.SetInitialPassword(userid, password)
|
||||
}
|
||||
|
||||
// MinPasswordLength is the floor for a console password.
|
||||
//
|
||||
// Six, matching the check `UpdateStaff` already applied at the controller — not
|
||||
// a new rule, the same one stated where both callers can reach it.
|
||||
const MinPasswordLength = 6
|
||||
|
||||
Reference in New Issue
Block a user