Skip to content

Commit

Permalink
Remove caseSensitivityKey invalidation (#20965)
Browse files Browse the repository at this point in the history
* removing caseSensitivityKey invalidation

* add changelog

* remove caseSensitivityKey

* removing loadCaseSensitivityKeyStore

* add go test to check caseSensitivityKey deletion

* edit return error messages during deletion of caseSensitivityKey

* adding log line to record caseSensitivityKey value

* modifying error check

* addressing comments
  • Loading branch information
akshya96 authored Jul 14, 2023
1 parent b9251da commit e1a9d85
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 62 deletions.
3 changes: 3 additions & 0 deletions changelog/20965.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
identity: Remove caseSensitivityKey to prevent errors while loading groups which could result in missing groups in memDB when duplicates are found.
```
4 changes: 0 additions & 4 deletions vault/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -593,10 +593,6 @@ type Core struct {
// active, or give up active as soon as it gets it
neverBecomeActive *uint32

// loadCaseSensitiveIdentityStore enforces the loading of identity store
// artifacts in a case sensitive manner. To be used only in testing.
loadCaseSensitiveIdentityStore bool

// clusterListener starts up and manages connections on the cluster ports
clusterListener *atomic.Value

Expand Down
66 changes: 22 additions & 44 deletions vault/identity_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,14 +576,31 @@ func (i *IdentityStore) initialize(ctx context.Context, req *logical.Initializat
return err
}

entry, err := logical.StorageEntryJSON(caseSensitivityKey, &casesensitivity{
DisableLowerCasedNames: i.disableLowerCasedNames,
})
// if the storage entry for caseSensitivityKey exists, remove it
storageEntry, err := i.view.Get(ctx, caseSensitivityKey)
if err != nil {
return err
i.logger.Error("could not get storage entry for case sensitivity key", "error", err)
return nil
}

if storageEntry != nil {
var setting casesensitivity
err := storageEntry.DecodeJSON(&setting)
switch err {
case nil:
i.logger.Debug("removing storage entry for case sensitivity key", "value", setting.DisableLowerCasedNames)
default:
i.logger.Error("failed to decode case sensitivity key, removing its storage entry anyway", "error", err)
}

err = i.view.Delete(ctx, caseSensitivityKey)
if err != nil {
i.logger.Error("could not delete storage entry for case sensitivity key", "error", err)
return nil
}
}

return i.view.Put(ctx, entry)
return nil
}

// Invalidate is a callback wherein the backend is informed that the value at
Expand All @@ -597,45 +614,6 @@ func (i *IdentityStore) Invalidate(ctx context.Context, key string) {
defer i.lock.Unlock()

switch {
case key == caseSensitivityKey:
entry, err := i.view.Get(ctx, caseSensitivityKey)
if err != nil {
i.logger.Error("failed to read case sensitivity setting during invalidation", "error", err)
return
}
if entry == nil {
return
}

var setting casesensitivity
if err := entry.DecodeJSON(&setting); err != nil {
i.logger.Error("failed to decode case sensitivity setting during invalidation", "error", err)
return
}

// Fast return if the setting is the same
if i.disableLowerCasedNames == setting.DisableLowerCasedNames {
return
}

// If the setting is different, reset memdb and reload all the artifacts
i.disableLowerCasedNames = setting.DisableLowerCasedNames
if err := i.resetDB(ctx); err != nil {
i.logger.Error("failed to reset memdb during invalidation", "error", err)
return
}
if err := i.loadEntities(ctx); err != nil {
i.logger.Error("failed to load entities during invalidation", "error", err)
return
}
if err := i.loadGroups(ctx); err != nil {
i.logger.Error("failed to load groups during invalidation", "error", err)
return
}
if err := i.loadOIDCClients(ctx); err != nil {
i.logger.Error("failed to load OIDC clients during invalidation", "error", err)
return
}
// Check if the key is a storage entry key for an entity bucket
case strings.HasPrefix(key, storagepacker.StoragePackerBucketsPrefix):
// Create a MemDB transaction
Expand Down
56 changes: 56 additions & 0 deletions vault/identity_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -857,3 +857,59 @@ func TestIdentityStore_UpdateAliasMetadataPerAccessor(t *testing.T) {
t.Fatalf("wrong alias index changed. Expected 1, got %d", i)
}
}

// TestIdentityStore_DeleteCaseSensitivityKey tests that
// casesensitivity key gets removed from storage if it exists upon
// initializing identity store.
func TestIdentityStore_DeleteCaseSensitivityKey(t *testing.T) {
c, unsealKey, root := TestCoreUnsealed(t)
ctx := context.Background()

// add caseSensitivityKey to storage
entry, err := logical.StorageEntryJSON(caseSensitivityKey, &casesensitivity{
DisableLowerCasedNames: true,
})
if err != nil {
t.Fatal(err)
}
err = c.identityStore.view.Put(ctx, entry)
if err != nil {
t.Fatal(err)
}

// check if the value is stored in storage
storageEntry, err := c.identityStore.view.Get(ctx, caseSensitivityKey)
if err != nil {
t.Fatal(err)
}

if storageEntry == nil {
t.Fatalf("bad: expected a non-nil entry for casesensitivity key")
}

// Seal and unseal to trigger identityStore initialize
if err = c.Seal(root); err != nil {
t.Fatal(err)
}

var unsealed bool
for i := 0; i < len(unsealKey); i++ {
unsealed, err = c.Unseal(unsealKey[i])
if err != nil {
t.Fatal(err)
}
}
if !unsealed {
t.Fatal("still sealed")
}

// check if caseSensitivityKey exists after initialize
storageEntry, err = c.identityStore.view.Get(ctx, caseSensitivityKey)
if err != nil {
t.Fatal(err)
}

if storageEntry != nil {
t.Fatalf("bad: expected no entry for casesensitivity key")
}
}
22 changes: 8 additions & 14 deletions vault/identity_store_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ var (
tmpSuffix = ".tmp"
)

func (c *Core) SetLoadCaseSensitiveIdentityStore(caseSensitive bool) {
c.loadCaseSensitiveIdentityStore = caseSensitive
}

func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error {
if c.identityStore == nil {
c.logger.Warn("identity store is not setup, skipping loading")
Expand All @@ -58,16 +54,14 @@ func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error {
return nil
}

if !c.loadCaseSensitiveIdentityStore {
// Load everything when memdb is set to operate on lower cased names
err := loadFunc(ctx)
switch {
case err == nil:
// If it succeeds, all is well
return nil
case !errwrap.Contains(err, errDuplicateIdentityName.Error()):
return err
}
// Load everything when memdb is set to operate on lower cased names
err := loadFunc(ctx)
switch {
case err == nil:
// If it succeeds, all is well
return nil
case !errwrap.Contains(err, errDuplicateIdentityName.Error()):
return err
}

c.identityStore.logger.Warn("enabling case sensitive identity names")
Expand Down

0 comments on commit e1a9d85

Please sign in to comment.