Remove caseSensitivityKey invalidation

[akshya96]

Jira issue: https://hashicorp.atlassian.net/browse/VAULT-16845
Ent PR: https://github.com/hashicorp/vault-enterprise/pull/4233

A problem occurs when the caseSensitivityKey is invalidated due to a change in its value from True to False in storage, and duplicate groups are discovered. As a result, the loadGroups function returns an error, preventing the loading of groups and potentially causing missing groups in the memDB.

To resolve this issue, the proposed solution is to eliminate the invalidation case for the caseSensitivityKey. Since the value in storage can only be changed on the primary active node during initialization, there is no need for further invalidation. A warning is anyways being logged in loadIdentityStoreArtifacts function during the unseal process before initialization, if duplicate groups are encountered.

[davidadeleon]

If we are removing the invalidation for this key in storage, should we also look to remove the key from storage in general moving forward? The only place we update this value in storage is here:

vault/vault/identity_store.go

Lines 649 to 656 in 0d14718

	entry, err := logical.StorageEntryJSON(caseSensitivityKey, &casesensitivity{
	DisableLowerCasedNames: i.disableLowerCasedNames,
	})
	if err != nil {
	return err
	}
	return i.view.Put(ctx, entry)

Which looks to also be (after this change) the last reference for this struct:

vault/vault/identity_store_structs.go

Lines 114 to 116 in 0d14718

	type casesensitivity struct {
	DisableLowerCasedNames bool `json:"disable_lower_cased_names"`
	}

And this variable:

vault/vault/identity_store.go

Line 34 in 0d14718

caseSensitivityKey = "casesensitivity"

[maxb]

Indeed, isn't acting as an invalidation signal the whole reason for this key's existence?

Are you willing/able to share more reason about this change publicly? Once upon a time (years ago), with a previous employer, I witnessed a spontaneous change in case sensitivity from False to True in a production cluster, and we never did figure out how that was even possible. I'm kind of curious whether more information has come to light! :-)

[akshya96]

If we are removing the invalidation for this key in storage, should we also look to remove the key from storage in general moving forward? The only place we update this value in storage is here:

vault/vault/identity_store.go

Lines 649 to 656 in 0d14718

	entry, err := logical.StorageEntryJSON(caseSensitivityKey, &casesensitivity{
	DisableLowerCasedNames: i.disableLowerCasedNames,
	})
	if err != nil {
	return err
	}
	return i.view.Put(ctx, entry)

Which looks to also be (after this change) the last reference for this struct:

vault/vault/identity_store_structs.go

Lines 114 to 116 in 0d14718

	type casesensitivity struct {
	DisableLowerCasedNames bool `json:"disable_lower_cased_names"`
	}

And this variable:

vault/vault/identity_store.go

Line 34 in 0d14718

caseSensitivityKey = "casesensitivity"

I agree that since the invalidation of the key is removed, we can remove the key itself but I think we will still need this field in the identitystore because it uses it to create the schema. I updated the PR with the changes.

[ncabatoff]

Are you willing/able to share more reason about this change publicly? Once upon a time (years ago), with a previous employer, I witnessed a spontaneous change in case sensitivity from False to True in a production cluster, and we never did figure out how that was even possible. I'm kind of curious whether more information has come to light! :-)

Until #20964 we allowed for duplicate groups to be created due to a bug. Once duplicate groups exist, on load we modify the in-memory setting, then during the initialize call of the identitystore we'll modify the stored setting.

[raskchanky]

I read the Slack discussion and the Jira and removing invalidation makes sense given the key isn't supposed to change, but I'm not following the part about removing the bits from initialize() that write the key to storage. If it's not in storage, where does the default value come from?

[davidadeleon]

I read the Slack discussion and the Jira and removing invalidation makes sense given the key isn't supposed to change, but I'm not following the part about removing the bits from initialize() that write the key to storage. If it's not in storage, where does the default value come from?

The IdentityStore struct has its own variable for case sensitivity which defaults to false. When we initialize a new Identity store it will be false, and only internally set to true if we find any duplicate groups when we load them on initialization.

vault/vault/identity_store_structs.go

Line 94 in 0d14718

disableLowerCasedNames bool

[raskchanky]

Based on the background info I read, I think this is ok?

[ncabatoff]

You don't need a Get, you can just Delete - it won't return an error if the thing is already missing.

[ncabatoff]

Although now that I think about it, let's keep this, and add a log line where we record the value it had at time of deletion. Could be useful in debugging at some point.

[akshya96]

added debug line to get caseSensitivityKey value at the time of deletion