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

feat: Make headers to add to request in openid-connect plugin configurable. #2903

Merged
merged 112 commits into from
Jan 5, 2021
Merged
Show file tree
Hide file tree
Changes from 109 commits
Commits
Show all changes
112 commits
Select commit Hold shift + click to select a range
4af92db
Make headers to add to request configurable.
jenskeiner Nov 30, 2020
fc57f35
Fix lines that were too long.
jenskeiner Nov 30, 2020
49e904f
Import global string module.
jenskeiner Nov 30, 2020
4c033d9
Fix an issue when trying to call a boolean value.
jenskeiner Dec 1, 2020
e997baa
Addressing several pull request comments.
jenskeiner Dec 1, 2020
fbca5d8
Remove old way of setting defaults for added configuration options.
jenskeiner Dec 1, 2020
b842247
Merge branch 'master' into 2764-oidc-configurable-headers
jenskeiner Dec 1, 2020
8eabdef
Fix lines that are too long.
jenskeiner Dec 1, 2020
6328c68
Remove trailing whitespace.
jenskeiner Dec 1, 2020
bfa4aa7
Add auxilliary method to insert/update request header while updating …
jenskeiner Dec 1, 2020
1aeec0b
Small fixes.
jenskeiner Dec 1, 2020
eb73cee
Merge branch 'master_upstream' into 2764-oidc-configurable-headers
jenskeiner Dec 16, 2020
f13fc9d
CHange backend from 127.0.0.1:1980 to 127.0.0.1:8888 to use mendhak/h…
jenskeiner Dec 16, 2020
f088a88
Fix port number.
jenskeiner Dec 16, 2020
a4d503d
Revert local port back to 1980.
jenskeiner Dec 16, 2020
76c6b8b
Add endpoint that reflects OIDC-relevant request headers back in resp…
jenskeiner Dec 16, 2020
ade0bec
Add test cases to check if request bearer token in Authoriztation hea…
jenskeiner Dec 16, 2020
19c62a7
Fix issue when not all desired headers were reflected.
jenskeiner Dec 16, 2020
84c8684
Fix header verification tests.
jenskeiner Dec 16, 2020
4ce30ac
Fix test cases.
jenskeiner Dec 16, 2020
1b9cf28
Temporarily run only openid-connect plugin tests.
jenskeiner Dec 17, 2020
de7600e
Use different endpoint to reflect headers back in response body.
jenskeiner Dec 17, 2020
2e19611
Use direct GET request.
jenskeiner Dec 17, 2020
989f6b9
Fix unit tests.
jenskeiner Dec 17, 2020
d865b5d
Fix test case.
jenskeiner Dec 17, 2020
032b0e8
Remove obsolete helper function.
jenskeiner Dec 17, 2020
0ead6da
Improve documentation. Start new test case that simulates the authori…
jenskeiner Dec 17, 2020
593853b
Further process response from authorization endpoint and print to res…
jenskeiner Dec 17, 2020
a5ece5d
Small fixes.
jenskeiner Dec 17, 2020
f916b7e
Print out cookies.
jenskeiner Dec 17, 2020
8194baa
Concatenate extracted cookies into request header format.
jenskeiner Dec 17, 2020
12cf142
Fix cookie header format.
jenskeiner Dec 17, 2020
a9f4248
Extract raw cookie values.
jenskeiner Dec 18, 2020
728c750
Log out response of initial call to /hello endpoint redirect to ID pr…
jenskeiner Dec 18, 2020
3de1aff
Fix missing content_by_lua_block.
jenskeiner Dec 18, 2020
866fcbd
Fix more errors.
jenskeiner Dec 18, 2020
a04ca52
More fixes.
jenskeiner Dec 18, 2020
86589f2
Reorganise test order.
jenskeiner Dec 18, 2020
0a1ee78
More fixes.
jenskeiner Dec 18, 2020
009c6bc
More fixes.
jenskeiner Dec 18, 2020
f872a58
Even more fixes.
jenskeiner Dec 18, 2020
797c44a
Extract nonce, state and session cookie.
jenskeiner Dec 18, 2020
4a9a5a3
Fixes.
jenskeiner Dec 18, 2020
3249f59
Fixes.
jenskeiner Dec 18, 2020
385de27
Add back in some code.
jenskeiner Dec 18, 2020
58b8e55
More debugging.
jenskeiner Dec 18, 2020
071da28
Debugging.
jenskeiner Dec 18, 2020
f39187a
Add back in nonce and state extraction.
jenskeiner Dec 18, 2020
8f669dc
Add back in call to authorization endpoint.
jenskeiner Dec 18, 2020
70e2407
Fix syntax error.
jenskeiner Dec 18, 2020
cd3cb32
Debugging.
jenskeiner Dec 18, 2020
ad1a527
Debugging.
jenskeiner Dec 18, 2020
3100ff7
Debugging.
jenskeiner Dec 18, 2020
3f3e447
Add back assembling of form submission request.
jenskeiner Dec 18, 2020
3d5596f
Send actual request to simulate form submission.
jenskeiner Dec 18, 2020
ec55584
Pass some parameters in URL, not request body.
jenskeiner Dec 18, 2020
7fac749
Set status first.
jenskeiner Dec 18, 2020
fe3493f
Minor fix.
jenskeiner Dec 18, 2020
930fd2d
Only print out Location header.
jenskeiner Dec 18, 2020
3572499
Add /authenticated endpoint to route.
jenskeiner Dec 18, 2020
636b518
Revert previous change.
jenskeiner Dec 18, 2020
3a93ce2
Use catch-all route.
jenskeiner Dec 18, 2020
c348143
Fix redirect URL.
jenskeiner Dec 18, 2020
bf948e8
Actually call redirect_uri.
jenskeiner Dec 18, 2020
5fe5fc2
Add other cookies.
jenskeiner Dec 18, 2020
926729b
Use redirect URI from Location header directly.
jenskeiner Dec 18, 2020
730ef07
Final redirect.
jenskeiner Dec 18, 2020
ce4d794
Debugging.
jenskeiner Dec 18, 2020
c17b359
Fix final URI. Update cookie.
jenskeiner Dec 18, 2020
431a0fe
Use /uri endpoint.
jenskeiner Dec 19, 2020
1e77d12
Start clean up.
jenskeiner Dec 21, 2020
78eddff
Continue clean up.
jenskeiner Dec 21, 2020
b98d609
Continue clean up.
jenskeiner Dec 21, 2020
76c613e
Continue clean up.
jenskeiner Dec 21, 2020
f0ce405
Continue clean up.
jenskeiner Dec 21, 2020
ec57b24
Fix stray end.
jenskeiner Dec 21, 2020
159672d
Make config parametrizable through NGINX variables.
jenskeiner Dec 21, 2020
ec8addd
Clean up test cases.
jenskeiner Dec 22, 2020
61cce4d
Fix test cases.
jenskeiner Dec 22, 2020
2afae7b
Fix test cases.
jenskeiner Dec 22, 2020
0f8a53d
Fix unit tests.
jenskeiner Dec 22, 2020
40bd353
Fix test cases.
jenskeiner Dec 22, 2020
8fb5bd3
Debugging.
jenskeiner Dec 22, 2020
d9bdcef
Take out another test for debugging.
jenskeiner Dec 22, 2020
eb36145
Add back case.
jenskeiner Dec 22, 2020
239f506
Fix test case.
jenskeiner Dec 22, 2020
8a43122
Add back another case.
jenskeiner Dec 22, 2020
0a8b046
Debugging.
jenskeiner Dec 22, 2020
61f6008
Debugging.
jenskeiner Dec 22, 2020
5e2ea0a
Debugging.
jenskeiner Dec 22, 2020
95cd514
Debuging.
jenskeiner Dec 22, 2020
3583c3a
Debugging.
jenskeiner Dec 22, 2020
1e7031f
Merge branch 'master_upstream' into 2764-oidc-configurable-headers
jenskeiner Dec 22, 2020
d12aa16
Add defaults check.
jenskeiner Dec 22, 2020
ba4260f
Re-enable all tests. Revert server.lua.
jenskeiner Dec 22, 2020
aed18dc
Clean up.
jenskeiner Dec 22, 2020
700d26c
Fix linting errors.
jenskeiner Dec 22, 2020
8bf1498
Clean up.
jenskeiner Dec 23, 2020
ae34c7f
Minor fixes.
jenskeiner Dec 26, 2020
d9ab5ba
Minor fix.
jenskeiner Dec 26, 2020
8ca9c8e
Fix test.
jenskeiner Dec 26, 2020
0e924d7
Remove duplicate test case.
jenskeiner Dec 28, 2020
18a188b
Check Authorization heade parse result.
jenskeiner Dec 28, 2020
5354eca
Fix stray return value.
jenskeiner Dec 28, 2020
207cfa1
Always try to extract access token from header if bearer_only flag is…
jenskeiner Dec 28, 2020
61ae9f3
Fix test cases.
jenskeiner Dec 28, 2020
d3b046b
Fix expected response header.
jenskeiner Dec 28, 2020
29a4904
Trivial change to re-start failed checks on GitHub.
jenskeiner Dec 28, 2020
307e07b
Move check into auxilliary function.
jenskeiner Dec 29, 2020
f51baf7
Update plugin documentation.
jenskeiner Jan 4, 2021
4ec11c6
Fix stray quotation mark.
jenskeiner Jan 4, 2021
6f93a1e
Merge branch 'master_upstream' into 2764-oidc-configurable-headers
jenskeiner Jan 4, 2021
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
208 changes: 165 additions & 43 deletions apisix/plugins/openid-connect.lua
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
-- See the License for the specific language governing permissions and
-- limitations under the License.
--
local string = string
local core = require("apisix.core")
local ngx_re = require("ngx.re")
local openidc = require("resty.openidc")
Expand Down Expand Up @@ -67,7 +68,31 @@ local schema = {
description = "use ngx.var.request_uri if not configured"
},
public_key = {type = "string"},
token_signing_alg_values_expected = {type = "string"}
token_signing_alg_values_expected = {type = "string"},
set_access_token_header = {
description = "Whether the access token should be added as a header to the request " ..
"for downstream",
type = "boolean",
default = true
},
access_token_in_authorization_header = {
description = "Whether the access token should be added in the Authorization " ..
"header as opposed to the X-Access-Token header.",
type = "boolean",
default = false
},
set_id_token_header = {
description = "Whether the ID token should be added in the X-ID-Token header to " ..
"the request for downstream.",
type = "boolean",
default = true
},
set_userinfo_header = {
description = "Whether the user info token should be added in the X-Userinfo " ..
"header to the request for downstream.",
type = "boolean",
default = true
}
},
required = {"client_id", "client_secret", "discovery"}
}
Expand All @@ -80,6 +105,7 @@ local _M = {
schema = schema,
}


