Skip to content
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

Merged
Merged
22 changes: 22 additions & 0 deletions apisix/plugins/openid-connect.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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 ",
Copy link
Contributor

@monkeyDluffy6017 monkeyDluffy6017 Aug 7, 2023

Choose a reason for hiding this comment

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

Typo?

Copy link
Contributor Author

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" ?

Copy link
Contributor

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)?

Copy link
Contributor

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

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]",
},
},
}
},
encrypt_fields = {"client_secret"},
Expand Down
1 change: 1 addition & 0 deletions docs/en/latest/plugins/openid-connect.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Copy link
Contributor

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?

Copy link
Contributor Author

@darkSheep404 darkSheep404 Aug 7, 2023

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

Copy link
Contributor

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

Copy link
Contributor Author

@darkSheep404 darkSheep404 Aug 7, 2023

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

hi @monkeyDluffy6017

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?
image

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.

Copy link
Contributor

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.",
                }
            },

Copy link
Contributor Author

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

Copy link
Contributor Author

@darkSheep404 darkSheep404 Aug 9, 2023

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
image

We may need to confirm this disagreement


NOTE: `encrypt_fields = {"client_secret"}` is also defined in the schema, which means that the field will be stored encrypted in etcd. See [encrypted storage fields](../plugin-develop.md#encrypted-storage-fields).

Expand Down
1 change: 1 addition & 0 deletions docs/zh/latest/plugins/openid-connect.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ description: OpenID Connect(OIDC)是基于 OAuth 2.0 的身份认证协议
| set_refresh_token_header | boolean | 否 | false | | 当设置为 `true` 并且刷新令牌可用时,则会将该属性设置在`X-Refresh-Token`请求头中。 |
| session | object | 否 | | | 当设置 bearer_only 为 false 时,openid-connect 插件将使用 Authorization Code 在 IDP 上进行认证,因此你必须设置 session 相关设置。 |
| session.secret | string | 是 | 自动生成 | 16 个以上字符 | 用于 session 加密和 HMAC 计算的密钥。 |
| proxy_opts | object | 否 | | | 给 openid-connect 插件配置一个 proxy。 |

注意:schema 中还定义了 `encrypt_fields = {"client_secret"}`,这意味着该字段将会被加密存储在 etcd 中。具体参考 [加密存储字段](../plugin-develop.md#加密存储字段)。

Expand Down
111 changes: 111 additions & 0 deletions t/plugin/openid-connect3.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
#
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
use t::APISIX 'no_plan';

log_level('debug');
repeat_each(1);
no_long_string();
no_root_location();
no_shuffle();

add_block_preprocessor(sub {
my ($block) = @_;

if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
$block->set_value("no_error_log", "[error]");
}

if (!defined $block->request) {
$block->set_value("request", "GET /t");
}
});

run_tests();

__DATA__

=== TEST 1: Set up new route access the auth server via http proxy
--- 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": "https://samples.auth0.com/.well-known/openid-configuration",
"redirect_uri": "https://iresty.com",
"ssl_verify": false,
"timeout": 10,
"scope": "apisix",
"proxy_opts": {
"http_proxy": "http://127.0.0.1:8080",
"http_proxy_authorization": "Basic dXNlcm5hbWU6cGFzc3dvcmQK"
},
"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



=== TEST 2: Access route w/o bearer token. Should redirect to authentication endpoint of ID provider.
--- config
location /t {
content_by_lua_block {
local http = require "resty.http"
local httpc = http.new()
local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello"
local res, err = httpc:request_uri(uri, {method = "GET"})
ngx.status = res.status
local location = res.headers['Location']
if location and string.find(location, 'https://samples.auth0.com/authorize') ~= -1 and
string.find(location, 'scope=apisix') ~= -1 and
string.find(location, 'client_id=kbyuFDidLLm280LIwVFiazOqjO3ty8KH') ~= -1 and
string.find(location, 'response_type=code') ~= -1 and
string.find(location, 'redirect_uri=https://iresty.com') ~= -1 then
ngx.say(true)
end
}
}
--- timeout: 10s
--- response_body
true
--- error_code: 302
--- error_log
use http proxy
Loading