Skip to content

feat: add required scopes configuration property to openid-connect plugin#10493

Merged
juststillthinking merged 9 commits into
apache:masterfrom
csotiriou:feature/oidc-required-scopes
Nov 21, 2023
Merged

feat: add required scopes configuration property to openid-connect plugin#10493
juststillthinking merged 9 commits into
apache:masterfrom
csotiriou:feature/oidc-required-scopes

Conversation

@csotiriou

Copy link
Copy Markdown
Contributor

Fixes #10352

This adds the optional required_scopes configuration property in the openid-connect plugin configuration. In cases where the introspection endpoint of the OIDC server is called, the plugin will check if all required scopes are present in the scopes returned by the introspection endpoint.

I have also updated the documentation to reflect this, in case anyone finds this PR valuable.

@moonming

Copy link
Copy Markdown
Member

@csotiriou thanks for this PR. It will be great if you can add test cases for it :)

@csotiriou

Copy link
Copy Markdown
Contributor Author

Hello @moonming I'm trying to wrap my head around the test implementation, my familiarity with lua is limited. However, I have reached the point where I'm making a sample test.

I noticed that for the openid-connect plugin there are some sample oidc providers with a dummy client-id and client-secret used. Must I use the same and have the same configuration in my keycloak? If not, how are tests going to pass during automation?

@moonming

Copy link
Copy Markdown
Member

@Revolyssup Please check how to write a more appropriate test case. I am not familiar with this plug-in.

@Revolyssup
Revolyssup self-requested a review November 16, 2023 18:24
@Revolyssup Revolyssup self-assigned this Nov 16, 2023
@Revolyssup

Revolyssup commented Nov 16, 2023

Copy link
Copy Markdown
Contributor

@moonming @csotiriou I have added the test case and fixed the lint tests to save time.

Signed-off-by: Ashish Tiwari <[email protected]>
Signed-off-by: Ashish Tiwari <[email protected]>
moonming
moonming previously approved these changes Nov 17, 2023
@moonming

Copy link
Copy Markdown
Member

@csotiriou thanks for your great work 👍

Comment thread apisix/plugins/openid-connect.lua
@soulbird

Copy link
Copy Markdown
Contributor

I think you need to add a test case for scope validation

@Revolyssup

Copy link
Copy Markdown
Contributor

I think you need to add a test case for scope validation

@soulbird Do you mean the case where the correct scope is passed and the request succeeds?

@moonming

Copy link
Copy Markdown
Member

Conflicting files
docs/en/latest/plugins/openid-connect.md

@csotiriou please fix this when you have time.

Signed-off-by: Ashish Tiwari <[email protected]>
@soulbird

Copy link
Copy Markdown
Contributor

I think you need to add a test case for scope validation

@soulbird Do you mean the case where the correct scope is passed and the request succeeds?

yes

@csotiriou

csotiriou commented Nov 20, 2023

Copy link
Copy Markdown
Contributor Author

Conflicting files
docs/en/latest/plugins/openid-connect.md

@csotiriou please fix this when you have time.

Thanks for the notification, @moonming , It seems that the merge conflicts in the conflicting documentation files have been resolved by @Revolyssup faster, thank you for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

oidc-connect plugin, and restricting access to endpoints using scope

6 participants