From aba2b2fb3544d9e42991237c13d8cada421deda5 Mon Sep 17 00:00:00 2001 From: Greg Steuck Date: Tue, 15 Jun 2021 11:37:27 -0700 Subject: dashboard/app: document the values in maps and rename local vars The previous names confused a couple of unrelated concepts. --- dashboard/app/api.go | 11 ++++++----- dashboard/app/app_test.go | 16 ++++++++-------- dashboard/app/config.go | 5 ++++- dashboard/app/repro_test.go | 2 +- dashboard/app/util_test.go | 4 ++-- 5 files changed, 21 insertions(+), 17 deletions(-) diff --git a/dashboard/app/api.go b/dashboard/app/api.go index dc09a32fc..99dd0e7fd 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -104,6 +104,7 @@ func handleAPI(c context.Context, r *http.Request) (reply interface{}, err error client := r.PostFormValue("client") method := r.PostFormValue("method") log.Infof(c, "api %q from %q", method, client) + // Somewhat confusingly the "key" parameter is the password. ns, err := checkClient(c, client, r.PostFormValue("key")) if err != nil { if client != "" { @@ -142,19 +143,19 @@ func handleAPI(c context.Context, r *http.Request) (reply interface{}, err error return nsHandler(c, ns, r, payload) } -func checkClient(c context.Context, name0, key0 string) (string, error) { - for name, key := range config.Clients { +func checkClient(c context.Context, name0, password0 string) (string, error) { + for name, password := range config.Clients { if name == name0 { - if key != key0 { + if password != password0 { return "", ErrAccess } return "", nil } } for ns, cfg := range config.Namespaces { - for name, key := range cfg.Clients { + for name, password := range cfg.Clients { if name == name0 { - if key != key0 { + if password != password0 { return "", ErrAccess } return ns, nil diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go index 98bd185a4..489732118 100644 --- a/dashboard/app/app_test.go +++ b/dashboard/app/app_test.go @@ -56,7 +56,7 @@ var testConfig = &GlobalConfig{ Key: "test1keytest1keytest1key", FixBisectionAutoClose: true, Clients: map[string]string{ - client1: key1, + client1: password1, }, Repos: []KernelRepo{ { @@ -105,7 +105,7 @@ var testConfig = &GlobalConfig{ AccessLevel: AccessAdmin, Key: "test2keytest2keytest2key", Clients: map[string]string{ - client2: key2, + client2: password2, }, Repos: []KernelRepo{ { @@ -262,8 +262,8 @@ var testConfig = &GlobalConfig{ const ( client1 = "client1" client2 = "client2" - key1 = "client1keyclient1keyclient1key" - key2 = "client2keyclient2keyclient2key" + password1 = "client1keyclient1keyclient1key" + password2 = "client2keyclient2keyclient2key" clientAdmin = "client-admin" keyAdmin = "clientadminkeyclientadminkey" clientUser = "client-user" @@ -355,8 +355,8 @@ func TestApp(t *testing.T) { c.expectOK(c.GET("/test1")) - apiClient1 := c.makeClient(client1, key1, false) - apiClient2 := c.makeClient(client2, key2, false) + apiClient1 := c.makeClient(client1, password1, false) + apiClient2 := c.makeClient(client2, password2, false) c.expectFail("unknown api method", apiClient1.Query("unsupported_method", nil, nil)) c.client.LogError("name", "msg %s", "arg") @@ -367,8 +367,8 @@ func TestApp(t *testing.T) { // Some bad combinations of client/key. c.expectFail("unauthorized", c.makeClient(client1, "", false).Query("upload_build", build, nil)) - c.expectFail("unauthorized", c.makeClient("unknown", key1, false).Query("upload_build", build, nil)) - c.expectFail("unauthorized", c.makeClient(client1, key2, false).Query("upload_build", build, nil)) + c.expectFail("unauthorized", c.makeClient("unknown", password1, false).Query("upload_build", build, nil)) + c.expectFail("unauthorized", c.makeClient(client1, password2, false).Query("upload_build", build, nil)) crash1 := testCrash(build, 1) c.client.ReportCrash(crash1) diff --git a/dashboard/app/config.go b/dashboard/app/config.go index f4f4d1ac9..3e0c16731 100644 --- a/dashboard/app/config.go +++ b/dashboard/app/config.go @@ -31,6 +31,7 @@ type GlobalConfig struct { // syz-ci can upload these reports to GCS. CoverPath string // Global API clients that work across namespaces (e.g. external reporting). + // The keys are client identities (names), the values are their passwords. Clients map[string]string // List of emails blocked from issuing test requests. EmailBlocklist []string @@ -67,8 +68,10 @@ type Config struct { // Similar bugs are shown only across namespaces with the same value of SimilarityDomain. SimilarityDomain string // Per-namespace clients that act only on a particular namespace. + // The keys are client identities (names), the values are their passwords. Clients map[string]string - // A unique key for hashing, can be anything. + // A random string used for hashing, can be anything, but once fixed it can't + // be changed as it becomes a part of persistent bug identifiers. Key string // Mail bugs without reports (e.g. "no output"). MailWithoutReport bool diff --git a/dashboard/app/repro_test.go b/dashboard/app/repro_test.go index f488d9d66..84a6b69ee 100644 --- a/dashboard/app/repro_test.go +++ b/dashboard/app/repro_test.go @@ -222,7 +222,7 @@ func TestNeedReproMissing(t *testing.T) { c := NewCtx(t) defer c.Close() - client := c.makeClient(client1, key1, false) + client := c.makeClient(client1, password1, false) cid := &dashapi.CrashID{ BuildID: "some missing build", diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go index bfb14c54b..628f77425 100644 --- a/dashboard/app/util_test.go +++ b/dashboard/app/util_test.go @@ -72,8 +72,8 @@ func NewCtx(t *testing.T) *Ctx { mockedTime: time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC), emailSink: make(chan *aemail.Message, 100), } - c.client = c.makeClient(client1, key1, true) - c.client2 = c.makeClient(client2, key2, true) + c.client = c.makeClient(client1, password1, true) + c.client2 = c.makeClient(client2, password2, true) registerContext(r, c) return c } -- cgit mrf-deployment