Merge branch 'issue_63' of https://github.com/jeremyfaller/exposure-n…

…otifications-verification-server into issue_63

pkg/api/api.go

@@ -102,8 +102,10 @@ type IssueCodeRequest struct {
 	SymptomDate string `json:"symptomDate"` // ISO 8601 formatted date, YYYY-MM-DD
 	TestDate    string `json:"testDate"`
 	TestType    string `json:"testType"`
-	TZOffset    int    `json:"tzOffset"` // offset in minutes of the user's timezone.
-	Phone       string `json:"phone"`
+	// Offset in minutes of the user's timezone. Positive, negative, 0, or ommitted
+	// (using the default of 0) are all valid. 0 is considered to be UTC.
+	TZOffset int    `json:"tzOffset"`
+	Phone    string `json:"phone"`
 }
 
 // IssueCodeResponse defines the response type for IssueCodeRequest.

pkg/controller/issueapi/issue.go

@@ -15,6 +15,7 @@
 package issueapi
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -27,6 +28,49 @@ import (
 	"github.com/google/exposure-notifications-verification-server/pkg/sms"
 )
 
+// Cache the UTC time.Location, to speed runtime.
+var utc *time.Location
+
+func init() {
+	var err error
+	utc, err = time.LoadLocation("UTC")
+	if err != nil {
+		panic("should have found UTC")
+	}
+}
+
+// validateDate validates the date given -- returning the time or an error.
+func validateDate(date, minDate, maxDate time.Time, tzOffset int) (*time.Time, error) {
+	// Check that all our dates are utc.
+	if date.Location() != utc || minDate.Location() != utc || maxDate.Location() != utc {
+		return nil, errors.New("dates weren't in UTC")
+	}
+
+	// If we're dealing with a timezone where the offset is earlier than this one,
+	// we loosen up the lower bound. We might have the following circumstance:
+	//
+	//    Server time: UTC Aug 1, 12:01 AM
+	//    Client time: UTC July 30, 11:01 PM (ie, tzOffset = -30)
+	//
+	// In this circumstance, we'll have the following:
+	//
+	//    minTime: UTC July 31, maxTime: Aug 1, clientTime: July 30.
+	//
+	// which would be an error. Loosening up the lower bound, by a day, keeps us
+	// all ok.
+	if tzOffset < 0 {
+		if m := minDate.Add(-24 * time.Hour); m.After(date) {
+			return nil, fmt.Errorf("date %v before min %v", date, m)
+		}
+	} else if minDate.After(date) {
+		return nil, fmt.Errorf("date %v before min %v", date, minDate)
+	}
+	if date.After(maxDate) {
+		return nil, fmt.Errorf("date %v after max %v", date, maxDate)
+	}
+	return &date, nil
+}
+
 func (c *Controller) HandleIssue() http.Handler {
 	logger := c.logger.Named("issueapi.HandleIssue")
 
@@ -96,26 +140,26 @@ func (c *Controller) HandleIssue() http.Handler {
 			return
 		}
 
-		// Max date is today (UTC time) and min date is AllowedTestAge ago, truncated.
-		maxDate := time.Now().UTC()
-		minDate := maxDate.Add(-1 * c.config.GetAllowedSymptomAge()).Truncate(24 * time.Hour)
-
 		var symptomDate *time.Time
 		if request.SymptomDate != "" {
 			if parsed, err := time.Parse("2006-01-02", request.SymptomDate); err != nil {
 				c.h.RenderJSON(w, http.StatusBadRequest, api.Errorf("failed to process symptom onset date: %v", err))
 				return
 			} else {
-				parsed = parsed.Local()
-				parsed = parsed.Add(time.Duration(request.TZOffset) * time.Minute) // adjust to UTC
-				if minDate.After(parsed) || parsed.After(maxDate) {
-					err := fmt.Errorf("symptom onset date must be on/after %v and on/before %v",
+				// Max date is today (UTC time) and min date is AllowedTestAge ago, truncated.
+				maxDate := time.Now().UTC().Truncate(24 * time.Hour)
+				minDate := maxDate.Add(-1 * c.config.GetAllowedSymptomAge()).Truncate(24 * time.Hour)
+
+				symptomDate, err = validateDate(parsed, minDate, maxDate, request.TZOffset)
+				if err != nil {
+					err := fmt.Errorf("symptom onset date must be on/after %v and on/before %v %v",
 						minDate.Format("2006-01-02"),
-						maxDate.Format("2006-01-02"))
+						maxDate.Format("2006-01-02"),
+						parsed.Format("2006-01-02"),
+					)
 					c.h.RenderJSON(w, http.StatusBadRequest, api.Error(err))
 					return
 				}
-				symptomDate = &parsed
 			}
 		}
 

pkg/controller/issueapi/issue_test.go

@@ -0,0 +1,68 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package issueapi
+
+import (
+	"testing"
+	"time"
+)
+
+func TestDateValidationn(t *testing.T) {
+	utc, err := time.LoadLocation("UTC")
+	if err != nil {
+		t.Fatalf("error loading utc")
+	}
+	var aug1 time.Time
+	aug1, err = time.ParseInLocation("2006-01-02", "2020-08-01", utc)
+	if err != nil {
+		t.Fatalf("error parsing date")
+	}
+
+	tests := []struct {
+		v         string
+		max       time.Time
+		tzOffset  int
+		shouldErr bool
+		expected  string
+	}{
+		{"2020-08-01", aug1, 0, false, "2020-08-01"},
+		{"2020-08-01", aug1, 60, false, "2020-08-01"},
+		{"2020-08-01", aug1, 60 * 12, false, "2020-08-01"},
+		{"2020-07-31", aug1, 60, false, "2020-07-31"},
+		{"2020-08-01", aug1, -60, false, "2020-08-01"},
+		{"2020-07-31", aug1, -60, false, "2020-07-31"},
+		{"2020-07-30", aug1, -60, false, "2020-07-30"},
+		{"2020-07-29", aug1, -60, true, "2020-07-30"},
+	}
+	for i, test := range tests {
+		date, err := time.ParseInLocation("2006-01-02", test.v, utc)
+		if err != nil {
+			t.Fatalf("[%d] error parsing date %q", i, test.v)
+		}
+		min := test.max.Add(-24 * time.Hour)
+		var newDate *time.Time
+		if newDate, err = validateDate(date, min, test.max, test.tzOffset); newDate == nil {
+			if err != nil {
+				if !test.shouldErr {
+					t.Fatalf("[%d] validateDate returned an unexpected error: %q", i, err)
+				}
+			} else {
+				t.Fatalf("[%d] expected error", i)
+			}
+		} else if s := newDate.Format("2006-01-02"); s != test.expected {
+			t.Fatalf("[%d] validateDate returned a different date %q != %q", i, s, test.expected)
+		}
+	}
+}