Fetch quota by name before updating it (hashicorp#9466)

* Fix quotas update

* Update doc

vault/external_tests/quotas/quotas_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/sdk/logical"
+	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/vault/builtin/credential/userpass"
 	"github.com/hashicorp/vault/builtin/logical/pki"
@@ -126,6 +127,42 @@ func waitForRemovalOrTimeout(c *api.Client, path string, tick, to time.Duration)
 	}
 }
 
+func TestQuotas_RateLimitQuota_DupName(t *testing.T) {
+	conf, opts := teststorage.ClusterSetup(coreConfig, nil, nil)
+	cluster := vault.NewTestCluster(t, conf, opts)
+	cluster.Start()
+	defer cluster.Cleanup()
+	core := cluster.Cores[0].Core
+	client := cluster.Cores[0].Client
+	vault.TestWaitActive(t, core)
+
+	// create a rate limit quota w/ 'secret' path
+	_, err := client.Logical().Write("sys/quotas/rate-limit/secret-rlq", map[string]interface{}{
+		"rate":  7.7,
+		"burst": 8,
+		"path":  "secret",
+	})
+	require.NoError(t, err)
+
+	s, err := client.Logical().Read("sys/quotas/rate-limit/secret-rlq")
+	require.NoError(t, err)
+	require.NotEmpty(t, s.Data)
+
+	// create a rate limit quota w/ empty path (same name)
+	_, err = client.Logical().Write("sys/quotas/rate-limit/secret-rlq", map[string]interface{}{
+		"rate":  7.7,
+		"burst": 8,
+		"path":  "",
+	})
+	require.NoError(t, err)
+
+	// list again and verify that only 1 item is returned
+	s, err = client.Logical().List("sys/quotas/rate-limit")
+	require.NoError(t, err)
+
+	require.Len(t, s.Data, 1, "incorrect number of quotas")
+}
+
 func TestQuotas_RateLimitQuota_Mount(t *testing.T) {
 	conf, opts := teststorage.ClusterSetup(coreConfig, nil, nil)
 	cluster := vault.NewTestCluster(t, conf, opts)

vault/logical_system_quotas.go

@@ -162,18 +162,24 @@ func (b *SystemBackend) handleRateLimitQuotasUpdate() framework.OperationFunc {
 			}
 		}
 
-		// Disallow duplicate quotas with same precedence and similar
-		// properties.
-		quota, err := b.Core.quotaManager.QuotaByFactors(ctx, qType, ns.Path, mountPath)
+		// If a quota already exists, fetch and update it.
+		quota, err := b.Core.quotaManager.QuotaByName(qType, name)
 		if err != nil {
 			return nil, err
 		}
-		if quota != nil && quota.QuotaName() != name {
-			return logical.ErrorResponse("quota rule with similar properties exists under the name %q", quota.QuotaName()), nil
-		}
 
 		switch {
 		case quota == nil:
+			// Disallow creation of new quota that has properties similar to an
+			// existing quota.
+			quotaByFactors, err := b.Core.quotaManager.QuotaByFactors(ctx, qType, ns.Path, mountPath)
+			if err != nil {
+				return nil, err
+			}
+			if quotaByFactors != nil && quotaByFactors.QuotaName() != name {
+				return logical.ErrorResponse("quota rule with similar properties exists under the name %q", quotaByFactors.QuotaName()), nil
+			}
+
 			quota = quotas.NewRateLimitQuota(name, ns.Path, mountPath, rate, burst)
 		default:
 			rlq := quota.(*quotas.RateLimitQuota)

website/pages/api-docs/system/lease-count-quotas.mdx

@@ -27,7 +27,10 @@ that can either be a namespace or mount.
 - `path` `(string: "")` - Path of the mount or namespace to apply the quota.
   A blank path configures a global lease count quota. For example `namespace1/`
   adds a quota to a full namespace, `namespace1/auth/userpass` adds a quota to
-  `userpass` in `namespace1`.
+  `userpass` in `namespace1`. Updating this field on an existing quota can have
+  "moving" effects. For example, updating `auth/userpass` to
+  `namespace1/auth/userpass` moves this quota from being a global mount quota to a
+  namespace specific mount quota.
 - `max_leases` `(int: 0)` - Maximum number of leases allowed by the quota rule.
 
 ### Sample Payload

website/pages/api-docs/system/rate-limit-quotas.mdx

@@ -25,7 +25,10 @@ that can either be a namespace or mount.
 - `path` `(string: "")` - Path of the mount or namespace to apply the quota.
   A blank path configures a global rate limit quota. For example `namespace1/`
   adds a quota to a full namespace, `namespace1/auth/userpass` adds a quota to
-  `userpass` in `namespace1`. **Note, namespaces are supported in Enterprise only**.
+  `userpass` in `namespace1`. Updating this field on an existing quota can have
+  "moving" effects. For example, updating `auth/userpass` to
+  `namespace1/auth/userpass` moves this quota from being a global mount quota to a
+  namespace specific mount quota. **Note, namespaces are supported in Enterprise only**.
 - `rate` `(float: 0.0)` - The rate at which allowed requests are refilled per
   second by the quota rule. Internally, a token-bucket algorithm is used which
   has a size of `burst`, initially full. The quota limits requests to `rate`