function _M.check_schema(conf)
if conf.ssl_verify == "no" then
-- we used to set 'ssl_verify' to "no"
Expand All @@ -95,56 +121,117 @@ function _M.check_schema(conf)
end


local function has_bearer_access_token(ctx)
local function get_bearer_access_token(ctx)
-- Get Authorization header, maybe.
local auth_header = core.request.header(ctx, "Authorization")
if not auth_header then
return false
-- No Authorization header, get X-Access-Token header, maybe.
local access_token_header = core.request.header(ctx, "X-Access-Token")
if not access_token_header then
-- No X-Access-Token header neither.
return false, nil, nil
end

-- Return extracted header value.
return true, access_token_header, nil
end

-- Check format of Authorization header.
local res, err = ngx_re.split(auth_header, " ", nil, nil, 2)

if not res then
return false, err
-- No result was returned.
return false, nil, err
elseif #res < 2 then
-- Header doesn't split into enough tokens.
return false, nil, "Invalid Authorization header format."
end

if res[1] == "bearer" then
return true
if string.lower(res[1]) == "bearer" then
-- Return extracted token.
return true, res[2], nil
end

return false
return false, nil, nil
end


local function introspect(ctx, conf)
if has_bearer_access_token(ctx) or conf.bearer_only then
local res, err
-- Extract token, maybe.
local has_token, token, err = get_bearer_access_token(ctx)

