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: support hide the authentication header in basic-auth with a config #6039

Merged
merged 30 commits into from
Jan 14, 2022
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
dc2570a
fix:
Sep 1, 2021
f0d6f6f
merge from upstream master
Sep 29, 2021
3845a27
Merge remote-tracking branch 'upstream/master'
Oct 15, 2021
db0c9e7
feature: support hide the authentication header in basic-auth plugin
mangoGoForward Jan 6, 2022
0cee869
add test case with hide auth header
mangoGoForward Jan 7, 2022
79ca875
Revert "fix: when eureka server return compressed data, use lua-zlib …
mangoGoForward Jan 7, 2022
9a9565c
remove blank line
mangoGoForward Jan 7, 2022
10d35fd
change default value of hide_auth_header to `true`
mangoGoForward Jan 7, 2022
c642635
change default value of hide_auth_header to `true`
mangoGoForward Jan 7, 2022
29d3077
fix code lint error
mangoGoForward Jan 7, 2022
1c42db0
fix code lint error
mangoGoForward Jan 7, 2022
7a663fa
Merge pull request #1 from mangoGoForward/feature/hide-auth-header
mangoGoForward Jan 7, 2022
6e8d684
hide Authentication request header if hide_auth_header is true
mangoGoForward Jan 7, 2022
ec3bec6
hide Authentication request header if hide_auth_header is true
mangoGoForward Jan 7, 2022
37076a7
Merge branch 'apache:master' into master
mangoGoForward Jan 10, 2022
484d05d
change config item `hide_auth_header` to `hide_credentials` and updat…
mangoGoForward Jan 10, 2022
63aa704
change config item `hide_auth_header` to `hide_credentials` and updat…
mangoGoForward Jan 10, 2022
210c9d5
add test cases to cover it
mangoGoForward Jan 10, 2022
cb53193
add test cases to cover it
mangoGoForward Jan 10, 2022
395ac1a
add test cases
mangoGoForward Jan 12, 2022
25602a7
add test cases
mangoGoForward Jan 12, 2022
41bb17e
update basic-auth doc
mangoGoForward Jan 12, 2022
49b7850
remove log_level
mangoGoForward Jan 12, 2022
a1deeef
Revert "feat: support hide the authentication header in basic-auth"
mangoGoForward Jan 14, 2022
4fc743f
Merge pull request #2 from mangoGoForward/revert-1-feature/hide-auth-…
mangoGoForward Jan 14, 2022
8440045
Merge branch 'master' into feature/hide-auth-header
mangoGoForward Jan 14, 2022
8531227
Revert "Revert "feat: support hide the authentication header in basic…
mangoGoForward Jan 14, 2022
31af04d
change en doc
mangoGoForward Jan 14, 2022
ccc46b6
change zh doc
mangoGoForward Jan 14, 2022
1b0ffe4
Merge branch 'apache:master' into feature/hide-auth-header
mangoGoForward Jan 14, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion apisix/plugins/basic-auth.lua
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ local consumers_lrucache = core.lrucache.new({
local schema = {
type = "object",
title = "work with route or service object",
properties = {},
properties = {
hide_credentials = {
type = "boolean",
default = false,
}
},
}

local consumer_schema = {
Expand Down Expand Up @@ -161,6 +166,11 @@ function _M.rewrite(conf, ctx)
return 401, { message = "Password is error" }
end

-- 5. hide `Authorization` request header if `hide_credentials` is `true`
if conf.hide_credentials then
core.request.set_header(ctx, "Authorization", nil)
end

consumer.attach_consumer(ctx, cur_consumer, consumer_conf)

core.log.info("hit basic-auth access")
Expand Down
20 changes: 14 additions & 6 deletions docs/en/latest/plugins/basic-auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,18 @@ For more information on Basic authentication, refer to [Wiki](https://en.wikiped

## Attributes

| Name | Type | Requirement | Default | Valid | Description |
| -------- | ------ | ----------- | ------- | ----- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| username | string | required | | | Different `consumer` should have different value which is unique. When different `consumer` use a same `username`, a request matching exception would be raised. |
| password | string | required | | | the user's password |
For consumer side:

| Name | Type | Requirement | Default | Valid | Description |
| -------- | ------ | ----------- | ------- | ----- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| username | string | required | | | Different `consumer` should have different value which is unique. When different `consumer` use a same `username`, a request matching exception would be raised. |
| password | string | required | | | the user's password |
Copy link
Member

Choose a reason for hiding this comment

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

@mangoGoForward IMHO there's no need to change this. How about revert it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


For route side:

| Name | Type | Requirement | Default | Valid | Description |
| -------- | ------ | ----------- | ------- | ----- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| hide_credentials | boolean | optional | false | | Whether to pass the Authorization request headers to the upstream. |

## How To Enable

Expand Down Expand Up @@ -129,8 +137,8 @@ hello, world
## Disable Plugin

When you want to disable the `basic-auth` plugin, it is very simple,
you can delete the corresponding json configuration in the plugin configuration,
no need to restart the service, it will take effect immediately:
you can delete the corresponding json configuration in the plugin configuration,
no need to restart the service, it will take effect immediately:

```shell
$ curl http://127.0.0.1:9080/apisix/admin/routes/1 -X PUT -d '
Expand Down
8 changes: 8 additions & 0 deletions docs/zh/latest/plugins/basic-auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,19 @@ title: basic-auth

## 属性

consumer 端配置:

| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
| -------- | ------ | ------ | ------ | ------ | ------------------------------------------------------------------------------------------------------------------ |
| username | string | 必须 | | | 不同的 `consumer` 对象应有不同的值,它应当是唯一的。不同 consumer 使用了相同的 `username` ,将会出现请求匹配异常。 |
| password | string | 必须 | | | 用户的密码 |

router 端配置:

| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
| -------- | ------ | ------ | ------ | ------ | ------------------------------------------------------------------------------------------------------------------ |
| hide_credentials | boolean | 可选 | false | | 是否将 Authorization 请求头传递给 upstream。 |

## 如何启用

### 1. 创建一个 consumer 对象,并设置插件 `basic-auth` 的值。
Expand Down
101 changes: 101 additions & 0 deletions t/plugin/basic-auth.t
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ repeat_each(2);
no_long_string();
no_root_location();
no_shuffle();
log_level('info');
Copy link
Member

Choose a reason for hiding this comment

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

@mangoGoForward please take a check whether this change is needed. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems unnecessary, removed.

run_tests;

__DATA__
Expand Down Expand Up @@ -356,3 +357,103 @@ GET /t
GET /t
--- no_error_log
[error]



=== TEST 15: enable basic auth plugin using admin api, set hide_credentials = true
--- 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": {
"basic-auth": {
"hide_credentials": true
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/echo"
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed
--- no_error_log
[error]



=== TEST 16: verify Authorization request header is hidden
--- request
GET /echo
--- more_headers
Authorization: Basic Zm9vOmJhcg==
--- response_headers
!Authorization
--- no_error_log
[error]



=== TEST 17: enable basic auth plugin using admin api, hide_credentials = false
--- 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": {
"basic-auth": {
"hide_credentials": false
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/echo"
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed
--- no_error_log
[error]



=== TEST 18: verify Authorization request header should not hidden
--- request
GET /echo
--- more_headers
Authorization: Basic Zm9vOmJhcg==
--- response_headers
Authorization: Basic Zm9vOmJhcg==
--- no_error_log
[error]