Refactor statistics and visualizations

This commit refactors stats and stats APIs to return standardized responses. Instead of munging JSON on the server and the client, the server defines a custom JSON marshaler and the client uses the JSON API to request the chart data. This greatly simplifies the HTML template and controller logic.

In the same vein, there's a custom MarshalCSV interface that all the stats providers implement, and new renderer functions for creating CSVs.

The biggest part of this refactor is the way stats are calculated. Previously stats were naively calculated with a JOIN, but that left zeros or missing data in the structure. The new stats calculation ensures no gaps and fills in zero values where data is missing.

docs/api.md

@@ -195,6 +195,17 @@ Request a verification code to be issued. Accepts [optional] symptom date and te
   the padding.
 * `uuid` is optional as request input. The server will generate a uuid on response if omitted.
   * This is a handle which allows the issuer to track status of the issued verification code.
+* `externalIssuerID` is a optional information supplied by the API caller to
+	uniquely identify the entity making this request. This is useful where
+	callers are using a single API key behind an ERP, or when callers are using
+	the verification server as an API with a different authentication system.
+	This field is optional.
+
+  * The information provided is stored exactly as-is. If the identifier is
+    uniquely identifying PII (such as an email address, employee ID, SSN, etc),
+    the caller should apply a cryptographic hash before sending that data. **The
+    system does not sanitize or encrypt these external IDs, it is the caller's
+    responsibility to do so.**
 
 **IssueCodeResponse**
 

internal/icsv/icsv.go

@@ -0,0 +1,22 @@
+// 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 icsv defines an interface for things that can export as CSV.
+package icsv
+
+// Marshaler is an interface for items that can convert to CSV.
+type Marshaler interface {
+	// MarshalCSV produces CSV.
+	MarshalCSV() ([]byte, error)
+}

internal/routes/server.go

@@ -390,10 +390,9 @@ func realmadminRoutes(r *mux.Router, c *realmadmin.Controller) {
 	r.Handle("/settings", c.HandleSettings()).Methods("GET", "POST")
 	r.Handle("/settings/enable-express", c.HandleEnableExpress()).Methods("POST")
 	r.Handle("/settings/disable-express", c.HandleDisableExpress()).Methods("POST")
-	r.Handle("/stats", c.HandleShow(realmadmin.HTML)).Methods("GET")
-	r.Handle("/stats.json", c.HandleShow(realmadmin.JSON)).Methods("GET")
-	r.Handle("/stats.csv", c.HandleShow(realmadmin.CSV)).Methods("GET")
-	r.Handle("/stats/{date}", c.HandleStats()).Methods("GET")
+	r.Handle("/stats", c.HandleShow()).Methods("GET")
+	r.Handle("/stats.csv", c.HandleShow()).Methods("GET")
+	r.Handle("/stats.json", c.HandleShow()).Methods("GET")
 	r.Handle("/events", c.HandleEvents()).Methods("GET")
 }
 

internal/routes/server_test.go

@@ -228,9 +228,6 @@ func TestRoutes_realmadminRoutes(t *testing.T) {
 		{
 			req: httptest.NewRequest("GET", "/stats.csv", nil),
 		},
-		{
-			req: httptest.NewRequest("GET", "/stats/20201112", nil),
-		},
 		{
 			req: httptest.NewRequest("GET", "/events", nil),
 		},

pkg/api/api.go

@@ -184,12 +184,18 @@ type IssueCodeRequest struct {
 	// of the issued verification code. If omitted the server will generate the UUID.
 	UUID string `json:"uuid"`
 
-	// A unique handle to the caller for statistics tracking. May be used as a
-	// finer-grained identifier to one or many issuers (where API-key is too
-	// broad to be useful). If an identifier is used that might uniquely
-	// identify a person, the caller should send a string that does not convey
-	// meaning to the server such as a UUID or one-way-hash.
-	AuditID string `json:"auditID"`
+	// ExternalIssuerID is a optional information supplied by the API caller to
+	// uniquely identify the entity making this request. This is useful where
+	// callers are using a single API key behind an ERP, or when callers are using
+	// the verification server as an API with a different authentication system.
+	// This field is optional.
+
+	// The information provided is stored exactly as-is. If the identifier is
+	// uniquely identifying PII (such as an email address, employee ID, SSN, etc),
+	// the caller should apply a cryptographic hash before sending that data. The
+	// system does not sanitize or encrypt these external IDs, it is the caller's
+	// responsibility to do so.
+	ExternalIssuerID string `json:"externalIssuerID"`
 }
 
 // IssueCodeResponse defines the response type for IssueCodeRequest.

pkg/clients/apis.go

@@ -26,14 +26,8 @@ import (
 
 // IssueCode uses the ADMIN API to issue a verification code.
 // Currently does not accept the SMS param.
-func IssueCode(ctx context.Context, hostname string, apiKey, testType, symptomDate, auditID string, tzMinOffset int, timeout time.Duration) (*api.IssueCodeRequest, *api.IssueCodeResponse, error) {
+func IssueCode(ctx context.Context, hostname, apiKey string, request *api.IssueCodeRequest) (*api.IssueCodeResponse, error) {
 	url := hostname + "/api/issue"
-	request := api.IssueCodeRequest{
-		TestType:    testType,
-		SymptomDate: symptomDate,
-		TZOffset:    float32(tzMinOffset),
-		AuditID:     auditID,
-	}
 	client := &http.Client{
 		Timeout: timeout,
 	}
@@ -44,9 +38,9 @@ func IssueCode(ctx context.Context, hostname string, apiKey, testType, symptomDa
 	headers.Add("X-API-Key", apiKey)
 
 	if err := jsonclient.MakeRequest(ctx, client, url, headers, request, &response); err != nil {
-		return &request, nil, err
+		return nil, err
 	}
-	return &request, &response, nil
+	return &response, nil
 }
 
 // CheckCodeStatus uses the ADMIN API to retrieve the status of an OTP code.

pkg/clients/e2e.go

@@ -90,8 +90,16 @@ func RunEndToEnd(ctx context.Context, config *config.E2ETestConfig) error {
 		}
 		code, err := func() (*api.IssueCodeResponse, error) {
 			defer recordLatency(ctx, time.Now(), "/api/issue")
+
 			// Issue the verification code.
-			codeRequest, code, err := IssueCode(ctx, config.VerificationAdminAPIServer, config.VerificationAdminAPIKey, testType, symptomDate, adminID, 0, timeout)
+			codeRequest := &api.IssueCodeRequest{
+				TestType:         testType,
+				SymptomDate:      symptomDate,
+				TZOffset:         0,
+				ExternalIssuerID: adminID,
+			}
+
+			code, err := IssueCode(ctx, config.VerificationAdminAPIServer, config.VerificationAdminAPIKey, codeRequest)
 			if err != nil {
 				result = observability.ResultNotOK()
 				return nil, fmt.Errorf("error issuing verification code: %w", err)

pkg/controller/issueapi/issue.go

@@ -304,11 +304,12 @@ func (c *Controller) HandleIssue() http.Handler {
 			SymptomDate:    parsedDates[0],
 			TestDate:       parsedDates[1],
 			MaxSymptomAge:  c.config.GetAllowedSymptomAge(),
-			IssuingUser:    user,
-			IssuingApp:     authApp,
 			RealmID:        realm.ID,
 			UUID:           rUUID,
-			AuditID:        request.AuditID,
+
+			IssuingUser:       user,
+			IssuingApp:        authApp,
+			IssuingExternalID: request.ExternalIssuerID,
 		}
 
 		code, longCode, uuid, err := codeRequest.Issue(ctx, c.config.GetCollisionRetryCount())

pkg/controller/middleware/i18n.go

@@ -37,15 +37,13 @@ func ProcessLocale(locales *i18n.LocaleMap) mux.MiddlewareFunc {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			ctx := r.Context()
 
-			// TODO(sethvargo): extract from session/cookie as well
 			param := r.URL.Query().Get(QueryKeyLanguage)
-			cookie := ""
 			header := r.Header.Get(HeaderAcceptLanguage)
 
 			// Find the "best" language from the given parameters. They are in
 			// priority order.
 			m := controller.TemplateMapFromContext(ctx)
-			m["locale"] = locales.Lookup(param, cookie, header)
+			m["locale"] = locales.Lookup(param, header)
 
 			next.ServeHTTP(w, r)
 		})

pkg/controller/modeler/modeler_test.go

@@ -89,7 +89,7 @@ func TestRebuildModel(t *testing.T) {
 		line := []uint{50, 50, 50, 50, 50, 50, 50, 50, 50, 50, 50, 50, 50, 50, 50, 50, 50, 50, 50}
 		for _, y := range line {
 			if err := db.RawDB().
-				Create(&database.RealmStats{
+				Create(&database.RealmStat{
 					Date:        nextDate(),
 					RealmID:     realm.ID,
 					CodesIssued: y,
@@ -124,7 +124,7 @@ func TestRebuildModel(t *testing.T) {
 		line := []uint{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20}
 		for _, y := range line {
 			if err := db.RawDB().
-				Create(&database.RealmStats{
+				Create(&database.RealmStat{
 					Date:        nextDate(),
 					RealmID:     realm.ID,
 					CodesIssued: y,
@@ -159,7 +159,7 @@ func TestRebuildModel(t *testing.T) {
 		line := []uint{1, 26, 61, 13, 19, 50, 9, 20, 91, 187, 39, 4, 2, 5, 1}
 		for _, y := range line {
 			if err := db.RawDB().
-				Create(&database.RealmStats{
+				Create(&database.RealmStat{
 					Date:        nextDate(),
 					RealmID:     realm.ID,
 					CodesIssued: y,
@@ -194,7 +194,7 @@ func TestRebuildModel(t *testing.T) {
 		line := []uint{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
 		for _, y := range line {
 			if err := db.RawDB().
-				Create(&database.RealmStats{
+				Create(&database.RealmStat{
 					Date:        nextDate(),
 					RealmID:     realm.ID,
 					CodesIssued: y,
@@ -233,7 +233,7 @@ func TestRebuildModel(t *testing.T) {
 
 		for _, y := range line {
 			if err := db.RawDB().
-				Create(&database.RealmStats{
+				Create(&database.RealmStat{
 					Date:        nextDate(),
 					RealmID:     realm.ID,
 					CodesIssued: y,