VAULT-40297 log warning when using allowed/denied params (#10444) (#10483)

* log warning when using allowed/denied params

* add changelog

Co-authored-by: Bruno Oliveira de Souza <bruno.souza@hashicorp.com>

changelog/_10444.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+policies: add warning about list comparison when using allowed_parameters or denied_parameters
+```

vault/policy_store.go

@@ -196,6 +196,9 @@ type PolicyStore struct {
 
 	// logger is the server logger copied over from core
 	logger log.Logger
+
+	// deniedParamWarn tracks if we've already logged about the system using allowed_parameters or denied_parameters
+	deniedParamWarnOnce sync.Once
 }
 
 // PolicyEntry is used to store a policy by name
@@ -412,6 +415,8 @@ func (ps *PolicyStore) setPolicyInternal(ctx context.Context, p *Policy) error {
 			ps.tokenPoliciesLRU.Add(index, p)
 		}
 
+		ps.logDeniedParamWarning(p)
+
 	case PolicyTypeRGP:
 		aclView := ps.getACLView(p.namespace)
 		acl, err := aclView.Get(ctx, entry.Key)
@@ -941,3 +946,27 @@ func (ps *PolicyStore) sanitizeName(name string) string {
 func (ps *PolicyStore) cacheKey(ns *namespace.Namespace, name string) string {
 	return path.Join(ns.ID, name)
 }
+
+// logDeniedParamWarning logs a warning if the given policy uses allowed_parameters or denied_parameters
+// TODO (DENIED_PARAMETERS_CHANGE): Remove this function after deprecation is done
+func (ps *PolicyStore) logDeniedParamWarning(p *Policy) {
+	if p == nil {
+		return
+	}
+	// check if the policy uses allowed_parameters or denied_parameters
+	var usesAllowedOrDenied bool
+	for _, path := range p.Paths {
+		if path.Permissions != nil {
+			if len(path.Permissions.AllowedParameters) > 0 || len(path.Permissions.DeniedParameters) > 0 {
+				usesAllowedOrDenied = true
+				break
+			}
+		}
+	}
+
+	if usesAllowedOrDenied {
+		ps.deniedParamWarnOnce.Do(func() {
+			ps.logger.Warn("you're using 'allowed_parameters' or 'denied_parameters' in one or more policies, note that Vault 1.21 introduced a behavior change. For details see https://developer.hashicorp.com/vault/docs/v1.21.x/updates/important-changes#item-by-item-list-comparison-for-allowed_parameters-and-denied_parameters")
+		})
+	}
+}

vault/policy_store_test.go

@@ -495,3 +495,86 @@ path "foo" {
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "failed to parse policy: failed to parse policy: The argument \"capabilities\" at 61:2 was already set. Each argument can only be defined once")
 }
+
+// TestPolicyStore_AllowedParametersWarning tests that a warning is logged when a policy containing
+// allowed_parameters or denied_parameters is set in the policy store.
+// TODO (DENIED_PARAMETERS_CHANGE): Remove this test after deprecation is done
+func TestPolicyStore_AllowedParametersWarning(t *testing.T) {
+	tests := []struct {
+		name           string
+		policyFragment string
+		expectLog      bool
+	}{
+		{
+			name: "allowed_parameters",
+			policyFragment: `
+path "foo" {
+ allowed_parameters = {
+  "param1" = ["val1", "val2"]
+ }
+ capabilities = ["read"]
+}
+`,
+			expectLog: true,
+		},
+		{
+			name: "denied_parameters",
+			policyFragment: `
+path "foo" {
+ denied_parameters = {
+  "param1" = ["val1", "val2"]
+ }
+ capabilities = ["read"]
+}
+`,
+			expectLog: true,
+		},
+		{
+			name: "no_parameters",
+			policyFragment: `
+path "foo" {
+ capabilities = ["read"]
+}
+`,
+			expectLog: false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			logOut := new(bytes.Buffer)
+			conf := &CoreConfig{
+				Logger: log.New(&log.LoggerOptions{
+					Mutex:  &sync.Mutex{},
+					Level:  log.Warn,
+					Output: logOut,
+				}),
+			}
+			core, _, _ := TestCoreUnsealedWithConfig(t, conf)
+			ps := core.policyStore
+
+			// First policy
+			policy := aclPolicy + tc.policyFragment
+			parsedPolicy, err := ParseACLPolicy(namespace.RootNamespace, policy)
+			require.NoError(t, err)
+
+			ctx := namespace.RootContext(context.Background())
+			err = ps.SetPolicy(ctx, parsedPolicy)
+			require.NoError(t, err)
+
+			if tc.expectLog {
+				require.Contains(t, logOut.String(), "you're using 'allowed_parameters' or 'denied_parameters' in one or more policies")
+			} else {
+				require.NotContains(t, logOut.String(), "you're using 'allowed_parameters' or 'denied_parameters' in one or more policies")
+			}
+
+			// Reset log output and add a second policy
+			logOut.Reset()
+			err = ps.SetPolicy(ctx, parsedPolicy)
+			require.NoError(t, err)
+
+			// Ensure no additional log is generated for the second policy
+			require.NotContains(t, logOut.String(), "you're using 'allowed_parameters' or 'denied_parameters' in one or more policies")
+		})
+	}
+}