Use less chromedp on codes api (#1862)

google · Feb 25, 2021 · c9272a7 · c9272a7
1 parent 17e0f5c
commit c9272a7
Show file tree

Hide file tree

Showing 10 changed files with 463 additions and 272 deletions.
diff --git a/internal/routes/adminapi.go b/internal/routes/adminapi.go
@@ -111,7 +111,7 @@ func AdminAPI(
 		sub.Handle("/issue", issueapiController.HandleIssueAPI()).Methods(http.MethodPost)
 		sub.Handle("/batch-issue", issueapiController.HandleBatchIssueAPI()).Methods(http.MethodPost)
 
-		codesController := codes.NewAPI(ctx, cfg, db, h)
+		codesController := codes.NewAPI(cfg, db, h)
 		sub.Handle("/checkcodestatus", codesController.HandleCheckCodeStatus()).Methods(http.MethodPost)
 		sub.Handle("/expirecode", codesController.HandleExpireAPI()).Methods(http.MethodPost)
 	}

diff --git a/internal/routes/server.go b/internal/routes/server.go
@@ -226,7 +226,7 @@ func Server(
 		sub.Handle("/issue", issueapiController.HandleIssueUI()).Methods(http.MethodPost)
 		sub.Handle("/batch-issue", issueapiController.HandleBatchIssueUI()).Methods(http.MethodPost)
 
-		codesController := codes.NewServer(ctx, cfg, db, h)
+		codesController := codes.NewServer(cfg, db, h)
 		codesRoutes(sub, codesController)
 	}
 

diff --git a/pkg/controller/admin/events_test.go b/pkg/controller/admin/events_test.go
@@ -30,9 +30,6 @@ import (
 	"github.com/gorilla/sessions"
 )
 
-// This goes to the value of a <input type="datetime-local">
-const rfc3339PartialLocal = "2006-01-02T15:04:05"
-
 func TestAdminEvents(t *testing.T) {
 	t.Parallel()
 

diff --git a/pkg/controller/codes/bulk_issue_test.go b/pkg/controller/codes/bulk_issue_test.go
@@ -15,62 +15,149 @@
 package codes_test
 
 import (
-	"context"
+	"net/http"
 	"testing"
 
-	"github.com/chromedp/chromedp"
-	"github.com/google/exposure-notifications-verification-server/internal/browser"
 	"github.com/google/exposure-notifications-verification-server/internal/envstest"
+	"github.com/google/exposure-notifications-verification-server/internal/i18n"
 	"github.com/google/exposure-notifications-verification-server/internal/project"
+	"github.com/google/exposure-notifications-verification-server/pkg/controller"
+	"github.com/google/exposure-notifications-verification-server/pkg/controller/codes"
+	"github.com/google/exposure-notifications-verification-server/pkg/controller/middleware"
 	"github.com/google/exposure-notifications-verification-server/pkg/database"
 	"github.com/google/exposure-notifications-verification-server/pkg/rbac"
 	"github.com/google/exposure-notifications-verification-server/pkg/sms"
+	"github.com/gorilla/sessions"
 )
 
 func TestRenderBulkIssue(t *testing.T) {
 	t.Parallel()
 
-	harness := envstest.NewServer(t, testDatabaseInstance)
+	ctx := project.TestContext(t)
 
-	realm, user, session, err := harness.ProvisionAndLogin()
-	if err != nil {
-		t.Fatal(err)
-	}
-	cookie, err := harness.SessionCookie(session)
+	harness := envstest.NewServerConfig(t, testDatabaseInstance)
+
+	locales, err := i18n.Load(harness.Config.LocalesPath)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	realm.AllowBulkUpload = true
-	if err := harness.Database.SaveRealm(realm, database.SystemTest); err != nil {
-		t.Fatal(err)
-	}
+	c := codes.NewServer(harness.Config, harness.Database, harness.Renderer)
+	handler := middleware.ProcessLocale(locales)(c.HandleBulkIssue())
 
-	if err := user.AddToRealm(harness.Database, realm,
-		rbac.LegacyRealmAdmin|rbac.CodeBulkIssue, database.SystemTest); err != nil {
-		t.Fatal(err)
-	}
+	t.Run("middleware", func(t *testing.T) {
+		t.Parallel()
 
-	sms := &database.SMSConfig{
-		RealmID:          realm.ID,
-		ProviderType:     sms.ProviderType("TWILIO"),
-		TwilioAccountSid: "abc123",
-		TwilioAuthToken:  "def123",
-		TwilioFromNumber: "+11234567890",
-	}
-	if err := harness.Database.SaveSMSConfig(sms); err != nil {
-		t.Fatalf("failed to save SMSConfig: %v", err)
-	}
+		envstest.ExerciseSessionMissing(t, handler)
+		envstest.ExerciseMembershipMissing(t, handler)
+		envstest.ExercisePermissionMissing(t, handler)
+	})
 
-	browserCtx := browser.New(t)
-	taskCtx, done := context.WithTimeout(browserCtx, project.TestTimeout())
-	defer done()
+	t.Run("internal_error", func(t *testing.T) {
+		t.Parallel()
 
-	if err := chromedp.Run(taskCtx,
-		browser.SetCookie(cookie),
-		chromedp.Navigate(`http://`+harness.Server.Addr()+`/codes/bulk-issue`),
-		chromedp.WaitVisible(`body#bulk-issue`, chromedp.ByQuery),
-	); err != nil {
-		t.Fatal(err)
-	}
+		c := codes.NewServer(harness.Config, harness.BadDatabase, harness.Renderer)
+		handler := middleware.ProcessLocale(locales)(c.HandleBulkIssue())
+
+		ctx := ctx
+		ctx = controller.WithSession(ctx, &sessions.Session{})
+		ctx = controller.WithMembership(ctx, &database.Membership{
+			Realm: &database.Realm{
+				AllowBulkUpload: true,
+			},
+			User:        &database.User{},
+			Permissions: rbac.LegacyRealmAdmin | rbac.CodeBulkIssue,
+		})
+
+		w, r := envstest.BuildFormRequest(ctx, t, http.MethodGet, "/", nil)
+		handler.ServeHTTP(w, r)
+
+		if got, want := w.Code, http.StatusInternalServerError; got != want {
+			t.Errorf("Expected %d to be %d", got, want)
+		}
+	})
+
+	t.Run("not_enabled", func(t *testing.T) {
+		t.Parallel()
+
+		ctx := ctx
+		ctx = controller.WithSession(ctx, &sessions.Session{})
+		ctx = controller.WithMembership(ctx, &database.Membership{
+			Realm: &database.Realm{
+				AllowBulkUpload: false,
+			},
+			User:        &database.User{},
+			Permissions: rbac.LegacyRealmAdmin | rbac.CodeBulkIssue,
+		})
+
+		w, r := envstest.BuildFormRequest(ctx, t, http.MethodGet, "/", nil)
+		handler.ServeHTTP(w, r)
+
+		if got, want := w.Code, http.StatusSeeOther; got != want {
+			t.Errorf("Expected %d to be %d: %s", got, want, w.Body.String())
+		}
+	})
+
+	t.Run("success", func(t *testing.T) {
+		t.Parallel()
+
+		ctx := ctx
+		ctx = controller.WithSession(ctx, &sessions.Session{})
+		ctx = controller.WithMembership(ctx, &database.Membership{
+			Realm: &database.Realm{
+				AllowBulkUpload: true,
+			},
+			User:        &database.User{},
+			Permissions: rbac.LegacyRealmAdmin | rbac.CodeBulkIssue,
+		})
+
+		w, r := envstest.BuildFormRequest(ctx, t, http.MethodGet, "/", nil)
+		handler.ServeHTTP(w, r)
+
+		if got, want := w.Code, http.StatusOK; got != want {
+			t.Errorf("Expected %d to be %d: %s", got, want, w.Body.String())
+		}
+	})
+
+	t.Run("with_sms", func(t *testing.T) {
+		t.Parallel()
+
+		realm, err := harness.Database.FindRealm(1)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		realm.AllowBulkUpload = true
+		if err := harness.Database.SaveRealm(realm, database.SystemTest); err != nil {
+			t.Fatal(err)
+		}
+
+		sms := &database.SMSConfig{
+			RealmID:          realm.ID,
+			ProviderType:     sms.ProviderType("TWILIO"),
+			TwilioAccountSid: "abc123",
+			TwilioAuthToken:  "def123",
+			TwilioFromNumber: "+11234567890",
+		}
+		if err := harness.Database.SaveSMSConfig(sms); err != nil {
+			t.Fatalf("failed to save SMSConfig: %v", err)
+		}
+
+		ctx := ctx
+		ctx = controller.WithSession(ctx, &sessions.Session{})
+		ctx = controller.WithMembership(ctx, &database.Membership{
+			Realm: &database.Realm{
+				AllowBulkUpload: true,
+			},
+			User:        &database.User{},
+			Permissions: rbac.LegacyRealmAdmin | rbac.CodeBulkIssue,
+		})
+
+		w, r := envstest.BuildFormRequest(ctx, t, http.MethodGet, "/", nil)
+		handler.ServeHTTP(w, r)
+
+		if got, want := w.Code, http.StatusOK; got != want {
+			t.Errorf("Expected %d to be %d: %s", got, want, w.Body.String())
+		}
+	})
 }
diff --git a/pkg/controller/codes/controller.go b/pkg/controller/codes/controller.go
@@ -18,8 +18,6 @@
 package codes
 
 import (
-	"context"
-
 	"github.com/google/exposure-notifications-verification-server/pkg/config"
 	"github.com/google/exposure-notifications-verification-server/pkg/database"
 	"github.com/google/exposure-notifications-verification-server/pkg/render"
@@ -33,18 +31,18 @@ type Controller struct {
 }
 
 // NewServer creates a new controller for serving admin server requests.
-func NewServer(ctx context.Context, config *config.ServerConfig, db *database.Database, h *render.Renderer) *Controller {
+func NewServer(cfg *config.ServerConfig, db *database.Database, h *render.Renderer) *Controller {
 	return &Controller{
-		serverconfig: config,
+		serverconfig: cfg,
 		db:           db,
 		h:            h,
 	}
 }
 
 // NewAPI creates a new controller serving API requests.
-func NewAPI(ctx context.Context, config *config.AdminAPIServerConfig, db *database.Database, h *render.Renderer) *Controller {
+func NewAPI(cfg *config.AdminAPIServerConfig, db *database.Database, h *render.Renderer) *Controller {
 	return &Controller{
-		apiconfig: config,
+		apiconfig: cfg,
 		db:        db,
 		h:         h,
 	}

diff --git a/pkg/controller/codes/expire.go b/pkg/controller/codes/expire.go
@@ -59,6 +59,13 @@ func (c *Controller) HandleExpirePage() http.Handler {
 		ctx := r.Context()
 		vars := mux.Vars(r)
 
+		session := controller.SessionFromContext(ctx)
+		if session == nil {
+			controller.MissingSession(w, r, c.h)
+			return
+		}
+		flash := controller.Flash(session)
+
 		membership := controller.MembershipFromContext(ctx)
 		if membership == nil {
 			controller.MissingMembership(w, r, c.h)
@@ -72,13 +79,6 @@ func (c *Controller) HandleExpirePage() http.Handler {
 		currentRealm := membership.Realm
 		currentUser := membership.User
 
-		session := controller.SessionFromContext(ctx)
-		if session == nil {
-			controller.MissingSession(w, r, c.h)
-			return
-		}
-		flash := controller.Flash(session)
-
 		// Retrieve once to check permissions.
 		code, _, apiErr := c.checkCodeStatus(r, vars["uuid"])
 		if apiErr != nil {