feat: support secret fetching in authz-keycloak #10291

Revolyssup · 2023-10-05T01:37:44Z

Description

Currently $secret uri cannot be used in authz-keycloak plugin. This PR adds support for it.

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)

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

apisix/plugins/authz-keycloak.lua

t/plugin/authz-keycloak.t

monkeyDluffy6017 · 2023-10-07T03:54:01Z

Please make the ci pass

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

Revolyssup · 2023-10-09T09:30:15Z

Please make the ci pass

done

shreemaan-abhishek · 2023-10-10T03:10:59Z

apisix/plugins/authz-keycloak.lua

@@ -16,11 +16,13 @@
 --
 local core      = require("apisix.core")
 local http      = require "resty.http"
+local secret    = require("apisix.secret")


I'm not sure but importing just fetch_secrets() (instead of the whole module) might be more efficient. cc: @monkeyDluffy6017

@Revolyssup please check this

t/plugin/authz-keycloak4.t

Co-authored-by: Abhishek Choudhary <shreemaan.abhishek@gmail.com>

Sn0rt · 2023-10-12T03:44:59Z

pls add user document for guide

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

Revolyssup · 2023-10-12T15:36:31Z

pls add user document for guide

Updated the auth-keycloak plugin doc

shreemaan-abhishek · 2023-10-13T01:51:55Z

docs/en/latest/plugins/authz-keycloak.md