if err then
return ngx.HTTP_BAD_REQUEST, err, nil, nil
end

if not has_token then
-- Could not find token.

if conf.public_key then
res, err = openidc.bearer_jwt_verify(conf)
if res then
return res
end
else
res, err = openidc.introspect(conf)
if err then
return ngx.HTTP_UNAUTHORIZED, err
else
return res
end
end
if conf.bearer_only then
ngx.header["WWW-Authenticate"] = 'Bearer realm="' .. conf.realm
.. '",error="' .. err .. '"'
return ngx.HTTP_UNAUTHORIZED, err
-- Token strictly required in request.
ngx.header["WWW-Authenticate"] = 'Bearer realm="' .. conf.realm .. '"'
return ngx.HTTP_UNAUTHORIZED, "No bearer token found in request.", nil, nil
else
-- Return empty result.
return nil, nil, nil, nil
end
end

return nil
-- If we get here, token was found in request.

if conf.public_key then
-- Validate token against public key.
-- TODO: In the called method, the openidc module will try to extract
-- the token by itself again -- from a request header or session cookie.
-- It is inefficient that we also need to extract it (just from headers)
-- so we can add it in the configured header. Find a way to use openidc
-- module's internal methods to extract the token.
local res, err = openidc.bearer_jwt_verify(conf)

if err then
-- Error while validating or token invalid.
ngx.header["WWW-Authenticate"] = 'Bearer realm="' .. conf.realm ..
'", error="invalid_token", error_description="' .. err .. '"'
return ngx.HTTP_UNAUTHORIZED, err, nil, nil
end

