Skip to content

feat: Add authorization params to openid-connect plugin#10058

Merged
juststillthinking merged 4 commits into
apache:masterfrom
TrevorSmith-msr:openid-connect-authorization-params
Oct 23, 2023
Merged

feat: Add authorization params to openid-connect plugin#10058
juststillthinking merged 4 commits into
apache:masterfrom
TrevorSmith-msr:openid-connect-authorization-params

Conversation

@TrevorSmith-msr

@TrevorSmith-msr TrevorSmith-msr commented Aug 18, 2023

Copy link
Copy Markdown
Contributor

Description

Add the ability to configure additional authorization params included in the openid-connect plugin.

Fixes #10057

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)

If tests are required, I may need assistance in writing those.

@TrevorSmith-msr TrevorSmith-msr changed the title Add authorization params to openid-connect plugin feat: Add authorization params to openid-connect plugin Aug 18, 2023
Comment thread apisix/plugins/openid-connect.lua
@Revolyssup Revolyssup added the wait for update wait for the author's response in this issue/PR label Aug 21, 2023
@Revolyssup Revolyssup removed the wait for update wait for the author's response in this issue/PR label Aug 21, 2023
},
authorization_params = {
description = "Extra authorization params to the authorize endpoint",
type = "object"

Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juststillthinking juststillthinking added the wait for update wait for the author's response in this issue/PR label Aug 22, 2023
@moonming

moonming commented Oct 9, 2023

Copy link
Copy Markdown
Member

@TrevorSmith-msr thanks for this PR, it will be great if you can add some test cases for it.
please ping me and @monkeyDluffy6017 if you need any help when writing test cases.

@TrevorSmith-msr

Copy link
Copy Markdown
Contributor Author

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.

@Revolyssup

Copy link
Copy Markdown
Contributor

@TrevorSmith-msr Maybe if you give me write permissions on this branch, I can help you out with tests. :)

@juststillthinking

Copy link
Copy Markdown
Contributor

@Revolyssup please make the ci pass

@Revolyssup

Copy link
Copy Markdown
Contributor

@monkeyDluffy6017 done

| 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. |

Copy link
Copy Markdown
Contributor

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 <[email protected]>
@juststillthinking

Copy link
Copy Markdown
Contributor

@Revolyssup Good job!

@juststillthinking juststillthinking added approved and removed wait for update wait for the author's response in this issue/PR labels Oct 23, 2023
@juststillthinking
juststillthinking merged commit 88406dc into apache:master Oct 23, 2023
@TrevorSmith-msr

Copy link
Copy Markdown
Contributor Author

Thank you @Revolyssup and @Sn0rt for your help finishing this up.

@TrevorSmith-msr
TrevorSmith-msr deleted the openid-connect-authorization-params branch October 23, 2023 17:34
hongbinhsu pushed a commit to fitphp/apix that referenced this pull request Nov 1, 2023
* 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)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

feat: As a user, I want to be able to configure additional authorization params in the OIDC plugin, so that I can be compliant with my identity provider

5 participants