diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 738aae7..d7449f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,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 1f04f8f..14d6d8b 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 diff --git a/functions/challenge.go b/functions/challenge.go index 11c23dc..7271ca3 100644 --- a/functions/challenge.go +++ b/functions/challenge.go @@ -9,7 +9,7 @@ import ( // ChallengeHandler is a handler for the /challenge endpoint. func ChallengeHandler(ctx *util.Context) util.StatusError { - if err := util.ValidateRequestMethod(ctx, "GET", ""); err != nil { + if err := ctx.ValidateRequestMethod("GET", ""); err != nil { return err } diff --git a/functions/challenge_test.go b/functions/challenge_test.go new file mode 100644 index 0000000..c50108e --- /dev/null +++ b/functions/challenge_test.go @@ -0,0 +1,39 @@ +package functions + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + + "functions/internal/pow" + "functions/internal/util" +) + +func TestChallenge(t *testing.T) { + firestore := util.NewTestFirestore(t) + + c := util.WithTestFirestore(context.Background(), firestore) + req, err := http.NewRequestWithContext(c, "GET", "/challenge", nil) + assert.Nil(t, err) + r := httptest.NewRecorder() + ctx, err := util.NewContext(r, req) + 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 115b9cb..2fe6ae4 100644 --- a/functions/cmd/main.go +++ b/functions/cmd/main.go @@ -50,6 +50,6 @@ func writeStatusError(w http.ResponseWriter, r *http.Request, err util.StatusErr 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\"", - r.RemoteAddr, r.Method, r.URL, err.HTTPStatusCode(), 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) } diff --git a/functions/go.mod b/functions/go.mod index e1c07b4..3f26c62 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 cc735e1..7af45fd 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..5588fc4 --- /dev/null +++ b/functions/internal/util/firestore.go @@ -0,0 +1,163 @@ +package util + +import ( + "bufio" + "bytes" + "context" + "io" + "log" + "net" + "os/exec" + "runtime" + "strings" + "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. +// +// When a TestFirestore is garbage collected, the subprocess is killed. Make +// sure to keep a TestFirestore reachable so long as code is attempting to use +// the emulator so the emulator won't be killed. +type TestFirestore struct { + // The emulator subprocess. + emulator *exec.Cmd + host string +} + +func NewTestFirestore(t *testing.T) *TestFirestore { + // 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) + } + + firestore := &TestFirestore{emulator: cmd} + // Set a finalizer so that the subprocess is killed when we're done with it. + runtime.SetFinalizer(firestore, func(firestore *TestFirestore) { + err := firestore.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) + } + + firestore.host = net.JoinHostPort(host[:colon], host[colon+1:]) + return firestore +} + +// 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, "test", opt) +} + +func (t *TestFirestore) clientOption() (option.ClientOption, error) { + conn, err := grpc.Dial(t.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 +} diff --git a/functions/internal/util/firestore_test.go b/functions/internal/util/firestore_test.go new file mode 100644 index 0000000..1c8831a --- /dev/null +++ b/functions/internal/util/firestore_test.go @@ -0,0 +1,12 @@ +package util + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNewFirestore(t *testing.T) { + f := NewTestFirestore(t) + assert.NotEqual(t, f.host, "") +} diff --git a/functions/internal/util/util.go b/functions/internal/util/util.go index 120c233..ee2a77d 100644 --- a/functions/internal/util/util.go +++ b/functions/internal/util/util.go @@ -11,10 +11,25 @@ import ( "os" "cloud.google.com/go/firestore" + "google.golang.org/api/option" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) +// testFirestoreContextKey is a key used to identify a *TestFirestore object. +// Since it is not exported, there is no way for code outside of this package to +// overwrite or access values stored with this key. +type testFirestoreContextKey struct{} + +// WithTestFirestore wraps parent, producing a context.Context which stores the +// given *TestFirestore. If NewContext is called with an *http.Request whose +// context.Context was produced using this function, it will connect to the +// TestFirestore instead of consulting the environment to figure out how to +// connect to a Firestore instance, as is its normal behavior. +func WithTestFirestore(parent context.Context, store *TestFirestore) context.Context { + return context.WithValue(parent, testFirestoreContextKey{}, store) +} + // Context is a context.Context that provides extra utilities for common // operations. type Context struct { @@ -27,23 +42,47 @@ type Context struct { // NewContext constructs a new Context from an http.ResponseWriter and an // *http.Request. +// +// NewContext automatically figures out how to connect to a Firestore instance. +// In particular: +// - In a production environment, it uses the firestore.DetectProjectID +// project, which causes firestore.NewClient to use environment variables +// to detect the appropriate Firestore configuration. +// - If r.Context() was created using WithTestFirestore, NewContext will connect +// to that TestFirestore instance. +// - If the `FIRESTORE_EMULATOR_HOST` environment variable is set, +// firestore.NewClient will attempt to connect to that host. 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") != "" { + // Attempt to extract a *TestFirestore stored by withTestFirestore. + testFirestore := ctx.Value(testFirestoreContextKey{}) + var clientOptions []option.ClientOption + if os.Getenv("FIRESTORE_EMULATOR_HOST") != "" || testFirestore != nil { // 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" + if store, ok := testFirestore.(*TestFirestore); ok { + opt, err := store.clientOption() + if err != nil { + return Context{}, NewInternalServerError(err) + } + clientOptions = append(clientOptions, opt) + } } - client, err := firestore.NewClient(ctx, projectID) + client, err := firestore.NewClient(ctx, projectID, clientOptions...) 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 } // HTTPRequest returns the *http.Request that was used to construct this @@ -63,10 +102,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) } @@ -138,14 +177,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) @@ -157,6 +200,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)