feat: add cas-auth plugin by kingluo · Pull Request #7932 · apache/apisix

kingluo · 2022-09-16T08:30:40Z

Description

Implement cas-auth plugin

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
I have updated the documentation to reflect this change
I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

spacewander · 2022-09-18T11:57:35Z

Please make the CI pass, thanks!

spacewander · 2022-09-19T05:59:10Z

.github/workflows/build.yml


+      - name: download keycloak cas provider
+        run: |
+          sudo wget https://github.com/jacekkow/keycloak-protocol-cas/releases/download/18.0.2/keycloak-protocol-cas-18.0.2.jar -O /opt/keycloak-protocol-cas-18.0.2.jar


Let's move it to https://github.com/apache/apisix/blob/master/ci/init-plugin-test-service.sh

@spacewander No, that scirpt runs after docker compose, but this download file must be set as volume in docker compose file.

@kingluo
What about moving it into Start CI env (PLUGIN_TEST)?

spacewander · 2022-09-19T06:09:57Z

apisix/plugins/cas-auth.lua

+end
+
+local function uri_without_ticket(conf)
+    return ngx.var.scheme .. "://" .. ngx.var.host .. ":" ..


Could you use ctx.var instead of ngx.var?

spacewander · 2022-09-19T06:51:57Z

apisix/plugins/cas-auth.lua

+    store:delete(session_id)
+    set_our_cookie(COOKIE_NAME, "deleted; Max-Age=0")
+
+    ngx.redirect(conf.idp_uri .. "/logout")


ngx.redirect works like ngx.exit. As we avoid ngx.exit, we prefer to do it by ourselves, see

apisix/apisix/plugins/redirect.lua

Line 257 in 9129572

core.response.set_header("Location", new_uri)

spacewander · 2022-09-19T06:54:17Z

apisix/plugins/cas-auth.lua

+end
+
+local function set_cookie(cookie_str)
+    local h = to_table(ngx.header['Set-Cookie'])


We can use core.response.add_header if we don't check if the same cookie exists?

spacewander · 2022-09-19T06:54:37Z

apisix/plugins/cas-auth.lua

+local function first_access(conf)
+    local login_uri = conf.idp_uri .. "/login?" ..
+        ngx.encode_args({ service = uri_without_ticket(conf) })
+    ngx.log(ngx.INFO, "first access: ", login_uri,


Please use core.log

spacewander · 2022-09-19T06:55:32Z

apisix/plugins/cas-auth.lua

+        "/serviceValidate", { query = { ticket = ticket, service = uri_without_ticket(conf) } })
+
+    if res and res.status == ngx.HTTP_OK and res.body ~= nil then
+        if string.find(res.body, "<cas:authenticationSuccess>") then


Let's use core.string

spacewander · 2022-09-19T06:57:50Z

apisix/plugins/cas-auth.lua

+                return m[1]
+            end
+        else
+            ngx.log(ngx.INFO, "CAS serviceValidate failed: " .. res.body)


"CAS serviceValidate failed: ", res.body is enough

tzssangglass · 2022-09-19T10:26:54Z

apisix/plugins/cas-auth.lua

+end
+
+function _M.access(conf, ctx)
+    local method = ngx.req.get_method()


Suggested change

local method = ngx.req.get_method()

local method = core.request.get_method()

tzssangglass · 2022-09-19T10:28:58Z

apisix/plugins/cas-auth.lua

+    if method == "GET" and uri == conf.logout_uri then
+        return logout(conf, ctx)
+    elseif method == "POST" and uri == conf.cas_callback_uri then


Suggested change

if method == "GET" and uri == conf.logout_uri then

return logout(conf, ctx)

elseif method == "POST" and uri == conf.cas_callback_uri then

if method == "GET" and uri == conf.logout_uri then

return logout(conf, ctx)

end

if method == "POST" and uri == conf.cas_callback_uri then

is ok?

tzssangglass · 2022-09-19T10:31:43Z

apisix/plugins/cas-auth.lua

+        ngx.req.read_body()
+        local data = ngx.req.get_body_data()


use core.request.get_body()

tzssangglass · 2022-09-19T10:35:00Z

apisix/plugins/cas-auth.lua

+        if err == "no memory" then
+            core.log.emerg("CAS cookie store is out of memory")
+        elseif err == "exists" then
+            core.log.error("Same CAS ticket validated twice, this should never happen!")
+        end


log the origin err?

tzssangglass · 2022-09-19T10:44:13Z

docs/en/latest/plugins/cas-auth.md

+        }
+    }
+}'
+```


Can we add the process of how to start and configure IdP, as well as show some key images.

I think that should be left to blog. I would link the blog into the doc later.

spacewander · 2022-09-20T05:55:43Z

apisix/plugins/cas-auth.lua

+
+    if res and res.status == ngx.HTTP_OK and res.body ~= nil then
+        if core.string.find(res.body, "<cas:authenticationSuccess>") then
+            local m = ngx_re_match(res.body, "<cas:user>(.*?)</cas:user>");


Missing jo in the re match?

spacewander · 2022-09-20T05:56:45Z

apisix/plugins/cas-auth.lua

+            core.log.info("CAS serviceValidate failed: ", res.body)
+        end
+    else
+        core.log.error("validate ticket failed: res=", res, ", err=", err)


This may cause an error if res.status ~= ngx.HTTP_OK?

Only if res and res.status == ngx.HTTP_OK and res.body ~= nil, the validate process is successful. Otherwise, log the res and err.

@kingluo
Yes, but if res is not nil, will it raise an error when logging a table directly?

spacewander · 2022-09-20T09:22:38Z

Please address #7932 (comment)

spacewander · 2022-09-21T07:29:10Z

Before we can merge it, what is your idea about #7932 (comment)?

spacewander reviewed Sep 19, 2022

View reviewed changes

tzssangglass self-requested a review September 19, 2022 10:16

tzssangglass reviewed Sep 19, 2022

View reviewed changes

spacewander reviewed Sep 20, 2022

View reviewed changes

tzssangglass previously approved these changes Sep 20, 2022

View reviewed changes

kingluo dismissed tzssangglass’s stale review via aa04f04 September 20, 2022 09:27

kingluo added 11 commits September 22, 2022 09:51

feat: add cas-auth plugin

05244c0

fix indent

1107340

fix global var

b48a495

add cas-auth.md and some bugfix

89dd0ea

fix PR

3d4dab4

fix code lint

88ab81e

fix PR

7a7bdbe

use jo in ngx.re.match

152d2e0

fix PR

17dfca6

fix PR

6a88fa5

fix lint

5e3d329

kingluo force-pushed the cas_plugin branch from b4c9c7e to 5e3d329 Compare September 22, 2022 01:52

tzssangglass previously approved these changes Sep 22, 2022

View reviewed changes

fix PR

8933873

kingluo dismissed tzssangglass’s stale review via 8933873 September 22, 2022 07:12

spacewander approved these changes Sep 22, 2022

View reviewed changes

tzssangglass approved these changes Sep 22, 2022

View reviewed changes

spacewander merged commit 987f7be into apache:master Sep 22, 2022

Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request Nov 4, 2022

feat: add cas-auth plugin (apache#7932)

42e332a

	local method = ngx.req.get_method()
	local method = core.request.get_method()

Conversation

kingluo commented Sep 16, 2022

Description

Checklist

Uh oh!

spacewander commented Sep 18, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spacewander commented Sep 20, 2022

Uh oh!

spacewander commented Sep 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants