-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add authorization params to openid-connect plugin #10058
feat: Add authorization params to openid-connect plugin #10058
Conversation
}, | ||
authorization_params = { | ||
description = "Extra authorization params to the authorize endpoint", | ||
type = "object" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add test cases for this option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I'm working on learning the testing framework now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can refer to these test cases: https://github.com/apache/apisix/blob/master/t/plugin/openid-connect.t
@TrevorSmith-msr thanks for this PR, it will be great if you can add some test cases for it. |
Hey @moonming, thanks for commenting. I've been having trouble getting my environment working for tests and as a result haven't been prioritizing this very highly. I think it would be great to get some help at some point. |
@TrevorSmith-msr Maybe if you give me write permissions on this branch, I can help you out with tests. :) |
@Revolyssup please make the ci pass |
@monkeyDluffy6017 done |
@@ -67,6 +67,7 @@ description: OpenID Connect allows the client to obtain user information from th | |||
| proxy_opts.http_proxy_authorization | string | False | | Basic [base64 username:password] | Default `Proxy-Authorization` header value to be used with `http_proxy`. | | |||
| proxy_opts.https_proxy_authorization | string | False | | Basic [base64 username:password] | As `http_proxy_authorization` but for use with `https_proxy` (since with HTTPS the authorisation is done when connecting, this one cannot be overridden by passing the `Proxy-Authorization` request header). | | |||
| proxy_opts.no_proxy | string | False | | | Comma separated list of hosts that should not be proxied. | | |||
| authorization_params | object | False | | | Additional parameters to send in the in the request to the authorization endpoint. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chinese doc too
Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>
@Revolyssup Good job! |
Thank you @Revolyssup and @Sn0rt for your help finishing this up. |
* upstream/master: (83 commits) fix: make install failed on mac (apache#10403) feat(zipkin): add variable (apache#10361) test(clickhouse-logger): to show that different endpoints will be chosen randomly (apache#8777) chore(deps): bump actions/setup-node from 3.8.1 to 4.0.0 (apache#10381) ci: fix the grpc test error (apache#10388) ci: trigger ci when doc-lint.yml changes (apache#10382) docs: fix usage of incorrect default admin api port (apache#10391) feat: Add authorization params to openid-connect plugin (apache#10058) feat: integrate authz-keycloak with secrets resource (apache#10353) fix(traffic-split): post_arg match fails because content-type contains charset (apache#10372) fix(consul): worker will not exit while reload or quit (apache#10342) chore: update rules for unresponded issues (apache#10354) docs: Update APISIX usecases in README (apache#10358) test: use http2 to test limit-req plugin (apache#10334) test: use http2 to test limit-conn plugin (apache#10332) chore: remove stream_proxy.only in config-default.yaml (apache#10337) docs: update underscore to hyphen in HTTP headers in `response-rewrite` plugin (apache#10347) fix: typos in comments (apache#10330) feat: support config stream_route upstream in service (apache#10298) fix: keep healthcheck target state when upstream changes (apache#10312) ...
Description
Add the ability to configure additional authorization params included in the openid-connect plugin.
Fixes #10057
Checklist
If tests are required, I may need assistance in writing those.