bisakhmondal commented on a change in pull request #6201:
URL: https://github.com/apache/apisix/pull/6201#discussion_r794309154



##########
File path: apisix/plugins/csrf.lua
##########
@@ -129,6 +135,10 @@ function _M.access(conf, ctx)
         return
     end
 
+    if conf.expires == 0 then
+        return
+    end

Review comment:
       Ahh, sorry I confused you. I didn't mean to ignore csrf token 
validation. I meant we can ignore `token has expired check`.

##########
File path: docs/en/latest/plugins/csrf.md
##########
@@ -43,6 +43,8 @@ In the following we define `GET`, `HEAD` and `OPTIONS` as the 
`safe-methods` and
 | expires |  number | optional | `7200` | | Expiration time(s) of csrf cookie. 
|
 | key | string | required |  |  | The secret key used to encrypt the cookie. |
 
+**Note: When expires is set to 0 the plugin will ignore all checks**

Review comment:
       ```suggestion
   **Note: When expires is set to 0 the plugin will ignore checking if the 
token is expired or not.**
   ```

##########
File path: apisix/plugins/csrf.lua
##########
@@ -112,6 +113,11 @@ local function check_csrf_token(conf, ctx, token)
         core.log.error("no expires in token")
         return false
     end
+    local time_now = ngx_time()
+    if time_now - expires > conf.expires then
+        core.log.error("token has expired")
+        return false
+    end

Review comment:
       ```suggestion
       if conf.expires > 0 and time_now - expires > conf.expires then
           core.log.error("token has expired")
           return false
       end
   ```
   Just that's it.

##########
File path: t/plugin/csrf.t
##########
@@ -158,5 +159,92 @@ Cookie: 
apisix-csrf-token=eyJleHBpcmVzIjo3MjAwLCJyYW5kb20iOjAuMjE2ODAxOTYyNTEwND
 --- request
 POST /hello
 --- more_headers
-apisix-csrf-token: 
eyJzaWduIjoiZTlhNWVkOTBmZDc2YjRhMTYyMzg1ZDU2Y2ZhZDI1N2MxNmI0MWY1MjFjZWUwODczNzExM2NlYzZkZDQwMWJmNyIsInJhbmRvbSI6MC4zNjcxNDg2NDI2MjE0MywiZXhwaXJlcyI6NzIwMH0=
-Cookie: 
apisix-csrf-token=eyJzaWduIjoiZTlhNWVkOTBmZDc2YjRhMTYyMzg1ZDU2Y2ZhZDI1N2MxNmI0MWY1MjFjZWUwODczNzExM2NlYzZkZDQwMWJmNyIsInJhbmRvbSI6MC4zNjcxNDg2NDI2MjE0MywiZXhwaXJlcyI6NzIwMH0=
+apisix-csrf-token: 
eyJyYW5kb20iOjAuNDI5ODYzMTk3MTYxMzksInNpZ24iOiI0ODRlMDY4NTkxMWQ5NmJhMDc5YzQ1ZGI0OTE2NmZkYjQ0ODhjODVkNWQ0NmE1Y2FhM2UwMmFhZDliNjE5OTQ2IiwiZXhwaXJlcyI6MjY0MzExOTYyNH0=
+Cookie: 
apisix-csrf-token=eyJyYW5kb20iOjAuNDI5ODYzMTk3MTYxMzksInNpZ24iOiI0ODRlMDY4NTkxMWQ5NmJhMDc5YzQ1ZGI0OTE2NmZkYjQ0ODhjODVkNWQ0NmE1Y2FhM2UwMmFhZDliNjE5OTQ2IiwiZXhwaXJlcyI6MjY0MzExOTYyNH0=
+
+
+
+=== TEST 10: change expired
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                ngx.HTTP_PUT,
+                [[{
+                    "uri": "/hello",
+                    "upstream": {
+                        "type": "roundrobin",
+                        "nodes": {
+                            "127.0.0.1:1980": 1
+                        }
+                    },
+                    "plugins": {
+                        "csrf": {
+                            "key": "userkey",
+                            "expires": 1
+                        }
+                    }
+                }]]
+            )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 11: expired csrf token
+--- request
+POST /hello
+--- more_headers
+apisix-csrf-token: 
eyJyYW5kb20iOjAuMDY3NjAxMDQwMDM5MzI4LCJzaWduIjoiOTE1Yjg2MjBhNTg1N2FjZmIzNjIxOTNhYWVlN2RkYjY5NmM0NWYwZjE5YjY5Zjg3NjM4ZTllNGNjNjYxYjQwNiIsImV4cGlyZXMiOjE2NDMxMjAxOTN9
+Cookie: 
apisix-csrf-token=eyJyYW5kb20iOjAuMDY3NjAxMDQwMDM5MzI4LCJzaWduIjoiOTE1Yjg2MjBhNTg1N2FjZmIzNjIxOTNhYWVlN2RkYjY5NmM0NWYwZjE5YjY5Zjg3NjM4ZTllNGNjNjYxYjQwNiIsImV4cGlyZXMiOjE2NDMxMjAxOTN9
+--- error_code: 401
+--- error_log: token has expired
+
+
+
+=== TEST 12: set expires 0

Review comment:
       please update it accordingly. Split it into 2 cases
   - expires = 1 sec, sleep = 2 sec after drawing a token then check `token has 
expired`
   - expires = 0, sleep = 1, no expiration




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to