Transit UX improvements: show key policy, configs on write (hashicorp…

…#20652) * Respond with cache size on config write Signed-off-by: Alexander Scheel <[email protected]> * Respond with key policy on write This includes creating a key, but also trimming or rotating an existing key. Signed-off-by: Alexander Scheel <[email protected]> * Add changelog entry Signed-off-by: Alexander Scheel <[email protected]> * Correctly handle locking around policy formatting Signed-off-by: Alexander Scheel <[email protected]> * Validate that responses are non-empty Signed-off-by: Alexander Scheel <[email protected]> --------- Signed-off-by: Alexander Scheel <[email protected]>
Nordix · May 18, 2023 · d52d307 · d52d307
1 parent a98e239
commit d52d307
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 35 deletions.
diff --git a/builtin/logical/transit/backend_test.go b/builtin/logical/transit/backend_test.go
@@ -1071,9 +1071,7 @@ func testConvergentEncryptionCommon(t *testing.T, ver int, keyType keysutil.KeyT
 	if err != nil {
 		t.Fatal(err)
 	}
-	if resp != nil {
-		t.Fatal("expected nil response")
-	}
+	require.NotNil(t, resp, "expected populated request")
 
 	p, err := keysutil.LoadPolicy(context.Background(), storage, path.Join("policy", "testkey"))
 	if err != nil {
@@ -1559,9 +1557,7 @@ func TestBadInput(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if resp != nil {
-		t.Fatal("expected nil response")
-	}
+	require.NotNil(t, resp, "expected populated request")
 
 	req.Path = "decrypt/test"
 	req.Data = map[string]interface{}{
@@ -1650,9 +1646,7 @@ func TestTransit_AutoRotateKeys(t *testing.T) {
 				if err != nil {
 					t.Fatal(err)
 				}
-				if resp != nil {
-					t.Fatal("expected nil response")
-				}
+				require.NotNil(t, resp, "expected populated request")
 
 				// Write a key with an auto rotate value one day in the future
 				req = &logical.Request{
@@ -1667,9 +1661,7 @@ func TestTransit_AutoRotateKeys(t *testing.T) {
 				if err != nil {
 					t.Fatal(err)
 				}
-				if resp != nil {
-					t.Fatal("expected nil response")
-				}
+				require.NotNil(t, resp, "expected populated request")
 
 				// Run the rotation check and ensure none of the keys have rotated
 				b.checkAutoRotateAfter = time.Now()

diff --git a/builtin/logical/transit/path_cache_config.go b/builtin/logical/transit/path_cache_config.go
@@ -84,7 +84,11 @@ func (b *backend) pathCacheConfigWrite(ctx context.Context, req *logical.Request
 		return nil, err
 	}
 
-	return nil, nil
+	return &logical.Response{
+		Data: map[string]interface{}{
+			"size": cacheSize,
+		},
+	}, nil
 }
 
 type configCache struct {

diff --git a/builtin/logical/transit/path_keys.go b/builtin/logical/transit/path_keys.go
@@ -257,12 +257,14 @@ func (b *backend) pathPolicyWrite(ctx context.Context, req *logical.Request, d *
 		p.Unlock()
 	}
 
-	resp := &logical.Response{}
+	resp, err := b.formatKeyPolicy(p, nil)
+	if err != nil {
+		return nil, err
+	}
 	if !upserted {
 		resp.AddWarning(fmt.Sprintf("key %s already existed", name))
 	}
-
-	return nil, nil
+	return resp, nil
 }
 
 // Built-in helper type for returning asymmetric keys
@@ -290,6 +292,19 @@ func (b *backend) pathPolicyRead(ctx context.Context, req *logical.Request, d *f
 	}
 	defer p.Unlock()
 
+	contextRaw := d.Get("context").(string)
+	var context []byte
+	if len(contextRaw) != 0 {
+		context, err = base64.StdEncoding.DecodeString(contextRaw)
+		if err != nil {
+			return logical.ErrorResponse("failed to base64-decode context"), logical.ErrInvalidRequest
+		}
+	}
+
+	return b.formatKeyPolicy(p, context)
+}
+
+func (b *backend) formatKeyPolicy(p *keysutil.Policy, context []byte) (*logical.Response, error) {
 	// Return the response
 	resp := &logical.Response{
 		Data: map[string]interface{}{
@@ -346,15 +361,6 @@ func (b *backend) pathPolicyRead(ctx context.Context, req *logical.Request, d *f
 		}
 	}
 
-	contextRaw := d.Get("context").(string)
-	var context []byte
-	if len(contextRaw) != 0 {
-		context, err = base64.StdEncoding.DecodeString(contextRaw)
-		if err != nil {
-			return logical.ErrorResponse("failed to base64-decode context"), logical.ErrInvalidRequest
-		}
-	}
-
 	switch p.Type {
 	case keysutil.KeyType_AES128_GCM96, keysutil.KeyType_AES256_GCM96, keysutil.KeyType_ChaCha20_Poly1305:
 		retKeys := map[string]int64{}

diff --git a/builtin/logical/transit/path_keys_config.go b/builtin/logical/transit/path_keys_config.go
@@ -97,6 +97,8 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request,
 	}
 	defer p.Unlock()
 
+	var warning string
+
 	originalMinDecryptionVersion := p.MinDecryptionVersion
 	originalMinEncryptionVersion := p.MinEncryptionVersion
 	originalDeletionAllowed := p.DeletionAllowed
@@ -113,8 +115,6 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request,
 		}
 	}()
 
-	resp = &logical.Response{}
-
 	persistNeeded := false
 
 	minDecryptionVersionRaw, ok := d.GetOk("min_decryption_version")
@@ -127,7 +127,7 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request,
 
 		if minDecryptionVersion == 0 {
 			minDecryptionVersion = 1
-			resp.AddWarning("since Vault 0.3, transit key numbering starts at 1; forcing minimum to 1")
+			warning = "since Vault 0.3, transit key numbering starts at 1; forcing minimum to 1"
 		}
 
 		if minDecryptionVersion != p.MinDecryptionVersion {
@@ -221,7 +221,14 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request,
 	}
 
 	if !persistNeeded {
-		return nil, nil
+		resp, err := b.formatKeyPolicy(p, nil)
+		if err != nil {
+			return nil, err
+		}
+		if warning != "" {
+			resp.AddWarning(warning)
+		}
+		return resp, nil
 	}
 
 	switch {
@@ -231,11 +238,18 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request,
 		return logical.ErrorResponse("min decryption version should not be less then min available version"), nil
 	}
 
-	if len(resp.Warnings) == 0 {
-		return nil, p.Persist(ctx, req.Storage)
+	if err := p.Persist(ctx, req.Storage); err != nil {
+		return nil, err
 	}
 
-	return resp, p.Persist(ctx, req.Storage)
+	resp, err = b.formatKeyPolicy(p, nil)
+	if err != nil {
+		return nil, err
+	}
+	if warning != "" {
+		resp.AddWarning(warning)
+	}
+	return resp, nil
 }
 
 const pathKeysConfigHelpSyn = `Configure a named encryption key`

diff --git a/builtin/logical/transit/path_rotate.go b/builtin/logical/transit/path_rotate.go
@@ -64,6 +64,7 @@ func (b *backend) pathRotateWrite(ctx context.Context, req *logical.Request, d *
 	if !b.System().CachingDisabled() {
 		p.Lock(true)
 	}
+	defer p.Unlock()
 
 	if p.Type == keysutil.KeyType_MANAGED_KEY {
 		var keyId string
@@ -78,8 +79,11 @@ func (b *backend) pathRotateWrite(ctx context.Context, req *logical.Request, d *
 		err = p.Rotate(ctx, req.Storage, b.GetRandomReader())
 	}
 
-	p.Unlock()
-	return nil, err
+	if err != nil {
+		return nil, err
+	}
+
+	return b.formatKeyPolicy(p, nil)
 }
 
 const pathRotateHelpSyn = `Rotate named encryption key`

diff --git a/builtin/logical/transit/path_trim.go b/builtin/logical/transit/path_trim.go
@@ -100,7 +100,7 @@ func (b *backend) pathTrimUpdate() framework.OperationFunc {
 			return nil, err
 		}
 
-		return nil, nil
+		return b.formatKeyPolicy(p, nil)
 	}
 }
 

diff --git a/changelog/20652.txt b/changelog/20652.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/transit: Respond to writes with updated key policy, cache configuration.
+```
-Original file line number
+Diff line change
@@ Expand Up @@
     			return nil, err
     		}
-    		return nil, nil
+    		return b.formatKeyPolicy(p, nil)
     	}
     }
@@ Expand Down @@