From 25476568d635dfe6746287c3a9ac5f0121b90f40 Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Fri, 9 Aug 2024 16:36:48 -0400 Subject: [PATCH] 21901: Add clientToken to fileEventLog As an aid to log deduplication. Arvados-DCO-1.1-Signed-off-by: Brett Smith --- services/keep-web/handler.go | 5 ++- services/keep-web/handler_test.go | 58 ++++++++++++++++--------------- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go index 94a3767fc6..374be113eb 100644 --- a/services/keep-web/handler.go +++ b/services/keep-web/handler.go @@ -957,6 +957,7 @@ type fileEventLog struct { collPDH string collFilePath string clientAddr string + clientToken string } func newFileEventLog( @@ -965,6 +966,7 @@ func newFileEventLog( filepath string, collection *arvados.Collection, user *arvados.User, + token string, ) *fileEventLog { var eventType string switch r.Method { @@ -993,6 +995,7 @@ func newFileEventLog( requestPath: r.URL.Path, eventType: eventType, clientAddr: clientAddr, + clientToken: token, } if user != nil { @@ -1065,7 +1068,7 @@ func (h *handler) logUploadOrDownload( if collection == nil && fs != nil { collection, filepath = h.determineCollection(fs, filepath) } - event := newFileEventLog(h, r, filepath, collection, user) + event := newFileEventLog(h, r, filepath, collection, user, client.ApiToken) if event == nil { return // Nothing to log } diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go index 60ca52ba0e..3aa1d30d09 100644 --- a/services/keep-web/handler_test.go +++ b/services/keep-web/handler_test.go @@ -93,7 +93,7 @@ func (s *UnitSuite) TestLogEventTypes(c *check.C) { } { filePath := "/" + method req := newRequest(method, collURL+filePath) - actual := newFileEventLog(s.handler, req, filePath, nil, nil) + actual := newFileEventLog(s.handler, req, filePath, nil, nil, "") if !c.Check(actual, check.NotNil) { continue } @@ -106,7 +106,7 @@ func (s *UnitSuite) TestUnloggedEventTypes(c *check.C) { for _, method := range []string{"DELETE", "HEAD", "OPTIONS", "PATCH"} { filePath := "/" + method req := newRequest(method, collURL+filePath) - actual := newFileEventLog(s.handler, req, filePath, nil, nil) + actual := newFileEventLog(s.handler, req, filePath, nil, nil, "") c.Check(actual, check.IsNil, check.Commentf("%s request made a log event", method)) } @@ -117,7 +117,7 @@ func (s *UnitSuite) TestLogFilePath(c *check.C) { collURL := "http://keep-web.example/c=" + arvadostest.FooCollection for _, filePath := range []string{"/foo", "/Foo", "/foo/bar"} { req := newRequest("GET", collURL+filePath) - actual := newFileEventLog(s.handler, req, filePath, coll, nil) + actual := newFileEventLog(s.handler, req, filePath, coll, nil, "") if !c.Check(actual, check.NotNil) { continue } @@ -132,7 +132,7 @@ func (s *UnitSuite) TestLogRemoteAddr(c *check.C) { for _, addr := range []string{"10.20.30.55", "192.168.144.120", "192.0.2.4"} { req.RemoteAddr = addr + ":57914" - actual := newFileEventLog(s.handler, req, filePath, nil, nil) + actual := newFileEventLog(s.handler, req, filePath, nil, nil, "") if !c.Check(actual, check.NotNil) { continue } @@ -141,7 +141,7 @@ func (s *UnitSuite) TestLogRemoteAddr(c *check.C) { for _, addr := range []string{"100::20:30:40", "2001:db8::90:100", "3fff::30"} { req.RemoteAddr = fmt.Sprintf("[%s]:57916", addr) - actual := newFileEventLog(s.handler, req, filePath, nil, nil) + actual := newFileEventLog(s.handler, req, filePath, nil, nil, "") if !c.Check(actual, check.NotNil) { continue } @@ -170,7 +170,7 @@ func (s *UnitSuite) TestLogXForwardedFor(c *check.C) { ",100::321,10.30.20.10": "100::321", } { req.Header.Set("X-Forwarded-For", xff) - actual := newFileEventLog(s.handler, req, filePath, nil, nil) + actual := newFileEventLog(s.handler, req, filePath, nil, nil, "") if !c.Check(actual, check.NotNil) { continue } @@ -184,7 +184,7 @@ func (s *UnitSuite) TestLogXForwardedForMalformed(c *check.C) { req := newRequest("GET", collURL+filePath) for _, xff := range []string{"", ",", "10.20,30.40", "foo, bar"} { req.Header.Set("X-Forwarded-For", xff) - actual := newFileEventLog(s.handler, req, filePath, nil, nil) + actual := newFileEventLog(s.handler, req, filePath, nil, nil, "") if !c.Check(actual, check.NotNil) { continue } @@ -199,7 +199,7 @@ func (s *UnitSuite) TestLogXForwardedForMultivalue(c *check.C) { req.Header.Set("X-Forwarded-For", ", ") req.Header.Add("X-Forwarded-For", "2001:db8::db9:dbd") req.Header.Add("X-Forwarded-For", "10.20.30.90") - actual := newFileEventLog(s.handler, req, filePath, nil, nil) + actual := newFileEventLog(s.handler, req, filePath, nil, nil, "") c.Assert(actual, check.NotNil) c.Check(actual.clientAddr, check.Equals, "2001:db8::db9:dbd") } @@ -211,13 +211,13 @@ func (s *UnitSuite) TestLogClientAddressCanonicalization(c *check.C) { expected := "2001:db8::12:0" req.RemoteAddr = "[2001:db8::012:0000]:57918" - a := newFileEventLog(s.handler, req, filePath, nil, nil) + a := newFileEventLog(s.handler, req, filePath, nil, nil, "") c.Assert(a, check.NotNil) c.Check(a.clientAddr, check.Equals, expected) req.RemoteAddr = "10.20.30.40:57919" req.Header.Set("X-Forwarded-For", "2001:db8:0::0:12:00") - b := newFileEventLog(s.handler, req, filePath, nil, nil) + b := newFileEventLog(s.handler, req, filePath, nil, nil, "") c.Assert(b, check.NotNil) c.Check(b.clientAddr, check.Equals, expected) } @@ -226,30 +226,32 @@ func (s *UnitSuite) TestLogAnonymousUser(c *check.C) { collURL := "http://keep-web.example/c=" + arvadostest.FooCollection filePath := "/foo" req := newRequest("GET", collURL+filePath) - actual := newFileEventLog(s.handler, req, filePath, nil, nil) + actual := newFileEventLog(s.handler, req, filePath, nil, nil, arvadostest.AnonymousToken) c.Assert(actual, check.NotNil) c.Check(actual.userUUID, check.Equals, s.handler.Cluster.ClusterID+"-tpzed-anonymouspublic") c.Check(actual.userFullName, check.Equals, "") + c.Check(actual.clientToken, check.Equals, arvadostest.AnonymousToken) } func (s *UnitSuite) TestLogUser(c *check.C) { collURL := "http://keep-web.example/c=" + arvadostest.FooCollection - for userUUID, userFullName := range map[string]string{ - arvadostest.ActiveUserUUID: "Active User", - arvadostest.SpectatorUserUUID: "Spectator User", + for _, trial := range []struct{ uuid, fullName, token string }{ + {arvadostest.ActiveUserUUID, "Active User", arvadostest.ActiveToken}, + {arvadostest.SpectatorUserUUID, "Spectator User", arvadostest.SpectatorToken}, } { - filePath := "/" + userUUID + filePath := "/" + trial.uuid req := newRequest("GET", collURL+filePath) user := &arvados.User{ - UUID: userUUID, - FullName: userFullName, + UUID: trial.uuid, + FullName: trial.fullName, } - actual := newFileEventLog(s.handler, req, filePath, nil, user) + actual := newFileEventLog(s.handler, req, filePath, nil, user, trial.token) if !c.Check(actual, check.NotNil) { continue } - c.Check(actual.userUUID, check.Equals, userUUID) - c.Check(actual.userFullName, check.Equals, userFullName) + c.Check(actual.userUUID, check.Equals, trial.uuid) + c.Check(actual.userFullName, check.Equals, trial.fullName) + c.Check(actual.clientToken, check.Equals, trial.token) } } @@ -259,7 +261,7 @@ func (s *UnitSuite) TestLogCollectionByUUID(c *check.C) { filePath := "/" + collUUID req := newRequest("GET", collURL+filePath) coll := newCollection(collUUID) - actual := newFileEventLog(s.handler, req, filePath, coll, nil) + actual := newFileEventLog(s.handler, req, filePath, coll, nil, "") if !c.Check(actual, check.NotNil) { continue } @@ -274,7 +276,7 @@ func (s *UnitSuite) TestLogCollectionByPDH(c *check.C) { filePath := "/PDHFile" req := newRequest("GET", collURL+filePath) coll := newCollection(collPDH) - actual := newFileEventLog(s.handler, req, filePath, coll, nil) + actual := newFileEventLog(s.handler, req, filePath, coll, nil, "") if !c.Check(actual, check.NotNil) { continue } @@ -288,7 +290,7 @@ func (s *UnitSuite) TestLogGETUUIDAsDict(c *check.C) { reqPath := "/c=" + arvadostest.FooCollection + filePath req := newRequest("GET", "http://keep-web.example"+reqPath) coll := newCollection(arvadostest.FooCollection) - logEvent := newFileEventLog(s.handler, req, filePath, coll, nil) + logEvent := newFileEventLog(s.handler, req, filePath, coll, nil, "") c.Assert(logEvent, check.NotNil) c.Check(logEvent.asDict(), check.DeepEquals, arvadosclient.Dict{ "event_type": "file_download", @@ -311,7 +313,7 @@ func (s *UnitSuite) TestLogGETPDHAsDict(c *check.C) { UUID: arvadostest.ActiveUserUUID, FullName: "Active User", } - logEvent := newFileEventLog(s.handler, req, filePath, coll, user) + logEvent := newFileEventLog(s.handler, req, filePath, coll, user, "") c.Assert(logEvent, check.NotNil) c.Check(logEvent.asDict(), check.DeepEquals, arvadosclient.Dict{ "event_type": "file_download", @@ -335,7 +337,7 @@ func (s *UnitSuite) TestLogUploadAsDict(c *check.C) { filePath := "/" + method + "File" reqPath := "/c=" + arvadostest.FooCollection + filePath req := newRequest(method, "http://keep-web.example"+reqPath) - logEvent := newFileEventLog(s.handler, req, filePath, coll, user) + logEvent := newFileEventLog(s.handler, req, filePath, coll, user, "") if !c.Check(logEvent, check.NotNil) { continue } @@ -356,7 +358,7 @@ func (s *UnitSuite) TestLogGETUUIDAsFields(c *check.C) { reqPath := "/c=" + arvadostest.FooCollection + filePath req := newRequest("GET", "http://keep-web.example"+reqPath) coll := newCollection(arvadostest.FooCollection) - logEvent := newFileEventLog(s.handler, req, filePath, coll, nil) + logEvent := newFileEventLog(s.handler, req, filePath, coll, nil, "") c.Assert(logEvent, check.NotNil) c.Check(logEvent.asFields(), check.DeepEquals, logrus.Fields{ "user_uuid": s.handler.Cluster.ClusterID + "-tpzed-anonymouspublic", @@ -375,7 +377,7 @@ func (s *UnitSuite) TestLogGETPDHAsFields(c *check.C) { UUID: arvadostest.ActiveUserUUID, FullName: "Active User", } - logEvent := newFileEventLog(s.handler, req, filePath, coll, user) + logEvent := newFileEventLog(s.handler, req, filePath, coll, user, "") c.Assert(logEvent, check.NotNil) c.Check(logEvent.asFields(), check.DeepEquals, logrus.Fields{ "user_uuid": arvadostest.ActiveUserUUID, @@ -396,7 +398,7 @@ func (s *UnitSuite) TestLogUploadAsFields(c *check.C) { filePath := "/" + method + "File" reqPath := "/c=" + arvadostest.FooCollection + filePath req := newRequest(method, "http://keep-web.example"+reqPath) - logEvent := newFileEventLog(s.handler, req, filePath, coll, user) + logEvent := newFileEventLog(s.handler, req, filePath, coll, user, "") if !c.Check(logEvent, check.NotNil) { continue } -- 2.39.5