diff --git a/repositories/userRepository.go b/repositories/userRepository.go index 764bcee..60a4cf7 100644 --- a/repositories/userRepository.go +++ b/repositories/userRepository.go @@ -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 } - - diff --git a/scratch/posseparation/main.go b/scratch/posseparation/main.go index 6495f39..183d9ea 100644 --- a/scratch/posseparation/main.go +++ b/scratch/posseparation/main.go @@ -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", diff --git a/services/userLogin_test.go b/services/userLogin_test.go new file mode 100644 index 0000000..83cecad --- /dev/null +++ b/services/userLogin_test.go @@ -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) + } +} diff --git a/services/userService.go b/services/userService.go index dcd3a40..6a6922c 100644 --- a/services/userService.go +++ b/services/userService.go @@ -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,