From 8f01486dbef3f29f0a819048e9487b70a09ae7e1 Mon Sep 17 00:00:00 2001 From: Joshua Liebow-Feeser Date: Sat, 9 May 2020 00:26:17 -0700 Subject: [PATCH] [test] Add framework for running Firestore emulator for unit tests --- .github/workflows/pull_master.yml | 3 + README.md | 27 ++- deploy.sh | 3 +- functions/challenge.go | 13 +- functions/challenge_test.go | 38 ++++ functions/cmd/main.go | 7 +- functions/go.mod | 3 +- functions/internal/pow/pow.go | 8 +- functions/internal/pow/pow_test.go | 4 +- functions/internal/util/firestore.go | 212 ++++++++++++++++++++++ functions/internal/util/firestore_test.go | 12 ++ functions/internal/util/handler.go | 31 +++- functions/internal/util/util.go | 160 ++++++++++------ 13 files changed, 429 insertions(+), 92 deletions(-) create mode 100644 functions/challenge_test.go create mode 100644 functions/internal/util/firestore.go create mode 100644 functions/internal/util/firestore_test.go diff --git a/.github/workflows/pull_master.yml b/.github/workflows/pull_master.yml index b7924ec..e9ed50e 100644 --- a/.github/workflows/pull_master.yml +++ b/.github/workflows/pull_master.yml @@ -15,6 +15,9 @@ jobs: fail-fast: false runs-on: ${{ matrix.platform }} steps: + - uses: GoogleCloudPlatform/github-actions/setup-gcloud@master + - name: Install Google Cloud SDK components + run: yes | gcloud components install beta cloud-firestore-emulator - name: Install Go uses: actions/setup-go@v2 with: diff --git a/README.md b/README.md index d68cc42..c27c11e 100644 --- a/README.md +++ b/README.md @@ -12,12 +12,9 @@ reports. 1. Install [Go](https://golang.org/) 1.13 or higher 2. [Install the Google Cloud SDK](https://cloud.google.com/sdk/install). -3. Tests use the Firestore emulator, which requires some components from the - Google Cloud SDK which are not installed by default. In order to install - them, run the emulator once using `gcloud beta emulators firestore start`. It - will prompt to install any missing components. Once all the necessary - components have been installed and the emulator is actually running, you can - kill it. +3. Install components necessary to run the Firestore emulator: `gcloud + components install beta cloud-firestore-emulator`. Unit tests use the + emulator, so if these components are not installed, unit tests will fail. ## Run Tests @@ -59,7 +56,8 @@ the new one and cause confusion). If the `--host-port` flag is omitted, the emulator will choose a random port, which has a high likelihood of not being in use. The emulator will output which -address to use by displaying a line like `export FIRESTORE_EMULATOR_HOST=::1:8195`. +address to use by displaying a line like `export +FIRESTORE_EMULATOR_HOST=::1:8195`. Note that, if the local IP address is an IPv6 address (like `::1`), then you will need to put square brackets around the address for compatibility with Go's @@ -79,13 +77,13 @@ curl --request GET 'http://localhost:8080/challenge' \ ### HTTPS -The local dev server uses HTTP so you must send a fake HTTPS header to prevent the -request from being rejected. +The local dev server uses HTTP so you must send a fake HTTPS header to prevent +the request from being rejected. -If you see an error message like this with HTTP Code 426: +If you see an error message like this with HTTP Code 418: ``` -{"Error":"Please use HTTPS"} +{"message":"unsupported protocol HTTP; only HTTPS is supported"} ``` You should add either: @@ -102,8 +100,9 @@ Forwarded: for=\"localhost\";proto=https ## Firebase Security -Unauthenticated Firestore access is disabled. If you want to bypass the firestore.rules -on the emulator REST interface just supply the auth bearer header value "owner": +Unauthenticated Firestore access is disabled. If you want to bypass the +firestore.rules on the emulator REST interface just supply the auth bearer +header value "owner": ``` Authorization: Bearer owner @@ -119,5 +118,5 @@ $ ./deploy.sh ### Postman Collection -You can import the COVID-Watch.postman_collection.json to play with the local +You can import the `Covid Watch.postman_collection.json` to play with the local Cloud Functions and Firestore Emulator. diff --git a/deploy.sh b/deploy.sh index 1ce8538..43f2e78 100755 --- a/deploy.sh +++ b/deploy.sh @@ -1,2 +1,3 @@ #!/bin/sh -cd functions && gcloud functions deploy challenge --runtime go113 --trigger-http --entry-point ChallengeHandler --allow-unauthenticated +cd functions && gcloud functions deploy challenge --runtime go113 --trigger-http \ + --entry-point ChallengeHTTPHandler --allow-unauthenticated diff --git a/functions/challenge.go b/functions/challenge.go index 5170b41..2dcaa1e 100644 --- a/functions/challenge.go +++ b/functions/challenge.go @@ -7,11 +7,14 @@ import ( "upload-token.functions/internal/util" ) -// ChallengeHandler is a handler for the /challenge endpoint. -var ChallengeHandler = util.MakeHTTPHandler(challengeHandler) +// ChallengeHTTPHandler is an HTTP handler for the /challenge endpoint. It is +// intended to be registered as a Google Cloud Function by using the +// --entry-point flag to the `gcloud functions deploy` command. +var ChallengeHTTPHandler = util.MakeHTTPHandler(ChallengeHandler) -func challengeHandler(ctx *util.Context) util.StatusError { - if err := util.ValidateRequestMethod(ctx, "GET", ""); err != nil { +// ChallengeHandler is a handler for the /challenge endpoint. +func ChallengeHandler(ctx *util.Context) util.StatusError { + if err := ctx.ValidateRequestMethod("GET", ""); err != nil { return err } @@ -22,4 +25,4 @@ func challengeHandler(ctx *util.Context) util.StatusError { json.NewEncoder(ctx.HTTPResponseWriter()).Encode(c) return nil -} \ No newline at end of file +} diff --git a/functions/challenge_test.go b/functions/challenge_test.go new file mode 100644 index 0000000..3c70627 --- /dev/null +++ b/functions/challenge_test.go @@ -0,0 +1,38 @@ +package functions + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + + "upload-token.functions/internal/pow" + "upload-token.functions/internal/util" +) + +func TestChallenge(t *testing.T) { + firestore := util.NewTestFirestore(t) + + req, err := http.NewRequestWithContext(context.Background(), "GET", "/challenge", nil) + assert.Nil(t, err) + r := httptest.NewRecorder() + ctx, err := util.NewTestContext(r, req, firestore) + assert.Nil(t, err) + + ChallengeHandler(&ctx) + + // First, unmarshal using pow.Challenge in order to benefit from its + // validation. + var c0 pow.Challenge + err = json.Unmarshal(r.Body.Bytes(), &c0) + assert.Nil(t, err) + + // Second, unmarshal into a map so that we can inspect its contents. + var c1 map[string]interface{} + err = json.Unmarshal(r.Body.Bytes(), &c1) + assert.Nil(t, err) + assert.Equal(t, c1["work_factor"], float64(pow.DefaultWorkFactor)) +} diff --git a/functions/cmd/main.go b/functions/cmd/main.go index 08ef01c..ad541c3 100644 --- a/functions/cmd/main.go +++ b/functions/cmd/main.go @@ -5,13 +5,14 @@ import ( "log" "os" - "upload-token.functions" - "github.com/GoogleCloudPlatform/functions-framework-go/funcframework" + + functions "upload-token.functions" + "upload-token.functions/internal/util" ) func main() { - funcframework.RegisterHTTPFunction("/challenge", functions.ChallengeHandler) + funcframework.RegisterHTTPFunction("/challenge", util.MakeTestHTTPHandler(functions.ChallengeHandler)) // Use PORT environment variable, or default to 8080. port := "8080" if envPort := os.Getenv("PORT"); envPort != "" { diff --git a/functions/go.mod b/functions/go.mod index 7e2f11c..5c9ee73 100644 --- a/functions/go.mod +++ b/functions/go.mod @@ -11,9 +11,10 @@ require ( github.com/stretchr/testify v1.5.1 golang.org/x/crypto v0.0.0-20200429183012-4b2356b1ed79 golang.org/x/net v0.0.0-20200506145744-7e3656a0809f // indirect + golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d golang.org/x/sys v0.0.0-20200501145240-bc7a7d42d5c3 // indirect golang.org/x/tools v0.0.0-20200507205054-480da3ebd79c // indirect - google.golang.org/api v0.23.0 // indirect + google.golang.org/api v0.23.0 google.golang.org/genproto v0.0.0-20200507105951-43844f6eee31 // indirect google.golang.org/grpc v1.29.1 ) diff --git a/functions/internal/pow/pow.go b/functions/internal/pow/pow.go index 1b15510..f96b84c 100644 --- a/functions/internal/pow/pow.go +++ b/functions/internal/pow/pow.go @@ -24,8 +24,10 @@ const ( // Allow challenges to remain valid for one minute to allow for slow // connections. We may need to increase this if we find that there's a tail // of clients whose connections are bad enough that this is too short. - expirationPeriod = 60 * time.Second - defaultWorkFactor = 1024 + expirationPeriod = 60 * time.Second + // DefaultWorkFactor is the default work factor used when generating + // challenges. + DefaultWorkFactor = 1024 // The name of the Firestore collection of challenges. challengeCollection = "challenges" @@ -148,7 +150,7 @@ type challengeDoc struct { // GenerateChallenge generates a new challenge and stores it in the database. func GenerateChallenge(ctx *util.Context) (*Challenge, error) { - c := generateChallenge(defaultWorkFactor) + c := generateChallenge(DefaultWorkFactor) doc := challengeDoc{Expiration: time.Now().Add(expirationPeriod)} _, err := ctx.FirestoreClient().Collection(challengeCollection).Doc(c.docID()).Create(ctx, doc) diff --git a/functions/internal/pow/pow_test.go b/functions/internal/pow/pow_test.go index 8cc3e5c..0f36168 100644 --- a/functions/internal/pow/pow_test.go +++ b/functions/internal/pow/pow_test.go @@ -29,7 +29,7 @@ func TestValidate(t *testing.T) { // On a 2018 MacBook Pro, this takes ~930us per validation. func BenchmarkValidate(b *testing.B) { - c := generateChallenge(defaultWorkFactor) + c := generateChallenge(DefaultWorkFactor) var s Solution for { _, err := rand.Read(s.inner.Nonce[:]) @@ -48,6 +48,6 @@ func BenchmarkValidate(b *testing.B) { // On a 2018 MacBook Pro, this takes ~1100ns per validation. func BenchmarkGenerate(b *testing.B) { for i := 0; i < b.N; i++ { - generateChallenge(defaultWorkFactor) + generateChallenge(DefaultWorkFactor) } } diff --git a/functions/internal/util/firestore.go b/functions/internal/util/firestore.go new file mode 100644 index 0000000..f0a1fa4 --- /dev/null +++ b/functions/internal/util/firestore.go @@ -0,0 +1,212 @@ +package util + +import ( + "bufio" + "bytes" + "context" + "encoding/hex" + "io" + "log" + "net" + "os/exec" + "runtime" + "strings" + "sync" + "testing" + "time" + + "cloud.google.com/go/firestore" + "google.golang.org/api/option" + "google.golang.org/grpc" +) + +// TestFirestore is a handle to a Firestore emulator running as a subprocess. +// +// In order to make their use more efficient, Multiple TestFirestores may exist +// which use the same emulator under the hood. However, each will connect using +// a different project ID, so their views of the database will be +// non-overlapping. +type TestFirestore struct { + emulator *firestoreEmulator + projectID string +} + +// NewTestFirestore creates a new TestFirestore. It calls t.Fatal if it is not +// able to start a Firestore emulator subprocess or re-use an existing emulator +// subprocess. +func NewTestFirestore(t *testing.T) *TestFirestore { + emul := getGlobalEmulator(t) + var bytes [16]byte + ReadCryptoRandBytes(bytes[:]) + return &TestFirestore{ + emulator: emul, + projectID: "test-" + hex.EncodeToString(bytes[:]), + } +} + +// FirestoreClient creates a *firestore.Client which connects to this Firestore. +func (t *TestFirestore) FirestoreClient(ctx context.Context) (*firestore.Client, error) { + opt, err := t.clientOption() + if err != nil { + return nil, err + } + return firestore.NewClient(ctx, t.projectID, opt) +} + +func (t *TestFirestore) clientOption() (option.ClientOption, error) { + conn, err := grpc.Dial(t.emulator.host, grpc.WithInsecure(), grpc.WithPerRPCCredentials(emulatorCreds{})) + if err != nil { + return nil, err + } + return option.WithGRPCConn(conn), nil +} + +// emulatorCreds is taken from cloud.google.com/go/firestore/client.go. +// +// TODO(https://github.com/googleapis/google-cloud-go/issues/1978): Switch to a +// first-class API if one is provided. + +// emulatorCreds is an instance of grpc.PerRPCCredentials that will configure a +// client to act as an admin for the Firestore emulator. It always hardcodes +// the "authorization" metadata field to contain "Bearer owner", which the +// Firestore emulator accepts as valid admin credentials. +type emulatorCreds struct{} + +func (ec emulatorCreds) GetRequestMetadata(ctx context.Context, uri ...string) (map[string]string, error) { + return map[string]string{"authorization": "Bearer owner"}, nil +} + +func (ec emulatorCreds) RequireTransportSecurity() bool { + return false +} + +// A global *firestoreEmulator instance for all TestFirestores to share. Use +// getGlobalEmulator to access. +var globalEmulator *firestoreEmulator + +// This sync.Once initializes globalEmulator. Once it has completed, then +// globalEmulator is guaranteed to be initialized. If it is still nil, then that +// means that the initialization failed. +// +// TODO(joshlf): This solution will keep the emulator alive for the remainder of +// the process' lifetime. If we ever get into a situation in which we want to +// create an emulator, and then continue running the process for a significant +// amount of time after no code needs to access it, we should implement a +// fancier solution that detects when there are no more handles to the global +// emulator. +var globalEmulatorOnce sync.Once + +func getGlobalEmulator(t *testing.T) *firestoreEmulator { + globalEmulatorOnce.Do(func() { + globalEmulator = newFirestoreEmulator(t) + }) + + if globalEmulator == nil { + t.Fatal("previous attempt to initialize global emulator failed") + } + + return globalEmulator +} + +type firestoreEmulator struct { + // The emulator subprocess. + emulator *exec.Cmd + host string +} + +func newFirestoreEmulator(t *testing.T) *firestoreEmulator { + // For some reason, if the emulator crashes, the stderr does not close, and + // without any other intervention, our stderr scanning code hangs + // indefinitely. This timeout ensures that, if that happens, this timeout + // will eventually trigger, the process will be killed (thus closing + // stderr), and we will return an error. + // + // TODO(joshlf): Implement a more sophisticated solution to this problem. + // The right way to do this is probably to spawn two goroutines - one to + // scan stderr, and one to monitor the process to see if it quits. + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + // Do this separately ahead of time (rather than just passing "gcloud" to + // exec.CommandContext) to give more precise errors. + gcloudPath, err := exec.LookPath("gcloud") + if err != nil { + t.Fatalf("could not find gcloud: %v", err) + } + // When we don't specify a local address, a random port is chosen + // automatically. + cmd := exec.CommandContext(ctx, gcloudPath, "beta", "emulators", "firestore", "start") + stderr, err := cmd.StderrPipe() + if err != nil { + t.Fatalf("could not start Firestore emulator: %v", err) + } + defer stderr.Close() + + if err := cmd.Start(); err != nil { + t.Fatalf("could not start Firestore emulator: %v", err) + } + + emul := &firestoreEmulator{emulator: cmd} + // Set a finalizer so that the subprocess is killed when we're done with it. + runtime.SetFinalizer(emul, func(emul *firestoreEmulator) { + err := emul.emulator.Process.Kill() + if err != nil { + log.Print("could not kill emulator process:", err) + } + }) + + // Parse the command's stderr until we find the line indicating what local + // address the emulator is listening on. Example output: + // + // Executing: /Users/dvader/google-cloud-sdk/platform/cloud-firestore-emulator/cloud_firestore_emulator start --host=::1 --port=8007 + // [firestore] API endpoint: http://::1:8007 + // [firestore] If you are using a library that supports the FIRESTORE_EMULATOR_HOST environment variable, run: + // [firestore] + // [firestore] export FIRESTORE_EMULATOR_HOST=::1:8007 + // [firestore] + // [firestore] Dev App Server is now running. + // [firestore] + // + // In particular, we look for "export FIRESTORE_EMULATOR_HOST=". + + // Tee all of stderr to a buffer so we can provide more informative error + // messages if need be. + cached := bytes.NewBuffer(nil) + s := bufio.NewScanner(io.TeeReader(stderr, cached)) + var host string +loop: + for s.Scan() { + parts := strings.Split(s.Text(), "export FIRESTORE_EMULATOR_HOST=") + switch len(parts) { + case 1: + if strings.Contains(s.Text(), "Dev App Server is now running") { + break loop + } + case 2: + host = parts[1] + default: + t.Fatalf("got unexpected line from stderr output: \"%v\"", s.Text()) + } + } + + if err := s.Err(); err != nil { + t.Fatalf("error reading output: %v; contents of stderr:\n%v", err, string(cached.Bytes())) + } + + if host == "" { + t.Fatalf("emulator started without outputting its listening address; contents of stderr:\n%s", string(cached.Bytes())) + } + + // Instead of just storing the host as is, we split it into two and then + // recombine it using net.JoinHostPort. The reason is that, when using IPv6, + // the host looks like "::1:8007", which cannot be parsed as an IP + // address/port pair by the Go Firestore client. net.JoinHostPort takes care + // of wrapping IPv6 addresses in square brackets. + colon := strings.LastIndex(host, ":") + if colon == -1 { + t.Fatalf("could not parse host: %v", host) + } + + emul.host = net.JoinHostPort(host[:colon], host[colon+1:]) + return emul +} diff --git a/functions/internal/util/firestore_test.go b/functions/internal/util/firestore_test.go new file mode 100644 index 0000000..6df5547 --- /dev/null +++ b/functions/internal/util/firestore_test.go @@ -0,0 +1,12 @@ +package util + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNewFirestoreEmulator(t *testing.T) { + e := newFirestoreEmulator(t) + assert.NotEqual(t, e.host, "") +} diff --git a/functions/internal/util/handler.go b/functions/internal/util/handler.go index 3129945..ecf36b2 100644 --- a/functions/internal/util/handler.go +++ b/functions/internal/util/handler.go @@ -6,16 +6,33 @@ import ( "net/http" ) -// Handler is a handler for a request to this service. Use MakeHTTPHandler to -// wrap a Handler with the logic necessary to produce a handler which can be -// registered with the "net/http" package. +// Handler is a handler for a request to this service. Use MakeHTTPHandler or +// MakeTestHTTPHandler to wrap a Handler with the logic necessary to produce a +// handler which can be registered with the "net/http" package. type Handler = func(ctx *Context) StatusError // MakeHTTPHandler wraps a Handler, producing a handler which can be registered // with the "net/http" package. The returned handler is responsible for: // - Constructing a *Context // - Converting any errors into an HTTP response -func MakeHTTPHandler(handler func(ctx *Context) StatusError) func(http.ResponseWriter, *http.Request) { +func MakeHTTPHandler(handler Handler) func(http.ResponseWriter, *http.Request) { + return makeHTTPHandler(NewContext, handler) +} + +// MakeTestHTTPHandler is like MakeHTTPHandler, except that the generated +// handler will attempt to connect to the Firestore emulator at the +// FIRESTORE_EMULATOR_HOST environment variable rather than attempting to +// connect to a production Firestore instance. +func MakeTestHTTPHandler(handler Handler) func(http.ResponseWriter, *http.Request) { + return makeHTTPHandler(func(w http.ResponseWriter, r *http.Request) (Context, StatusError) { + return NewTestContext(w, r, nil) + }, handler) +} + +func makeHTTPHandler( + newContext func(w http.ResponseWriter, r *http.Request) (Context, StatusError), + handler Handler, +) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, r *http.Request) { // Add HSTS header. addHSTS(w) @@ -26,7 +43,7 @@ func MakeHTTPHandler(handler func(ctx *Context) StatusError) func(http.ResponseW return } - ctx, err := NewContext(w, r) + ctx, err := newContext(w, r) if err != nil { writeStatusError(w, r, err) return @@ -47,6 +64,6 @@ func writeStatusError(w http.ResponseWriter, r *http.Request, err StatusError) { w.WriteHeader(err.HTTPStatusCode()) json.NewEncoder(w).Encode(response{Message: err.Message()}) - log.Printf("[%v %v %v]: responding with error code %v and message \"%v\" (error: %v)", - r.RemoteAddr, r.Method, r.URL, err.HTTPStatusCode(), err.Message(), err) + log.Printf("[%v %v]: responding with error code %v and message \"%v\" (error: %v)", + r.Method, r.URL, err.HTTPStatusCode(), err.Message(), err) } diff --git a/functions/internal/util/util.go b/functions/internal/util/util.go index 5e454fc..d0cee9c 100644 --- a/functions/internal/util/util.go +++ b/functions/internal/util/util.go @@ -9,10 +9,11 @@ import ( "io" "net/http" "os" - "strings" "regexp" + "strings" "cloud.google.com/go/firestore" + "google.golang.org/api/option" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -28,25 +29,64 @@ type Context struct { } // NewContext constructs a new Context from an http.ResponseWriter and an -// *http.Request. +// *http.Request. It uses the firestore.DetectProjectID Firestore project, which +// instructs firestore.NewClient to use environment variables to detect the +// appropriate Firestore configuration. func NewContext(w http.ResponseWriter, r *http.Request) (Context, StatusError) { ctx := r.Context() - - // In production, automatically detect credentials from the environment. - projectID := firestore.DetectProjectID - if os.Getenv("FIRESTORE_EMULATOR_HOST") != "" { - // If we're not in production, then `firestore.DetectProjectID` will - // cause `NewClient` to look for credentials which aren't there, and so - // the call will fail. - projectID = "test" - } - client, err := firestore.NewClient(ctx, projectID) + client, err := firestore.NewClient(ctx, firestore.DetectProjectID) if err != nil { err := NewInternalServerError(err) return Context{}, err } - return Context{w, r, client, ctx}, nil + return Context{ + resp: w, + req: r, + client: client, + Context: ctx, + }, nil +} + +// NewTestContext constructs a new Context from an http.ResponseWriter and an +// *http.Request. Unlike NewContext, it is intended for use in testing, and uses +// a Firestore emulator rather than a production Firestore instance. In +// particular: +// - If store is nil, then NewTestContext checks that `FIRESTORE_EMULATOR_HOST` +// is set. If it is, then it connects to the Firestore emulator at that host. +// If it is not set, an error is returned. +// - If store is not nil, then NewTestContext connects to that Firestore +// emulator. +func NewTestContext(w http.ResponseWriter, r *http.Request, store *TestFirestore) (Context, StatusError) { + const emulatorHostEnvVar = "FIRESTORE_EMULATOR_HOST" + + projectID := "test" + var clientOptions []option.ClientOption + if store != nil { + opt, err := store.clientOption() + if err != nil { + return Context{}, NewInternalServerError(err) + } + clientOptions = append(clientOptions, opt) + projectID = store.projectID + } else if os.Getenv(emulatorHostEnvVar) == "" { + return Context{}, NewInternalServerError(fmt.Errorf( + "could not connect to Firestore emulator; %v environment variable not present", emulatorHostEnvVar, + )) + } + + ctx := r.Context() + client, err := firestore.NewClient(ctx, projectID, clientOptions...) + if err != nil { + return Context{}, NewInternalServerError(err) + } + + return Context{ + resp: w, + req: r, + client: client, + Context: ctx, + }, nil } // HTTPRequest returns the *http.Request that was used to construct this @@ -66,10 +106,10 @@ func (c *Context) FirestoreClient() *firestore.Client { return c.client } -// ValidateRequestMethod validates that ctx.HTTPRequest().Method == method, and -// if not, returns an appropriate StatusError. -func ValidateRequestMethod(ctx *Context, method, err string) StatusError { - m := ctx.HTTPRequest().Method +// ValidateRequestMethod validates that c.HTTPRequest().Method == method, and if +// not, returns an appropriate StatusError. +func (c *Context) ValidateRequestMethod(method, err string) StatusError { + m := c.HTTPRequest().Method if m != method { return NewMethodNotAllowedError(m) } @@ -133,7 +173,6 @@ func NewBadRequestError(err error) StatusError { // NewMethodNotAllowedError wraps err in a StatusError whose HTTPStatusCode // method returns http.StatusMethodNotAllowed and whose Message method returns // "unsupported method: " followed by the given method string. - func NewMethodNotAllowedError(method string) StatusError { return statusError{ code: http.StatusMethodNotAllowed, @@ -142,14 +181,18 @@ func NewMethodNotAllowedError(method string) StatusError { } var ( - notFoundError = NewBadRequestError(errors.New("not found")) + // NotFoundError is an error returned when a resource is not found. + NotFoundError = NewBadRequestError(errors.New("not found")) ) // FirestoreToStatusError converts an error returned from the // "cloud.google.com/go/firestore" package to a StatusError. func FirestoreToStatusError(err error) StatusError { + if err, ok := err.(StatusError); ok { + return err + } if status.Code(err) == codes.NotFound { - return notFoundError + return NotFoundError } return NewInternalServerError(err) @@ -161,6 +204,8 @@ func FirestoreToStatusError(err error) StatusError { // internal server errors. func JSONToStatusError(err error) StatusError { switch err := err.(type) { + case StatusError: + return err case *json.MarshalerError, *json.SyntaxError, *json.UnmarshalFieldError, *json.UnmarshalTypeError, *json.UnsupportedTypeError, *json.UnsupportedValueError: return NewBadRequestError(err) @@ -184,13 +229,13 @@ func ReadCryptoRandBytes(b []byte) { // newStatusError constructs a new statusError with the given code and error. // The given error will be used as the message returned by StatusError.Message. func newStatusError(code int, err error) statusError { - return statusError { - code: code, + return statusError{ + code: code, error: err, // Leave empty so that error.Error() will be used as the return value // from Message. - message: "", - } + message: "", + } } // checkHTTPS retrieves the scheme from the X-Forwarded-Proto or RFC7239 @@ -205,8 +250,8 @@ var ( // De-facto standard header keys. xForwardedProto = http.CanonicalHeaderKey("X-Forwarded-Proto") forwarded = http.CanonicalHeaderKey("Forwarded") // RFC7239 - - protoRegex = regexp.MustCompile(`(?i)(?:proto=)(https|http)`) + + protoRegex = regexp.MustCompile(`(?i)(?:proto=)(https|http)`) ) func checkHTTPS(r *http.Request) StatusError { @@ -228,29 +273,28 @@ func checkHTTPS(r *http.Request) StatusError { } } - // We want to ensure that clients always use HTTPS. Even if we don't - // serve our API over HTTP, if clients use HTTP, they are vulnerable - // to man-in-the-middle attacks in which the attacker communicates - // with our service over HTTPS. In order to prevent this, it is not - // sufficient to simply auto-upgrade to HTTPS (e.g., via a redirect - // status code in the 300s). If we do this, then code which - // erroneously uses HTTP will continue to work, and so it might get - // deployed. Instead, we have to ensure that such code breaks - // completely, alerting the code's developers to the issue, and - // ensuring that they will change the code to use HTTPS directly. - // Thus, we want an error code with the following properties: - // - Guaranteed that smart clients (such as web browsers) will not - // attempt to automatically upgrade to HTTPS - // - Doesn't have another meaning which might cause developers to - // overlook the error or lead them down the wrong path (e.g., if - // we chose 400 - bad request - they might go down the path of - // debugging their request format) + // We want to ensure that clients always use HTTPS. Even if we don't serve + // our API over HTTP, if clients use HTTP, they are vulnerable to + // man-in-the-middle attacks in which the attacker communicates with our + // service over HTTPS. In order to prevent this, it is not sufficient to + // simply auto-upgrade to HTTPS (e.g., via a redirect status code in the + // 300s). If we do this, then code which erroneously uses HTTP will continue + // to work, and so it might get deployed. Instead, we have to ensure that + // such code breaks completely, alerting the code's developers to the issue, + // and ensuring that they will change the code to use HTTPS directly. Thus, + // we want an error code with the following properties: + // - Guaranteed that smart clients (such as web browsers) will not attempt + // to automatically upgrade to HTTPS + // - Doesn't have another meaning which might cause developers to overlook + // the error or lead them down the wrong path (e.g., if we chose 400 - + // bad request - they might go down the path of debugging their request + // format) // - // For these reasons, we choose error code 418 - server is a teapot. - // It is as unlikely as any other error code to cause the client to - // automatically upgrade to HTTPS, and it is guaranteed to get a - // developer's attention, hopefully getting them to look at the - // response body, which will contain the relevant information. + // For these reasons, we choose error code 418 - server is a teapot. It is + // as unlikely as any other error code to cause the client to automatically + // upgrade to HTTPS, and it is guaranteed to get a developer's attention, + // hopefully getting them to look at the response body, which will contain + // the relevant information. if scheme != "https" { return newStatusError(http.StatusTeapot, errors.New("unsupported protocol HTTP; only HTTPS is supported")) @@ -258,15 +302,19 @@ func checkHTTPS(r *http.Request) StatusError { return nil } -// Add HSTS to force HTTPS usage. -// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security -// In the following example, max-age is set to 2 years, raised from what was a former -// limit max-age of 1 year. Note that 1 year is acceptable for a domain to be included -// in browsers' HSTS preload lists. 2 years is, however, the recommended goal as a -// website's final HSTS configuration as explained on https://hstspreload.org. -// It also suffixed with preload which is necessary for inclusion in most major web -// browsers' HSTS preload lists, e.g. Chromium, Edge, & Firefox. var headerHSTS = http.CanonicalHeaderKey("Strict-Transport-Security") + +// addHSTS adds HSTS [1] to force HTTPS usage. max-age is set to 2 years, raised +// from what was a former limit max-age of 1 year. Note that 1 year is +// acceptable for a domain to be included in browsers' HSTS preload lists. 2 +// years is, however, the recommended goal as a website's final HSTS +// configuration as explained on [2]. It also suffixed with preload which is +// necessary for inclusion in most major web browsers' HSTS preload lists, e.g. +// Chromium, Edge, & Firefox. +// +// [1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security +// +// [2] https://hstspreload.org func addHSTS(w http.ResponseWriter) { w.Header().Set(headerHSTS, "max-age=63072000; includeSubDomains; preload") }