You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "illidan33 (via GitHub)" <gi...@apache.org> on 2024/02/06 07:33:39 UTC
[PR] fix: Set the session configuration [apisix]
illidan33 opened a new pull request, #10919:
URL: https://github.com/apache/apisix/pull/10919
### Description
Added the configuration items related to resty.session. You can set the behavior and expiration time of the session through the configuration items to avoid the problem that the access token does not expire but the session expires when the expiration time of the access token is longer than the session.
Fixes # (issue)
https://github.com/apache/apisix/issues/10797
### Checklist
- [ ] I have explained the need for this PR and the problem it solves
- [ ] 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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "illidan33 (via GitHub)" <gi...@apache.org>.
illidan33 commented on PR #10919:
URL: https://github.com/apache/apisix/pull/10919#issuecomment-1978054645
> 
>
> The quality of the english doc is subpar and I cannot understand what it's saying. Please hold the review to a higher standard.
>
> My questions:
>
> * `This can be configured with Nginx set $session_cookie_lifetime 3600`: does this mean this new configuration option just makes it feasible to configure `session_cookie_lifetime` dynamically at runtime?
> * The next sentence is grammatically incorrect and I cannot understand, @shreemaan-abhishek please rephrase it makes sense to you.
> * `it is used if the cookies are configured persistent with session.cookie.persistent == true`: where is this configuration? it is not in this plugin.
> * The Chinese doc also says `for more information see ssl_session_timeout`. Where is this `ssl_session_timeout`
hi kayx23, the documentation is sourced from https://github.com/bungle/lua-resty-session/tree/v3.10, where it describes the `session.cookie.lifetime`. This update only adds the configuration for `lifetime`, and the description should only focus on the role of `lifetime`. I have modified the documentation. Do I need to submit another 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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10919:
URL: https://github.com/apache/apisix/pull/10919#discussion_r1505488422
##########
t/plugin/openid-connect6.t:
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+# no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: Check configuration of cookie
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {
+ client_id = "course_management",
+ client_secret = "tbsmDOpsHwdgIqYl2NltGRTKzjIzvEmT",
+ discovery = "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ session = {
+ secret = "6S8IO+Pydgb33LIor8T9ClER0T/sglFAjClFeAF3RsY=",
+ cookie = {
+ lifetime = 100
+ }
+ }
+ },
+ }
+ local plugin = require("apisix.plugins.openid-connect")
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+
+
+
+=== TEST 2: Set up new route access the auth server
+--- 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,
+ [[{
+ "plugins": {
+ "openid-connect": {
+ "discovery": "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "bearer_only" : false,
+ "timeout": 10,
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "required_scopes": ["profile"],
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "introspection_endpoint": "http://127.0.0.1:8080/realms/University/protocol/openid-connect/token/introspect",
+ "set_access_token_header": true,
+ "access_token_in_authorization_header": false,
+ "set_id_token_header": true,
+ "set_userinfo_header": true,
+ "set_refresh_token_header": true,
+ "session": {
+ "secret": "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "cookie": {
+ "lifetime": 100
+ }
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 3: Call to route to get session
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies = require("lib.keycloak").concatenate_cookies
+
+ local current_time = os.time()
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello"
+
+ local res, err = login_keycloak(uri, "teacher@gmail.com", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+ local parts = {}
+ for part in string.gmatch(cookie_str, "[^|]+") do
+ table.insert(parts, part)
+ end
+ local target_number = tonumber(parts[2], 10) - 100
Review Comment:
If you don't set the `cookie.lifetime`, what will the `target_number` be?
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10919:
URL: https://github.com/apache/apisix/pull/10919#discussion_r1507103630
##########
docs/en/latest/plugins/openid-connect.md:
##########
@@ -61,6 +61,8 @@ description: OpenID Connect allows the client to obtain user information from th
| set_refresh_token_header | boolean | False | false | | When set to true and a refresh token object is available, sets it in the `X-Refresh-Token` request header. |
| session | object | False | | | When bearer_only is set to false, openid-connect will use Authorization Code flow to authenticate on the IDP, so you need to set the session-related configuration. |
| session.secret | string | True | Automatic generation | 16 or more characters | The key used for session encrypt and HMAC operation. |
+| session.cookie | integer | False | | | |
Review Comment:
It's a map to hold the `lifetime`, does it need some description?
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on code in PR #10919:
URL: https://github.com/apache/apisix/pull/10919#discussion_r1507095021
##########
docs/en/latest/plugins/openid-connect.md:
##########
@@ -61,6 +61,8 @@ description: OpenID Connect allows the client to obtain user information from th
| set_refresh_token_header | boolean | False | false | | When set to true and a refresh token object is available, sets it in the `X-Refresh-Token` request header. |
| session | object | False | | | When bearer_only is set to false, openid-connect will use Authorization Code flow to authenticate on the IDP, so you need to set the session-related configuration. |
| session.secret | string | True | Automatic generation | 16 or more characters | The key used for session encrypt and HMAC operation. |
+| session.cookie | integer | False | | | |
Review Comment:
missing description for `session.cookie`
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek merged PR #10919:
URL: https://github.com/apache/apisix/pull/10919
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "illidan33 (via GitHub)" <gi...@apache.org>.
illidan33 commented on code in PR #10919:
URL: https://github.com/apache/apisix/pull/10919#discussion_r1503852399
##########
t/plugin/openid-connect6.t:
##########
@@ -0,0 +1,154 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+# no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: Check configuration of cookie
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {
+ client_id = "course_management",
+ client_secret = "tbsmDOpsHwdgIqYl2NltGRTKzjIzvEmT",
+ discovery = "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ session = {
+ secret = "6S8IO+Pydgb33LIor8T9ClER0T/sglFAjClFeAF3RsY=",
+ cookie = {
+ lifetime = 100
+ }
+ }
+ },
+ }
+ local plugin = require("apisix.plugins.openid-connect")
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+
+=== TEST 2: Set up new route access the auth server
+--- 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,
+ [[{
+ "plugins": {
+ "openid-connect": {
+ "discovery": "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "bearer_only" : false,
+ "timeout": 10,
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "required_scopes": ["profile"],
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "introspection_endpoint": "http://127.0.0.1:8080/realms/University/protocol/openid-connect/token/introspect",
+ "set_access_token_header": true,
+ "access_token_in_authorization_header": false,
+ "set_id_token_header": true,
+ "set_userinfo_header": true,
+ "set_refresh_token_header": true,
+ "session": {
+ "secret": "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "cookie": {
+ "lifetime": 100
+ }
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+=== TEST 3: Call to route to get session
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies = require("lib.keycloak").concatenate_cookies
+
+ local current_time = os.time()
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello"
+
+ local res, err = login_keycloak(uri, "teacher@gmail.com", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+ local parts = {}
+ for part in string.gmatch(cookie_str, "[^|]+") do
+ table.insert(parts, part)
+ end
+ local target_number = tonumber(parts[2], 10) - 100
+ -- ngx.say(target_number, current_time)
+ -- It takes time to execute the request, and there may be delays. So subtract one second.
+ if target_number == current_time or target_number -1 == current_time then
Review Comment:
@monkeyDluffy6017 Thank you for your suggestions, and these problems have been 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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10919:
URL: https://github.com/apache/apisix/pull/10919#discussion_r1505488422
##########
t/plugin/openid-connect6.t:
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+# no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: Check configuration of cookie
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {
+ client_id = "course_management",
+ client_secret = "tbsmDOpsHwdgIqYl2NltGRTKzjIzvEmT",
+ discovery = "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ session = {
+ secret = "6S8IO+Pydgb33LIor8T9ClER0T/sglFAjClFeAF3RsY=",
+ cookie = {
+ lifetime = 100
+ }
+ }
+ },
+ }
+ local plugin = require("apisix.plugins.openid-connect")
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+
+
+
+=== TEST 2: Set up new route access the auth server
+--- 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,
+ [[{
+ "plugins": {
+ "openid-connect": {
+ "discovery": "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "bearer_only" : false,
+ "timeout": 10,
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "required_scopes": ["profile"],
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "introspection_endpoint": "http://127.0.0.1:8080/realms/University/protocol/openid-connect/token/introspect",
+ "set_access_token_header": true,
+ "access_token_in_authorization_header": false,
+ "set_id_token_header": true,
+ "set_userinfo_header": true,
+ "set_refresh_token_header": true,
+ "session": {
+ "secret": "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "cookie": {
+ "lifetime": 100
+ }
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 3: Call to route to get session
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies = require("lib.keycloak").concatenate_cookies
+
+ local current_time = os.time()
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello"
+
+ local res, err = login_keycloak(uri, "teacher@gmail.com", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+ local parts = {}
+ for part in string.gmatch(cookie_str, "[^|]+") do
+ table.insert(parts, part)
+ end
+ local target_number = tonumber(parts[2], 10) - 100
Review Comment:
If you don't set the `cookie.lifetime`, what will the `target_number` be?
If we want to use the condition `if target_number >= current_time then`, the default value should be less than 100
--
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
Re: [PR] fix: Set the session configuration [apisix]
Posted by "illidan33 (via GitHub)" <gi...@apache.org>.
illidan33 commented on PR #10919:
URL: https://github.com/apache/apisix/pull/10919#issuecomment-1963234091
> @illidan33 you can find the keycloak service defined here:
>
> https://github.com/apache/apisix/blob/21d9ceeac9715dcdad37335fe402e325ee74a43d/ci/pod/docker-compose.plugin.yml#L40C1-L59C91
@shreemaan-abhishek hi, I updated the configuration to keep it the same as the official one.
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "illidan33 (via GitHub)" <gi...@apache.org>.
illidan33 commented on code in PR #10919:
URL: https://github.com/apache/apisix/pull/10919#discussion_r1507449554
##########
docs/en/latest/plugins/openid-connect.md:
##########
@@ -61,6 +61,8 @@ description: OpenID Connect allows the client to obtain user information from th
| set_refresh_token_header | boolean | False | false | | When set to true and a refresh token object is available, sets it in the `X-Refresh-Token` request header. |
| session | object | False | | | When bearer_only is set to false, openid-connect will use Authorization Code flow to authenticate on the IDP, so you need to set the session-related configuration. |
| session.secret | string | True | Automatic generation | 16 or more characters | The key used for session encrypt and HMAC operation. |
+| session.cookie | integer | False | | | |
Review Comment:
> missing description for `session.cookie`
As @monkeyDluffy6017 said, it is a map. I fixed its type to object, does it still need some description?
--
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
Re: [PR] fix: Set the session configuration [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on PR #10919:
URL: https://github.com/apache/apisix/pull/10919#issuecomment-1952645411
@illidan33 you can find the keycloak service defined here:
https://github.com/apache/apisix/blob/21d9ceeac9715dcdad37335fe402e325ee74a43d/ci/pod/docker-compose.plugin.yml#L40C1-L59C91
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #10919:
URL: https://github.com/apache/apisix/pull/10919#issuecomment-1978044207
@shreemaan-abhishek could you create a pr to change what @kayx23 mentioned?
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "illidan33 (via GitHub)" <gi...@apache.org>.
illidan33 commented on PR #10919:
URL: https://github.com/apache/apisix/pull/10919#issuecomment-1975761896
@shreemaan-abhishek @monkeyDluffy6017 hi, could you help to check the ci's error. It doesn't look like it failed because of the test code I submitted
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #10919:
URL: https://github.com/apache/apisix/pull/10919#issuecomment-1978056082
@illidan33 yeah, of course, another pr is needed, and i will invite @kayx23 to review
--
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
Re: [PR] fix: Set the session configuration [apisix]
Posted by "illidan33 (via GitHub)" <gi...@apache.org>.
illidan33 commented on PR #10919:
URL: https://github.com/apache/apisix/pull/10919#issuecomment-1951065448
> Hello, I think we may need additional testing, and documentation to ensure it is available in the plugin
Sure
@smileby @shreemaan-abhishek I have added test code and documentation, and successfully executed it on my local machine. However, I have a question. I am using a Docker setup of Keycloak/etcd on my local machine to validate the test code, referencing test code files from other contributors that use the same realm and client. I am unsure if this approach is standard. Does the official provide standard testing support services? For example, does the official provide a designated Keycloak authentication service for testing purposes?
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10919:
URL: https://github.com/apache/apisix/pull/10919#discussion_r1505488422
##########
t/plugin/openid-connect6.t:
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+# no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: Check configuration of cookie
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {
+ client_id = "course_management",
+ client_secret = "tbsmDOpsHwdgIqYl2NltGRTKzjIzvEmT",
+ discovery = "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ session = {
+ secret = "6S8IO+Pydgb33LIor8T9ClER0T/sglFAjClFeAF3RsY=",
+ cookie = {
+ lifetime = 100
+ }
+ }
+ },
+ }
+ local plugin = require("apisix.plugins.openid-connect")
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+
+
+
+=== TEST 2: Set up new route access the auth server
+--- 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,
+ [[{
+ "plugins": {
+ "openid-connect": {
+ "discovery": "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "bearer_only" : false,
+ "timeout": 10,
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "required_scopes": ["profile"],
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "introspection_endpoint": "http://127.0.0.1:8080/realms/University/protocol/openid-connect/token/introspect",
+ "set_access_token_header": true,
+ "access_token_in_authorization_header": false,
+ "set_id_token_header": true,
+ "set_userinfo_header": true,
+ "set_refresh_token_header": true,
+ "session": {
+ "secret": "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "cookie": {
+ "lifetime": 100
+ }
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 3: Call to route to get session
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies = require("lib.keycloak").concatenate_cookies
+
+ local current_time = os.time()
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello"
+
+ local res, err = login_keycloak(uri, "teacher@gmail.com", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+ local parts = {}
+ for part in string.gmatch(cookie_str, "[^|]+") do
+ table.insert(parts, part)
+ end
+ local target_number = tonumber(parts[2], 10) - 100
Review Comment:
If you don't set the `cookie.lifetime`, what will the `target_number` be?
If we want to use the condition `if target_number >= current_time then`, the default value of `cookie.lifetime` should be less than 100
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10919:
URL: https://github.com/apache/apisix/pull/10919#discussion_r1505488422
##########
t/plugin/openid-connect6.t:
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+# no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: Check configuration of cookie
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {
+ client_id = "course_management",
+ client_secret = "tbsmDOpsHwdgIqYl2NltGRTKzjIzvEmT",
+ discovery = "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ session = {
+ secret = "6S8IO+Pydgb33LIor8T9ClER0T/sglFAjClFeAF3RsY=",
+ cookie = {
+ lifetime = 100
+ }
+ }
+ },
+ }
+ local plugin = require("apisix.plugins.openid-connect")
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+
+
+
+=== TEST 2: Set up new route access the auth server
+--- 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,
+ [[{
+ "plugins": {
+ "openid-connect": {
+ "discovery": "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "bearer_only" : false,
+ "timeout": 10,
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "required_scopes": ["profile"],
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "introspection_endpoint": "http://127.0.0.1:8080/realms/University/protocol/openid-connect/token/introspect",
+ "set_access_token_header": true,
+ "access_token_in_authorization_header": false,
+ "set_id_token_header": true,
+ "set_userinfo_header": true,
+ "set_refresh_token_header": true,
+ "session": {
+ "secret": "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "cookie": {
+ "lifetime": 100
+ }
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 3: Call to route to get session
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies = require("lib.keycloak").concatenate_cookies
+
+ local current_time = os.time()
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello"
+
+ local res, err = login_keycloak(uri, "teacher@gmail.com", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+ local parts = {}
+ for part in string.gmatch(cookie_str, "[^|]+") do
+ table.insert(parts, part)
+ end
+ local target_number = tonumber(parts[2], 10) - 100
Review Comment:
If you don't set the `lifetime`, what is the `target_number`?
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "illidan33 (via GitHub)" <gi...@apache.org>.
illidan33 commented on PR #10919:
URL: https://github.com/apache/apisix/pull/10919#issuecomment-1978061306
> @illidan33 yeah, of course, another pr is needed, and i will invite @kayx23 to review
hi @monkeyDluffy6017 @kayx23 , the new pr's link is [10994](https://github.com/apache/apisix/pull/10994)
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on PR #10919:
URL: https://github.com/apache/apisix/pull/10919#issuecomment-1977876939
The quality of the english doc is subpar and I cannot understand what it's saying. Please hold the review to a higher standard.
My questions:
* `This can be configured with Nginx set $session_cookie_lifetime 3600`: does this mean this new configuration option just makes it feasible to configure `session_cookie_lifetime` dynamically at runtime?
* The next sentence is grammatically incorrect and I cannot understand, @shreemaan-abhishek please rephrase it makes sense to you.
* `it is used if the cookies are configured persistent with session.cookie.persistent == true`: where is this configuration? it is not in this plugin.
--
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
Re: [PR] fix: Set the session configuration [apisix]
Posted by "smileby (via GitHub)" <gi...@apache.org>.
smileby commented on PR #10919:
URL: https://github.com/apache/apisix/pull/10919#issuecomment-1930082145
Hello, I think we may need additional testing, and documentation to ensure it is available in the plugin
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10919:
URL: https://github.com/apache/apisix/pull/10919#discussion_r1503601217
##########
t/plugin/openid-connect6.t:
##########
@@ -0,0 +1,154 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+# no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: Check configuration of cookie
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {
+ client_id = "course_management",
+ client_secret = "tbsmDOpsHwdgIqYl2NltGRTKzjIzvEmT",
+ discovery = "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ session = {
+ secret = "6S8IO+Pydgb33LIor8T9ClER0T/sglFAjClFeAF3RsY=",
+ cookie = {
+ lifetime = 100
+ }
+ }
+ },
+ }
+ local plugin = require("apisix.plugins.openid-connect")
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+
+=== TEST 2: Set up new route access the auth server
+--- 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,
+ [[{
+ "plugins": {
+ "openid-connect": {
+ "discovery": "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "bearer_only" : false,
+ "timeout": 10,
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "required_scopes": ["profile"],
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "introspection_endpoint": "http://127.0.0.1:8080/realms/University/protocol/openid-connect/token/introspect",
+ "set_access_token_header": true,
+ "access_token_in_authorization_header": false,
+ "set_id_token_header": true,
+ "set_userinfo_header": true,
+ "set_refresh_token_header": true,
+ "session": {
+ "secret": "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "cookie": {
+ "lifetime": 100
+ }
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+=== TEST 3: Call to route to get session
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies = require("lib.keycloak").concatenate_cookies
+
+ local current_time = os.time()
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello"
+
+ local res, err = login_keycloak(uri, "teacher@gmail.com", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+ local parts = {}
+ for part in string.gmatch(cookie_str, "[^|]+") do
+ table.insert(parts, part)
+ end
+ local target_number = tonumber(parts[2], 10) - 100
+ -- ngx.say(target_number, current_time)
+ -- It takes time to execute the request, and there may be delays. So subtract one second.
+ if target_number == current_time or target_number -1 == current_time then
Review Comment:
How about
```
if target_number >= current_time then
```
If the judgements are equal, this may lead to occasional errors
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10919:
URL: https://github.com/apache/apisix/pull/10919#discussion_r1503595332
##########
t/plugin/openid-connect6.t:
##########
@@ -0,0 +1,154 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+# no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: Check configuration of cookie
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {
+ client_id = "course_management",
+ client_secret = "tbsmDOpsHwdgIqYl2NltGRTKzjIzvEmT",
+ discovery = "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ session = {
+ secret = "6S8IO+Pydgb33LIor8T9ClER0T/sglFAjClFeAF3RsY=",
+ cookie = {
+ lifetime = 100
+ }
+ }
+ },
+ }
+ local plugin = require("apisix.plugins.openid-connect")
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+
Review Comment:
Three blanks are needed between test cases
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10919:
URL: https://github.com/apache/apisix/pull/10919#discussion_r1505488422
##########
t/plugin/openid-connect6.t:
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+# no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: Check configuration of cookie
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {
+ client_id = "course_management",
+ client_secret = "tbsmDOpsHwdgIqYl2NltGRTKzjIzvEmT",
+ discovery = "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ session = {
+ secret = "6S8IO+Pydgb33LIor8T9ClER0T/sglFAjClFeAF3RsY=",
+ cookie = {
+ lifetime = 100
+ }
+ }
+ },
+ }
+ local plugin = require("apisix.plugins.openid-connect")
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+
+
+
+=== TEST 2: Set up new route access the auth server
+--- 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,
+ [[{
+ "plugins": {
+ "openid-connect": {
+ "discovery": "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "bearer_only" : false,
+ "timeout": 10,
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "required_scopes": ["profile"],
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "introspection_endpoint": "http://127.0.0.1:8080/realms/University/protocol/openid-connect/token/introspect",
+ "set_access_token_header": true,
+ "access_token_in_authorization_header": false,
+ "set_id_token_header": true,
+ "set_userinfo_header": true,
+ "set_refresh_token_header": true,
+ "session": {
+ "secret": "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "cookie": {
+ "lifetime": 100
+ }
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 3: Call to route to get session
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies = require("lib.keycloak").concatenate_cookies
+
+ local current_time = os.time()
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello"
+
+ local res, err = login_keycloak(uri, "teacher@gmail.com", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+ local parts = {}
+ for part in string.gmatch(cookie_str, "[^|]+") do
+ table.insert(parts, part)
+ end
+ local target_number = tonumber(parts[2], 10) - 100
Review Comment:
If you don't set the `lifetime`, what will the `target_number` be?
--
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
Re: [PR] feat(openid-connect): add session.cookie configuration [apisix]
Posted by "illidan33 (via GitHub)" <gi...@apache.org>.
illidan33 commented on code in PR #10919:
URL: https://github.com/apache/apisix/pull/10919#discussion_r1505689021
##########
t/plugin/openid-connect6.t:
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+# no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: Check configuration of cookie
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {
+ client_id = "course_management",
+ client_secret = "tbsmDOpsHwdgIqYl2NltGRTKzjIzvEmT",
+ discovery = "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ session = {
+ secret = "6S8IO+Pydgb33LIor8T9ClER0T/sglFAjClFeAF3RsY=",
+ cookie = {
+ lifetime = 100
+ }
+ }
+ },
+ }
+ local plugin = require("apisix.plugins.openid-connect")
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+
+
+
+=== TEST 2: Set up new route access the auth server
+--- 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,
+ [[{
+ "plugins": {
+ "openid-connect": {
+ "discovery": "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
+ "realm": "University",
+ "client_id": "course_management",
+ "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+ "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+ "ssl_verify": false,
+ "bearer_only" : false,
+ "timeout": 10,
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "required_scopes": ["profile"],
+ "introspection_endpoint_auth_method": "client_secret_post",
+ "introspection_endpoint": "http://127.0.0.1:8080/realms/University/protocol/openid-connect/token/introspect",
+ "set_access_token_header": true,
+ "access_token_in_authorization_header": false,
+ "set_id_token_header": true,
+ "set_userinfo_header": true,
+ "set_refresh_token_header": true,
+ "session": {
+ "secret": "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK",
+ "cookie": {
+ "lifetime": 100
+ }
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/*"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 3: Call to route to get session
+--- config
+ location /t {
+ content_by_lua_block {
+ local http = require "resty.http"
+ local login_keycloak = require("lib.keycloak").login_keycloak
+ local concatenate_cookies = require("lib.keycloak").concatenate_cookies
+
+ local current_time = os.time()
+ local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello"
+
+ local res, err = login_keycloak(uri, "teacher@gmail.com", "123456")
+ if err then
+ ngx.status = 500
+ ngx.say(err)
+ return
+ end
+
+ local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])
+ local parts = {}
+ for part in string.gmatch(cookie_str, "[^|]+") do
+ table.insert(parts, part)
+ end
+ local target_number = tonumber(parts[2], 10) - 100
Review Comment:
The default value of `cookie.lifetime` is 3600.
Yes, you are right. 100 is not the right condition, I will change it to a larger number than the default value.
--
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