Stop reporting a failed login lookup as "Invalid Email"
GetUserByAuthname / GetUserByContactNo / GetUserLogin discarded the Scan error, so a database that could not answer — down, pool exhausted, or booted without its config (2026-07-20) — came back as uid 0 and every user was told their email was wrong. One lookup, GetUserLogin, now returns an error; sql.ErrNoRows is "not found" and anything else reaches the service, which answers 500 "Login is temporarily unavailable" and logs the cause. 409 "Invalid Email" is unchanged for a genuine no-match: the console reads that exact shape as "not registered". NULL password/role columns scan through sql.Null* so they do not become 500s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,8 @@
|
||||
package repositories
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
@@ -15,13 +17,11 @@ type UserRepository interface {
|
||||
Login(user models.User) (models.UserInfo, error)
|
||||
FindUserID(authname, contactno string, configid int) (int, error)
|
||||
UpdateStaff(user models.User) error
|
||||
GetUserByAuthname(authname string, configid int) (int, string, string)
|
||||
GetUserByContactNo(contactno string, configid int) (int, string, string)
|
||||
UpdateFCMToken(userid int, token string) error
|
||||
GetTenantUserById(userid int) models.TenantUserInfo
|
||||
CreateUser(user models.User) (int, error)
|
||||
GetUserById(uid int) (models.UserInfo, error)
|
||||
GetUserLogin(field, value string, configid int) (int, string, string, int)
|
||||
GetUserLogin(field, value string, configid int) (int, string, string, int, error)
|
||||
UpdateUserFcmToken(uid int, token string) error
|
||||
GetLocationStatus(locationid int) string
|
||||
DeleteUser(userid int) error
|
||||
@@ -164,7 +164,6 @@ func (r *userRepository) Login(user models.User) (models.UserInfo, error) {
|
||||
return userInfo, nil
|
||||
}
|
||||
|
||||
|
||||
func (r *userRepository) FindUserID(authname, contactno string, configid int) (int, error) {
|
||||
var uid int
|
||||
var query string
|
||||
@@ -187,39 +186,10 @@ func (r *userRepository) FindUserID(authname, contactno string, configid int) (i
|
||||
return uid, nil
|
||||
}
|
||||
|
||||
|
||||
|
||||
func (r *userRepository) UpdateStaff(user models.User) error {
|
||||
return r.db.Table("app_users").Where("userid = ?", user.Userid).Updates(&user).Error
|
||||
}
|
||||
|
||||
// A till account is not a Nearle Daily user. The two products share this table
|
||||
// and nothing else, so every way into the application excludes roles 7 and 8 in
|
||||
// the lookup itself: a cashier is not "refused", they are simply not found.
|
||||
//
|
||||
// Doing it in the query rather than after it is deliberate. A check bolted on
|
||||
// afterwards has to be repeated at each of these call sites and is one edit away
|
||||
// from being forgotten at one of them, and that one would be the hole.
|
||||
func (r *userRepository) GetUserByAuthname(authname string, configid int) (int, string, string) {
|
||||
var uid int
|
||||
var password, status string
|
||||
query := `SELECT userid, password, status FROM app_users
|
||||
WHERE authname = ? AND configid = ?
|
||||
AND COALESCE(roleid, 0) NOT IN (7, 8)`
|
||||
r.db.Raw(query, authname, configid).Row().Scan(&uid, &password, &status)
|
||||
return uid, password, status
|
||||
}
|
||||
|
||||
func (r *userRepository) GetUserByContactNo(contactno string, configid int) (int, string, string) {
|
||||
var uid int
|
||||
var password, status string
|
||||
query := `SELECT userid, password, status FROM app_users
|
||||
WHERE contactno = ? AND configid = ?
|
||||
AND COALESCE(roleid, 0) NOT IN (7, 8)`
|
||||
r.db.Raw(query, contactno, configid).Row().Scan(&uid, &password, &status)
|
||||
return uid, password, status
|
||||
}
|
||||
|
||||
func (r *userRepository) UpdateFCMToken(userid int, token string) error {
|
||||
query := `UPDATE app_users SET userfcmtoken = ? WHERE userid = ?`
|
||||
return r.db.Exec(query, token, userid).Error
|
||||
@@ -329,9 +299,40 @@ func (r *userRepository) GetUserById(uid int) (models.UserInfo, error) {
|
||||
return user, nil
|
||||
}
|
||||
|
||||
func (r *userRepository) GetUserLogin(field, value string, configid int) (int, string, string, int) {
|
||||
var uid, roleid int
|
||||
var password, status string
|
||||
// GetUserLogin is the one sign-in lookup, for the app and the console alike.
|
||||
//
|
||||
// `field` is the column matched — "authname" or "contactno", nothing else is
|
||||
// accepted — and it is interpolated, so the whitelist is what keeps this from
|
||||
// being an injection point.
|
||||
//
|
||||
// A till account is not a Nearle Daily user. The two products share this table
|
||||
// and nothing else, so the lookup itself excludes roles 7 and 8: a cashier is
|
||||
// not "refused", they are simply not found. Doing it in the query rather than
|
||||
// after it is deliberate — a check bolted on afterwards has to be repeated at
|
||||
// every call site and is one edit away from being forgotten at one of them.
|
||||
//
|
||||
// Three outcomes, and the caller must tell them apart:
|
||||
//
|
||||
// - found: uid > 0, err == nil
|
||||
// - not found: uid == 0, err == nil
|
||||
// - failed: err != nil — the database could not answer at all
|
||||
//
|
||||
// The third used to be invisible. `Row().Scan`'s error was discarded, so a
|
||||
// database that was down, a connection pool that was exhausted or a
|
||||
// misconfigured `configid` all came back as uid 0 — which the service then
|
||||
// reported as "Invalid Email". On 2026-07-20 the deployment lost its
|
||||
// ConfigMaps/Secrets and every user on the platform was told their email was
|
||||
// wrong, and nothing in the logs said otherwise.
|
||||
func (r *userRepository) GetUserLogin(field, value string, configid int) (int, string, string, int, error) {
|
||||
switch field {
|
||||
case "authname", "contactno":
|
||||
default:
|
||||
return 0, "", "", 0, fmt.Errorf("login: %q is not a sign-in field", field)
|
||||
}
|
||||
|
||||
var uid int
|
||||
var password, status sql.NullString
|
||||
var roleid sql.NullInt64
|
||||
|
||||
query := fmt.Sprintf(`
|
||||
SELECT userid, password, status, roleid
|
||||
@@ -339,9 +340,16 @@ func (r *userRepository) GetUserLogin(field, value string, configid int) (int, s
|
||||
WHERE %s = ? AND configid = ?
|
||||
AND COALESCE(roleid, 0) NOT IN (7, 8)`, field)
|
||||
|
||||
r.db.Raw(query, value, configid).Row().Scan(&uid, &password, &status, &roleid)
|
||||
|
||||
return uid, password, status, roleid
|
||||
err := r.db.Raw(query, value, configid).Row().Scan(&uid, &password, &status, &roleid)
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return 0, "", "", 0, nil
|
||||
}
|
||||
if err != nil {
|
||||
return 0, "", "", 0, err
|
||||
}
|
||||
// Nullable columns scanned through sql.Null* so that a NULL password or
|
||||
// role — both exist on real rows — does not itself read as a failed query.
|
||||
return uid, password.String, status.String, int(roleid.Int64), nil
|
||||
}
|
||||
|
||||
func (r *userRepository) UpdateUserFcmToken(userid int, fcmToken string) error {
|
||||
@@ -359,5 +367,3 @@ 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
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -137,15 +137,11 @@ func main() {
|
||||
}
|
||||
|
||||
fmt.Println("\n3. a till account cannot reach the Nearle Daily application")
|
||||
uid, _, _ := users.GetUserByAuthname(sup.Authname, sup.Configid)
|
||||
check("applogin lookup does not find the supervisor",
|
||||
uid == 0,
|
||||
fmt.Sprintf("GetUserByAuthname(%s) -> userid %d", sup.Authname, uid))
|
||||
|
||||
uid2, _, _, _ := users.GetUserLogin("authname", sup.Authname, sup.Configid)
|
||||
check("tenant web login does not find the supervisor",
|
||||
uid2 == 0,
|
||||
fmt.Sprintf("GetUserLogin(%s) -> userid %d", sup.Authname, uid2))
|
||||
// Both the app and the console sign-in now go through GetUserLogin.
|
||||
uid2, _, _, _, lookupErr := users.GetUserLogin("authname", sup.Authname, sup.Configid)
|
||||
check("app and tenant web login do not find the supervisor",
|
||||
uid2 == 0 && lookupErr == nil,
|
||||
fmt.Sprintf("GetUserLogin(%s) -> userid %d err=%v", sup.Authname, uid2, lookupErr))
|
||||
|
||||
uid3, _ := users.FindUserID(sup.Authname, "", sup.Configid)
|
||||
check("password-setup lookup does not find the supervisor",
|
||||
|
||||
139
services/userLogin_test.go
Normal file
139
services/userLogin_test.go
Normal file
@@ -0,0 +1,139 @@
|
||||
package services
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"nearle/models"
|
||||
"nearle/repositories"
|
||||
)
|
||||
|
||||
/*
|
||||
"Invalid Email" has to mean one thing: the sign-in query ran and matched
|
||||
nobody.
|
||||
|
||||
It used to also mean "the database did not answer". The repository discarded
|
||||
the Scan error, so an outage, an exhausted pool or a deployment that had lost
|
||||
its environment all surfaced as uid 0 — and every user on the platform was told
|
||||
their email was wrong (2026-07-20). The console makes it worse: it reads a 409
|
||||
as "this address is not registered" and sends the person to sign-up.
|
||||
*/
|
||||
|
||||
// loginRepo is a UserRepository that answers only the sign-in path. Anything
|
||||
// else panics on the nil embedded interface, which is the point: these tests
|
||||
// must not reach further than the lookup.
|
||||
type loginRepo struct {
|
||||
repositories.UserRepository
|
||||
uid int
|
||||
password string
|
||||
status string
|
||||
roleid int
|
||||
err error
|
||||
|
||||
field, value string
|
||||
configid int
|
||||
}
|
||||
|
||||
func (r *loginRepo) GetUserLogin(field, value string, configid int) (int, string, string, int, error) {
|
||||
r.field, r.value, r.configid = field, value, configid
|
||||
if r.err != nil {
|
||||
return 0, "", "", 0, r.err
|
||||
}
|
||||
return r.uid, r.password, r.status, r.roleid, nil
|
||||
}
|
||||
|
||||
func (r *loginRepo) UpdateFCMToken(int, string) error { return nil }
|
||||
func (r *loginRepo) UpdateUserFcmToken(int, string) error { return nil }
|
||||
func (r *loginRepo) GetTenantUserById(uid int) models.TenantUserInfo {
|
||||
return models.TenantUserInfo{Userid: uid}
|
||||
}
|
||||
|
||||
func TestADatabaseFailureIsNotAnInvalidEmail(t *testing.T) {
|
||||
repo := &loginRepo{err: errors.New("dial tcp 10.0.0.5:5433: connection refused")}
|
||||
svc := NewUserService(repo)
|
||||
user := models.User{Authname: "owner@shop.example", Password: "pw", Configid: 1}
|
||||
|
||||
t.Run("app login", func(t *testing.T) {
|
||||
_, resp, err := svc.AppLogin(user)
|
||||
if err == nil {
|
||||
t.Fatal("a failed lookup must be an error to the controller")
|
||||
}
|
||||
if resp["code"] != 500 || resp["status"] != false {
|
||||
t.Fatalf("want a 500/false envelope, got %v", resp)
|
||||
}
|
||||
if resp["message"] == "Invalid Email" {
|
||||
t.Fatal("the database being down was reported as a wrong email")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("tenant web login", func(t *testing.T) {
|
||||
_, resp := svc.TenantWebLogin(user)
|
||||
if resp["code"] != 500 || resp["status"] != false {
|
||||
t.Fatalf("want a 500/false envelope, got %v", resp)
|
||||
}
|
||||
if resp["message"] == "Invalid Email" {
|
||||
t.Fatal("the database being down was reported as a wrong email")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// The console depends on this exact shape — code 409 — to tell "not
|
||||
// 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)
|
||||
user := models.User{Authname: "nobody@shop.example", Password: "pw", Configid: 1}
|
||||
|
||||
_, resp, err := svc.AppLogin(user)
|
||||
if err == nil || resp["code"] != 409 || resp["message"] != "Invalid Email" {
|
||||
t.Fatalf("app login: want 409 Invalid Email, got %v / %v", resp, err)
|
||||
}
|
||||
|
||||
_, wresp := svc.TenantWebLogin(user)
|
||||
if wresp["code"] != 409 || wresp["message"] != "Invalid Email" {
|
||||
t.Fatalf("web login: want 409 Invalid Email, got %v", wresp)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLookupPrefersAuthnameAndFallsBackToContactNo(t *testing.T) {
|
||||
repo := &loginRepo{uid: 7, password: "pw", status: "Active"}
|
||||
svc := NewUserService(repo)
|
||||
|
||||
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 {
|
||||
t.Fatalf("authname should win when both are sent, looked up %s=%q configid=%d", repo.field, repo.value, repo.configid)
|
||||
}
|
||||
|
||||
svc.AppLogin(models.User{Contactno: "9999999999", Password: "pw", Configid: 1})
|
||||
if repo.field != "contactno" || repo.value != "9999999999" {
|
||||
t.Fatalf("contactno should be used when authname is blank, looked up %s=%q", repo.field, repo.value)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNeitherIdentifierIsRefusedBeforeTheLookup(t *testing.T) {
|
||||
repo := &loginRepo{err: errors.New("must not be called")}
|
||||
svc := NewUserService(repo)
|
||||
|
||||
_, resp, err := svc.AppLogin(models.User{Password: "pw", Configid: 1})
|
||||
if err == nil || resp["code"] != 400 {
|
||||
t.Fatalf("want 400, got %v / %v", resp, err)
|
||||
}
|
||||
if repo.field != "" {
|
||||
t.Fatal("the repository was queried with nothing to match on")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAMatchedAccountStillSignsIn(t *testing.T) {
|
||||
repo := &loginRepo{uid: 42, password: "secret", status: "Active", roleid: 2}
|
||||
svc := NewUserService(repo)
|
||||
|
||||
info, resp, err := svc.AppLogin(models.User{Authname: "owner@shop.example", Password: "secret", Configid: 1})
|
||||
if err != nil || resp["code"] != 200 || info.Userid != 42 {
|
||||
t.Fatalf("app login: want success for userid 42, got %v / %v / %+v", resp, err, info)
|
||||
}
|
||||
|
||||
winfo, wresp := svc.TenantWebLogin(models.User{Authname: "owner@shop.example", Password: "secret", Configid: 1, Roleid: 2})
|
||||
if wresp["code"] != 200 || winfo.Userid != 42 {
|
||||
t.Fatalf("web login: want success for userid 42, got %v / %+v", wresp, winfo)
|
||||
}
|
||||
}
|
||||
@@ -2,6 +2,7 @@ package services
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"log"
|
||||
"nearle/models"
|
||||
"nearle/repositories"
|
||||
"strings"
|
||||
@@ -9,6 +10,48 @@ import (
|
||||
"github.com/gofiber/fiber"
|
||||
)
|
||||
|
||||
// errLoginUnavailable is returned by lookupLogin when the database could not
|
||||
// answer the sign-in query. It is deliberately not "invalid user": the account
|
||||
// may well exist, and the person needs to be told to try again, not to check
|
||||
// their spelling.
|
||||
var errLoginUnavailable = errors.New("login lookup failed")
|
||||
|
||||
// loginUnavailableResponse is the body for that case. 500 rather than 409,
|
||||
// because the console reads `409` as "this email does not exist" (see
|
||||
// daily_merchant_web/src/services/auth.ts) and would otherwise send a
|
||||
// perfectly good account to the sign-up form while the database was down.
|
||||
func loginUnavailableResponse() map[string]interface{} {
|
||||
return map[string]interface{}{
|
||||
"status": false,
|
||||
"code": 500,
|
||||
"message": "Login is temporarily unavailable. Please try again in a moment.",
|
||||
}
|
||||
}
|
||||
|
||||
// lookupLogin resolves who is signing in, by authname first and contact number
|
||||
// second, and separates "not found" from "could not look".
|
||||
//
|
||||
// Both login paths used to run their own copy of this and both discarded the
|
||||
// repository's error, so a database that was down came back as uid 0 and was
|
||||
// reported as "Invalid Email". That message now means exactly one thing: the
|
||||
// query ran and matched nobody.
|
||||
func (s *userService) lookupLogin(user models.User) (uid int, password, status string, roleid int, err error) {
|
||||
field, value := "authname", user.Authname
|
||||
if user.Authname == "" {
|
||||
field, value = "contactno", user.Contactno
|
||||
}
|
||||
|
||||
uid, password, status, roleid, err = s.repo.GetUserLogin(field, value, user.Configid)
|
||||
if err != nil {
|
||||
// The value is what the caller typed — an email or a phone number —
|
||||
// and is safe to log; the password never reaches this function's
|
||||
// output.
|
||||
log.Printf("login: lookup by %s=%q configid=%d failed: %v", field, value, user.Configid, err)
|
||||
return 0, "", "", 0, errLoginUnavailable
|
||||
}
|
||||
return uid, password, status, roleid, nil
|
||||
}
|
||||
|
||||
type UserService interface {
|
||||
GetAllUsers(roleID, tenantID, pageno, pagesize int, keyword string) ([]models.UserInfo, error)
|
||||
GetUserByID(uid int) (models.UserInfo, error)
|
||||
@@ -80,15 +123,7 @@ func (s *userService) UpdateStaff(user models.User) error {
|
||||
}
|
||||
|
||||
func (s *userService) AppLogin(user models.User) (models.TenantUserInfo, fiber.Map, error) {
|
||||
var uid int
|
||||
var status, dbPassword string
|
||||
|
||||
// Get user by authname or contactno
|
||||
if user.Authname != "" {
|
||||
uid, dbPassword, status = s.repo.GetUserByAuthname(user.Authname, user.Configid)
|
||||
} else if user.Contactno != "" {
|
||||
uid, dbPassword, status = s.repo.GetUserByContactNo(user.Contactno, user.Configid)
|
||||
} else {
|
||||
if user.Authname == "" && user.Contactno == "" {
|
||||
resp := fiber.Map{
|
||||
"code": 400,
|
||||
"status": false,
|
||||
@@ -97,7 +132,12 @@ func (s *userService) AppLogin(user models.User) (models.TenantUserInfo, fiber.M
|
||||
return models.TenantUserInfo{}, resp, errors.New("missing authname or contactno")
|
||||
}
|
||||
|
||||
// Invalid user
|
||||
uid, dbPassword, status, _, err := s.lookupLogin(user)
|
||||
if err != nil {
|
||||
return models.TenantUserInfo{}, fiber.Map(loginUnavailableResponse()), err
|
||||
}
|
||||
|
||||
// Nobody matched. This is the only way to reach "Invalid Email" now.
|
||||
if uid == 0 {
|
||||
resp := fiber.Map{
|
||||
"status": false,
|
||||
@@ -212,14 +252,8 @@ func (s *userService) CreateUser(user models.User) (models.UserInfo, error) {
|
||||
func (s *userService) TenantWebLogin(user models.User) (models.TenantUserInfo, map[string]interface{}) {
|
||||
tenantFormExists := true
|
||||
|
||||
uid, dbPassword, status, roleid := 0, "", "", 0
|
||||
|
||||
// Step 1: Login by authname or contactno
|
||||
if user.Authname != "" {
|
||||
uid, dbPassword, status, roleid = s.repo.GetUserLogin("authname", user.Authname, user.Configid)
|
||||
} else if user.Contactno != "" {
|
||||
uid, dbPassword, status, roleid = s.repo.GetUserLogin("contactno", user.Contactno, user.Configid)
|
||||
} else {
|
||||
if user.Authname == "" && user.Contactno == "" {
|
||||
return models.TenantUserInfo{}, map[string]interface{}{
|
||||
"status": true,
|
||||
"code": 400,
|
||||
@@ -227,7 +261,13 @@ func (s *userService) TenantWebLogin(user models.User) (models.TenantUserInfo, m
|
||||
}
|
||||
}
|
||||
|
||||
// Step 2: Validate user
|
||||
uid, dbPassword, status, roleid, err := s.lookupLogin(user)
|
||||
if err != nil {
|
||||
return models.TenantUserInfo{}, loginUnavailableResponse()
|
||||
}
|
||||
|
||||
// Step 2: Validate user. Nobody matched — the only way to reach
|
||||
// "Invalid Email" now; a database that could not answer is a 500 above.
|
||||
if uid == 0 {
|
||||
return models.TenantUserInfo{}, map[string]interface{}{
|
||||
"status": false,
|
||||
|
||||
Reference in New Issue
Block a user