diff --git a/controllers/customerController.go b/controllers/customerController.go index 0ba874e..7dbadf6 100644 --- a/controllers/customerController.go +++ b/controllers/customerController.go @@ -573,6 +573,52 @@ func CreateCustomerBooking(c *fiber.Ctx) error { return utils.Created(c, booking) } +// fillConsignmentFacts attaches the consignment's tracking number and live +// status to each booking that has one. +// +// A booking freezes at Converted_To_Consignment the moment it is collected, +// while the parcel keeps moving on the consignment — so without this a customer +// sees a status that stopped updating the instant their parcel was picked up. +// +// The tracking number matters more: GET /customer/track/:trackingno is keyed on +// it, and nothing else the customer app can read exposes it. Tracking was +// unreachable from the app not because the endpoint was missing but because the +// number never travelled to it. +// +// Batched — one query for the whole page, not one per row. +func fillConsignmentFacts(bookings []models.PickupBooking) { + ids := make([]int, 0, len(bookings)) + for _, b := range bookings { + if b.Consignmentid != nil { + ids = append(ids, *b.Consignmentid) + } + } + if len(ids) == 0 { + return + } + + var consignments []models.Consignment + if err := db.DB.Select("consignmentid, trackingno, status"). + Where("consignmentid IN ?", ids).Find(&consignments).Error; err != nil { + utils.Warn("fillConsignmentFacts: could not load consignments", "error", err) + return + } + + byID := make(map[int]models.Consignment, len(consignments)) + for _, cn := range consignments { + byID[cn.Consignmentid] = cn + } + for i := range bookings { + if bookings[i].Consignmentid == nil { + continue + } + if cn, ok := byID[*bookings[i].Consignmentid]; ok { + bookings[i].Trackingno = cn.Trackingno + bookings[i].Consignmentstatus = cn.Status + } + } +} + func GetCustomerBookings(c *fiber.Ctx) error { customerID := c.Locals("userid").(int) @@ -581,6 +627,8 @@ func GetCustomerBookings(c *fiber.Ctx) error { return utils.Internal(c, "failed to fetch bookings") } + fillConsignmentFacts(bookings) + return utils.List(c, bookings, int64(len(bookings))) } @@ -596,6 +644,10 @@ func GetCustomerBookingDetails(c *fiber.Ctx) error { return utils.NotFound(c, "booking not found") } + one := []models.PickupBooking{booking} + fillConsignmentFacts(one) + booking = one[0] + return utils.OK(c, booking) } diff --git a/cors_test.go b/cors_test.go new file mode 100644 index 0000000..73efb19 --- /dev/null +++ b/cors_test.go @@ -0,0 +1,80 @@ +package main + +import "testing" + +// Which Origin headers count as "this machine". +// +// This exists because Flutter Web's dev server binds a random high port on every +// launch, so no fixed allowlist can name it — the app hit +// PreflightMissingAllowOriginHeader from http://localhost:65256 and would have +// hit it again from a different port tomorrow. +// +// The reason it is worth a test rather than a one-line helper: the obvious +// implementation is a prefix or substring match on "localhost", and that quietly +// admits http://localhost.attacker.com — a completely different machine that +// merely starts with the right word. Combined with AllowCredentials, that would +// let an attacker-controlled page make authenticated calls as the signed-in user. +// A parsed-host comparison is the only version that is actually safe, and this +// pins it. + +func TestLoopbackOriginsAreAllowed(t *testing.T) { + for _, origin := range []string{ + "http://localhost:65256", // the port Flutter Web picked; it changes every run + "http://localhost:5173", + "http://localhost", + "https://localhost:8443", + "http://127.0.0.1:3000", + "http://127.0.0.1", + "http://[::1]:8080", + } { + if !isLoopbackOrigin(origin) { + t.Errorf("%q is this machine and should be allowed in development", origin) + } + } +} + +func TestLookalikeOriginsAreRefused(t *testing.T) { + // Every one of these contains "localhost" or "127.0.0.1" as a substring and + // is a different host. A prefix or Contains check would admit all of them. + for _, origin := range []string{ + "http://localhost.attacker.com", + "https://localhost.evil.io:443", + "http://notlocalhost", + "http://mylocalhost:3000", + "http://127.0.0.1.attacker.com", + "http://evil.com/?x=http://localhost:3000", + "http://evil.com#localhost", + } { + if isLoopbackOrigin(origin) { + t.Errorf("%q is NOT this machine and must be refused", origin) + } + } +} + +func TestNonHTTPSchemesAreRefused(t *testing.T) { + // An Origin is a scheme/host/port triple. Anything else is either a browser + // that will not send it or something forged, and neither should be trusted. + for _, origin := range []string{ + "file://localhost", + "ftp://localhost:21", + "javascript:alert(1)", + "chrome-extension://abcdefghijklmnop", + } { + if isLoopbackOrigin(origin) { + t.Errorf("%q is not an http(s) origin and must be refused", origin) + } + } +} + +func TestMalformedOriginsAreRefused(t *testing.T) { + for _, origin := range []string{ + "", + "null", // what a sandboxed iframe sends + "not a url at all", + "://missing-scheme", + } { + if isLoopbackOrigin(origin) { + t.Errorf("%q is not a usable origin and must be refused", origin) + } + } +} diff --git a/main.go b/main.go index bd43cf8..6566c26 100644 --- a/main.go +++ b/main.go @@ -2,6 +2,7 @@ package main import ( "errors" + "net/url" "os" "os/signal" "strings" @@ -31,6 +32,30 @@ import ( // turned into an error by the recover middleware — into the same // {success, message} envelope the utils helpers emit, so clients never receive // Fiber's default plain-text error body. +// isLoopbackOrigin reports whether an Origin header names this machine, on any +// port. It exists for Flutter Web, whose dev server picks a fresh random port +// on every launch — no fixed allowlist can name it in advance. +// +// Deliberately strict about what counts as loopback: the host must be exactly +// localhost, 127.0.0.1 or [::1]. A prefix match would admit +// http://localhost.attacker.com, which is a different machine entirely and is +// precisely the mistake this kind of check usually makes. +func isLoopbackOrigin(origin string) bool { + u, err := url.Parse(origin) + if err != nil { + return false + } + if u.Scheme != "http" && u.Scheme != "https" { + return false + } + switch u.Hostname() { + case "localhost", "127.0.0.1", "::1": + return true + default: + return false + } +} + func errorHandler(c *fiber.Ctx, err error) error { code := fiber.StatusInternalServerError msg := "internal server error" @@ -107,10 +132,37 @@ func main() { // nil-pointer dereference in any handler takes the whole process down. app.Use(recover.New(recover.Config{EnableStackTrace: true})) - // CORS policy + // CORS policy. + // + // The named list is production and the well-known dev-server ports. It cannot + // cover local development on its own: `flutter run -d chrome` binds a RANDOM + // high port on every launch (65256 one run, something else the next), so a + // fixed allowlist misses it every time and the browser rejects the request + // with PreflightMissingAllowOriginHeader before the handler is ever reached. + // + // AllowOriginsFunc is consulted only when the static list has already missed, + // so it widens nothing in production — it just admits loopback origins on any + // port while developing. It is NOT enabled when ENV=production: a live API + // that accepts credentialed requests from any localhost page is a real, if + // modest, hole — a developer visiting a hostile page served from their own + // machine would have that page able to call this API as them. + // + // A wildcard is not an option regardless: AllowCredentials with + // AllowOrigins "*" is rejected by the CORS spec, and Fiber panics on it. + allowLoopbackOrigins := !strings.EqualFold(cfg.Env, "production") + if allowLoopbackOrigins { + utils.Info("CORS: loopback origins on any port are allowed (non-production)", "env", cfg.Env) + } + app.Use(cors.New(cors.Config{ - AllowHeaders: "Origin,Content-Type,Accept,Authorization", - AllowOrigins: "http://localhost:5173,http://localhost:5174,http://localhost:3000,http://localhost:3001,http://localhost:3002,http://localhost:8080,http://localhost:8081,https://doormile.com,https://www.doormile.com,https://admin.doormile.com,https://api.doormile.com,https://crm.doormile.com,https://console.doormile.com,https://app.doormile.com,https://hub.doormile.com", + // Idempotency-Key is sent by the rider app on pickup-complete, payment + // and the base handover. A browser client that could not send it would + // lose retry safety on exactly the calls that most need it. + AllowHeaders: "Origin,Content-Type,Accept,Authorization,Idempotency-Key", + AllowOrigins: "http://localhost:5173,http://localhost:5174,http://localhost:3000,http://localhost:3001,http://localhost:3002,http://localhost:8080,http://localhost:8081,https://doormile.com,https://www.doormile.com,https://admin.doormile.com,https://api.doormile.com,https://crm.doormile.com,https://console.doormile.com,https://app.doormile.com,https://hub.doormile.com", + AllowOriginsFunc: func(origin string) bool { + return allowLoopbackOrigins && isLoopbackOrigin(origin) + }, AllowCredentials: true, AllowMethods: "GET,POST,PUT,DELETE,PATCH,OPTIONS", })) diff --git a/models/booking.go b/models/booking.go index c1e24a8..d1d4205 100644 --- a/models/booking.go +++ b/models/booking.go @@ -73,9 +73,15 @@ type PickupBooking struct { // list, so a "Converted_To_Consignment" booking can still show // Out_for_Delivery / Delivered). omitempty keeps it out of every other // PickupBooking response that does not populate it. - Consignmentstatus string `json:"consignmentstatus,omitempty" gorm:"-"` - Createdat time.Time `json:"createdat" gorm:"column:createdat;default:CURRENT_TIMESTAMP"` - Updatedat time.Time `json:"updatedat" gorm:"column:updatedat;default:CURRENT_TIMESTAMP"` + Consignmentstatus string `json:"consignmentstatus,omitempty" gorm:"-"` + // Trackingno is not a column either — it lives on the consignment, which only + // exists once the parcel is collected. It is filled in by handlers that serve + // a customer, because without it the customer app has no way to reach + // GET /customer/track/:trackingno at all: a booking is addressed by id, a + // shipment by tracking number, and nothing joined the two. + Trackingno string `json:"trackingno,omitempty" gorm:"-"` + Createdat time.Time `json:"createdat" gorm:"column:createdat;default:CURRENT_TIMESTAMP"` + Updatedat time.Time `json:"updatedat" gorm:"column:updatedat;default:CURRENT_TIMESTAMP"` // Relations Parcels []BookingParcel `json:"parcels" gorm:"foreignKey:Bookingid"`