Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: close session in case of error to avoid blocked session #11089

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apisix/plugins/openid-connect.lua
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,9 @@ function _M.rewrite(plugin_conf, ctx)
response, err, _, session = openidc.authenticate(conf, nil, unauth_action, conf.session)

if err then
if session then
session:close()
end
if err == "unauthorized request" then
if conf.unauth_action == "pass" then
return nil
Expand Down
94 changes: 94 additions & 0 deletions t/plugin/openid-connect5.t
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,97 @@ __DATA__
}
--- response_body_like
hello world



=== TEST 2: Call to route with locking session storage, no authentication and unauth_action 'deny' should not block subsequent requests on same session
--- config
set $session_storage redis;
set $session_redis_prefix sessions;
set $session_redis_database 0;
set $session_redis_connect_timeout 1000; # (in milliseconds)
set $session_redis_send_timeout 1000; # (in milliseconds)
set $session_redis_read_timeout 1000; # (in milliseconds)
set $session_redis_host 127.0.0.1;
set $session_redis_port 6379;
set $session_redis_ssl off;
set $session_redis_ssl_verify off;
set $session_redis_uselocking on;
set $session_redis_spinlockwait 150; # (in milliseconds)
set $session_redis_maxlockwait 30; # (in seconds)

location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local http = require "resty.http"
local login_keycloak = require("lib.keycloak").login_keycloak
local concatenate_cookies = require("lib.keycloak").concatenate_cookies

local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"plugins": {
"openid-connect": {
"discovery": "http://127.0.0.1:8080/realms/University/.well-known/openid-configuration",
"realm": "University",
"client_id": "course_management",
"client_secret": "d1ec69e9-55d2-4109-a3ea-befa071579d5",
"redirect_uri": "http://127.0.0.1:]] .. ngx.var.server_port .. [[/authenticated",
"ssl_verify": false,
"bearer_only" : false,
"timeout": 10,
"introspection_endpoint_auth_method": "client_secret_post",
"introspection_endpoint": "http://127.0.0.1:8080/realms/University/protocol/openid-connect/token/introspect",
"set_access_token_header": true,
"access_token_in_authorization_header": false,
"set_id_token_header": true,
"set_userinfo_header": true,
"set_refresh_token_header": true,
"unauth_action": "deny"
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/*"
}]]
)

local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello"

-- Make the final call to protected route WITHOUT cookie
local httpc = http.new()
local res, err = httpc:request_uri(uri, {method = "GET"})

-- Extract cookie which is not authenticated
local cookie_str = concatenate_cookies(res.headers['Set-Cookie'])

-- Make the call to protected route cookie
local function firstRequest()
local httpc = http.new()
httpc:request_uri(uri, {
method = "GET",
headers = {
["Cookie"] = cookie_str
}
})
end
ngx.thread.spawn(firstRequest)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can you ensure that the first request is arrived before the second one?
I think maybe you can give a return value of the firstRequest function , and use ngx.thread.wait to get the return value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@starsz Thank you for the suggestion, I have implemented a wait for the completion of the first request.


-- Make second call to protected route and same cookie which should not timeout due ton a blocked session
local httpc = http.new()
httpc:set_timeout(2000)

res, err = httpc:request_uri(uri, {
method = "GET",
headers = {
["Cookie"] = cookie_str
}
})
ngx.status = res.status
}
}
--- error_code: 401
Loading