-- Token successfully validated.
return res, err, token, nil
else
-- Validate token against introspection endpoint.
-- TODO: Same as above for public key validation.
local res, err = openidc.introspect(conf)

if err then
ngx.header["WWW-Authenticate"] = 'Bearer realm="' .. conf.realm ..
'", error="invalid_token", error_description="' .. err .. '"'
return ngx.HTTP_UNAUTHORIZED, err, nil, nil
end

-- Token successfully validated and response from the introspection
-- endpoint contains the userinfo.
return res, err, token, res
end
end


local function add_user_header(user)
local userinfo = core.json.encode(user)
ngx.req.set_header("X-Userinfo", ngx_encode_base64(userinfo))
local function add_access_token_header(ctx, conf, token)
jenskeiner marked this conversation as resolved.
Show resolved Hide resolved
if token then
-- Add Authorization or X-Access-Token header, respectively, if not already set.
if conf.set_access_token_header then
if conf.access_token_in_authorization_header then
if not core.request.header(ctx, "Authorization") then
-- Add Authorization header.
core.request.set_header(ctx, "Authorization", "Bearer " .. token)
end
else
if not core.request.header(ctx, "X-Access-Token") then
-- Add X-Access-Token header.
core.request.set_header(ctx, "X-Access-Token", token)
end
end
end
end
end


Expand All @@ -160,40 +247,75 @@ function _M.rewrite(plugin_conf, ctx)
if not conf.redirect_uri then
conf.redirect_uri = ctx.var.request_uri
end

if not conf.ssl_verify then
-- openidc use "no" to disable ssl verification
conf.ssl_verify = "no"
end

local response, err
if conf.introspection_endpoint or conf.public_key then
response, err = introspect(ctx, conf)

if conf.bearer_only or conf.introspection_endpoint or conf.public_key then
-- An introspection endpoint or a public key has been configured. Try to
-- validate the access token from the request, if it is present in a
-- request header. Otherwise, return a nil response. See below for
-- handling of the case where the access token is stored in a session cookie.
local access_token, userinfo
response, err, access_token, userinfo = introspect(ctx, conf)

if err then
core.log.error("failed to introspect in openidc: ", err)
-- Error while validating token or invalid token.
core.log.error("OIDC introspection failed: ", err)
return response
end

if response then
Copy link
Member

Choose a reason for hiding this comment

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

Why remove this part?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason is that adding the user info header is now handled in the introspect method. The header will still be added, if so configured, and introspection is done via the actual endpoint as opposed to just checking against the public key. Let me know if you see any problem with this approach.

add_user_header(response)
-- Add configured access token header, maybe.
add_access_token_header(ctx, conf, access_token)

if userinfo and conf.set_userinfo_header then
-- Set X-Userinfo header to introspection endpoint response.
core.request.set_header(ctx, "X-Userinfo",
ngx_encode_base64(core.json.encode(userinfo)))
end
end
end

if not response then
local response, err = openidc.authenticate(conf)
-- Either token validation via introspection endpoint or public key is
-- not configured, and/or token could not be extracted from the request.

-- Authenticate the request. This will validate the access token if it
-- is stored in a session cookie, and also renew the token if required.
-- If no token can be extracted, the response will redirect to the ID
-- provider's authorization endpoint to initiate the Relying Party flow.
-- This code path also handles when the ID provider then redirects to
-- the configured redirect URI after successful authentication.
response, err = openidc.authenticate(conf)

if err then
core.log.error("failed to authenticate in openidc: ", err)
core.log.error("OIDC authentication failed: ", err)
return 500
end

if response then
if response.user then
add_user_header(response.user)
end
if response.access_token then
ngx.req.set_header("X-Access-Token", response.access_token)
end
if response.id_token then
-- If the openidc module has returned a response, it may contain,
-- respectively, the access token, the ID token, and the userinfo.
-- Add respective headers to the request, if so configured.

-- Add configured access token header, maybe.
add_access_token_header(ctx, conf, response.access_token)
jenskeiner marked this conversation as resolved.
Show resolved Hide resolved

-- Add X-ID-Token header, maybe.
if response.id_token and conf.set_id_token_header then
local token = core.json.encode(response.id_token)
ngx.req.set_header("X-ID-Token", ngx.encode_base64(token))
core.request.set_header(ctx, "X-ID-Token", ngx.encode_base64(token))
end

-- Add X-Userinfo header, maybe.
if response.user and conf.set_userinfo_header then
core.request.set_header(ctx, "X-Userinfo",
ngx_encode_base64(core.json.encode(response.user)))
end
end
end
Expand Down
Loading