Updating revoke/renew to prefer PUT method (#2646)

vault/logical_system.go

@@ -325,9 +325,13 @@ func NewSystemBackend(core *Core, config *logical.BackendConfig) (logical.Backen
 			},
 
 			&framework.Path{
-				Pattern: "revoke/(?P<lease_id>.+)",
+				Pattern: "revoke" + framework.OptionalParamRegex("url_lease_id"),
 
 				Fields: map[string]*framework.FieldSchema{
+					"url_lease_id": &framework.FieldSchema{
+						Type:        framework.TypeString,
+						Description: strings.TrimSpace(sysHelp["lease_id"][0]),
+					},
 					"lease_id": &framework.FieldSchema{
 						Type:        framework.TypeString,
 						Description: strings.TrimSpace(sysHelp["lease_id"][0]),
@@ -1278,6 +1282,10 @@ func (b *SystemBackend) handleRenew(
 	if leaseID == "" {
 		leaseID = data.Get("url_lease_id").(string)
 	}
+	if leaseID == "" {
+		return logical.ErrorResponse("lease_id must be specified"),
+			logical.ErrInvalidRequest
+	}
 	incrementRaw := data.Get("increment").(int)
 
 	// Convert the increment
@@ -1297,6 +1305,13 @@ func (b *SystemBackend) handleRevoke(
 	req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
 	// Get all the options
 	leaseID := data.Get("lease_id").(string)
+	if leaseID == "" {
+		leaseID = data.Get("url_lease_id").(string)
+	}
+	if leaseID == "" {
+		return logical.ErrorResponse("lease_id must be specified"),
+			logical.ErrInvalidRequest
+	}
 
 	// Invoke the expiration manager directly
 	if err := b.Core.expiration.Revoke(leaseID); err != nil {

vault/logical_system_test.go

@@ -423,6 +423,17 @@ func TestSystemBackend_renew_invalidID(t *testing.T) {
 	if resp.Data["error"] != "lease not found or lease is not renewable" {
 		t.Fatalf("bad: %v", resp)
 	}
+
+	// Attempt renew with other method
+	req = logical.TestRequest(t, logical.UpdateOperation, "renew")
+	req.Data["lease_id"] = "foobarbaz"
+	resp, err = b.HandleRequest(req)
+	if err != logical.ErrInvalidRequest {
+		t.Fatalf("err: %v", err)
+	}
+	if resp.Data["error"] != "lease not found or lease is not renewable" {
+		t.Fatalf("bad: %v", resp)
+	}
 }
 
 func TestSystemBackend_revoke(t *testing.T) {
@@ -471,12 +482,34 @@ func TestSystemBackend_revoke(t *testing.T) {
 	if resp3.Data["error"] != "lease not found or lease is not renewable" {
 		t.Fatalf("bad: %v", resp)
 	}
+
+	// Read a key with a LeaseID
+	req = logical.TestRequest(t, logical.ReadOperation, "secret/foo")
+	req.ClientToken = root
+	resp, err = core.HandleRequest(req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if resp == nil || resp.Secret == nil || resp.Secret.LeaseID == "" {
+		t.Fatalf("bad: %#v", resp)
+	}
+
+	// Test the other route path
+	req2 = logical.TestRequest(t, logical.UpdateOperation, "revoke")
+	req2.Data["lease_id"] = resp.Secret.LeaseID
+	resp2, err = b.HandleRequest(req2)
+	if err != nil {
+		t.Fatalf("err: %v %#v", err, resp2)
+	}
+	if resp2 != nil {
+		t.Fatalf("bad: %#v", resp)
+	}
 }
 
 func TestSystemBackend_revoke_invalidID(t *testing.T) {
 	b := testSystemBackend(t)
 
-	// Attempt renew
+	// Attempt revoke
 	req := logical.TestRequest(t, logical.UpdateOperation, "revoke/foobarbaz")
 	resp, err := b.HandleRequest(req)
 	if err != nil {
@@ -485,6 +518,17 @@ func TestSystemBackend_revoke_invalidID(t *testing.T) {
 	if resp != nil {
 		t.Fatalf("bad: %v", resp)
 	}
+
+	// Attempt revoke with other method
+	req = logical.TestRequest(t, logical.UpdateOperation, "revoke")
+	req.Data["lease_id"] = "foobarbaz"
+	resp, err = b.HandleRequest(req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if resp != nil {
+		t.Fatalf("bad: %v", resp)
+	}
 }
 
 func TestSystemBackend_revokePrefix(t *testing.T) {

website/source/api/system/renew.html.md

@@ -16,7 +16,7 @@ This endpoint renews a secret, requesting to extend the lease.
 
 | Method   | Path                         | Produces               |
 | :------- | :--------------------------- | :--------------------- |
-| `PUT`    | `/sys/renew/(:lease_id)`     | `200 application/json` |
+| `PUT`    | `/sys/renew`                 | `200 application/json` |
 
 ### Parameters
 
@@ -30,23 +30,13 @@ This endpoint renews a secret, requesting to extend the lease.
 
 ```json
 {
-  "lease_id": "postgresql/creds/readonly/abcd-1234...",
+  "lease_id": "aws/creds/deploy/abcd-1234...",
   "increment": 1800
 }
 ```
 
 ### Sample Request
 
-With the `lease_id` as part of the URL:
-
-```
-$ curl \
-    --header "X-Vault-Token: ..." \
-    --request PUT \
-    --data @payload.json \
-    https://vault.rocks/v1/sys/renew/postgresql/creds/readonly/abcd-1234
-```
-
 With the `lease_id` in the request body:
 
 ```
@@ -61,7 +51,7 @@ $ curl \
 
 ```json
 {
-  "lease_id": "aws/creds/deploy/e31b1145-ff27-e62c-cba2-934e9f0d1dbc",
+  "lease_id": "aws/creds/deploy/abcd-1234...",
   "renewable": true,
   "lease_duration": 2764790
 }

website/source/api/system/revoke.html.md

@@ -16,17 +16,26 @@ This endpoint revokes a secret immediately.
 
 | Method   | Path                         | Produces               |
 | :------- | :--------------------------- | :--------------------- |
-| `PUT`    | `/sys/revoke/:lease_id`      | `204 (empty body)`     |
+| `PUT`    | `/sys/revoke`                | `204 (empty body)`     |
 
 ### Parameters
 
 - `lease_id` `(string: <required>)` – Specifies the ID of the lease to revoke.
 
+### Sample Payload
+
+```json
+{
+  "lease_id": "postgresql/creds/readonly/abcd-1234..."
+}
+```
+
 ### Sample Request
 
 ```
 $ curl \
     --header "X-Vault-Token: ..." \
     --request PUT \
-    https://vault.rocks/v1/sys/revoke/aws/creds/readonly-acbd1234
+    --data @payload.json \
+    https://vault.rocks/v1/sys/revoke
 ```