feat: add auth plugin for casdoor #6382
Conversation
6bfe556
to
753238b
Compare
753238b
to
2f138c8
Compare
465685d
to
6656e06
Compare
apisix/plugins/auth-casdoor.lua
Outdated
| local schema = { | ||
| type = "object", | ||
| properties = { | ||
| casdoor_endpoint = { type = "string" }, |
There was a problem hiding this comment.
Let's remove the casdoor_ prefix
There was a problem hiding this comment.
@ComradeProgrammer
Please request me to review it once you are ready. Thanks!
There was a problem hiding this comment.
Let's remove the casdoor_ prefix
all other casdoor_ prefixes are removed except casdoor_endpoint, because i think just using "endpoint" may be a little confusing.
apisix/plugins/auth-casdoor.lua
Outdated
| local schema = { | ||
| type = "object", | ||
| properties = { | ||
| casdoor_endpoint = { type = "string" }, |
There was a problem hiding this comment.
Let's remove the casdoor_ prefix
apisix/plugins/auth-casdoor.lua
Outdated
| local core = require("apisix.core") | ||
| local log = core.log | ||
| local ngx = ngx | ||
| local session = require("resty.session") |
There was a problem hiding this comment.
Let's sort the imports like
apisix/apisix/plugins/authz-casbin.lua
Line 20 in 4624135
conf/config-default.yaml
Outdated
| @@ -340,6 +340,7 @@ plugins: # plugin list (sorted by priority) | |||
| - request-validation # priority: 2800 | |||
| - openid-connect # priority: 2599 | |||
| - authz-casbin # priority: 2560 | |||
| - auth-casdoor # priority: 2561 | |||
There was a problem hiding this comment.
Please sort according to the priority and fix the indent
apisix/plugins/auth-casdoor.lua
Outdated
|
|
||
| local _M = { | ||
| version = 0.1, | ||
| priority = 12, |
apisix/plugins/auth-casdoor.lua
Outdated
|
|
||
| if session_obj_read.data.access_token then | ||
| -- session exists | ||
| log.warn("session exists") |
There was a problem hiding this comment.
Ditto. Warn log should not be used in per-req level
apisix/plugins/auth-casdoor.lua
Outdated
| local args = ngx.req.get_uri_args() | ||
| if not args.code or not args.state then | ||
| log.err("failed when accessing token. Invalid code or state ") | ||
| return nil |
There was a problem hiding this comment.
| return nil | |
| return nil, err |
and log the err outside would be better
apisix/plugins/auth-casdoor.lua
Outdated
| end | ||
| local client = http.new() | ||
| local res, err = client:request_uri( | ||
| get_path(conf.casdoor_endpoint) .. "/api/login/oauth/access_token", |
There was a problem hiding this comment.
Let's check if the endpoint contains ? in the schema instead of split it at runtime
apisix/plugins/auth-casdoor.lua
Outdated
| log.err("failed when accessing token. ", err) | ||
| return nil | ||
| end | ||
| local data = cjson.decode(res.body) |
There was a problem hiding this comment.
Need to check if decode succeed
|
|
||
| ``` | ||
|
|
||
| By doing this all requests toward `/anything/*` will be intercepted and whether this user has logged in via Casdoor will be checked (by checking session). If not, the user will be redirected to login page of casdoor, thus implementing authentication without modifying server's code. |
There was a problem hiding this comment.
Could you provide a step-by-step example?
I still can't fully understand it after reading it twice...
It looks like the callback_url needs to contain part of the request host & path?
|
I am extremely flattered, delighted and grateful for this extremely detailed reply and advice from you. Revisions will be made accordingly soon and tests will be added soon together. @spacewander |
3173296
to
653cfc6
Compare
apisix/plugins/auth-casdoor.lua
Outdated
| client_id = { type = "string" }, | ||
| client_secret = { type = "string" }, | ||
| callback_url = { type = "string" }, | ||
| test = { type= "boolean", default = false } |
There was a problem hiding this comment.
test is not in Attributes of plugin docs
apisix/plugins/auth-casdoor.lua
Outdated
| end | ||
|
|
||
| function _M.access(conf, ctx) | ||
| log.info("hit auth-casdoor access") |
There was a problem hiding this comment.
This line looks like it's only for debugging?
apisix/plugins/auth-casdoor.lua
Outdated
| local function fetch_access_token(conf) | ||
| local args = core.request.get_uri_args() | ||
| if not args.code or not args.state then | ||
| log.err() |
There was a problem hiding this comment.
Do you mean this function? Extract "code" and "state" parameters and use them to launch to casdoor api, in order to fetch "access_token" so that we can confirm whether the user have really logged in
There was a problem hiding this comment.
The err doesn't have parameters in it
apisix/plugins/auth-casdoor.lua
Outdated
| end | ||
|
|
||
| local function fetch_access_token(conf) | ||
| local args = core.request.get_uri_args() |
There was a problem hiding this comment.
| local args = core.request.get_uri_args() | |
| local args = core.request.get_uri_args(ctx) |
and should pass ctx to this function
apisix/plugins/auth-casdoor.lua
Outdated
|
|
||
| local function fetch_access_token(conf) | ||
| local args = core.request.get_uri_args() | ||
| if not args.code or not args.state then |
There was a problem hiding this comment.
it's better to determine if args is empty
apisix/plugins/auth-casdoor.lua
Outdated
| end | ||
|
|
||
| function _M.check_schema(conf) | ||
| local ok,err=core.schema.check(schema, conf) |
There was a problem hiding this comment.
| local ok,err=core.schema.check(schema, conf) | |
| local ok, err = core.schema.check(schema, conf) |
apisix/plugins/auth-casdoor.lua
Outdated
| local ok,err=core.schema.check(schema, conf) | ||
| if ok then | ||
| --check whether contains extra ? or / | ||
| if string.find(conf.callback_url,"?",1) then |
There was a problem hiding this comment.
use ngx.re.find is better?
apisix/plugins/auth-casdoor.lua
Outdated
| if ok then | ||
| --check whether contains extra ? or / | ||
| if string.find(conf.callback_url,"?",1) then | ||
| return false,"callback_url should not contain get parameters or end up with /" | ||
| else | ||
| return true,nil | ||
| end | ||
| else | ||
| return false,err | ||
| end |
There was a problem hiding this comment.
| if ok then | |
| --check whether contains extra ? or / | |
| if string.find(conf.callback_url,"?",1) then | |
| return false,"callback_url should not contain get parameters or end up with /" | |
| else | |
| return true,nil | |
| end | |
| else | |
| return false,err | |
| end | |
| if not ok then | |
| return false, err | |
| end | |
| --check whether contains extra ? or / | |
| local has_extra = string.find(conf.callback_url,"?",1) | |
| if has_extra then | |
| return false, "callback_url should not contain get parameters or end up with /" | |
| end | |
| return true, nil |
is better?
apisix/plugins/auth-casdoor.lua
Outdated
| if access_token then | ||
| local original_url = session_obj_read.data.original_uri | ||
| if not original_url then | ||
| return 503,"no original_url found in session" |
There was a problem hiding this comment.
| return 503,"no original_url found in session" | |
| return 503, "no original_url found in session" |
apisix/plugins/auth-casdoor.lua
Outdated
| session_obj_write.data.access_token = access_token | ||
| session_obj_write:save() | ||
| ngx.redirect(original_url, 302) | ||
| return |
There was a problem hiding this comment.
It seems a bit strange that here return does not reverse anything, but just exits the current function?
653cfc6
to
b5953d7
Compare
|
@spacewander @tzssangglass Most of the revisions in your comments have been implemented, and tests are also added. I think this code is ready to be reviewed now, THX. |
hi @ComradeProgrammer, after the CI run is finished, please fix the failure of the CI. |
52eea7e
to
c699468
Compare
I have made changes according to the CI information, and I wonder whether I can have CI run another time , so that I can confirm whether I can pass CI now? |
hi @ComradeProgrammer , there are still some code lint check failed. |
c699468
to
cc75f06
Compare
apisix/plugins/auth-casdoor.lua
Outdated
| local schema = { | ||
| type = "object", | ||
| properties = { | ||
| casdoor_endpoint = {type = "string"}, |
There was a problem hiding this comment.
| casdoor_endpoint = {type = "string"}, | |
| endpoint_addr = {type = "string"}, |
would be better?
apisix/plugins/auth-casdoor.lua
Outdated
| } | ||
| } | ||
|
|
||
| local _M = {version = 0.1, priority = 2559, name = plugin_name, schema = schema} |
There was a problem hiding this comment.
please put each field per line, like other plugins
apisix/plugins/auth-casdoor.lua
Outdated
| local function fetch_access_token(conf) | ||
| local args = core.request.get_uri_args() | ||
| if not args.code or not args.state then | ||
| log.err() |
There was a problem hiding this comment.
The err doesn't have parameters in it
apisix/plugins/auth-casdoor.lua
Outdated
| return nil, "failed when accessing token. Invalid code or state" | ||
| end | ||
| local client = http.new() | ||
| local url = get_path(conf.casdoor_endpoint) .. |
There was a problem hiding this comment.
Could we require the endpoint not to have ? so we don't need to use get_path?
apisix/plugins/auth-casdoor.lua
Outdated
| local ok, err = core.schema.check(schema, conf) | ||
| if not ok then return false, err end | ||
| -- check whether contains extra ? or / | ||
| local has_extra = string.find(conf.callback_url, "?", 1) |
There was a problem hiding this comment.
We can do the check in the schema via "pattern":
Line 34 in acf7a0c
|
|
||
| The first one is "callback_url". This is exactly the callback url in OAuth2. It should be emphasized that this callback url **must belong to the "uri" you specified for the route**, for example, in this example, http://localhost:9080/anything/callback obviously belong to "/anything/*" Only by this way can the visit toward callback_url can be intercepted and utilized by the plugin(so that the plugin can get the code and state in Oauth2). The logic of callback_url is implemented competely by plugin so that there is no need to modify the server implement this callback. | ||
|
|
||
| The second parameter "callback_url" is obviously the url of Casdoor. The third and fourth parameters are "client_id" and "client_secret", which you can acquire from Casdoor when you register an app. |
There was a problem hiding this comment.
| The second parameter "callback_url" is obviously the url of Casdoor. The third and fourth parameters are "client_id" and "client_secret", which you can acquire from Casdoor when you register an app. | |
| The second parameter "endpoint_addr" is obviously the url of Casdoor. The third and fourth parameters are "client_id" and "client_secret", which you can acquire from Casdoor when you register an app. |
|
|
||
| #### How it works? | ||
|
|
||
| Suppose a new user who have never visited this route before is going to visit it (http://localhost:9080/anything/d?param1=foo¶m2=bar), considering that "auth-casdoor" is enabled, this visit would be processed by "auth-casdoor" plugin first. After checking the session and confirming that this use hasn't been authentication, the visit will be intercepted. After remembering the original url user wants to visit, he will be redirected to the login page of Casdoor. |
There was a problem hiding this comment.
| Suppose a new user who have never visited this route before is going to visit it (http://localhost:9080/anything/d?param1=foo¶m2=bar), considering that "auth-casdoor" is enabled, this visit would be processed by "auth-casdoor" plugin first. After checking the session and confirming that this use hasn't been authentication, the visit will be intercepted. After remembering the original url user wants to visit, he will be redirected to the login page of Casdoor. | |
| Suppose a new user who has never visited this route before is going to visit it (http://localhost:9080/anything/d?param1=foo¶m2=bar), considering that "auth-casdoor" is enabled, this visit would be processed by "auth-casdoor" plugin first. After checking the session and confirming that this user hasn't been authenticated, the visit will be intercepted. With the original url user wants to visit kept, he will be redirected to the login page of Casdoor. |
|
|
||
| After successfully logging in with username and password(or whatever method he use), Casdoor will redirect this user to the "callback_url" with GET parameter "code " and "state" specified. Because the "callback_url" is known by the plugin, so when the visit toward the "callback_url" is intercepted this time, the logic of "Authorization code Grant Flow" in Oauth2 will be triggered, which means this plugin will request the access token to confirm whether this user is really logged in. After this confirmation, this plugin will redirect this user to the original url user wants to visit, which was remembered by us previously. The logged in status will also be remembered by session | ||
|
|
||
| Next time this user want to visit url behind this route (for example, http://localhost:9080/anything/d), after discovering that this user have been authentication previously, this plugin won't redirect this user any more so that this user can visit whatever he wants under this route without being interfered |
There was a problem hiding this comment.
| Next time this user want to visit url behind this route (for example, http://localhost:9080/anything/d), after discovering that this user have been authentication previously, this plugin won't redirect this user any more so that this user can visit whatever he wants under this route without being interfered | |
| Next time this user want to visit url behind this route (for example, http://localhost:9080/anything/d), after discovering that this user has been authenticated previously, this plugin won't redirect this user anymore so that this user can visit whatever he wants under this route without being interfered |
|
|
||
| Suppose a new user who have never visited this route before is going to visit it (http://localhost:9080/anything/d?param1=foo¶m2=bar), considering that "auth-casdoor" is enabled, this visit would be processed by "auth-casdoor" plugin first. After checking the session and confirming that this use hasn't been authentication, the visit will be intercepted. After remembering the original url user wants to visit, he will be redirected to the login page of Casdoor. | ||
|
|
||
| After successfully logging in with username and password(or whatever method he use), Casdoor will redirect this user to the "callback_url" with GET parameter "code " and "state" specified. Because the "callback_url" is known by the plugin, so when the visit toward the "callback_url" is intercepted this time, the logic of "Authorization code Grant Flow" in Oauth2 will be triggered, which means this plugin will request the access token to confirm whether this user is really logged in. After this confirmation, this plugin will redirect this user to the original url user wants to visit, which was remembered by us previously. The logged in status will also be remembered by session |
There was a problem hiding this comment.
| After successfully logging in with username and password(or whatever method he use), Casdoor will redirect this user to the "callback_url" with GET parameter "code " and "state" specified. Because the "callback_url" is known by the plugin, so when the visit toward the "callback_url" is intercepted this time, the logic of "Authorization code Grant Flow" in Oauth2 will be triggered, which means this plugin will request the access token to confirm whether this user is really logged in. After this confirmation, this plugin will redirect this user to the original url user wants to visit, which was remembered by us previously. The logged in status will also be remembered by session | |
| After successfully logging in with username and password(or whatever method he uses), Casdoor will redirect this user to the "callback_url" with GET parameter "code " and "state" specified. Because the "callback_url" is known by the plugin, when the visit toward the "callback_url" is intercepted this time, the logic of "Authorization code Grant Flow" in Oauth2 will be triggered, which means this plugin will request the access token to confirm whether this user is really logged in. After this confirmation, this plugin will redirect this user to the original url user wants to visit, which was kept by us previously. The logged-in status will also be kept in the session. |
apisix/plugins/auth-casdoor.lua
Outdated
|
|
||
| local _M = {version = 0.1, priority = 2559, name = plugin_name, schema = schema} | ||
|
|
||
| local function get_path(uri) |
There was a problem hiding this comment.
We can use
apisix/apisix/plugins/http-logger.lua
Line 88 in acf7a0c
to get the path instead of writing own solution.
cc75f06
to
19e132b
Compare
|
@spacewander revisions have been made |
apisix/plugins/auth-casdoor.lua
Outdated
| function _M.access(conf, ctx) | ||
| -- log.info("hit auth-casdoor access") | ||
| local current_uri = ctx.var.uri | ||
| local session_obj_read = session.open() |
There was a problem hiding this comment.
should we check if session exist? ref: bungle/lua-resty-session#12 (comment)
apisix/plugins/auth-casdoor.lua
Outdated
| -- step 1: check whether hits the callback | ||
|
|
||
| if current_uri == conf.callback_url:match(".-//[^/]+(/.*)") then |
There was a problem hiding this comment.
| -- step 1: check whether hits the callback | |
| if current_uri == conf.callback_url:match(".-//[^/]+(/.*)") then | |
| -- step 1: check whether hits the callback | |
| if current_uri == conf.callback_url:match(".-//[^/]+(/.*)") then |
|
|
||
| ## Name | ||
|
|
||
| `auth-casdoor` is an authorization plugin based on [Casdoor](https://casdoor.org/). Casdoor is a centralized authentication / Single-Sign-On (SSO) platform supporting OAuth 2.0, OIDC and SAML, integrated with Casbin RBAC and ABAC permission management |
There was a problem hiding this comment.
| `auth-casdoor` is an authorization plugin based on [Casdoor](https://casdoor.org/). Casdoor is a centralized authentication / Single-Sign-On (SSO) platform supporting OAuth 2.0, OIDC and SAML, integrated with Casbin RBAC and ABAC permission management | |
| `auth-casdoor` is an authorization plugin based on [Casdoor](https://casdoor.org/). Casdoor is a centralized authentication / Single-Sign-On (SSO) platform supporting OAuth 2.0, OIDC and SAML, integrated with Casbin RBAC and ABAC permission management. |
t/plugin/auth-casdoor.t
Outdated
| local cookie = res1.headers["Set-Cookie"] | ||
|
|
||
|
|
||
| local res2,err2=httpc:request_uri(callback_url, { |
There was a problem hiding this comment.
| local res2,err2=httpc:request_uri(callback_url, { | |
| local res2,err2 = httpc:request_uri(callback_url, { |
Please pay more attention to similar formatting issues in this PR.
70a497b
to
edd583a
Compare
|
@spacewander @tzssangglass what about this time? (revisions have been made) |
|
@spacewander @tzssangglass what kind of "chaos-test" is this? Besides, comparing with my last previous commit, the only change was format in test file t/plugin/auth-casdoor.t, but I passed all the tests in the previous commit. |
edd583a
to
70a497b
Compare
|
what is apisix/utils/batch-processor.lua? This file seemed to lead to the failure of lint but it's irrelevant with me...... |
Please merge the APISIX master branch into your PR branch. |
44e9d82
to
897c7a9
Compare
|
rebase made(force-pushed was caused by rebase) |
apisix/plugins/authz-casdoor.lua
Outdated
| end | ||
| local access_token, lifetime, err = | ||
| fetch_access_token(args.code, conf) | ||
| if err then |
There was a problem hiding this comment.
| if err then | |
| if not access_token then |
apisix/plugins/authz-casdoor.lua
Outdated
| core.log.error(err) | ||
| return 503 | ||
| end | ||
| if access_token then |
apisix/plugins/authz-casdoor.lua
Outdated
| session_obj_write.data.state = state | ||
| session_obj_write:save() | ||
|
|
||
| local redirect_url=conf.endpoint_addr .. "/login/oauth/authorize?" .. ngx.encode_args({ |
There was a problem hiding this comment.
| local redirect_url=conf.endpoint_addr .. "/login/oauth/authorize?" .. ngx.encode_args({ | |
| local redirect_url = conf.endpoint_addr .. "/login/oauth/authorize?" .. ngx.encode_args({ |
|
revisions have been made |
There was a problem hiding this comment.
@spacewander @ComradeProgrammer The documentation in the PR still needs to be optimized, let's merge this PR first and deal with the documentation in the next PR ?
|
OK |
|
|
@ComradeProgrammer |
|
Hi @ComradeProgrammer , thank you for your contribution! Here is the Contributor T-shirt form[1], if you're interested, kindly take a look :) [1] https://github.com/apache/apisix/blob/master/CONTRIBUTING.md#contributor-t-shirt |
What this PR does / why we need it:
auth-casdooris an authorization plugin based on Casdoor. Casdoor is a centralized authentication / Single-Sign-On (SSO) platformPre-submission checklist: