diff --git a/vault/activity_log_test.go b/vault/activity_log_test.go index 69887a39d497..6248e9667e7a 100644 --- a/vault/activity_log_test.go +++ b/vault/activity_log_test.go @@ -17,6 +17,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + "github.com/hashicorp/go-uuid" "github.com/axiomhq/hyperloglog" @@ -3987,6 +3989,55 @@ func TestActivityLog_partialMonthClientCountUsingHandleQuery(t *testing.T) { } } +// TestActivityLog_handleQuery_normalizedMountPaths ensures that the mount paths returned by the activity log always have a trailing slash and client accounting is done correctly when there's no trailing slash. +// Two clients that have the same mount path, but one has a trailing slash, should be considered part of the same mount path +func TestActivityLog_handleQuery_normalizedMountPaths(t *testing.T) { + timeutil.SkipAtEndOfMonth(t) + + core, _, _ := TestCoreUnsealed(t) + _, barrier, _ := mockBarrier(t) + view := NewBarrierView(barrier, "auth/") + ctx := namespace.RootContext(nil) + now := time.Now().UTC() + a := core.activityLog + a.SetEnable(true) + + uuid1, err := uuid.GenerateUUID() + require.NoError(t, err) + uuid2, err := uuid.GenerateUUID() + require.NoError(t, err) + accessor1 := "accessor1" + accessor2 := "accessor2" + pathWithSlash := "auth/foo/" + pathWithoutSlash := "auth/foo" + + // create two mounts of the same name. One has a trailing slash, the other doesn't + err = core.router.Mount(&NoopBackend{}, "auth/foo", &MountEntry{UUID: uuid1, Accessor: accessor1, NamespaceID: namespace.RootNamespaceID, namespace: namespace.RootNamespace, Path: pathWithSlash}, view) + require.NoError(t, err) + err = core.router.Mount(&NoopBackend{}, "auth/bar", &MountEntry{UUID: uuid2, Accessor: accessor2, NamespaceID: namespace.RootNamespaceID, namespace: namespace.RootNamespace, Path: pathWithoutSlash}, view) + require.NoError(t, err) + + // handle token usage for each of the mount paths + a.HandleTokenUsage(ctx, &logical.TokenEntry{Path: pathWithSlash, NamespaceID: namespace.RootNamespaceID}, "id1", false) + a.HandleTokenUsage(ctx, &logical.TokenEntry{Path: pathWithoutSlash, NamespaceID: namespace.RootNamespaceID}, "id2", false) + // and have client 2 use both mount paths + a.HandleTokenUsage(ctx, &logical.TokenEntry{Path: pathWithSlash, NamespaceID: namespace.RootNamespaceID}, "id2", false) + + // query the data for the month + results, err := a.handleQuery(ctx, timeutil.StartOfMonth(now), timeutil.EndOfMonth(now), 0) + require.NoError(t, err) + + byNamespace := results["by_namespace"].([]*ResponseNamespace) + require.Len(t, byNamespace, 1) + byMount := byNamespace[0].Mounts + require.Len(t, byMount, 1) + mountPath := byMount[0].MountPath + + // verify that both clients are recorded for the mount path with the slash + require.Equal(t, mountPath, pathWithSlash) + require.Equal(t, byMount[0].Counts.Clients, 2) +} + // TestActivityLog_partialMonthClientCountWithMultipleMountPaths verifies that logic in refreshFromStoredLog includes all mount paths // in its mount data. In this test we create 3 entity records with different mount accessors: one is empty, one is // valid, one can't be found (so it's assumed the mount is deleted). These records are written to storage, then this data is @@ -4006,7 +4057,7 @@ func TestActivityLog_partialMonthClientCountWithMultipleMountPaths(t *testing.T) } a := core.activityLog - path := "auth/foo/bar" + path := "auth/foo/bar/" accessor := "authfooaccessor" // we mount a path using the accessor 'authfooaccessor' which has mount path "auth/foo/bar" diff --git a/vault/activity_log_util_common.go b/vault/activity_log_util_common.go index 256b17aaf1cc..acdf51dcaa6d 100644 --- a/vault/activity_log_util_common.go +++ b/vault/activity_log_util_common.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "sort" + "strings" "time" "github.com/axiomhq/hyperloglog" @@ -277,6 +278,9 @@ func (a *ActivityLog) mountAccessorToMountPath(mountAccessor string) string { displayPath = fmt.Sprintf(deletedMountFmt, mountAccessor) } else { displayPath = valResp.MountPath + if !strings.HasSuffix(displayPath, "/") { + displayPath += "/" + } } } return displayPath