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/02/26 01:34:53 UTC

[GitHub] [apisix] starsz opened a new pull request #6455: feat: support post_logout_redirect_uri config in openid-connect plugin

starsz opened a new pull request #6455:
URL: https://github.com/apache/apisix/pull/6455


   ### What this PR does / why we need it:
   <!--- Why is this change required? What problem does it solve? -->
   <!--- If it fixes an open issue, please link to the issue here. -->
   Hi, this PR is used to support post_logout_redirect_uri config in openid-connect plugin.
   So that we can redirect the uri when request the logout path of openid-connect plugin.
   Fix: https://github.com/apache/apisix/issues/6345, https://github.com/apache/apisix/issues/6362
   
   
   Since the lua-resty-openidc has support post_logout_redirect_uri already, we can just add post_logout_redirect_uri  config in our plugin.
   Refer: https://github.com/zmartzone/lua-resty-openidc/blob/3aac462f8206b8553cd1f34bb99b1788efbc2140/lib/resty/openidc.lua#L1331-L1339
   
   ### Pre-submission checklist:
   
   <!--
   Please follow the PR manners:
   1. Use Draft if the PR is not ready to be reviewed
   2. Test is required for the feat/fix PR, unless you have a good reason
   3. Doc is required for the feat PR
   4. Use a new commit to resolve review instead of `push -f`
   5. If you need to resolve merge conflicts after the PR is reviewed, please merge master but do not rebase
   6. Use "request review" to notify the reviewer once you have resolved the review
   7. Only reviewer can click "Resolve conversation" to mark the reviewer's review resolved
   -->
   
   * [x] Did you explain what problem does this PR solve? Or what new features have been added?
   * [x] 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.

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 change in pull request #6455: feat: support post_logout_redirect_uri config in openid-connect plugin

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



##########
File path: t/plugin/openid-connect.t
##########
@@ -1665,3 +1665,319 @@ GET /t
 false
 --- error_log
 OIDC introspection failed: invalid jwt: invalid jwt string
