-
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(openid-connect): add proxy_opts attribute #9948
feat(openid-connect): add proxy_opts attribute #9948
Conversation
hi @Sn0rt Can you please help me to review this PR 你可以帮我review这个PR吗 |
Thanks for your PR
|
yes,this pr only add a proxy_opts attribute to doc ,if use git tools which can ignore format and emptylines will be more clearly,The indentation above is affected because markdown's table has been modified by adding a new line 是的 这个PR 其实只加了一个属性到文档,如果使用忽略格式化和空行的git 工具可以更清晰的看到更改. 因为向markdown的的table加了一行, 所以影响到了上面的缩进 或者我可以重新提一个影响代码更少的pr |
问题是你加的这个选项为啥会工作呢? 可以解释一下么? |
hi @Sn0rt local httpc = http.new()
openidc_configure_timeouts(httpc, opts.timeout)
openidc_configure_proxy(httpc, opts.proxy_opts) 其中 local function openidc_configure_proxy(httpc, proxy_opts)
if httpc and proxy_opts and type(proxy_opts) == "table" then
log(DEBUG, "openidc_configure_proxy : use http proxy")
httpc:set_proxy_options(proxy_opts)
else
log(DEBUG, "openidc_configure_proxy : don't use http proxy")
end
end
调用
所以 openid-connect插件配置 |
看上去不错. 修改一下文档? |
dc717ca
to
9021bc8
Compare
hi @Sn0rt |
需要多等待一下.
|
好的 thk thk~ |
你测试了这个格式么 "http://username:[email protected]:8080" 么? |
hi 我这边 只用到了 不需要认证的代理服务器 |
添加一个 test 吧, 内容如下. === TEST 5: Set up new route access the auth server via http proxy
--- ONLY
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"plugins": {
"openid-connect": {
"client_id": "kbyuFDidLLm280LIwVFiazOqjO3ty8KH",
"client_secret": "60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa",
"discovery": "http://127.0.0.1:1980/.well-known/openid-configuration",
"redirect_uri": "https://iresty.com",
"ssl_verify": false,
"timeout": 10,
"scope": "apisix",
"proxy_opts": "http://username:[email protected]:8080",
"use_pkce": false
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)
if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- response_body
passed 可以使用下面指令格式化测试文件 ~/w/apisix *master> ./utils/reindex t/plugin/openid-connect.t
reindex: t/plugin/openid-connect.t: skipped. |
讲道理, 既然想把这个选项暴露出来为啥没有更新这个地方? local schema = {
type = "object",
properties = {
client_id = {type = "string"},
client_secret = {type = "string"},
discovery = {type = "string"},
scope = {
type = "string",
default = "openid",
},
ssl_verify = {
type = "boolean",
default = false,
},
timeout = {
type = "integer",
minimum = 1,
default = 3,
description = "timeout in seconds",
},
introspection_endpoint = {
type = "string"
},
introspection_endpoint_auth_method = {
type = "string",
default = "client_secret_basic"
},
bearer_only = {
type = "boolean",
default = false,
},
session = {
type = "object",
properties = {
secret = {
type = "string",
description = "the key used for the encrypt and HMAC calculation",
minLength = 16,
},
},
required = {"secret"},
additionalProperties = false,
},
realm = {
type = "string",
default = "apisix",
},
logout_path = {
type = "string",
default = "/logout",
},
redirect_uri = {
type = "string",
description = "use ngx.var.request_uri if not configured"
},
post_logout_redirect_uri = {
type = "string",
description = "the URI will be redirect when request logout_path",
},
unauth_action = {
type = "string",
default = "auth",
enum = {"auth", "deny", "pass"},
description = "The action performed when client is not authorized. Use auth to " ..
"redirect user to identity provider, deny to respond with 401 Unauthorized, and " ..
"pass to allow the request regardless."
},
public_key = {type = "string"},
token_signing_alg_values_expected = {type = "string"},
use_pkce = {
description = "when set to true the PKEC(Proof Key for Code Exchange) will be used.",
type = "boolean",
default = false
},
set_access_token_header = {
description = "Whether the access token should be added as a header to the request " ..
"for downstream",
type = "boolean",
default = true
},
access_token_in_authorization_header = {
description = "Whether the access token should be added in the Authorization " ..
"header as opposed to the X-Access-Token header.",
type = "boolean",
default = false
},
set_id_token_header = {
description = "Whether the ID token should be added in the X-ID-Token header to " ..
"the request for downstream.",
type = "boolean",
default = true
},
set_userinfo_header = {
description = "Whether the user info token should be added in the X-Userinfo " ..
"header to the request for downstream.",
type = "boolean",
default = true
},
set_refresh_token_header = {
description = "Whether the refresh token should be added in the X-Refresh-Token " ..
"header to the request for downstream.",
type = "boolean",
default = false
}
},
encrypt_fields = {"client_secret"},
required = {"client_id", "client_secret", "discovery"}
}
|
hi @Sn0rt 我添加完测试了 |
https://apisix.apache.org/docs/apisix/building-apisix/ 我想这个链接可能对你有用. 试一下 |
确实没有想到去更新这个地方 |
Looking forward to you becoming a new contributor to APISIX 期待你成为APISIX 新的贡献者 |
1a43a97
to
d2cf653
Compare
hi @Sn0rt 我更新了插件的schema 可以麻烦你帮我运行一下格式化命令? |
可以, 我将会在你 commit 之后添加 commit 来完善你的需求. 需要先关闭这个 PR, 重新开一个. |
需要我先关闭这个PR吗 或者你可以在我fork仓库的分支再commit一次然后push到我这个分支 ? |
不出意外,我是没有权限往你的分支推送commit. |
我刚刚尝试 邀请你到我fork的那个github仓库 不知道是否可行 |
Signed-off-by: Sn0rt <[email protected]>
Signed-off-by: Sn0rt <[email protected]>
Please use english, thks |
@Sn0rt Is this pr ready? |
yep. wait the CI. |
Sorry, is this proxy attribute causing the ladp test to fail?😶 |
the LDAP is upstream error. I will merge the master branch and run CI again. |
noted, thk thk~ |
Signed-off-by: Sn0rt <[email protected]>
Signed-off-by: Sn0rt <[email protected]>
apisix/plugins/openid-connect.lua
Outdated
@@ -130,6 +130,28 @@ local schema = { | |||
"header to the request for downstream.", | |||
type = "boolean", | |||
default = false | |||
}, | |||
proxy_opts = { | |||
description = "access via openid server via a proxy server ", |
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.
Typo?
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.
Typo?
seems to, i just fixed it
by the way, do you think it will be better by using "oauth2 server" instead of "openid server" ?
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.
Do you mean OpenID Provider(OP)
?
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.
This PR will be handed over to you to follow up later, I have to fix other problems. Can you? @darkSheep404
@@ -61,6 +61,7 @@ description: OpenID Connect allows the client to obtain user information from th | |||
| session | object | False | | | When bearer_only is set to false, openid-connect will use Authorization Code flow to authenticate on the IDP, so you need to set the session-related configuration. | | |||
| session.secret | string | True | Automatic generation | 16 or more characters | The key used for session encrypt and HMAC operation. | | |||
| unauth_action | string | False | "auth" | | Specify the response type on unauthenticated requests. "auth" redirects to identity provider, "deny" results in a 401 response, "pass" will allow the request without authentication. | | |||
| proxy_opts | object | False | | | Configure an HTTP proxy to be used with the openid-connect plugin. | |
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 list all options?
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 list all options?
done,pls help to review
add no-proxy option refer to
https://github.com/ledgetech/lua-resty-http#set_proxy_options
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.
I didn't see the options in proxy_opts
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.
I didn't see the options in
proxy_opts
i had add no_proxy option before
base on doc in lua-resty-http
, that is all options now
Or you may mean add all options of proxy_opts in docs instead of just in plugin schema?
doc in lua-resty-http
set_proxy_options
syntax: httpc:set_proxy_options(opts)
Configure an HTTP proxy to be used with this client instance. The opts table expects the following fields:
http_proxy: an URI to a proxy server to be used with HTTP requests
http_proxy_authorization: a default Proxy-Authorization header value to be used with http_proxy, e.g. Basic ZGVtbzp0ZXN0, which will be overriden if the Proxy-Authorization request header is present.
https_proxy: an URI to a proxy server to be used with HTTPS requests
https_proxy_authorization: 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).
no_proxy: a comma separated list of hosts that should not be proxied.
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.
The option you added in the doc proxy_opts
, it should have some attributes as you described in the schema.
The attributes should be added to the doc too, or the users don't know how to use the proxy_opts
proxy_opts = {
description = "access openid server via a proxy server ",
type = "object",
properties = {
http_proxy = {
type = "string",
description = "Http proxy: http://proxy-server:80",
},
https_proxy = {
type = "string",
description = "Https proxy: https://proxy-server:80",
},
http_proxy_authorization = {
type = "string",
description = "Basic [base64 username:password].",
},
https_proxy_authorization = {
type = "string",
description = "Basic [base64 username:password].",
},
no_proxy = {
type = "string",
description = "A comma separated list of hosts that should not be proxied.",
}
},
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.
hi @monkeyDluffy6017
I just submitted the modification, please help to review it again when it is convenient
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.
hi @Sn0rt
by the way
I noticed that you modified the desc in openid-connect schema
by using https://proxy-server
instead of http://proxy-server
for https_proxy
.
I changed it back in this commit
Because when I did that before in our project, it caused an exception protocol https not supported for proxy connections
I fix it by set https_proxy
= http://proxy-server
instead of https://proxy-server
.
In addition I noticed some hardcoded special handling in the lua-resty-http
source code say https is not support,like below
We may need to confirm this disagreement
@Sn0rt @monkeyDluffy6017 please take a look |
* upstream/master: (77 commits) docs: Update admin-api.md (apache#10056) ci: fix a bug that can not open nginx.pid (apache#10061) feat: remove rust dependency by rollback lua-resty-ldap on master (apache#9936) docs: fix grpc-transcode.md error (apache#10059) feat: upgrade lua dependencies (apache#10051) fix: rollback lua-resty-session to 3.10 (apache#10046) feat: upgrade resty-redis-cluster from 1.02-4->1.05-1 (apache#10041) feat: update lua library (apache#10037) fix: worker not exited when executing quit or reload command (apache#9909) fix: traffic split plugin not validating upstream_id (apache#10008) ci: update the timeout value in cli.yml (apache#10026) fix(tencent-cloud-cls): DNS parsing failure (apache#9843) chore(deps): bump actions/setup-node from 3.7.0 to 3.8.0 (apache#10025) feat(openid-connect): add proxy_opts attribute (apache#9948) perf(log-rotate): replace string.sub with string.byte (apache#9984) fix(ci): replace github action in update-labels.yml (apache#9987) fix: can't sync etcd data if key has special character (apache#9967) perf(aws-lambda): cache the index of the array (apache#9944) fix: add support for dependency installation on endeavouros (apache#9985) chore(ci): automate management of unresponded issues (apache#9927) ...
Description
update openid-connect docs, add attribute proxy-opts,which enable openid-connect access oauth2 server with http-proxy sever
更新openid-connect 文档 添加属性 proxy-opts,通过配置此属性 可以使得openid-connect 使用代理服务器访问 配置的oauth2服务器
Fixes # (issue)
#9922
Here is a simple diagram,the plugin config of
openid-connect
will be passed toopenidc.lua
asopts
,and will be used to set_proxy_options这里是一个简单的图,openid-connect 插件的配置 将会被传递给
openidc.lua
文件作为opts
,并被用来 指定代理服务器Checklist