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 2020/11/26 11:30:19 UTC

[GitHub] [apisix] jenskeiner opened a new pull request #2859: fix: authz keycloak should run in access phase

jenskeiner opened a new pull request #2859:
URL: https://github.com/apache/apisix/pull/2859


   ### What this PR does / why we need it:
   See #2857.
   
   ### Pre-submission checklist:
   
   * [x] Did you explain what problem does this PR solve? Or what new features have been added?
   * [ ] Have you added corresponding test cases?
   * [x] Have you modified the corresponding document?
   * [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
   


----------------------------------------------------------------
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.

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



[GitHub] [apisix] jenskeiner commented on a change in pull request #2859: fix: authz-keycloak plugin should not be marked as auth-type and should run in access phase, basic-auth plugin should run in rewrite phase.

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r533169121



##########
File path: apisix/plugins/openid-connect.lua
##########
@@ -49,6 +49,7 @@ local schema = {
 local _M = {
     version = 0.1,
     priority = 2599,
+    type = 'auth',

Review comment:
       Sure, you're very welcome. Thanks for the info. I'll try to put the remaining outstanding changes from this one into a separate new PR and then we take it from there.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] jenskeiner commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r532423332



##########
File path: doc/plugin-develop.md
##########
@@ -168,8 +168,8 @@ local _M = {
 Determine which phase to run, generally access or rewrite. If you don't know the [Openresty life cycle](https://openresty-reference.readthedocs.io/en/latest/Directives/), it's
 recommended to know it in advance. For example key-auth is an authentication plugin, thus the authentication should be completed
 before forwarding the request to any upstream service. Therefore, the plugin can be executed in the rewrite and access phases.
-In APISIX, the authentication logic is implemented in the rewrite phase. Generally, IP access and interface
-permission are completed in the access phase.
+In APISIX, the authentication logic is implemented in the access phase. Generally, IP access and interface

Review comment:
       Done.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] membphis commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r532518453



##########
File path: apisix/plugins/basic-auth.lua
##########
@@ -125,7 +125,7 @@ do
     end
 end
 
-function _M.access(conf, ctx)
+function _M.rewrite(conf, ctx)

Review comment:
       why do we need to change another plugin?
   
   one PR for one thing, we should only update something about plugin `authz-keycloak`
   
   please revert this code

##########
File path: apisix/plugins/openid-connect.lua
##########
@@ -49,6 +49,7 @@ local schema = {
 local _M = {
     version = 0.1,
     priority = 2599,
+    type = 'auth',

Review comment:
       ditto
   
   you can create a new PR to fix this




----------------------------------------------------------------
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.

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



[GitHub] [apisix] jenskeiner commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r532422827



##########
File path: doc/plugin-develop.md
##########
@@ -168,8 +168,8 @@ local _M = {
 Determine which phase to run, generally access or rewrite. If you don't know the [Openresty life cycle](https://openresty-reference.readthedocs.io/en/latest/Directives/), it's
 recommended to know it in advance. For example key-auth is an authentication plugin, thus the authentication should be completed
 before forwarding the request to any upstream service. Therefore, the plugin can be executed in the rewrite and access phases.
-In APISIX, the authentication logic is implemented in the rewrite phase. Generally, IP access and interface
-permission are completed in the access phase.
+In APISIX, the authentication logic is implemented in the access phase. Generally, IP access and interface

Review comment:
       > We don't need to change it as authz-keycloak is for authorization.
   
   Reverted.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] jenskeiner commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r532423247



##########
File path: apisix/plugins/basic-auth.lua
##########
@@ -125,7 +125,7 @@ do
     end
 end
 
-function _M.access(conf, ctx)
+function _M.rewrite(conf, ctx)

Review comment:
       Happy to add a test case. I don't have much experience writing them yet, so if you could point to a similar example, that would be helpful.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] spacewander commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r531816904



##########
File path: apisix/plugins/basic-auth.lua
##########
@@ -125,7 +125,7 @@ do
     end
 end
 
-function _M.access(conf, ctx)
+function _M.rewrite(conf, ctx)

Review comment:
       Need a test case to check if the `ctx.consumer` set by this plugin can be used by other plugin runs in access phase.

##########
File path: doc/plugin-develop.md
##########
@@ -168,8 +168,8 @@ local _M = {
 Determine which phase to run, generally access or rewrite. If you don't know the [Openresty life cycle](https://openresty-reference.readthedocs.io/en/latest/Directives/), it's
 recommended to know it in advance. For example key-auth is an authentication plugin, thus the authentication should be completed
 before forwarding the request to any upstream service. Therefore, the plugin can be executed in the rewrite and access phases.
-In APISIX, the authentication logic is implemented in the rewrite phase. Generally, IP access and interface
-permission are completed in the access phase.
+In APISIX, the authentication logic is implemented in the access phase. Generally, IP access and interface

Review comment:
       Remember to revert this change.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] membphis commented on pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#issuecomment-734311999


   reply you later
   
   need more time to read your issue and pr first


----------------------------------------------------------------
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.

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



[GitHub] [apisix] jenskeiner commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r532438549



##########
File path: apisix/plugins/basic-auth.lua
##########
@@ -125,7 +125,7 @@ do
     end
 end
 
-function _M.access(conf, ctx)
+function _M.rewrite(conf, ctx)

Review comment:
       Ok, I think I've seen an example in the test cases for `key-auth`. I'll try to add test cases acceding for `basic-auth`.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] spacewander commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r532432466



##########
File path: apisix/plugins/basic-auth.lua
##########
@@ -125,7 +125,7 @@ do
     end
 end
 
-function _M.access(conf, ctx)
+function _M.rewrite(conf, ctx)

Review comment:
       You can add a stub:
   ```diff
   diff --git apisix/init.lua apisix/init.lua
   index 7333dd1..411cdd4 100644
   --- apisix/init.lua
   +++ apisix/init.lua
   @@ -514,6 +514,10 @@ function _M.http_access_phase()
                    api_ctx.consumer,
                    api_ctx
                )
   +
   +            core.log.info("find consumer ", api_ctx.consumer.username,
   +                          ", config changed: ", changed)
   +
                if changed then
                    core.table.clear(api_ctx.plugins)
                    api_ctx.plugins = plugin.filter(route, api_ctx.plugins)
   ```
   
   And then check it in:
   https://github.com/apache/apisix/blob/e66dd9a314e0b545e412f806b8b8d98f3b1a8f2d/t/plugin/basic-auth.t#L192
   
   Add something like:
   ```
   --- error_log
   find consumer foo
   ```




----------------------------------------------------------------
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.

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



[GitHub] [apisix] membphis commented on pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#issuecomment-734528857


   > @membphis @jenskeiner
   > `authz-keycloak` is for authorization but not authentication. It doesn't interact with consumers, so I think this change is fine.
   
   many thx for your explain. @spacewander 
   we can continue this PR


----------------------------------------------------------------
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.

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



[GitHub] [apisix] spacewander commented on pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#issuecomment-734525659


   @membphis @jenskeiner 
   `authz-keycloak` is for authorization but not authentication. It doesn't interact with consumers, so I think this change is fine.


----------------------------------------------------------------
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.

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



[GitHub] [apisix] jenskeiner commented on pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#issuecomment-734261094


   > Authentication type plug-ins should run in the rewrite phase
   
   How do you suggest this get's fixed then when `openid-connect` and other authentication plugins currently run in the access phase?
   
   The plugin development guidelines are currently not in line with the actual code base.
   
   Also, what's the rationale for running authentication/authorization in the *rewrite* phase when the *access* phase is the one that specifically caters for that purpose? It's not obvious why you would run in a different phase.


----------------------------------------------------------------
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.

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



[GitHub] [apisix] jenskeiner commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r531437514



##########
File path: doc/plugin-develop.md
##########
@@ -168,8 +168,8 @@ local _M = {
 Determine which phase to run, generally access or rewrite. If you don't know the [Openresty life cycle](https://openresty-reference.readthedocs.io/en/latest/Directives/), it's
 recommended to know it in advance. For example key-auth is an authentication plugin, thus the authentication should be completed
 before forwarding the request to any upstream service. Therefore, the plugin can be executed in the rewrite and access phases.
-In APISIX, the authentication logic is implemented in the rewrite phase. Generally, IP access and interface
-permission are completed in the access phase.
+In APISIX, the authentication logic is implemented in the access phase. Generally, IP access and interface

Review comment:
       Ok, I can revert this one. However, note that some authentication plugins currently run in the access phase, not the rewrite one. So the document doesn’t reflect the current state correctly, I believe.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] membphis commented on a change in pull request #2859: fix: authz-keycloak plugin should not be marked as auth-type and should run in access phase, basic-auth plugin should run in rewrite phase.

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r533061349



##########
File path: apisix/plugins/openid-connect.lua
##########
@@ -49,6 +49,7 @@ local schema = {
 local _M = {
     version = 0.1,
     priority = 2599,
+    type = 'auth',

Review comment:
       I found you have submitted new PR: https://github.com/apache/apisix/pull/2905/files
   
   Many thanks for your nice job.
   
   Small PR is important, it easy to find problems and we can merge it asap when it is small.
   
   By the way, it is a clearer way to show what we are doing.
   
   Many thx for nice contribution again @jenskeiner 




----------------------------------------------------------------
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.

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



[GitHub] [apisix] spacewander commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r531301855



##########
File path: doc/plugin-develop.md
##########
@@ -168,8 +168,8 @@ local _M = {
 Determine which phase to run, generally access or rewrite. If you don't know the [Openresty life cycle](https://openresty-reference.readthedocs.io/en/latest/Directives/), it's
 recommended to know it in advance. For example key-auth is an authentication plugin, thus the authentication should be completed
 before forwarding the request to any upstream service. Therefore, the plugin can be executed in the rewrite and access phases.
-In APISIX, the authentication logic is implemented in the rewrite phase. Generally, IP access and interface
-permission are completed in the access phase.
+In APISIX, the authentication logic is implemented in the access phase. Generally, IP access and interface

Review comment:
       We don't need to change it as authz-keycloak is for authorization.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] spacewander commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r531445397



##########
File path: doc/plugin-develop.md
##########
@@ -168,8 +168,8 @@ local _M = {
 Determine which phase to run, generally access or rewrite. If you don't know the [Openresty life cycle](https://openresty-reference.readthedocs.io/en/latest/Directives/), it's
 recommended to know it in advance. For example key-auth is an authentication plugin, thus the authentication should be completed
 before forwarding the request to any upstream service. Therefore, the plugin can be executed in the rewrite and access phases.
-In APISIX, the authentication logic is implemented in the rewrite phase. Generally, IP access and interface
-permission are completed in the access phase.
+In APISIX, the authentication logic is implemented in the access phase. Generally, IP access and interface

Review comment:
       @jenskeiner
   Thank you for the problem you point out. I just search for all plugins which has `type = 'auth'` (excluding the authz-keycloak) and find out `basic-auth` doesn't run the phase correctly.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] jenskeiner commented on a change in pull request #2859: fix: authz-keycloak plugin should not be marked as auth-type and should run in access phase, basic-auth plugin should run in rewrite phase.

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r533176699



##########
File path: apisix/plugins/openid-connect.lua
##########
@@ -49,6 +49,7 @@ local schema = {
 local _M = {
     version = 0.1,
     priority = 2599,
+    type = 'auth',

Review comment:
       Have created #2910 which should contain the outstanding changes from this PR.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] membphis commented on pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#issuecomment-734249801


   Authentication type plug-ins should run in the rewrite phase


----------------------------------------------------------------
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.

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



[GitHub] [apisix] jenskeiner commented on pull request #2859: fix: authz-keycloak plugin should not be marked as auth-type and should run in access phase, basic-auth plugin should run in rewrite phase.

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#issuecomment-736324964


   Closing this one as it has since been superseded by other PRs addressing the same issues.


----------------------------------------------------------------
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.

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



[GitHub] [apisix] jenskeiner closed pull request #2859: fix: authz-keycloak plugin should not be marked as auth-type and should run in access phase, basic-auth plugin should run in rewrite phase.

Posted by GitBox <gi...@apache.org>.
jenskeiner closed pull request #2859:
URL: https://github.com/apache/apisix/pull/2859


   


----------------------------------------------------------------
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.

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



[GitHub] [apisix] jenskeiner commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r532702571



##########
File path: apisix/plugins/openid-connect.lua
##########
@@ -49,6 +49,7 @@ local schema = {
 local _M = {
     version = 0.1,
     priority = 2599,
+    type = 'auth',

Review comment:
       I'm sorry but I currently can't afford the time to split this into separate PRs. I think the change set is reasonably small to be considered in one PR. If you prefer more fine-grained PRs, then please consider creating these yourself, e.g. based on the changes in this branch.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] jenskeiner commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r531735142



##########
File path: doc/plugin-develop.md
##########
@@ -168,8 +168,8 @@ local _M = {
 Determine which phase to run, generally access or rewrite. If you don't know the [Openresty life cycle](https://openresty-reference.readthedocs.io/en/latest/Directives/), it's
 recommended to know it in advance. For example key-auth is an authentication plugin, thus the authentication should be completed
 before forwarding the request to any upstream service. Therefore, the plugin can be executed in the rewrite and access phases.
-In APISIX, the authentication logic is implemented in the rewrite phase. Generally, IP access and interface
-permission are completed in the access phase.
+In APISIX, the authentication logic is implemented in the access phase. Generally, IP access and interface

Review comment:
       Ok, understood why you need to run in the rewrite phase. Then I would propose to change the phase for all authentication-type plugins to *rewrite*, instead of adjusting the phase for `authz-keycloak` from *rewrite* to *access*. Also, I think it would make the plugin development guidelines clearer if the reason they need to run in the rewrite phase was pointed out more clearly.
   
   If you're fine with this, I can update this PR accordingly. Just let me know what you think.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] spacewander commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r531447581



##########
File path: doc/plugin-develop.md
##########
@@ -168,8 +168,8 @@ local _M = {
 Determine which phase to run, generally access or rewrite. If you don't know the [Openresty life cycle](https://openresty-reference.readthedocs.io/en/latest/Directives/), it's
 recommended to know it in advance. For example key-auth is an authentication plugin, thus the authentication should be completed
 before forwarding the request to any upstream service. Therefore, the plugin can be executed in the rewrite and access phases.
-In APISIX, the authentication logic is implemented in the rewrite phase. Generally, IP access and interface
-permission are completed in the access phase.
+In APISIX, the authentication logic is implemented in the access phase. Generally, IP access and interface

Review comment:
       The reason why we need to run authentication plugins in the rewrite phases:
   https://github.com/apache/apisix/blob/f4161d39a495879edffbc75e2099f6e487bae81d/apisix/init.lua#L508-L520
   And:
   https://github.com/apache/apisix/blob/f4161d39a495879edffbc75e2099f6e487bae81d/apisix/admin/consumers.lua#L50
   
   Plugins has `type = 'auth'` should run in rewrite phase and set up the consumer for current api_ctx.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] spacewander commented on a change in pull request #2859: fix: authz keycloak should run in access phase

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2859:
URL: https://github.com/apache/apisix/pull/2859#discussion_r531443301



##########
File path: doc/plugin-develop.md
##########
@@ -168,8 +168,8 @@ local _M = {
 Determine which phase to run, generally access or rewrite. If you don't know the [Openresty life cycle](https://openresty-reference.readthedocs.io/en/latest/Directives/), it's
 recommended to know it in advance. For example key-auth is an authentication plugin, thus the authentication should be completed
 before forwarding the request to any upstream service. Therefore, the plugin can be executed in the rewrite and access phases.
-In APISIX, the authentication logic is implemented in the rewrite phase. Generally, IP access and interface
-permission are completed in the access phase.
+In APISIX, the authentication logic is implemented in the access phase. Generally, IP access and interface

Review comment:
       @jenskeiner
   We can fix it later, but in those plugins, not the documentation.




----------------------------------------------------------------
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.

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