@@ -48,7 +48,7 @@ Refer to [Authorization Services Guide](https://www.keycloak.org/docs/latest/aut
 | token_endpoint                               | string        | False    |                                               | https://host.domain/auth/realms/foo/protocol/openid-connect/token  | An OAuth2-compliant token endpoint that supports the `urn:ietf:params:oauth:grant-type:uma-ticket` grant type. If provided, overrides the value from discovery.                                                                                       |
 | resource_registration_endpoint               | string        | False    |                                               | https://host.domain/auth/realms/foo/authz/protection/resource_set  | A UMA-compliant resource registration endpoint. If provided, overrides the value from discovery.                                                                                                                                                      |
 | client_id                                    | string        | True     |                                               |                                                                    | The identifier of the resource server to which the client is seeking access.                                                                                                                                                                         |
-| client_secret                                | string        | False    |                                               |                                                                    | The client secret, if required.                                                                                                                                                                                                                       |
+| client_secret                                | string        | False    |                                               |                                                                    | The client secret, if required. You can use APISIX secret to store and reference this value. APISIX currently supports storing secrets in two ways. [Environment Variables](#use-environment-variables-to-manage-secrets) and [HashiCorp Vault](#use-vault-to-manage-secrets)                                                                                                                                                                                                                       |


The links are wrong. And please add an example as well.

I fixed the doc link. Though the lint points to the secret page which has the example in detail. So I think it will be redundant to add it again here.

Does this PR introduce support to use env vars for accessing secrets? If yes, please add tests as well.

Regarding the docs, it would be nice to have an example like this:

(existing example that uses hard-coded value for secrets)

You can also set secrets using the secret resource to avoid having sensitive information in plain text <-- add this line

(add a basic configuration here)

See (add link to secrets.md documentation) for more information.

@shreemaan-abhishek Though secret is the APISIX abstraction. Behind it can be vault or env variable or anything. So I thought just testing with any one of them should work. Okay I will add tests for env variable as well.

okay i will modify the test case

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

Sn0rt · 2023-10-16T03:40:48Z

Please take a look at this case, I think it might be better understood if you can directly check the value after reslove.

apisix/t/plugin/authz-keycloak3.t

Line 168 in 1434335

ngx.say(res.value.plugins["authz-keycloak"].client_secret)

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

Sn0rt · 2023-10-16T07:47:54Z

t/plugin/authz-keycloak4.t

+=== TEST 2: set client_secret as a reference to secret
+--- yaml_config
+apisix:
+    data_encryption:


why need this ?

the test case whose configuration I picked also had this so I copy pasted. Should I remove it?

Revolyssup · 2023-10-16T18:30:32Z

Please take a look at this case, I think it might be better understood if you can directly check the value after reslove.

apisix/t/plugin/authz-keycloak3.t

Line 168 in 1434335

ngx.say(res.value.plugins["authz-keycloak"].client_secret)

@Sn0rt This variable extracts the exact string which is not a resolved secret. So the value of this will be $secret://...something. Here accessing this field doesn't give the resolved secret. So I am reverting back to previous test.

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

Revolyssup · 2023-10-17T06:44:50Z

Closing this PR. Recreating this with clean commits.

feat: support secret fetching in authz-keycloak

5499bcd

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

shreemaan-abhishek requested changes Oct 5, 2023

View reviewed changes

apisix/plugins/authz-keycloak.lua Outdated Show resolved Hide resolved

t/plugin/authz-keycloak.t Outdated Show resolved Hide resolved

t/plugin/authz-keycloak.t Outdated Show resolved Hide resolved

monkeyDluffy6017 added the wait for update wait for the author's response in this issue/PR label Oct 7, 2023

Revolyssup added 2 commits October 8, 2023 16:08

fix if condition

d2b4a38

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

fix lint tests

4107904

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

Revolyssup self-assigned this Oct 8, 2023

Revolyssup added 3 commits October 9, 2023 12:23

separate out tests

e8e7db5

fix tests

830af3f

fix test

3bb3563

github-actions bot added the user responded label Oct 9, 2023

Revolyssup removed wait for update wait for the author's response in this issue/PR user responded labels Oct 9, 2023

apply suggestions

a236fe2

shreemaan-abhishek reviewed Oct 10, 2023

View reviewed changes

shreemaan-abhishek requested changes Oct 10, 2023

View reviewed changes

t/plugin/authz-keycloak4.t Show resolved Hide resolved

Update t/plugin/authz-keycloak4.t

5507b28

Co-authored-by: Abhishek Choudhary <shreemaan.abhishek@gmail.com>

monkeyDluffy6017 added the wait for update wait for the author's response in this issue/PR label Oct 12, 2023

apply suggestions

ada57bb

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

github-actions bot added the user responded label Oct 12, 2023

Revolyssup removed the wait for update wait for the author's response in this issue/PR label Oct 12, 2023

monkeyDluffy6017 previously approved these changes Oct 13, 2023

View reviewed changes

monkeyDluffy6017 added approved and removed user responded labels Oct 13, 2023

monkeyDluffy6017 requested a review from shreemaan-abhishek October 13, 2023 01:01

shreemaan-abhishek requested changes Oct 13, 2023

View reviewed changes

monkeyDluffy6017 removed the approved label Oct 13, 2023

monkeyDluffy6017 added the wait for update wait for the author's response in this issue/PR label Oct 13, 2023

fix doc lint

85a108a

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

Revolyssup dismissed monkeyDluffy6017’s stale review via 85a108a October 13, 2023 03:49

Sn0rt mentioned this pull request Oct 16, 2023

Secert Manager sub-task: new test case proves that the vault works as the secret manager backend. #10222

Closed

28 tasks

Revolyssup added 2 commits October 16, 2023 12:38

add test case

b13571f

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

fix test

89debed

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

Sn0rt approved these changes Oct 16, 2023

View reviewed changes

Revolyssup added 2 commits October 16, 2023 13:47

fix test

7b96bbc

fix tests

80927eb

github-actions bot added the user responded label Oct 16, 2023

Revolyssup added 2 commits October 17, 2023 00:04

revert to previous tests

63e7247

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

revert to previous tests

f22f535

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

Revolyssup closed this Oct 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support secret fetching in authz-keycloak #10291

feat: support secret fetching in authz-keycloak #10291

Revolyssup commented Oct 5, 2023 •

edited

Loading

monkeyDluffy6017 commented Oct 7, 2023

Revolyssup commented Oct 9, 2023

shreemaan-abhishek Oct 10, 2023

monkeyDluffy6017 Oct 12, 2023

Revolyssup Oct 12, 2023

Sn0rt commented Oct 12, 2023

Revolyssup commented Oct 12, 2023

shreemaan-abhishek Oct 13, 2023

Revolyssup Oct 13, 2023

shreemaan-abhishek Oct 14, 2023

Revolyssup Oct 16, 2023

Revolyssup Oct 16, 2023

Sn0rt commented Oct 16, 2023

Sn0rt Oct 16, 2023

Revolyssup Oct 16, 2023

Revolyssup commented Oct 16, 2023

Revolyssup commented Oct 17, 2023 •

edited

Loading

feat: support secret fetching in authz-keycloak #10291

feat: support secret fetching in authz-keycloak #10291

Conversation

Revolyssup commented Oct 5, 2023 • edited Loading

Description

Checklist

monkeyDluffy6017 commented Oct 7, 2023

Revolyssup commented Oct 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sn0rt commented Oct 12, 2023

Revolyssup commented Oct 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sn0rt commented Oct 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Revolyssup commented Oct 16, 2023

Revolyssup commented Oct 17, 2023 • edited Loading

Revolyssup commented Oct 5, 2023 •

edited

Loading

Revolyssup commented Oct 17, 2023 •

edited

Loading