diff --git a/docs/api.md b/docs/api.md index dc95f2b93..5e5a0b610 100644 --- a/docs/api.md +++ b/docs/api.md @@ -7,6 +7,11 @@ - [API Methods](#api-methods) - [`/api/verify`](#apiverify) - [`/api/certificate`](#apicertificate) +- [Admin APIs](#admin-apis) + - [`/api/issue`](#apiissue) + - [Client provided UUID to prevent duplicate SMS](#client-provided-uuid-to-prevent-duplicate-sms) + - [`/api/batch-issue`](#apibatch-issue) + - [Handling batch partial success/failure](#handling-batch-partial-successfailure) - [`/api/checkcodestatus`](#apicheckcodestatus) - [`/api/expirecode`](#apiexpirecode) - [`/api/stats/*` (preview)](#apistats-preview) @@ -275,6 +280,7 @@ Possible error code responses. New error codes may be added in future releases. | `unparsable_request` | 400 | No | Client sent an request the sever cannot parse | | `invalid_test_type` | 400 | No | The client sent an accept of an unrecognized test type | | `missing_date` | 400 | No | The realm requires either a test or symptom date, but none was provided. | +| `invalid_date` | 400 | No | The provided test or symptom date, was older or newer than the realm allows. | | `invalid_test_type` | 400 | No | The test type is not a valid test type (a string that is unknown to the server). | | `uuid_already_exists` | 409 | No | The UUID has already been used for an issued code | | `maintenance_mode ` | 429 | Yes | The server is temporarily down for maintenance. Wait and retry later. | diff --git a/pkg/api/api.go b/pkg/api/api.go index 8d85ecf9d..e30de3ac4 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -41,7 +41,7 @@ const ( // this could mean a database or RPC connection drop or some other internal outage. ErrInternal = "internal_server_error" - // Verify API responses + // Verify & Issue API responses // ErrVerifyCodeInvalid indicates the code entered is unknown or already used. ErrVerifyCodeInvalid = "code_invalid" @@ -60,6 +60,9 @@ const ( ErrInvalidTestType = "invalid_test_type" // ErrMissingDate indicates the realm requires a date, but none was supplied. ErrMissingDate = "missing_date" + // ErrInvalidDate indicates the realm requires a date, but the supplied date + // was older or newer than the allowed date ramge. + ErrInvalidDate = "invalid_date" // ErrUUIDAlreadyExists indicates that the UUID has already been used for an issued code. ErrUUIDAlreadyExists = "uuid_already_exists" // ErrMaintenanceMode indicates that the server is read-only for maintenance. diff --git a/pkg/controller/issueapi/issue.go b/pkg/controller/issueapi/issue.go index 1c740b8eb..1f3be69bb 100644 --- a/pkg/controller/issueapi/issue.go +++ b/pkg/controller/issueapi/issue.go @@ -52,7 +52,7 @@ func (c *Controller) HandleIssue() http.Handler { if err := controller.BindJSON(w, r, &request); err != nil { result.obsBlame = observability.BlameClient result.obsResult = observability.ResultError("FAILED_TO_PARSE_JSON_REQUEST") - c.h.RenderJSON(w, http.StatusBadRequest, api.Error(err)) + c.h.RenderJSON(w, http.StatusBadRequest, api.Error(err).WithCode(api.ErrUnparsableRequest)) return } diff --git a/pkg/controller/issueapi/issue_batch.go b/pkg/controller/issueapi/issue_batch.go index fbf30d82d..3776e7183 100644 --- a/pkg/controller/issueapi/issue_batch.go +++ b/pkg/controller/issueapi/issue_batch.go @@ -52,7 +52,7 @@ func (c *Controller) HandleBatchIssue() http.Handler { if err := controller.BindJSON(w, r, &request); err != nil { result.obsBlame = observability.BlameClient result.obsResult = observability.ResultError("FAILED_TO_PARSE_JSON_REQUEST") - c.h.RenderJSON(w, http.StatusBadRequest, api.Error(err)) + c.h.RenderJSON(w, http.StatusBadRequest, api.Error(err).WithCode(api.ErrUnparsableRequest)) return } diff --git a/pkg/controller/issueapi/issue_batch_test.go b/pkg/controller/issueapi/issue_batch_test.go index abb1a841a..ec3785b36 100644 --- a/pkg/controller/issueapi/issue_batch_test.go +++ b/pkg/controller/issueapi/issue_batch_test.go @@ -52,19 +52,6 @@ func TestIssueBatch(t *testing.T) { t.Fatal(err) } - existingCode := &database.VerificationCode{ - RealmID: realm.ID, - Code: "00000001", - LongCode: "00000001ABC", - Claimed: true, - TestType: "confirmed", - ExpiresAt: time.Now().Add(time.Hour), - LongExpiresAt: time.Now().Add(time.Hour), - } - if err := db.SaveVerificationCode(existingCode, time.Hour); err != nil { - t.Fatal(err) - } - symptomDate := time.Now().UTC().Add(-48 * time.Hour).Format(project.RFC3339Date) tzMinOffset := 0 @@ -131,9 +118,8 @@ func TestIssueBatch(t *testing.T) { }, { TestType: "confirmed", - SymptomDate: symptomDate, + SymptomDate: "unparsable date", TZOffset: float32(tzMinOffset), - UUID: existingCode.UUID, }, }, }, @@ -146,7 +132,7 @@ func TestIssueBatch(t *testing.T) { ErrorCode: api.ErrInvalidTestType, }, { - ErrorCode: api.ErrUUIDAlreadyExists, + ErrorCode: api.ErrUnparsableRequest, }, }, ErrorCode: api.ErrInvalidTestType, @@ -196,11 +182,7 @@ func TestIssueBatch(t *testing.T) { for i, issuedCode := range resp.Codes { if issuedCode.ErrorCode != tc.response.Codes[i].ErrorCode { - t.Errorf("did not receive expected inner errorCode. got %s, want %v", issuedCode.ErrorCode, tc.response.Codes[i].ErrorCode) - } - - if tc.request.Codes[i].UUID != "" && tc.response.Codes[i].UUID != issuedCode.UUID { - t.Errorf("expected stable client-issued UUID. got %s, want %v", issuedCode.UUID, tc.response.Codes[i].UUID) + t.Errorf("did not receive expected inner errorCode. got %q, want %q", issuedCode.ErrorCode, tc.response.Codes[i].ErrorCode) } } }) diff --git a/pkg/controller/issueapi/issue_test.go b/pkg/controller/issueapi/issue_test.go index 5e640c86f..e9f7decbb 100644 --- a/pkg/controller/issueapi/issue_test.go +++ b/pkg/controller/issueapi/issue_test.go @@ -12,59 +12,147 @@ // See the License for the specific language governing permissions and // limitations under the License. -package issueapi +package issueapi_test import ( + "context" + "net/http" "testing" "time" "github.com/google/exposure-notifications-verification-server/internal/project" + "github.com/google/exposure-notifications-verification-server/pkg/api" + "github.com/google/exposure-notifications-verification-server/pkg/database" + "github.com/google/exposure-notifications-verification-server/pkg/sms" + "github.com/google/exposure-notifications-verification-server/pkg/testsuite" ) -func TestDateValidation(t *testing.T) { - utc, err := time.LoadLocation("UTC") +func TestIssue(t *testing.T) { + t.Parallel() + + ctx := context.Background() + testSuite := testsuite.NewIntegrationSuite(t, ctx) + adminClient, err := testSuite.NewAdminAPIClient(ctx, t) if err != nil { - t.Fatalf("error loading utc") + t.Fatal(err) } - var aug1 time.Time - aug1, err = time.ParseInLocation(project.RFC3339Date, "2020-08-01", utc) - if err != nil { - t.Fatalf("error parsing date") + db := testSuite.DB + realm := testSuite.Realm + + realm.AllowedTestTypes = database.TestTypeConfirmed + if err := db.SaveRealm(realm, database.SystemTest); err != nil { + t.Fatal(err) + } + + smsConfig := &database.SMSConfig{ + RealmID: realm.ID, + ProviderType: sms.ProviderType(sms.ProviderTypeNoop), + } + if err := db.SaveSMSConfig(smsConfig); err != nil { + t.Fatal(err) + } + + existingCode := &database.VerificationCode{ + RealmID: realm.ID, + Code: "00000001", + LongCode: "00000001ABC", + Claimed: true, + TestType: "confirmed", + ExpiresAt: time.Now().Add(time.Hour), + LongExpiresAt: time.Now().Add(time.Hour), } + if err := db.SaveVerificationCode(existingCode, time.Hour); err != nil { + t.Fatal(err) + } + + symptomDate := time.Now().UTC().Add(-48 * time.Hour).Format(project.RFC3339Date) + tzMinOffset := 0 - tests := []struct { - v string - max time.Time - tzOffset int - shouldErr bool - expected string + cases := []struct { + name string + request api.IssueCodeRequest + responseErr string + httpStatusCode int }{ - {"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"}, + { + name: "success", + request: api.IssueCodeRequest{ + TestType: "confirmed", + SymptomDate: symptomDate, + TZOffset: float32(tzMinOffset), + }, + httpStatusCode: http.StatusOK, + }, + { + name: "failure", + request: api.IssueCodeRequest{ + TestType: "negative", // this realm only supports confirmed + SymptomDate: symptomDate, + TZOffset: float32(tzMinOffset), + }, + responseErr: api.ErrUnsupportedTestType, + httpStatusCode: http.StatusBadRequest, + }, + { + name: "no test date", + request: api.IssueCodeRequest{ + TestType: "confirmed", + TZOffset: float32(tzMinOffset), + }, + responseErr: api.ErrMissingDate, + httpStatusCode: http.StatusBadRequest, + }, + { + name: "unparsable test date", + request: api.IssueCodeRequest{ + TestType: "confirmed", + SymptomDate: "invalid date", + TZOffset: float32(tzMinOffset), + }, + responseErr: api.ErrUnparsableRequest, + httpStatusCode: http.StatusBadRequest, + }, + { + name: "really old test date", + request: api.IssueCodeRequest{ + TestType: "confirmed", + SymptomDate: "1988-09-14", + TZOffset: float32(tzMinOffset), + }, + responseErr: api.ErrInvalidDate, + httpStatusCode: http.StatusBadRequest, + }, + { + name: "conflict", + request: api.IssueCodeRequest{ + TestType: "confirmed", + SymptomDate: symptomDate, + TZOffset: float32(tzMinOffset), + UUID: existingCode.UUID, + }, + responseErr: api.ErrUUIDAlreadyExists, + httpStatusCode: http.StatusConflict, + }, } - for i, test := range tests { - date, err := time.ParseInLocation(project.RFC3339Date, 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 { + + for _, tc := range cases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + statusCode, resp, err := adminClient.IssueCode(tc.request) if err != nil { - if !test.shouldErr { - t.Fatalf("[%d] validateDate returned an unexpected error: %q", i, err) - } - } else { - t.Fatalf("[%d] expected error", i) + t.Fatal(err) + } + + // Check outer error + if statusCode != tc.httpStatusCode { + t.Errorf("incorrect error code. got %d, want %d", statusCode, tc.httpStatusCode) + } + if resp.ErrorCode != tc.responseErr { + t.Errorf("did not receive expected errorCode. got %q, want %q", resp.ErrorCode, tc.responseErr) } - } else if s := newDate.Format(project.RFC3339Date); s != test.expected { - t.Fatalf("[%d] validateDate returned a different date %q != %q", i, s, test.expected) - } + }) } } diff --git a/pkg/controller/issueapi/logic.go b/pkg/controller/issueapi/logic.go index 1c22b05fc..dc4b03053 100644 --- a/pkg/controller/issueapi/logic.go +++ b/pkg/controller/issueapi/logic.go @@ -160,7 +160,7 @@ func (c *Controller) issue(ctx context.Context, authApp *database.AuthorizedApp, obsBlame: observability.BlameClient, obsResult: observability.ResultError(dateSettings[i].ValidateError), httpCode: http.StatusBadRequest, - errorReturn: api.Error(err), + errorReturn: api.Error(err).WithCode(api.ErrInvalidDate), }, nil } parsedDates[i] = validatedDate diff --git a/pkg/controller/issueapi/parse_date_test.go b/pkg/controller/issueapi/parse_date_test.go new file mode 100644 index 000000000..5e640c86f --- /dev/null +++ b/pkg/controller/issueapi/parse_date_test.go @@ -0,0 +1,70 @@ +// 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" + + "github.com/google/exposure-notifications-verification-server/internal/project" +) + +func TestDateValidation(t *testing.T) { + utc, err := time.LoadLocation("UTC") + if err != nil { + t.Fatalf("error loading utc") + } + var aug1 time.Time + aug1, err = time.ParseInLocation(project.RFC3339Date, "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(project.RFC3339Date, 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(project.RFC3339Date); s != test.expected { + t.Fatalf("[%d] validateDate returned a different date %q != %q", i, s, test.expected) + } + } +} diff --git a/pkg/integration/integration_test.go b/pkg/integration/integration_test.go index fc542f73e..06f856c99 100644 --- a/pkg/integration/integration_test.go +++ b/pkg/integration/integration_test.go @@ -134,15 +134,17 @@ func (tc testCase) singleIssue(t *testing.T, adminClient *testsuite.AdminClient, TZOffset: float32(tzMinOffset), } - issueResp, err := adminClient.IssueCode(issueRequest) - if issueResp == nil || err != nil || issueResp.UUID == "" { + httpCode, issueResp, err := adminClient.IssueCode(issueRequest) + if issueResp == nil || err != nil || issueResp.UUID == "" || httpCode != http.StatusOK { t.Fatalf("adminClient.IssueCode(%+v) = expected nil, got resp %+v, err %v", issueRequest, issueResp, err) } // Try to issue the same code again (same UUID) issueRequest.UUID = issueResp.UUID - if _, err = adminClient.IssueCode(issueRequest); err == nil { - t.Fatalf("Expected conflict, got %s", err) + if httpCode, _, err = adminClient.IssueCode(issueRequest); httpCode != http.StatusConflict { + t.Fatalf("Expected 409 conflict, got %d", httpCode) + } else if err != nil { + t.Fatal(err) } verifyRequest := api.VerifyCodeRequest{ diff --git a/pkg/testsuite/clients.go b/pkg/testsuite/clients.go index 23de69f2f..43a334748 100644 --- a/pkg/testsuite/clients.go +++ b/pkg/testsuite/clients.go @@ -57,7 +57,7 @@ func (c *AdminClient) BatchIssueCode(req api.BatchIssueCodeRequest) (int, *api.B httpResp, err := c.client.Do(httpReq) if err != nil { - return 0, nil, fmt.Errorf("failed to POST /api/batch-issue: %w", err) + return 0, nil, fmt.Errorf("failed to POST %s: %w", url, err) } defer httpResp.Body.Close() @@ -75,33 +75,36 @@ func (c *AdminClient) BatchIssueCode(req api.BatchIssueCodeRequest) (int, *api.B } // IssueCode wraps the IssueCode API call. -func (c *AdminClient) IssueCode(req api.IssueCodeRequest) (*api.IssueCodeResponse, error) { +// returns the http status code, response. +// The caller may get a non-200 code even if the response contains some successful codes. +func (c *AdminClient) IssueCode(req api.IssueCodeRequest) (int, *api.IssueCodeResponse, error) { var resp *api.IssueCodeResponse var err error + httpCode := 0 if c.retry { finalErr := Eventually(c.retryTimes, c.retryInterval, func() error { - resp, err = c.issueCode(req) + httpCode, resp, err = c.issueCode(req) return err }) if finalErr != nil { - return nil, finalErr + return httpCode, nil, finalErr } - return resp, nil + return httpCode, resp, nil } return c.issueCode(req) } -func (c *AdminClient) issueCode(req api.IssueCodeRequest) (*api.IssueCodeResponse, error) { +func (c *AdminClient) issueCode(req api.IssueCodeRequest) (int, *api.IssueCodeResponse, error) { url := c.urlBase + "/api/issue" j, err := json.Marshal(req) if err != nil { - return nil, fmt.Errorf("failed to marshal json: %w", err) + return 0, nil, fmt.Errorf("failed to marshal json: %w", err) } httpReq, err := http.NewRequest("POST", url, bytes.NewReader(j)) if err != nil { - return nil, fmt.Errorf("failed to marshal json: %w", err) + return 0, nil, fmt.Errorf("failed to marshal json: %w", err) } httpReq.Header.Add("content-type", "application/json") @@ -109,20 +112,21 @@ func (c *AdminClient) issueCode(req api.IssueCodeRequest) (*api.IssueCodeRespons httpResp, err := c.client.Do(httpReq) if err != nil { - return nil, fmt.Errorf("failed to POST /api/issue: %w", err) + return 0, nil, fmt.Errorf("failed to POST %s: %w", url, err) } - body, err := checkResp(httpResp) + defer httpResp.Body.Close() + body, err := ioutil.ReadAll(httpResp.Body) if err != nil { - return nil, fmt.Errorf("failed to POST /api/issue: %w: %s", err, body) + return 0, nil, fmt.Errorf("failed to read response body: %w", err) } var pubResponse api.IssueCodeResponse if err := json.Unmarshal(body, &pubResponse); err != nil { - return nil, fmt.Errorf("bad publish response") + return 0, nil, fmt.Errorf("bad publish response") } - return &pubResponse, nil + return httpResp.StatusCode, &pubResponse, nil } // APIClient is a test client for verification API. @@ -151,12 +155,12 @@ func (c *APIClient) GetToken(req api.VerifyCodeRequest) (*api.VerifyCodeResponse httpResp, err := c.client.Do(httpReq) if err != nil { - return nil, fmt.Errorf("failed to POST /api/issue: %w", err) + return nil, fmt.Errorf("failed to POST %s: %w", url, err) } body, err := checkResp(httpResp) if err != nil { - return nil, fmt.Errorf("failed to POST /api/issue: %w: %s", err, body) + return nil, fmt.Errorf("failed to POST %s: %w: %s", url, err, body) } var pubResponse api.VerifyCodeResponse