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 2022/08/15 09:41:23 UTC

[GitHub] [apisix] tzssangglass opened a new pull request, #7683: change(authz-keycloak): remove deprecated audience

tzssangglass opened a new pull request, #7683:
URL: https://github.com/apache/apisix/pull/7683

   ### Description
   
   <!-- Please include a summary of the change and which issue is fixed. -->
   <!-- Please also include relevant motivation and context. -->
   
   Fixes #7482
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [ ] I have added tests corresponding to this change
   - [ ] I have updated the documentation to reflect this change
   - [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
   
   <!--
   
   Note
   
   1. Mark the PR as draft until it's ready to be reviewed.
   2. Always add/update tests for any changes unless you have a good reason.
   3. Always update the documentation to reflect the changes made in the PR.
   4. Make a new commit to resolve conversations instead of `push -f`.
   5. To resolve merge conflicts, merge master instead of rebasing.
   6. Use "request review" to notify the reviewer after making changes.
   7. Only a reviewer can mark a conversation as resolved.
   
   -->
   


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


[GitHub] [apisix] spacewander commented on a diff in pull request #7683: change(authz-keycloak): remove deprecated audience

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7683:
URL: https://github.com/apache/apisix/pull/7683#discussion_r946370232


##########
t/plugin/authz-keycloak.t:
##########
@@ -74,32 +74,7 @@ done
 
 
 
-=== TEST 3: minimal valid configuration with audience
---- config
-    location /t {
-        content_by_lua_block {
-            local plugin = require("apisix.plugins.authz-keycloak")
-            local ok, err = plugin.check_schema({
-                                audience = "foo",

Review Comment:
   Better to change it to use client_id instead of removing the test?



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


[GitHub] [apisix] tzssangglass commented on a diff in pull request #7683: change(authz-keycloak): remove deprecated audience

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7683:
URL: https://github.com/apache/apisix/pull/7683#discussion_r947383438


##########
t/plugin/authz-keycloak.t:
##########
@@ -74,32 +74,7 @@ done
 
 
 
-=== TEST 3: minimal valid configuration with audience
---- config
-    location /t {
-        content_by_lua_block {
-            local plugin = require("apisix.plugins.authz-keycloak")
-            local ok, err = plugin.check_schema({
-                                audience = "foo",

Review Comment:
   we have a test case cover `client_id`, ref: https://github.com/apache/apisix/blob/d3d6151c80fcdc162401429bb0920b381d3f2dd7/t/plugin/authz-keycloak.t#L52-L73
   
   so just remove this is ok.



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


[GitHub] [apisix] tzssangglass commented on a diff in pull request #7683: change(authz-keycloak): remove deprecated audience

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7683:
URL: https://github.com/apache/apisix/pull/7683#discussion_r947419651


##########
docs/en/latest/plugins/authz-keycloak.md:
##########
@@ -46,8 +46,7 @@ Refer to [Authorization Services Guide](https://www.keycloak.org/docs/latest/aut
 | discovery                                    | string        | False    |                                               | https://host.domain/auth/realms/foo/.well-known/uma2-configuration | URL to [discovery document](https://www.keycloak.org/docs/14.0/authorization_services/#_service_authorization_api) of Keycloak Authorization Services.                                                                                                |
 | token_endpoint                               | string        | False    |                                               | https://host.domain/auth/realms/foo/protocol/openid-connect/token  | An OAuth2-compliant token endpoint that supports the `urn:ietf:params:oauth:grant-type:uma-ticket` grant type. If provided, overrides the value from discovery.                                                                                       |
 | resource_registration_endpoint               | string        | False    |                                               | https://host.domain/auth/realms/foo/authz/protection/resource_set  | A UMA-compliant resource registration endpoint. If provided, overrides the value from discovery.                                                                                                                                                      |
-| client_id                                    | string        | False    |                                               |                                                                    | The identifier of the resource server to which the client is seeking access. Either `client_id` or `audience` is required.                                                                                                                            |
-| audience                                     | string        | False    |                                               |                                                                    | Legacy parameter now replaced by `client_id` kept for backwards compatibility. Either `client_id` or `audience` is required.                                                                                                                          |
+| client_id                                    | string        | False    |                                               |                                                                    | The identifier of the resource server to which the client is seeking access.                                                                                                                                                                         |

Review Comment:
   fixed



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


[GitHub] [apisix] spacewander merged pull request #7683: change(authz-keycloak): remove deprecated audience

Posted by GitBox <gi...@apache.org>.
spacewander merged PR #7683:
URL: https://github.com/apache/apisix/pull/7683


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


[GitHub] [apisix] tokers commented on a diff in pull request #7683: change(authz-keycloak): remove deprecated audience

Posted by GitBox <gi...@apache.org>.
tokers commented on code in PR #7683:
URL: https://github.com/apache/apisix/pull/7683#discussion_r947393088


##########
docs/en/latest/plugins/authz-keycloak.md:
##########
@@ -46,8 +46,7 @@ Refer to [Authorization Services Guide](https://www.keycloak.org/docs/latest/aut
 | discovery                                    | string        | False    |                                               | https://host.domain/auth/realms/foo/.well-known/uma2-configuration | URL to [discovery document](https://www.keycloak.org/docs/14.0/authorization_services/#_service_authorization_api) of Keycloak Authorization Services.                                                                                                |
 | token_endpoint                               | string        | False    |                                               | https://host.domain/auth/realms/foo/protocol/openid-connect/token  | An OAuth2-compliant token endpoint that supports the `urn:ietf:params:oauth:grant-type:uma-ticket` grant type. If provided, overrides the value from discovery.                                                                                       |
 | resource_registration_endpoint               | string        | False    |                                               | https://host.domain/auth/realms/foo/authz/protection/resource_set  | A UMA-compliant resource registration endpoint. If provided, overrides the value from discovery.                                                                                                                                                      |
-| client_id                                    | string        | False    |                                               |                                                                    | The identifier of the resource server to which the client is seeking access. Either `client_id` or `audience` is required.                                                                                                                            |
-| audience                                     | string        | False    |                                               |                                                                    | Legacy parameter now replaced by `client_id` kept for backwards compatibility. Either `client_id` or `audience` is required.                                                                                                                          |
+| client_id                                    | string        | False    |                                               |                                                                    | The identifier of the resource server to which the client is seeking access.                                                                                                                                                                         |

Review Comment:
   Now `client_id` is required.



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


[GitHub] [apisix] tzssangglass commented on a diff in pull request #7683: change(authz-keycloak): remove deprecated audience

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7683:
URL: https://github.com/apache/apisix/pull/7683#discussion_r948603841


##########
docs/zh/latest/plugins/authz-keycloak.md:
##########
@@ -73,7 +72,7 @@ description: 本文介绍了关于 Apache APISIX `authz-keycloak` 插件的基
     - 使用 `discovery` 属性后,`authz-keycloak` 插件就可以从其 URL 中发现 Keycloak API 的端点。该 URL 指向 Keyloak 针对相应领域授权服务的发现文档。
     - 如果发现文档可用,则插件将根据该文档确定令牌端点 URL。如果 URL 存在,则 `token_endpoint` 和 `resource_registration_endpoint` 的值将被其覆盖。
 - Client ID and secret
-    - 该插件需配置 `client_id` 或 `audience`(用于向后兼容)属性来标识自身,如果两者都已经配置,则 `client_id` 优先级更高。
+    - 该插件需配置 `client_id` 属性来标识自身,如果两者都已经配置,则 `client_id` 优先级更高。

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.

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

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


[GitHub] [apisix] starsz commented on a diff in pull request #7683: change(authz-keycloak): remove deprecated audience

Posted by GitBox <gi...@apache.org>.
starsz commented on code in PR #7683:
URL: https://github.com/apache/apisix/pull/7683#discussion_r948572810


##########
docs/zh/latest/plugins/authz-keycloak.md:
##########
@@ -73,7 +72,7 @@ description: 本文介绍了关于 Apache APISIX `authz-keycloak` 插件的基
     - 使用 `discovery` 属性后,`authz-keycloak` 插件就可以从其 URL 中发现 Keycloak API 的端点。该 URL 指向 Keyloak 针对相应领域授权服务的发现文档。
     - 如果发现文档可用,则插件将根据该文档确定令牌端点 URL。如果 URL 存在,则 `token_endpoint` 和 `resource_registration_endpoint` 的值将被其覆盖。
 - Client ID and secret
-    - 该插件需配置 `client_id` 或 `audience`(用于向后兼容)属性来标识自身,如果两者都已经配置,则 `client_id` 优先级更高。
+    - 该插件需配置 `client_id` 属性来标识自身,如果两者都已经配置,则 `client_id` 优先级更高。

Review Comment:
   ```suggestion
       - 该插件需配置 `client_id` 属性来标识自身
   ```



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