+
+
+
+=== TEST 28: Modify route to match catch-all URI `/*` and add post_logout_redirect_uri option.
+--- 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:8090/auth/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,
+                                "timeout": 10,
+                                "introspection_endpoint_auth_method": "client_secret_post",
+                                "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/*"
+                }]],
+                [[{
+                    "node": {

Review comment:
       Let me remove it.




-- 
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 change in pull request #6455: feat: support post_logout_redirect_uri config in openid-connect plugin

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



##########
File path: docs/en/latest/plugins/openid-connect.md
##########
@@ -47,6 +47,7 @@ The OAuth 2 / Open ID Connect(OIDC) plugin provides authentication and introspec
 | realm                                | string  | optional    | "apisix"              |         | Realm used for the authentication                                                                                               |
 | bearer_only                          | boolean | optional    | false                 |         | Setting this `true` will check for the authorization header in the request with a bearer token                                  |
 | logout_path                          | string  | optional    | "/logout"             |         |                                                                                                                                 |
+| post_logout_redirect_uri   | string | optional |              |  | URL want to redirect when request logout_path

Review comment:
       OK. Let me fix it.

##########
File path: t/plugin/openid-connect.t
##########
@@ -1665,3 +1665,319 @@ GET /t
 false
 --- error_log
 OIDC introspection failed: invalid jwt: invalid jwt string
+
+
+
+=== TEST 28: Modify route to match catch-all URI `/*` and add post_logout_redirect_uri option.
+--- 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:8090/auth/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,
+                                "timeout": 10,
+                                "introspection_endpoint_auth_method": "client_secret_post",
+                                "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/*"
+                }]],
+                [[{
+                    "node": {

Review comment:
       I think we should check the response data of this request like other route-creating tests.
   Why we don't need to check the response?
   




-- 
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 change in pull request #6455: feat: support post_logout_redirect_uri config in openid-connect plugin

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



##########
File path: t/plugin/openid-connect.t
##########
@@ -1665,3 +1665,319 @@ GET /t
 false
 --- error_log
 OIDC introspection failed: invalid jwt: invalid jwt string
+
+
+
+=== TEST 28: Modify route to match catch-all URI `/*` and add post_logout_redirect_uri option.
+--- 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:8090/auth/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,
+                                "timeout": 10,
+                                "introspection_endpoint_auth_method": "client_secret_post",
+                                "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/*"
+                }]],
+                [[{
+                    "node": {
+                        "value": {
+                            "plugins": {
+                                "openid-connect": {
+                                    "client_id": "course_management",
+                                    "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+                                    "discovery": "http://127.0.0.1:8090/auth/realms/University/.well-known/openid-configuration",
+                                    "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+                                    "ssl_verify": false,
+                                    "timeout": 10,
+                                    "realm": "University",
+                                    "introspection_endpoint_auth_method": "client_secret_post",
+                                    "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                    "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                                }
+                            },
+                            "upstream": {
+                                "nodes": {
+                                    "127.0.0.1:1980": 1
+                                },
+                                "type": "roundrobin"
+                            },
+                            "uri": "/*"
+                        },
+                        "key": "/apisix/routes/1"
+                    },
+                    "action": "set"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 29: Access route w/o bearer token and request logout to redirect to post_logout_redirect_uri.
+--- config
+    location /t {
+        content_by_lua_block {
+            local http = require "resty.http"
+            local httpc = http.new()
+
+            -- Invoke /uri endpoint w/o bearer token. Should receive redirect to Keycloak authorization endpoint.
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/uri"
+            local res, err = httpc:request_uri(uri, {method = "GET"})
+
+            if not res then
+                -- No response, must be an error.
+                ngx.status = 500
+                ngx.say(err)
+                return
+            elseif res.status ~= 302 then
+                -- Not a redirect which we expect.
+                -- Use 500 to indicate error.
+                ngx.status = 500
+                ngx.say("Initial request was not redirected to ID provider authorization endpoint.")
+                return
+            else
+                -- Redirect to ID provider's authorization endpoint.
+
+                -- Extract nonce and state from response header.
+                local nonce = res.headers['Location']:match('.*nonce=([^&]+).*')
+                local state = res.headers['Location']:match('.*state=([^&]+).*')
+
+                -- Extract cookies. Important since OIDC module tracks state with a session cookie.
+                local cookies = res.headers['Set-Cookie']
+
+                -- Concatenate cookies into one string as expected when sent in request header.
+                local cookie_str = ""
+
+                if type(cookies) == 'string' then
+                    cookie_str = cookies:match('([^;]*); .*')
+                else
+                    -- Must be a table.
+                    local len = #cookies
+                    if len > 0 then
+                        cookie_str = cookies[1]:match('([^;]*); .*')
+                        for i = 2, len do
+                            cookie_str = cookie_str .. "; " .. cookies[i]:match('([^;]*); .*')
+                        end
+                    end
+                end
+
+                -- Call authorization endpoint we were redirected to.
+                -- Note: This typically returns a login form which is the case here for Keycloak as well.
+                -- However, how we process the form to perform the login is specific to Keycloak and
+                -- possibly even the version used.
+                res, err = httpc:request_uri(res.headers['Location'], {method = "GET"})
+
+                if not res then
+                    -- No response, must be an error.
+                    ngx.status = 500
+                    ngx.say(err)
+                    return
+                elseif res.status ~= 200 then
+                    -- Unexpected response.
+                    ngx.status = res.status
+                    ngx.say(res.body)
+                    return
+                end
+
+                -- Check if response code was ok.
+                if res.status == 200 then
+                    -- From the returned form, extract the submit URI and parameters.
+                    local uri, params = res.body:match('.*action="(.*)%?(.*)" method="post">')
+
+                    -- Substitute escaped ampersand in parameters.
+                    params = params:gsub("&amp;", "&")
+
+                    -- Get all cookies returned. Probably not so important since not part of OIDC specification.
+                    local auth_cookies = res.headers['Set-Cookie']
+
+                    -- Concatenate cookies into one string as expected when sent in request header.
+                    local auth_cookie_str = ""
+
+                    if type(auth_cookies) == 'string' then
+                        auth_cookie_str = auth_cookies:match('([^;]*); .*')
+                    else
+                        -- Must be a table.
+                        local len = #auth_cookies
+                        if len > 0 then
+                            auth_cookie_str = auth_cookies[1]:match('([^;]*); .*')
+                            for i = 2, len do
+                                auth_cookie_str = auth_cookie_str .. "; " .. auth_cookies[i]:match('([^;]*); .*')
+                            end
+                        end
+                    end
+
+                    -- Invoke the submit URI with parameters and cookies, adding username and password in the body.
+                    -- Note: Username and password are specific to the Keycloak Docker image used.
+                    res, err = httpc:request_uri(uri .. "?" .. params, {
+                            method = "POST",
+                            body = "username=teacher@gmail.com&password=123456",
+                            headers = {
+                                ["Content-Type"] = "application/x-www-form-urlencoded",
+                                ["Cookie"] = auth_cookie_str
+                            }
+                        })
+
+                    if not res then
+                        -- No response, must be an error.
+                        ngx.status = 500
+                        ngx.say(err)
+                        return
+                    elseif res.status ~= 302 then
+                        -- Not a redirect which we expect.
+                        -- Use 500 to indicate error.
+                        ngx.status = 500
+                        ngx.say("Login form submission did not return redirect to redirect URI.")
+                        return
+                    end
+
+                    -- Extract the redirect URI from the response header.
+                    -- TODO: Consider validating this against the plugin configuration.
+                    local redirect_uri = res.headers['Location']
+
+                    -- Invoke the redirect URI (which contains the authorization code as an URL parameter).
+                    res, err = httpc:request_uri(redirect_uri, {
+                            method = "GET",
+                            headers = {
+                                ["Cookie"] = cookie_str
+                            }
+                        })
+
+                    if not res then
+                        -- No response, must be an error.
+                        ngx.status = 500
+                        ngx.say(err)
+                        return
+                    elseif res.status ~= 302 then
+                        -- Not a redirect which we expect.
+                        -- Use 500 to indicate error.
+                        ngx.status = 500
+                        ngx.say("Invoking redirect URI with authorization code did not return redirect to original URI.")
+                        return
+                    end
+
+                    -- Get all cookies returned. This should update the session cookie maintained by the OIDC module with the new state.
+                    -- E.g. the session cookie should now contain the access token, ID token and user info.
+                    -- The cookie itself should however be treated as opaque.
+                    cookies = res.headers['Set-Cookie']
+
+                    -- Concatenate cookies into one string as expected when sent in request header.
+                    if type(cookies) == 'string' then
+                        cookie_str = cookies:match('([^;]*); .*')
+                    else
+                        -- Must be a table.
+                        local len = #cookies
+                        if len > 0 then
+                            cookie_str = cookies[1]:match('([^;]*); .*')
+                            for i = 2, len do
+                                cookie_str = cookie_str .. "; " .. cookies[i]:match('([^;]*); .*')
+                            end
+                        end
+                    end
+
+                    -- Get the final URI out of the Location response header. This should be the original URI that was requested.

Review comment:
       https://github.com/apache/apisix/blob/acbb111afbbd096444bdd6dc0307d18187b19829/t/plugin/skywalking.t#L30
   You can define helper functions in extra_init_by_lua 




-- 
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 change in pull request #6455: feat: support post_logout_redirect_uri config in openid-connect plugin

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



##########
File path: t/plugin/openid-connect.t
##########
@@ -1665,3 +1665,319 @@ GET /t
 false
 --- error_log
 OIDC introspection failed: invalid jwt: invalid jwt string
+
+
+
+=== TEST 28: Modify route to match catch-all URI `/*` and add post_logout_redirect_uri option.
+--- 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:8090/auth/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,
+                                "timeout": 10,
+                                "introspection_endpoint_auth_method": "client_secret_post",
+                                "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/*"
+                }]],
+                [[{
+                    "node": {

Review comment:
       Yes, we check the response status code but not the data. The data is mostly an echo of the input.




-- 
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 change in pull request #6455: feat: support post_logout_redirect_uri config in openid-connect plugin

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



##########
File path: t/plugin/openid-connect.t
##########
@@ -1665,3 +1665,319 @@ GET /t
 false
 --- error_log
 OIDC introspection failed: invalid jwt: invalid jwt string
+
+
+
+=== TEST 28: Modify route to match catch-all URI `/*` and add post_logout_redirect_uri option.
+--- 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:8090/auth/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,
+                                "timeout": 10,
+                                "introspection_endpoint_auth_method": "client_secret_post",
+                                "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/*"
+                }]],
+                [[{
+                    "node": {
+                        "value": {
+                            "plugins": {
+                                "openid-connect": {
+                                    "client_id": "course_management",
+                                    "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+                                    "discovery": "http://127.0.0.1:8090/auth/realms/University/.well-known/openid-configuration",
+                                    "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+                                    "ssl_verify": false,
+                                    "timeout": 10,
+                                    "realm": "University",
+                                    "introspection_endpoint_auth_method": "client_secret_post",
+                                    "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                    "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                                }
+                            },
+                            "upstream": {
+                                "nodes": {
+                                    "127.0.0.1:1980": 1
+                                },
+                                "type": "roundrobin"
+                            },
+                            "uri": "/*"
+                        },
+                        "key": "/apisix/routes/1"
+                    },
+                    "action": "set"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 29: Access route w/o bearer token and request logout to redirect to post_logout_redirect_uri.
+--- config
+    location /t {
+        content_by_lua_block {
+            local http = require "resty.http"
+            local httpc = http.new()
+
+            -- Invoke /uri endpoint w/o bearer token. Should receive redirect to Keycloak authorization endpoint.
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/uri"
+            local res, err = httpc:request_uri(uri, {method = "GET"})
+
+            if not res then
+                -- No response, must be an error.
+                ngx.status = 500
+                ngx.say(err)
+                return
+            elseif res.status ~= 302 then
+                -- Not a redirect which we expect.
+                -- Use 500 to indicate error.
+                ngx.status = 500
+                ngx.say("Initial request was not redirected to ID provider authorization endpoint.")
+                return
+            else
+                -- Redirect to ID provider's authorization endpoint.
+
+                -- Extract nonce and state from response header.
+                local nonce = res.headers['Location']:match('.*nonce=([^&]+).*')
+                local state = res.headers['Location']:match('.*state=([^&]+).*')
+
+                -- Extract cookies. Important since OIDC module tracks state with a session cookie.
+                local cookies = res.headers['Set-Cookie']
+
+                -- Concatenate cookies into one string as expected when sent in request header.
+                local cookie_str = ""
+
+                if type(cookies) == 'string' then
+                    cookie_str = cookies:match('([^;]*); .*')
+                else
+                    -- Must be a table.
+                    local len = #cookies
+                    if len > 0 then
+                        cookie_str = cookies[1]:match('([^;]*); .*')
+                        for i = 2, len do
+                            cookie_str = cookie_str .. "; " .. cookies[i]:match('([^;]*); .*')
+                        end
+                    end
+                end
+
+                -- Call authorization endpoint we were redirected to.
+                -- Note: This typically returns a login form which is the case here for Keycloak as well.
+                -- However, how we process the form to perform the login is specific to Keycloak and
+                -- possibly even the version used.
+                res, err = httpc:request_uri(res.headers['Location'], {method = "GET"})
+
+                if not res then
+                    -- No response, must be an error.
+                    ngx.status = 500
+                    ngx.say(err)
+                    return
+                elseif res.status ~= 200 then
+                    -- Unexpected response.
+                    ngx.status = res.status
+                    ngx.say(res.body)
+                    return
+                end
+
+                -- Check if response code was ok.
+                if res.status == 200 then
+                    -- From the returned form, extract the submit URI and parameters.
+                    local uri, params = res.body:match('.*action="(.*)%?(.*)" method="post">')
+
+                    -- Substitute escaped ampersand in parameters.
+                    params = params:gsub("&amp;", "&")
+
+                    -- Get all cookies returned. Probably not so important since not part of OIDC specification.
+                    local auth_cookies = res.headers['Set-Cookie']
+
+                    -- Concatenate cookies into one string as expected when sent in request header.
+                    local auth_cookie_str = ""
+
+                    if type(auth_cookies) == 'string' then
+                        auth_cookie_str = auth_cookies:match('([^;]*); .*')
+                    else
+                        -- Must be a table.
+                        local len = #auth_cookies
+                        if len > 0 then
+                            auth_cookie_str = auth_cookies[1]:match('([^;]*); .*')
+                            for i = 2, len do
+                                auth_cookie_str = auth_cookie_str .. "; " .. auth_cookies[i]:match('([^;]*); .*')
+                            end
+                        end
+                    end
+
+                    -- Invoke the submit URI with parameters and cookies, adding username and password in the body.
+                    -- Note: Username and password are specific to the Keycloak Docker image used.
+                    res, err = httpc:request_uri(uri .. "?" .. params, {
+                            method = "POST",
+                            body = "username=teacher@gmail.com&password=123456",
+                            headers = {
+                                ["Content-Type"] = "application/x-www-form-urlencoded",
+                                ["Cookie"] = auth_cookie_str
+                            }
+                        })
+
+                    if not res then
+                        -- No response, must be an error.
+                        ngx.status = 500
+                        ngx.say(err)
+                        return
+                    elseif res.status ~= 302 then
+                        -- Not a redirect which we expect.
+                        -- Use 500 to indicate error.
+                        ngx.status = 500
+                        ngx.say("Login form submission did not return redirect to redirect URI.")
+                        return
+                    end
+
+                    -- Extract the redirect URI from the response header.
+                    -- TODO: Consider validating this against the plugin configuration.
+                    local redirect_uri = res.headers['Location']
+
+                    -- Invoke the redirect URI (which contains the authorization code as an URL parameter).
+                    res, err = httpc:request_uri(redirect_uri, {
+                            method = "GET",
+                            headers = {
+                                ["Cookie"] = cookie_str
+                            }
+                        })
+
+                    if not res then
+                        -- No response, must be an error.
+                        ngx.status = 500
+                        ngx.say(err)
+                        return
+                    elseif res.status ~= 302 then
+                        -- Not a redirect which we expect.
+                        -- Use 500 to indicate error.
+                        ngx.status = 500
+                        ngx.say("Invoking redirect URI with authorization code did not return redirect to original URI.")
+                        return
+                    end
+
+                    -- Get all cookies returned. This should update the session cookie maintained by the OIDC module with the new state.
+                    -- E.g. the session cookie should now contain the access token, ID token and user info.
+                    -- The cookie itself should however be treated as opaque.
+                    cookies = res.headers['Set-Cookie']
+
+                    -- Concatenate cookies into one string as expected when sent in request header.
+                    if type(cookies) == 'string' then
+                        cookie_str = cookies:match('([^;]*); .*')
+                    else
+                        -- Must be a table.
+                        local len = #cookies
+                        if len > 0 then
+                            cookie_str = cookies[1]:match('([^;]*); .*')
+                            for i = 2, len do
+                                cookie_str = cookie_str .. "; " .. cookies[i]:match('([^;]*); .*')
+                            end
+                        end
+                    end
+
+                    -- Get the final URI out of the Location response header. This should be the original URI that was requested.

Review comment:
       Fixed. Please review it again.




-- 
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 change in pull request #6455: feat: support post_logout_redirect_uri config in openid-connect plugin

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



##########
File path: t/plugin/openid-connect.t
##########
@@ -1665,3 +1665,319 @@ GET /t
 false
 --- error_log
 OIDC introspection failed: invalid jwt: invalid jwt string
+
+
+
+=== TEST 28: Modify route to match catch-all URI `/*` and add post_logout_redirect_uri option.
+--- 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:8090/auth/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,
+                                "timeout": 10,
+                                "introspection_endpoint_auth_method": "client_secret_post",
+                                "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/*"
+                }]],
+                [[{
+                    "node": {
+                        "value": {
+                            "plugins": {
+                                "openid-connect": {
+                                    "client_id": "course_management",
+                                    "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+                                    "discovery": "http://127.0.0.1:8090/auth/realms/University/.well-known/openid-configuration",
+                                    "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+                                    "ssl_verify": false,
+                                    "timeout": 10,
+                                    "realm": "University",
+                                    "introspection_endpoint_auth_method": "client_secret_post",
+                                    "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                    "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                                }
+                            },
+                            "upstream": {
+                                "nodes": {
+                                    "127.0.0.1:1980": 1
+                                },
+                                "type": "roundrobin"
+                            },
+                            "uri": "/*"
+                        },
+                        "key": "/apisix/routes/1"
+                    },
+                    "action": "set"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 29: Access route w/o bearer token and request logout to redirect to post_logout_redirect_uri.
+--- config
+    location /t {
+        content_by_lua_block {
+            local http = require "resty.http"
+            local httpc = http.new()
+
+            -- Invoke /uri endpoint w/o bearer token. Should receive redirect to Keycloak authorization endpoint.
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/uri"
+            local res, err = httpc:request_uri(uri, {method = "GET"})
+
+            if not res then
+                -- No response, must be an error.
+                ngx.status = 500
+                ngx.say(err)
+                return
+            elseif res.status ~= 302 then
+                -- Not a redirect which we expect.
+                -- Use 500 to indicate error.
+                ngx.status = 500
+                ngx.say("Initial request was not redirected to ID provider authorization endpoint.")
+                return
+            else
+                -- Redirect to ID provider's authorization endpoint.
+
+                -- Extract nonce and state from response header.
+                local nonce = res.headers['Location']:match('.*nonce=([^&]+).*')
+                local state = res.headers['Location']:match('.*state=([^&]+).*')
+
+                -- Extract cookies. Important since OIDC module tracks state with a session cookie.
+                local cookies = res.headers['Set-Cookie']
+
+                -- Concatenate cookies into one string as expected when sent in request header.
+                local cookie_str = ""
+
+                if type(cookies) == 'string' then
+                    cookie_str = cookies:match('([^;]*); .*')
+                else
+                    -- Must be a table.
+                    local len = #cookies
+                    if len > 0 then
+                        cookie_str = cookies[1]:match('([^;]*); .*')
+                        for i = 2, len do
+                            cookie_str = cookie_str .. "; " .. cookies[i]:match('([^;]*); .*')
+                        end
+                    end
+                end
+
+                -- Call authorization endpoint we were redirected to.
+                -- Note: This typically returns a login form which is the case here for Keycloak as well.
+                -- However, how we process the form to perform the login is specific to Keycloak and
+                -- possibly even the version used.
+                res, err = httpc:request_uri(res.headers['Location'], {method = "GET"})
+
+                if not res then
+                    -- No response, must be an error.
+                    ngx.status = 500
+                    ngx.say(err)
+                    return
+                elseif res.status ~= 200 then
+                    -- Unexpected response.
+                    ngx.status = res.status
+                    ngx.say(res.body)
+                    return
+                end
+
+                -- Check if response code was ok.
+                if res.status == 200 then
+                    -- From the returned form, extract the submit URI and parameters.
+                    local uri, params = res.body:match('.*action="(.*)%?(.*)" method="post">')
+
+                    -- Substitute escaped ampersand in parameters.
+                    params = params:gsub("&amp;", "&")
+
+                    -- Get all cookies returned. Probably not so important since not part of OIDC specification.
+                    local auth_cookies = res.headers['Set-Cookie']
+
+                    -- Concatenate cookies into one string as expected when sent in request header.
+                    local auth_cookie_str = ""
+
+                    if type(auth_cookies) == 'string' then
+                        auth_cookie_str = auth_cookies:match('([^;]*); .*')
+                    else
+                        -- Must be a table.
+                        local len = #auth_cookies
+                        if len > 0 then
+                            auth_cookie_str = auth_cookies[1]:match('([^;]*); .*')
+                            for i = 2, len do
+                                auth_cookie_str = auth_cookie_str .. "; " .. auth_cookies[i]:match('([^;]*); .*')
+                            end
+                        end
+                    end
+
+                    -- Invoke the submit URI with parameters and cookies, adding username and password in the body.
+                    -- Note: Username and password are specific to the Keycloak Docker image used.
+                    res, err = httpc:request_uri(uri .. "?" .. params, {
+                            method = "POST",
+                            body = "username=teacher@gmail.com&password=123456",
+                            headers = {
+                                ["Content-Type"] = "application/x-www-form-urlencoded",
+                                ["Cookie"] = auth_cookie_str
+                            }
+                        })
+
+                    if not res then
+                        -- No response, must be an error.
+                        ngx.status = 500
+                        ngx.say(err)
+                        return
+                    elseif res.status ~= 302 then
+                        -- Not a redirect which we expect.
+                        -- Use 500 to indicate error.
+                        ngx.status = 500
+                        ngx.say("Login form submission did not return redirect to redirect URI.")
+                        return
+                    end
+
+                    -- Extract the redirect URI from the response header.
+                    -- TODO: Consider validating this against the plugin configuration.
+                    local redirect_uri = res.headers['Location']
+
+                    -- Invoke the redirect URI (which contains the authorization code as an URL parameter).
+                    res, err = httpc:request_uri(redirect_uri, {
+                            method = "GET",
+                            headers = {
+                                ["Cookie"] = cookie_str
+                            }
+                        })
+
+                    if not res then
+                        -- No response, must be an error.
+                        ngx.status = 500
+                        ngx.say(err)
+                        return
+                    elseif res.status ~= 302 then
+                        -- Not a redirect which we expect.
+                        -- Use 500 to indicate error.
+                        ngx.status = 500
+                        ngx.say("Invoking redirect URI with authorization code did not return redirect to original URI.")
+                        return
+                    end
+
+                    -- Get all cookies returned. This should update the session cookie maintained by the OIDC module with the new state.
+                    -- E.g. the session cookie should now contain the access token, ID token and user info.
+                    -- The cookie itself should however be treated as opaque.
+                    cookies = res.headers['Set-Cookie']
+
+                    -- Concatenate cookies into one string as expected when sent in request header.
+                    if type(cookies) == 'string' then
+                        cookie_str = cookies:match('([^;]*); .*')
+                    else
+                        -- Must be a table.
+                        local len = #cookies
+                        if len > 0 then
+                            cookie_str = cookies[1]:match('([^;]*); .*')
+                            for i = 2, len do
+                                cookie_str = cookie_str .. "; " .. cookies[i]:match('([^;]*); .*')
+                            end
+                        end
+                    end
+
+                    -- Get the final URI out of the Location response header. This should be the original URI that was requested.

Review comment:
       Hi, @spacewander.
   > The similar Lua snipper is repeated in multiple tests.
   
   Yes, they are repeated. But I have no idea about refactoring it, can you give me some examples or docs? 




-- 
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 change in pull request #6455: feat: support post_logout_redirect_uri config in openid-connect plugin

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



##########
File path: t/plugin/openid-connect.t
##########
@@ -1665,3 +1665,319 @@ GET /t
 false
 --- error_log
 OIDC introspection failed: invalid jwt: invalid jwt string
+
+
+
+=== TEST 28: Modify route to match catch-all URI `/*` and add post_logout_redirect_uri option.
+--- 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:8090/auth/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,
+                                "timeout": 10,
+                                "introspection_endpoint_auth_method": "client_secret_post",
+                                "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/*"
+                }]],
+                [[{
+                    "node": {
+                        "value": {
+                            "plugins": {
+                                "openid-connect": {
+                                    "client_id": "course_management",
+                                    "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+                                    "discovery": "http://127.0.0.1:8090/auth/realms/University/.well-known/openid-configuration",
+                                    "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+                                    "ssl_verify": false,
+                                    "timeout": 10,
+                                    "realm": "University",
+                                    "introspection_endpoint_auth_method": "client_secret_post",
+                                    "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                    "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                                }
+                            },
+                            "upstream": {
+                                "nodes": {
+                                    "127.0.0.1:1980": 1
+                                },
+                                "type": "roundrobin"
+                            },
+                            "uri": "/*"
+                        },
+                        "key": "/apisix/routes/1"
+                    },
+                    "action": "set"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 29: Access route w/o bearer token and request logout to redirect to post_logout_redirect_uri.
+--- config
+    location /t {
+        content_by_lua_block {
+            local http = require "resty.http"
+            local httpc = http.new()
+
+            -- Invoke /uri endpoint w/o bearer token. Should receive redirect to Keycloak authorization endpoint.
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/uri"
+            local res, err = httpc:request_uri(uri, {method = "GET"})
+
+            if not res then
+                -- No response, must be an error.
+                ngx.status = 500
+                ngx.say(err)
+                return
+            elseif res.status ~= 302 then
+                -- Not a redirect which we expect.
+                -- Use 500 to indicate error.
+                ngx.status = 500
+                ngx.say("Initial request was not redirected to ID provider authorization endpoint.")
+                return
+            else
+                -- Redirect to ID provider's authorization endpoint.
+
+                -- Extract nonce and state from response header.
+                local nonce = res.headers['Location']:match('.*nonce=([^&]+).*')
+                local state = res.headers['Location']:match('.*state=([^&]+).*')
+
+                -- Extract cookies. Important since OIDC module tracks state with a session cookie.
+                local cookies = res.headers['Set-Cookie']
+
+                -- Concatenate cookies into one string as expected when sent in request header.
+                local cookie_str = ""
+
+                if type(cookies) == 'string' then
+                    cookie_str = cookies:match('([^;]*); .*')
+                else
+                    -- Must be a table.
+                    local len = #cookies
+                    if len > 0 then
+                        cookie_str = cookies[1]:match('([^;]*); .*')
+                        for i = 2, len do
+                            cookie_str = cookie_str .. "; " .. cookies[i]:match('([^;]*); .*')
+                        end
+                    end
+                end
+
+                -- Call authorization endpoint we were redirected to.
+                -- Note: This typically returns a login form which is the case here for Keycloak as well.
+                -- However, how we process the form to perform the login is specific to Keycloak and
+                -- possibly even the version used.
+                res, err = httpc:request_uri(res.headers['Location'], {method = "GET"})
+
+                if not res then
+                    -- No response, must be an error.
+                    ngx.status = 500
+                    ngx.say(err)
+                    return
+                elseif res.status ~= 200 then
+                    -- Unexpected response.
+                    ngx.status = res.status
+                    ngx.say(res.body)
+                    return
+                end
+
+                -- Check if response code was ok.
+                if res.status == 200 then
+                    -- From the returned form, extract the submit URI and parameters.
+                    local uri, params = res.body:match('.*action="(.*)%?(.*)" method="post">')
+
+                    -- Substitute escaped ampersand in parameters.
+                    params = params:gsub("&amp;", "&")
+
+                    -- Get all cookies returned. Probably not so important since not part of OIDC specification.
+                    local auth_cookies = res.headers['Set-Cookie']
+
+                    -- Concatenate cookies into one string as expected when sent in request header.
+                    local auth_cookie_str = ""
+
+                    if type(auth_cookies) == 'string' then
+                        auth_cookie_str = auth_cookies:match('([^;]*); .*')
+                    else
+                        -- Must be a table.
+                        local len = #auth_cookies
+                        if len > 0 then
+                            auth_cookie_str = auth_cookies[1]:match('([^;]*); .*')
+                            for i = 2, len do
+                                auth_cookie_str = auth_cookie_str .. "; " .. auth_cookies[i]:match('([^;]*); .*')
+                            end
+                        end
+                    end
+
+                    -- Invoke the submit URI with parameters and cookies, adding username and password in the body.
+                    -- Note: Username and password are specific to the Keycloak Docker image used.
+                    res, err = httpc:request_uri(uri .. "?" .. params, {
+                            method = "POST",
+                            body = "username=teacher@gmail.com&password=123456",
+                            headers = {
+                                ["Content-Type"] = "application/x-www-form-urlencoded",
+                                ["Cookie"] = auth_cookie_str
+                            }
+                        })
+
+                    if not res then
+                        -- No response, must be an error.
+                        ngx.status = 500
+                        ngx.say(err)
+                        return
+                    elseif res.status ~= 302 then
+                        -- Not a redirect which we expect.
+                        -- Use 500 to indicate error.
+                        ngx.status = 500
+                        ngx.say("Login form submission did not return redirect to redirect URI.")
+                        return
+                    end
+
+                    -- Extract the redirect URI from the response header.
+                    -- TODO: Consider validating this against the plugin configuration.
+                    local redirect_uri = res.headers['Location']
+
+                    -- Invoke the redirect URI (which contains the authorization code as an URL parameter).
+                    res, err = httpc:request_uri(redirect_uri, {
+                            method = "GET",
+                            headers = {
+                                ["Cookie"] = cookie_str
+                            }
+                        })
+
+                    if not res then
+                        -- No response, must be an error.
+                        ngx.status = 500
+                        ngx.say(err)
+                        return
+                    elseif res.status ~= 302 then
+                        -- Not a redirect which we expect.
+                        -- Use 500 to indicate error.
+                        ngx.status = 500
+                        ngx.say("Invoking redirect URI with authorization code did not return redirect to original URI.")
+                        return
+                    end
+
+                    -- Get all cookies returned. This should update the session cookie maintained by the OIDC module with the new state.
+                    -- E.g. the session cookie should now contain the access token, ID token and user info.
+                    -- The cookie itself should however be treated as opaque.
+                    cookies = res.headers['Set-Cookie']
+
+                    -- Concatenate cookies into one string as expected when sent in request header.
+                    if type(cookies) == 'string' then
+                        cookie_str = cookies:match('([^;]*); .*')
+                    else
+                        -- Must be a table.
+                        local len = #cookies
+                        if len > 0 then
+                            cookie_str = cookies[1]:match('([^;]*); .*')
+                            for i = 2, len do
+                                cookie_str = cookie_str .. "; " .. cookies[i]:match('([^;]*); .*')
+                            end
+                        end
+                    end
+
+                    -- Get the final URI out of the Location response header. This should be the original URI that was requested.

Review comment:
       Fixed. Please review it again.




-- 
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 change in pull request #6455: feat: support post_logout_redirect_uri config in openid-connect plugin

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



##########
File path: t/plugin/openid-connect.t
##########
@@ -1665,3 +1665,319 @@ GET /t
 false
 --- error_log
 OIDC introspection failed: invalid jwt: invalid jwt string
+
+
+
+=== TEST 28: Modify route to match catch-all URI `/*` and add post_logout_redirect_uri option.
+--- 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:8090/auth/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,
+                                "timeout": 10,
+                                "introspection_endpoint_auth_method": "client_secret_post",
+                                "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/*"
+                }]],
+                [[{
+                    "node": {
+                        "value": {
+                            "plugins": {
+                                "openid-connect": {
+                                    "client_id": "course_management",
+                                    "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+                                    "discovery": "http://127.0.0.1:8090/auth/realms/University/.well-known/openid-configuration",
+                                    "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+                                    "ssl_verify": false,
+                                    "timeout": 10,
+                                    "realm": "University",
+                                    "introspection_endpoint_auth_method": "client_secret_post",
+                                    "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                    "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                                }
+                            },
+                            "upstream": {
+                                "nodes": {
+                                    "127.0.0.1:1980": 1
+                                },
+                                "type": "roundrobin"
+                            },
+                            "uri": "/*"
+                        },
+                        "key": "/apisix/routes/1"
+                    },
+                    "action": "set"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 29: Access route w/o bearer token and request logout to redirect to post_logout_redirect_uri.
+--- config
+    location /t {
+        content_by_lua_block {
+            local http = require "resty.http"
+            local httpc = http.new()
+
+            -- Invoke /uri endpoint w/o bearer token. Should receive redirect to Keycloak authorization endpoint.
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/uri"
+            local res, err = httpc:request_uri(uri, {method = "GET"})
+
+            if not res then
+                -- No response, must be an error.
+                ngx.status = 500
+                ngx.say(err)
+                return
+            elseif res.status ~= 302 then
+                -- Not a redirect which we expect.
+                -- Use 500 to indicate error.
+                ngx.status = 500
+                ngx.say("Initial request was not redirected to ID provider authorization endpoint.")
+                return
+            else
+                -- Redirect to ID provider's authorization endpoint.
+
+                -- Extract nonce and state from response header.
+                local nonce = res.headers['Location']:match('.*nonce=([^&]+).*')
+                local state = res.headers['Location']:match('.*state=([^&]+).*')
+
+                -- Extract cookies. Important since OIDC module tracks state with a session cookie.
+                local cookies = res.headers['Set-Cookie']
+
+                -- Concatenate cookies into one string as expected when sent in request header.
+                local cookie_str = ""
+
+                if type(cookies) == 'string' then
+                    cookie_str = cookies:match('([^;]*); .*')
+                else
+                    -- Must be a table.
+                    local len = #cookies
+                    if len > 0 then
+                        cookie_str = cookies[1]:match('([^;]*); .*')
+                        for i = 2, len do
+                            cookie_str = cookie_str .. "; " .. cookies[i]:match('([^;]*); .*')
+                        end
+                    end
+                end
+
+                -- Call authorization endpoint we were redirected to.
+                -- Note: This typically returns a login form which is the case here for Keycloak as well.
+                -- However, how we process the form to perform the login is specific to Keycloak and
+                -- possibly even the version used.
+                res, err = httpc:request_uri(res.headers['Location'], {method = "GET"})
+
+                if not res then
+                    -- No response, must be an error.
+                    ngx.status = 500
+                    ngx.say(err)
+                    return
+                elseif res.status ~= 200 then
+                    -- Unexpected response.
+                    ngx.status = res.status
+                    ngx.say(res.body)
+                    return
+                end
+
+                -- Check if response code was ok.
+                if res.status == 200 then
+                    -- From the returned form, extract the submit URI and parameters.
+                    local uri, params = res.body:match('.*action="(.*)%?(.*)" method="post">')
+
+                    -- Substitute escaped ampersand in parameters.
+                    params = params:gsub("&amp;", "&")
+
+                    -- Get all cookies returned. Probably not so important since not part of OIDC specification.
+                    local auth_cookies = res.headers['Set-Cookie']
+
+                    -- Concatenate cookies into one string as expected when sent in request header.
+                    local auth_cookie_str = ""
+
+                    if type(auth_cookies) == 'string' then
+                        auth_cookie_str = auth_cookies:match('([^;]*); .*')
+                    else
+                        -- Must be a table.
+                        local len = #auth_cookies
+                        if len > 0 then
+                            auth_cookie_str = auth_cookies[1]:match('([^;]*); .*')
+                            for i = 2, len do
+                                auth_cookie_str = auth_cookie_str .. "; " .. auth_cookies[i]:match('([^;]*); .*')
+                            end
+                        end
+                    end
+
+                    -- Invoke the submit URI with parameters and cookies, adding username and password in the body.
+                    -- Note: Username and password are specific to the Keycloak Docker image used.
+                    res, err = httpc:request_uri(uri .. "?" .. params, {
+                            method = "POST",
+                            body = "username=teacher@gmail.com&password=123456",
+                            headers = {
+                                ["Content-Type"] = "application/x-www-form-urlencoded",
+                                ["Cookie"] = auth_cookie_str
+                            }
+                        })
+
+                    if not res then
+                        -- No response, must be an error.
+                        ngx.status = 500
+                        ngx.say(err)
+                        return
+                    elseif res.status ~= 302 then
+                        -- Not a redirect which we expect.
+                        -- Use 500 to indicate error.
+                        ngx.status = 500
+                        ngx.say("Login form submission did not return redirect to redirect URI.")
+                        return
+                    end
+
+                    -- Extract the redirect URI from the response header.
+                    -- TODO: Consider validating this against the plugin configuration.
+                    local redirect_uri = res.headers['Location']
+
+                    -- Invoke the redirect URI (which contains the authorization code as an URL parameter).
+                    res, err = httpc:request_uri(redirect_uri, {
+                            method = "GET",
+                            headers = {
+                                ["Cookie"] = cookie_str
+                            }
+                        })
+
+                    if not res then
+                        -- No response, must be an error.
+                        ngx.status = 500
+                        ngx.say(err)
+                        return
+                    elseif res.status ~= 302 then
+                        -- Not a redirect which we expect.
+                        -- Use 500 to indicate error.
+                        ngx.status = 500
+                        ngx.say("Invoking redirect URI with authorization code did not return redirect to original URI.")
+                        return
+                    end
+
+                    -- Get all cookies returned. This should update the session cookie maintained by the OIDC module with the new state.
+                    -- E.g. the session cookie should now contain the access token, ID token and user info.
+                    -- The cookie itself should however be treated as opaque.
+                    cookies = res.headers['Set-Cookie']
+
+                    -- Concatenate cookies into one string as expected when sent in request header.
+                    if type(cookies) == 'string' then
+                        cookie_str = cookies:match('([^;]*); .*')
+                    else
+                        -- Must be a table.
+                        local len = #cookies
+                        if len > 0 then
+                            cookie_str = cookies[1]:match('([^;]*); .*')
+                            for i = 2, len do
+                                cookie_str = cookie_str .. "; " .. cookies[i]:match('([^;]*); .*')
+                            end
+                        end
+                    end
+
+                    -- Get the final URI out of the Location response header. This should be the original URI that was requested.

Review comment:
       Hi, @spacewander.
   > The similar Lua snipper is repeated in multiple tests.
   Yes, they are repeated. But I have no idea about refactoring it, can you give me some examples or docs? 




-- 
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 change in pull request #6455: feat: support post_logout_redirect_uri config in openid-connect plugin

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



##########
File path: docs/en/latest/plugins/openid-connect.md
##########
@@ -47,6 +47,7 @@ The OAuth 2 / Open ID Connect(OIDC) plugin provides authentication and introspec
 | realm                                | string  | optional    | "apisix"              |         | Realm used for the authentication                                                                                               |
 | bearer_only                          | boolean | optional    | false                 |         | Setting this `true` will check for the authorization header in the request with a bearer token                                  |
 | logout_path                          | string  | optional    | "/logout"             |         |                                                                                                                                 |
+| post_logout_redirect_uri   | string | optional |              |  | URL want to redirect when request logout_path

Review comment:
       please align the table

##########
File path: t/plugin/openid-connect.t
##########
@@ -1665,3 +1665,319 @@ GET /t
 false
 --- error_log
 OIDC introspection failed: invalid jwt: invalid jwt string
+
+
+
+=== TEST 28: Modify route to match catch-all URI `/*` and add post_logout_redirect_uri option.
+--- 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:8090/auth/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,
+                                "timeout": 10,
+                                "introspection_endpoint_auth_method": "client_secret_post",
+                                "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/*"
+                }]],
+                [[{
+                    "node": {
+                        "value": {
+                            "plugins": {
+                                "openid-connect": {
+                                    "client_id": "course_management",
+                                    "client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
+                                    "discovery": "http://127.0.0.1:8090/auth/realms/University/.well-known/openid-configuration",
+                                    "redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
+                                    "ssl_verify": false,
+                                    "timeout": 10,
+                                    "realm": "University",
+                                    "introspection_endpoint_auth_method": "client_secret_post",
+                                    "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                    "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                                }
+                            },
+                            "upstream": {
+                                "nodes": {
+                                    "127.0.0.1:1980": 1
+                                },
+                                "type": "roundrobin"
+                            },
+                            "uri": "/*"
+                        },
+                        "key": "/apisix/routes/1"
+                    },
+                    "action": "set"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 29: Access route w/o bearer token and request logout to redirect to post_logout_redirect_uri.
+--- config
+    location /t {
+        content_by_lua_block {
+            local http = require "resty.http"
+            local httpc = http.new()
+
+            -- Invoke /uri endpoint w/o bearer token. Should receive redirect to Keycloak authorization endpoint.
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/uri"
+            local res, err = httpc:request_uri(uri, {method = "GET"})
+
+            if not res then
+                -- No response, must be an error.
+                ngx.status = 500
+                ngx.say(err)
+                return
+            elseif res.status ~= 302 then
+                -- Not a redirect which we expect.
+                -- Use 500 to indicate error.
+                ngx.status = 500
+                ngx.say("Initial request was not redirected to ID provider authorization endpoint.")
+                return
+            else
+                -- Redirect to ID provider's authorization endpoint.
+
+                -- Extract nonce and state from response header.
+                local nonce = res.headers['Location']:match('.*nonce=([^&]+).*')
+                local state = res.headers['Location']:match('.*state=([^&]+).*')
+
+                -- Extract cookies. Important since OIDC module tracks state with a session cookie.
+                local cookies = res.headers['Set-Cookie']
+
+                -- Concatenate cookies into one string as expected when sent in request header.
+                local cookie_str = ""
+
+                if type(cookies) == 'string' then
+                    cookie_str = cookies:match('([^;]*); .*')
+                else
+                    -- Must be a table.
+                    local len = #cookies
+                    if len > 0 then
+                        cookie_str = cookies[1]:match('([^;]*); .*')
+                        for i = 2, len do
+                            cookie_str = cookie_str .. "; " .. cookies[i]:match('([^;]*); .*')
+                        end
+                    end
+                end
+
+                -- Call authorization endpoint we were redirected to.
+                -- Note: This typically returns a login form which is the case here for Keycloak as well.
+                -- However, how we process the form to perform the login is specific to Keycloak and
+                -- possibly even the version used.
+                res, err = httpc:request_uri(res.headers['Location'], {method = "GET"})
+
+                if not res then
+                    -- No response, must be an error.
+                    ngx.status = 500
+                    ngx.say(err)
+                    return
+                elseif res.status ~= 200 then
+                    -- Unexpected response.
+                    ngx.status = res.status
+                    ngx.say(res.body)
+                    return
+                end
+
+                -- Check if response code was ok.
+                if res.status == 200 then
+                    -- From the returned form, extract the submit URI and parameters.
+                    local uri, params = res.body:match('.*action="(.*)%?(.*)" method="post">')
+
+                    -- Substitute escaped ampersand in parameters.
+                    params = params:gsub("&amp;", "&")
+
+                    -- Get all cookies returned. Probably not so important since not part of OIDC specification.
+                    local auth_cookies = res.headers['Set-Cookie']
+
+                    -- Concatenate cookies into one string as expected when sent in request header.
+                    local auth_cookie_str = ""
+
+                    if type(auth_cookies) == 'string' then
+                        auth_cookie_str = auth_cookies:match('([^;]*); .*')
+                    else
+                        -- Must be a table.
+                        local len = #auth_cookies
+                        if len > 0 then
+                            auth_cookie_str = auth_cookies[1]:match('([^;]*); .*')
+                            for i = 2, len do
+                                auth_cookie_str = auth_cookie_str .. "; " .. auth_cookies[i]:match('([^;]*); .*')
+                            end
+                        end
+                    end
+
+                    -- Invoke the submit URI with parameters and cookies, adding username and password in the body.
+                    -- Note: Username and password are specific to the Keycloak Docker image used.
+                    res, err = httpc:request_uri(uri .. "?" .. params, {
+                            method = "POST",
+                            body = "username=teacher@gmail.com&password=123456",
+                            headers = {
+                                ["Content-Type"] = "application/x-www-form-urlencoded",
+                                ["Cookie"] = auth_cookie_str
+                            }
+                        })
+
+                    if not res then
+                        -- No response, must be an error.
+                        ngx.status = 500
+                        ngx.say(err)
+                        return
+                    elseif res.status ~= 302 then
+                        -- Not a redirect which we expect.
+                        -- Use 500 to indicate error.
+                        ngx.status = 500
+                        ngx.say("Login form submission did not return redirect to redirect URI.")
+                        return
+                    end
+
+                    -- Extract the redirect URI from the response header.
+                    -- TODO: Consider validating this against the plugin configuration.
+                    local redirect_uri = res.headers['Location']
+
+                    -- Invoke the redirect URI (which contains the authorization code as an URL parameter).
+                    res, err = httpc:request_uri(redirect_uri, {
+                            method = "GET",
+                            headers = {
+                                ["Cookie"] = cookie_str
+                            }
+                        })
+
+                    if not res then
+                        -- No response, must be an error.
+                        ngx.status = 500
+                        ngx.say(err)
+                        return
+                    elseif res.status ~= 302 then
+                        -- Not a redirect which we expect.
+                        -- Use 500 to indicate error.
+                        ngx.status = 500
+                        ngx.say("Invoking redirect URI with authorization code did not return redirect to original URI.")
+                        return
+                    end
+
+                    -- Get all cookies returned. This should update the session cookie maintained by the OIDC module with the new state.
+                    -- E.g. the session cookie should now contain the access token, ID token and user info.
+                    -- The cookie itself should however be treated as opaque.
+                    cookies = res.headers['Set-Cookie']
+
+                    -- Concatenate cookies into one string as expected when sent in request header.
+                    if type(cookies) == 'string' then
+                        cookie_str = cookies:match('([^;]*); .*')
+                    else
+                        -- Must be a table.
+                        local len = #cookies
+                        if len > 0 then
+                            cookie_str = cookies[1]:match('([^;]*); .*')
+                            for i = 2, len do
+                                cookie_str = cookie_str .. "; " .. cookies[i]:match('([^;]*); .*')
+                            end
+                        end
+                    end
+
+                    -- Get the final URI out of the Location response header. This should be the original URI that was requested.

Review comment:
       The similar Lua snipper is repeated in multiple tests. Could we refactor it?

##########
File path: t/plugin/openid-connect.t
##########
@@ -1665,3 +1665,319 @@ GET /t
 false
 --- error_log
 OIDC introspection failed: invalid jwt: invalid jwt string
+
+
+
+=== TEST 28: Modify route to match catch-all URI `/*` and add post_logout_redirect_uri option.
+--- 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:8090/auth/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,
+                                "timeout": 10,
+                                "introspection_endpoint_auth_method": "client_secret_post",
+                                "introspection_endpoint": "http://127.0.0.1:8090/auth/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,
+                                "post_logout_redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/hello"
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/*"
+                }]],
+                [[{
+                    "node": {

Review comment:
       We don't need to check the response data of this request, right?




-- 
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 #6455: feat: support post_logout_redirect_uri config in openid-connect plugin

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


   


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