diff --git a/controllers/healthController.go b/controllers/healthController.go index 6e433e5..966e6ff 100644 --- a/controllers/healthController.go +++ b/controllers/healthController.go @@ -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. diff --git a/controllers/health_test.go b/controllers/health_test.go index c791421..a2d776d 100644 --- a/controllers/health_test.go +++ b/controllers/health_test.go @@ -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) + } +} diff --git a/controllers/setPassword_test.go b/controllers/setPassword_test.go new file mode 100644 index 0000000..025464c --- /dev/null +++ b/controllers/setPassword_test.go @@ -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 } diff --git a/controllers/userController.go b/controllers/userController.go index 9de475e..afbf868 100644 --- a/controllers/userController.go +++ b/controllers/userController.go @@ -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.", + }) +} diff --git a/repositories/userRepository.go b/repositories/userRepository.go index c99b5e6..984ed13 100644 --- a/repositories/userRepository.go +++ b/repositories/userRepository.go @@ -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 +} diff --git a/routes/userroutes.go b/routes/userroutes.go index 5c58107..15d3fb6 100644 --- a/routes/userroutes.go +++ b/routes/userroutes.go @@ -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) diff --git a/services/userService.go b/services/userService.go index 6a6922c..ad5d884 100644 --- a/services/userService.go +++ b/services/userService.go @@ -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