Introspection functionality #12
Conversation
kong/plugins/oidc/handler.lua
Outdated
| @@ -29,27 +29,52 @@ function CustomHandler:access(config) | |||
|
|
|||
| session.configure(config) | |||
|
|
|||
| local res, err = require("resty.openidc").authenticate(oidcConfig) | |||
| if tryIntrospect(oidcConfig) then | |||
There was a problem hiding this comment.
Please, update also README.md and explain "two legged authentication" usage maybe at "What is Kong OIDC plugin" section.
|
You need to rename also kong-oidc-1.0.2-0.rockspec to kong-oidc-1.0.3-0.rockspec and update version = "1.0.2-0" --> "1.0.3-0" and tag = "v1.0.2" --> "v1.0.3". Then, after the pull request is accepted and merged to the master, tag the master as v1.0.3 and update the latest version to luarocks server (I can help you in this step). So, then we will have this new feature published in release 1.0.3-0. |
|
LGTM |
|
Changed version number in rockspec and added brief description into README.md |
kong/plugins/oidc/handler.lua
Outdated
| end | ||
|
|
||
| if res and res.user then | ||
| utils.injectUser(res.user) |
There was a problem hiding this comment.
The user field isn't, when the user uses this flow. So Kong doesn't know, that the requests is already authenticated. You would need to set minimum the authenticated_consumer.id field from the subject claim.
There was a problem hiding this comment.
Fixed -> now providing sub field as consumer id
kong/plugins/oidc/handler.lua
Outdated
| if config.recovery_page_path then | ||
| ngx.log(ngx.DEBUG, "Entering recovery page: " .. config.recovery_page_path) | ||
| return ngx.redirect(config.recovery_page_path) | ||
| ngx.log(ngx.DEBUG, "In plugin CustomHandler:proceeding with two legged authentication, requested path: " .. ngx.var.request_uri) |
There was a problem hiding this comment.
This log is kinda misleading. Sending a token doesn't mean the client used a two legged flow. It's out of scope for this plugin. It should only check, if the token it gets (no matter, if it's a access token, AAT, RPT) was created by the Authorization Server.
Also the CustomHandler is misleading, but IMO this is out of scope for this MR.
There was a problem hiding this comment.
Fixed -> now it says that valid token can go through
| @@ -4,6 +4,7 @@ return { | |||
| client_id = { type = "string", required = true }, | |||
| client_secret = { type = "string", required = true }, | |||
| discovery = { type = "string", required = true, default = "https://.well-known/openid-configuration" }, | |||
| introspection_endpoint = { type = "string", required = false }, | |||
There was a problem hiding this comment.
IMO, the Introspection Endpoint should be fetched from the OIDC Discovery Endpoint.
There was a problem hiding this comment.
Nope -> in OAuth only AS you will not have discovery uri
There was a problem hiding this comment.
That's a plugin for OIDC, so you can assume, that there must be a Discovery Endpoint
There was a problem hiding this comment.
introspection however is an optional extension to OAuth 2.0 and OIDC Discovery in its turn is OpenID Connect specific and predates the standardization of the introspection OAuth 2.0 extension, so OIDC Discovery hasn't standardized a key for the introspection endpoint
now there's WIP here https://tools.ietf.org/html/draft-ietf-oauth-discovery that standardizes Discovery for OAuth 2.0 and that includes the introspection endpoint... thats still TBD
kong/plugins/oidc/handler.lua
Outdated
| @@ -29,27 +29,52 @@ function CustomHandler:access(config) | |||
|
|
|||
| session.configure(config) | |||
|
|
|||
| local res, err = require("resty.openidc").authenticate(oidcConfig) | |||
| if tryIntrospect(oidcConfig) then | |||
There was a problem hiding this comment.
Those nested if/else statements smell a bit. I would create a few functions, so it
[...] reads like well-written prose
There was a problem hiding this comment.
Please, reveal us those few functions you would write instead of nested ifs.
There was a problem hiding this comment.
created doAuthentication function to make code more clear
Story/stry0038018
added scope validation unit tests
No description provided.