fix: customer PIN-reset takeover, booking-quote and consignment-log IDORs

- POST /customer/reset-pin was unauthenticated and overwrote a customer's PIN
  given only their phone number — which is the login identifier, not a secret —
  so reset-pin followed by verify-pin took over any customer account. Exactly
  the miler flaw fixed in fd7cf3e, on the B2C side. It now requires the account's
  registered email to have been verified through the existing
  send-email-otp/verify-email-otp flow; the verification is recorded in Redis
  for 10 minutes and consumed on use, so one verification authorises one reset.
  Accounts with no email on file are directed to support rather than left open.

- GET /customer/bookings/:id/price had no ownership check, unlike every other
  customer booking route, so any signed-in customer could read the price quoted
  on anyone else's booking by walking the id.

- GET /miler/consignments/userlogs/:userid took the rider from the URL and never
  compared it to the caller, letting any miler read another miler's movement
  history.

Verified as already correct while sweeping: miler assignment and booking-flow
handlers all scope by mileruserid/assignedmileruserid, customer booking detail
and cancel scope by appcustomerid, /internal sits behind InternalKeyAuth, and
CreateHubStaffAccount already refuses non-Doormile staff.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Suriya
2026-08-05 18:20:47 +05:30
parent fd7cf3e35e
commit c8a9b5d797
3 changed files with 54 additions and 0 deletions

View File

@@ -189,6 +189,19 @@ func ResetCustomerPin(c *fiber.Ctx) error {
return utils.NotFound(c, "customer not found") return utils.NotFound(c, "customer not found")
} }
// Proof of identity is required before overwriting a login credential.
// Without it this endpoint reset any customer's PIN from their phone number
// alone — and phone numbers are the login identifier, not a secret — so
// reset-pin followed by verify-pin was a complete account takeover.
// The caller must first pass /customer/send-email-otp and
// /customer/verify-email-otp for this account's registered address.
if customer.Email == "" {
return utils.Forbidden(c, "this account has no registered email to verify against — contact support to reset the PIN")
}
if !ConsumeEmailVerification(customer.Email) {
return utils.Forbidden(c, "verify your registered email first via /customer/send-email-otp and /customer/verify-email-otp")
}
pinHash, err := utils.HashPassword(req.NewPin) pinHash, err := utils.HashPassword(req.NewPin)
if err != nil { if err != nil {
return utils.Internal(c, "failed to process PIN reset") return utils.Internal(c, "failed to process PIN reset")
@@ -620,11 +633,22 @@ func CancelCustomerBooking(c *fiber.Ctx) error {
} }
func GetCustomerBookingQuote(c *fiber.Ctx) error { func GetCustomerBookingQuote(c *fiber.Ctx) error {
customerID := c.Locals("userid").(int)
bookingID, err := strconv.Atoi(c.Params("bookingid")) bookingID, err := strconv.Atoi(c.Params("bookingid"))
if err != nil { if err != nil {
return utils.BadRequest(c, "invalid booking ID") return utils.BadRequest(c, "invalid booking ID")
} }
// Ownership is checked here as it is on the other booking routes — without
// it any signed-in customer could read the price quoted on anyone else's
// booking just by walking the id.
var booking models.PickupBooking
if err := db.DB.Select("bookingid").
Where("bookingid = ? AND appcustomerid = ?", bookingID, customerID).
First(&booking).Error; err != nil {
return utils.NotFound(c, "booking not found")
}
var serviceOpt models.BookingServiceOption var serviceOpt models.BookingServiceOption
if err := db.DB.Where("bookingid = ?", bookingID).Order("createdat DESC").First(&serviceOpt).Error; err != nil { if err := db.DB.Where("bookingid = ?", bookingID).Order("createdat DESC").First(&serviceOpt).Error; err != nil {
return utils.NotFound(c, "price quote not found for this booking") return utils.NotFound(c, "price quote not found for this booking")

View File

@@ -1204,6 +1204,13 @@ func GetUserConsignmentLogs(c *fiber.Ctx) error {
return utils.BadRequest(c, "invalid user ID") return utils.BadRequest(c, "invalid user ID")
} }
// The path names a rider, so it has to be checked against the caller —
// otherwise any miler could read another miler's movement history simply by
// changing the number in the URL.
if userID != c.Locals("userid").(int) {
return utils.Forbidden(c, "you can only read your own consignment logs")
}
if db.Rdb == nil { if db.Rdb == nil {
return utils.Internal(c, "cache service unavailable") return utils.Internal(c, "cache service unavailable")
} }

View File

@@ -20,6 +20,11 @@ import (
const ( const (
otpTTL = 5 * time.Minute otpTTL = 5 * time.Minute
otpMaxAttempts = 5 otpMaxAttempts = 5
// otpVerifiedTTL is how long a successful email verification stays usable as
// proof of identity for a follow-up action such as a PIN reset. Long enough
// to type a new PIN, short enough that a stale verification can't be
// redeemed later.
otpVerifiedTTL = 10 * time.Minute
) )
func generateOtpCode() string { func generateOtpCode() string {
@@ -33,6 +38,22 @@ func generateOtpCode() string {
func otpKey(email string) string { return fmt.Sprintf("otp:email:%s", email) } func otpKey(email string) string { return fmt.Sprintf("otp:email:%s", email) }
func otpAttemptsKey(email string) string { return fmt.Sprintf("otp:email:%s:attempts", email) } func otpAttemptsKey(email string) string { return fmt.Sprintf("otp:email:%s:attempts", email) }
// otpVerifiedKey marks an email as recently proven. Verification previously
// left no trace at all, so nothing downstream could require it — which is why
// ResetCustomerPin was able to overwrite a PIN on nothing but a phone number.
func otpVerifiedKey(email string) string { return fmt.Sprintf("otp:email:%s:verified", email) }
// ConsumeEmailVerification reports whether the email was verified recently, and
// clears the marker so a single verification can authorise exactly one action.
func ConsumeEmailVerification(email string) bool {
if db.Rdb == nil || email == "" {
return false
}
ctx := context.Background()
n, err := db.Rdb.Del(ctx, otpVerifiedKey(email)).Result()
return err == nil && n > 0
}
func SendCustomerEmailOtp(cfg *config.Config) fiber.Handler { func SendCustomerEmailOtp(cfg *config.Config) fiber.Handler {
return func(c *fiber.Ctx) error { return func(c *fiber.Ctx) error {
req := new(dto.SendEmailOtpRequest) req := new(dto.SendEmailOtpRequest)
@@ -100,6 +121,8 @@ func VerifyCustomerEmailOtp() fiber.Handler {
} }
db.Rdb.Del(ctx, key, otpAttemptsKey(req.Email)) db.Rdb.Del(ctx, key, otpAttemptsKey(req.Email))
// Recorded so a follow-up PIN reset can prove this email was verified.
db.Rdb.Set(ctx, otpVerifiedKey(req.Email), "1", otpVerifiedTTL)
return utils.Message(c, "email verified successfully") return utils.Message(c, "email verified successfully")
} }
} }