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: allow to use environment variables for openid-connect plugin #11451

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open
4 changes: 3 additions & 1 deletion apisix/plugins/openid-connect.lua
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ local core = require("apisix.core")
local ngx_re = require("ngx.re")
local openidc = require("resty.openidc")
local random = require("resty.random")
local fetch_secrets = require("apisix.secret").fetch_secrets
local string = string
local ngx = ngx
local ipairs = ipairs
Expand Down Expand Up @@ -471,7 +472,8 @@ local function required_scopes_present(required_scopes, http_scopes)
end

function _M.rewrite(plugin_conf, ctx)
local conf = core.table.clone(plugin_conf)
local conf_clone = core.table.clone(plugin_conf)
Copy link
Member

Choose a reason for hiding this comment

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

performance tips:

we can use a lrucache, avoid repeated rendering of plugin_conf configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

performance tips:

we can use a lrucache, avoid repeated rendering of plugin_conf configuration

Thank you, but I don't fully understand your suggestion

The original way, I just used fetch_secret without clone the config first , but with this suggestion, I add table.clone before fetch_secret

I think fetch_secret itself will return a clone of conf, and if you want to optimize it, maybe can just keep using fetch_secret directly, instead of add another extra lrucache

local conf = fetch_secrets(conf_clone)

-- Previously, we multiply conf.timeout before storing it in etcd.
-- If the timeout is too large, we should not multiply it again.
Expand Down
9 changes: 9 additions & 0 deletions docs/en/latest/plugins/openid-connect.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ description: OpenID Connect allows the client to obtain user information from th

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

In addition, you can use Environment Variables or APISIX secret to store and reference plugin attributes. APISIX currently supports storing secrets in two ways - [Environment Variables and HashiCorp Vault](../terminology/secret.md).

For example, use below command to set environment variable
`export keycloak_secret=abc`

and use it in plugin conf like below

`"client_secret": "$ENV://keycloak_secret"`

## Scenarios

:::tip
Expand Down
97 changes: 97 additions & 0 deletions t/plugin/openid-connect.t
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ add_block_preprocessor(sub {
}
});

BEGIN {
$ENV{CLIENT_SECRET_ENV} = "60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa";
$ENV{VAULT_TOKEN} = "root";
}

run_tests();

__DATA__
Expand Down Expand Up @@ -1550,3 +1555,95 @@ true
qr/token validate successfully by \w+/
--- grep_error_log_out
token validate successfully by jwks

=== TEST 41: configure oidc plugin with small public key using environment variable
--- 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": "$ENV://CLIENT_SECRET_ENV",
"discovery": "https://samples.auth0.com/.well-known/openid-configuration",
"redirect_uri": "https://iresty.com",
"ssl_verify": false,
"timeout": 10,
"bearer_only": true,
"scope": "apisix",
"public_key": "-----BEGIN PUBLIC KEY-----\n]] ..
[[MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBANW16kX5SMrMa2t7F2R1w6Bk/qpjS4QQ\n]] ..
[[hnrbED3Dpsl9JXAx90MYsIWp51hBxJSE/EPVK8WF/sjHK1xQbEuDfEECAwEAAQ==\n]] ..
[[-----END PUBLIC KEY-----",
"token_signing_alg_values_expected": "RS256"
}
},
"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 42: store secret into vault
--- exec
VAULT_TOKEN='root' VAULT_ADDR='http://0.0.0.0:8200' vault kv put kv/apisix/foo client_secret=60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa
--- response_body
Success! Data written to: kv/apisix/foo

=== TEST 43: configure oidc plugin with small public key using vault
--- 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": "$secret://vault/test1/foo/client_secret",
"discovery": "https://samples.auth0.com/.well-known/openid-configuration",
"redirect_uri": "https://iresty.com",
"ssl_verify": false,
"timeout": 10,
"bearer_only": true,
"scope": "apisix",
"public_key": "-----BEGIN PUBLIC KEY-----\n]] ..
[[MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBANW16kX5SMrMa2t7F2R1w6Bk/qpjS4QQ\n]] ..
[[hnrbED3Dpsl9JXAx90MYsIWp51hBxJSE/EPVK8WF/sjHK1xQbEuDfEECAwEAAQ==\n]] ..
[[-----END PUBLIC KEY-----",
"token_signing_alg_values_expected": "RS256"
}
},
"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
Loading