You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/09/14 02:28:09 UTC

[GitHub] [apisix] spacewander commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin

spacewander commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r707856262



##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -79,7 +82,17 @@ local consumer_schema = {
             type = "boolean",
             title = "Whether to escape the uri parameter",
             default = true,
-        }
+        },
+        validate_request_body = {
+            type = "boolean",
+            title = "A boolean value telling the plugin to enable body validation.",
+            default = false,
+        },
+        max_req_body = {
+            type = "number",

Review comment:
       ```suggestion
               type = "integer",
   ```

##########
File path: docs/en/latest/plugins/hmac-auth.md
##########
@@ -278,6 +290,29 @@ Accept-Ranges: bytes
 <html lang="cn">
 ```
 
+### Enable request body checking
+
+```shell
+$ curl -X "POST" "http://localhost:9080/index.html?age=36&name=james" \
+     -H 'X-HMAC-ACCESS-KEY: zyedu-hmac-01' \
+     -H 'X-HMAC-SIGNATURE: ivlwjZPoVdSVvdSSM4drEFk9q9HS2jeJ5cAN9JffmdA=' \
+     -H 'X-HMAC-ALGORITHM: hmac-sha256' \
+     -H 'Date: Tue, 24 Aug 2021 03:19:21 GMT' \
+     -H 'X-HMAC-SIGNED-HEADERS: User-Agent;Digest' \
+     -H 'User-Agent: curl/7.29.0' \
+     -H 'Digest: L9b/+QMvhvnoUlSw5vq+kHPqnZiHGl61T8oavMVTaC4=' \
+     -H 'Content-Type: text/plain; charset=utf-8' \
+     -d "{\"hello\":\"world\"}"
+
+HTTP/1.1 200 OK

Review comment:
       Better to only show the first line? APISIX 2.2 doesn't support this feature. We can avoid confusing users by only show the first line.

##########
File path: docs/en/latest/plugins/hmac-auth.md
##########
@@ -51,6 +51,8 @@ The `consumer` then adds its key to request header to verify its request.
 | signed_headers    | array[string] | optional    |               |                                             | Restrict the headers that are added to the encrypted calculation. After the specified, the client request can only specify the headers within this range. When this item is empty, all the headers specified by the client request will be added to the encrypted calculation |
 | keep_headers      | boolean       | optional    | false         | [ true, false ]                             | Whether it is necessary to keep the request headers of `X-HMAC-SIGNATURE`, `X-HMAC-ALGORITHM` and `X-HMAC-SIGNED-HEADERS` in the http request after successful authentication. true: means to keep the http request header, false: means to remove the http request header.   |
 | encode_uri_params | boolean       | optional    | true          | [ true, false ]                             | Whether to encode the uri parameter in the signature, for example: `params1=hello%2Cworld` is encoded, `params2=hello,world` is not encoded. true: means to encode the uri parameter in the signature, false: not to encode the uri parameter in the signature.               |
+| validate_request_body | boolean  | optional   | false         | [ true, false ]                             | Whether to check request body. |
+| max_req_body     | number        | optional   | 512KB         |                                             | Max allowed body size. |

Review comment:
       Better to use a number instead of string in the doc's default value cell

##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -29,13 +29,16 @@ local hmac       = require("resty.hmac")
 local consumer   = require("apisix.consumer")
 local plugin     = require("apisix.plugin")
 local ngx_decode_base64 = ngx.decode_base64
+local ngx_encode_base64 = ngx.encode_base64
 
+local DIGEST = "Digest"

Review comment:
       Can we make this header configurable and default to "X-HMAC-DIGEST"?
   
   There is already a Digest header in the HTTP standard and it is not relative to the hmac.
   https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Digest




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org