Skip to content

Commit 0e078db

Browse files
AlinsRanCopilot
andcommitted
fix: fix CI failures and address review comments
- Fix trailing whitespace in t/plugin/acl.t line 877 (eclint failure) - Fix TEST 36 error_log pattern: use eval qr// to properly escape regex metacharacters ([, (, )) in the expected PCRE error message - Rename extra_values_* -> extract_values_* for clarity - Change core.log.info -> core.log.debug for label logging to avoid leaking sensitive user/tenant attributes - Only apply external_user parser/sep config for ctx.external_user, not ctx.consumer, to prevent consumer label matching breakage - Fix TEST 51/52: change external_user_label_field_key to external_user_label_field_separator in test configs and expected response bodies so they actually test the separator field validation - Fix typo in comment: 'dose' -> 'does' - Clarify docs: external_user_label_field accepts JSONPath or plain field name (both English and Chinese docs) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 078e785 commit 0e078db

4 files changed

Lines changed: 22 additions & 22 deletions

File tree

apisix/plugins/acl.lua

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ local parsers = {
9292
}
9393

9494

95-
local function extra_values_with_parser(value, parser, sep)
95+
local function extract_values_with_parser(value, parser, sep)
9696
local values = {}
9797
if parser == parsers.SEGMENTED_TEXT then
9898
sep = "\\s*" .. sep .. "\\s*"
@@ -138,20 +138,20 @@ local function extra_values_with_parser(value, parser, sep)
138138
end
139139

140140

141-
local function extra_values_without_parser(value)
141+
local function extract_values_without_parser(value)
142142
local values = {}
143143
local typ = type(value)
144144

145145
if typ == "table" then
146-
return extra_values_with_parser(value, parsers.TABLE, "")
146+
return extract_values_with_parser(value, parsers.TABLE, "")
147147
end
148148

149149
if typ == "string" then
150150
if core.string.has_prefix(value, "[") then
151-
return extra_values_with_parser(value, parsers.JSON, "")
151+
return extract_values_with_parser(value, parsers.JSON, "")
152152
end
153153
if core.string.find(value, ",") then
154-
return extra_values_with_parser(value, parsers.SEGMENTED_TEXT, ",")
154+
return extract_values_with_parser(value, parsers.SEGMENTED_TEXT, ",")
155155
end
156156
core.log.info("the string value can not parsed by ", parsers.JSON,
157157
" or ",parsers.SEGMENTED_TEXT)
@@ -166,9 +166,9 @@ end
166166
local function contains_value(want_values, value, parser, sep)
167167
local values
168168
if parser then
169-
values = extra_values_with_parser(value, parser, sep)
169+
values = extract_values_with_parser(value, parser, sep)
170170
else
171-
values = extra_values_without_parser(value)
171+
values = extract_values_without_parser(value)
172172
end
173173

174174
for _, want in ipairs(want_values) do
@@ -211,6 +211,7 @@ end
211211

212212
function _M.access(conf, ctx)
213213
local labels
214+
local parser, sep
214215
if ctx.consumer then
215216
labels = ctx.consumer.labels
216217
elseif ctx.external_user then
@@ -220,14 +221,13 @@ function _M.access(conf, ctx)
220221
end
221222
local label_value = jp.value(ctx.external_user, conf.external_user_label_field)
222223
labels = { [label_key] = label_value }
224+
parser = conf.external_user_label_field_parser
225+
sep = conf.external_user_label_field_separator
223226
else
224227
return 401, { message = "Missing authentication."}
225228
end
226229

227-
core.log.info("consumer's or user's labels: ", core.json.delay_encode(labels))
228-
229-
local sep = conf.external_user_label_field_separator
230-
local parser = conf.external_user_label_field_parser
230+
core.log.debug("consumer's or user's labels: ", core.json.delay_encode(labels))
231231

232232
if conf.deny_labels then
233233
if contains_label(conf.deny_labels, labels, parser, sep) then

docs/en/latest/plugins/acl.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ At least one of `allow_labels` or `deny_labels` must be configured. When both ar
5151
| deny_labels | object | False* | | | Labels to deny. Keys are label names, values are arrays of denied label values. At least one of `allow_labels` or `deny_labels` must be configured. |
5252
| rejected_code | integer | False | 403 | >= 200 | HTTP status code returned when the request is rejected. |
5353
| rejected_msg | string | False | | | Custom rejection message body. If not set, defaults to `{"message":"The consumer is forbidden."}`. |
54-
| external_user_label_field | string | False | `groups` | | JSONPath expression used to extract the label value from `ctx.external_user`. |
54+
| external_user_label_field | string | False | `groups` | | JSONPath expression or plain field name used to extract the label value from `ctx.external_user`. For example, `$..groups` (JSONPath) or `groups` (plain field name). |
5555
| external_user_label_field_key | string | False | | | The label key name used for the extracted value. Defaults to the value of `external_user_label_field`. |
5656
| external_user_label_field_parser | string | False | | `segmented_text`, `json`, `table` | How to parse the extracted field value. If not set, the Plugin auto-detects the format. |
5757
| external_user_label_field_separator | string | False | | | Separator regex for the `segmented_text` parser. Required when `external_user_label_field_parser` is `segmented_text`. |

docs/zh/latest/plugins/acl.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ description: acl 插件基于标签实现访问控制,通过检查消费者标
5151
| deny_labels | object |* | | | 拒绝的标签。键为标签名,值为拒绝的标签值数组。`allow_labels``deny_labels` 至少需配置其中一个。 |
5252
| rejected_code | integer || 403 | >= 200 | 请求被拒绝时返回的 HTTP 状态码。 |
5353
| rejected_msg | string || | | 自定义拒绝消息体。若未设置,默认返回 `{"message":"The consumer is forbidden."}`|
54-
| external_user_label_field | string || `groups` | | 用于从 `ctx.external_user` 提取标签值的 JSONPath 表达式|
54+
| external_user_label_field | string || `groups` | | 用于从 `ctx.external_user` 提取标签值的 JSONPath 表达式或普通字段名称。例如,`$..groups`(JSONPath)或 `groups`(字段名称)|
5555
| external_user_label_field_key | string || | | 提取值所使用的标签键名。默认为 `external_user_label_field` 的值。 |
5656
| external_user_label_field_parser | string || | `segmented_text``json``table` | 提取字段值的解析方式。若未设置,插件自动检测格式。 |
5757
| external_user_label_field_separator | string || | | `segmented_text` 解析器使用的分隔符(正则表达式)。当 `external_user_label_field_parser``segmented_text` 时必填。 |

t/plugin/acl.t

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ passed
689689
690690
691691
=== TEST 30: test ACL with the invalid separator
692-
# User may want to split the text "cloud|infra" to be ["cloud", "infra"] by char "|", but it dose not.
692+
# User may want to split the text "cloud|infra" to be ["cloud", "infra"] by char "|", but it does not.
693693
# Because the char "|" is a regex expression, the text "cloud|infra" will be split to ['c','l','o','u','d','|','i','n','f','r','a'].
694694
# If you want to split text by "|" you should use "\\|".
695695
# This is a normal case, no error_log here.
@@ -873,8 +873,8 @@ passed
873873
--- request
874874
GET /hello
875875
--- error_code: 403
876-
--- error_log
877-
failed to split labels [cloud,infra], err: pcre_compile() failed: missing ) in "\s*(invalid(pattern\s*"
876+
--- error_log eval
877+
qr/failed to split labels \[cloud,infra\], err: pcre_compile\(\) failed: missing \) in/
878878
879879
880880
@@ -1336,7 +1336,7 @@ GET /t
13361336
},
13371337
"external_user_label_field": "team",
13381338
"external_user_label_field_parser": "segmented_text",
1339-
"external_user_label_field_key": {},
1339+
"external_user_label_field_separator": {},
13401340
"rejected_code": 403
13411341
}
13421342
}
@@ -1353,7 +1353,7 @@ GET /t
13531353
GET /t
13541354
--- error_code: 400
13551355
--- response_body
1356-
{"error_msg":"failed to check the configuration of plugin acl err: property \"external_user_label_field_key\" validation failed: wrong type: expected string, got table"}
1356+
{"error_msg":"failed to check the configuration of plugin acl err: property \"external_user_label_field_separator\" validation failed: wrong type: expected string, got table"}
13571357
13581358
13591359
@@ -1380,7 +1380,7 @@ GET /t
13801380
},
13811381
"external_user_label_field": "team",
13821382
"external_user_label_field_parser": "segmented_text",
1383-
"external_user_label_field_key": "",
1383+
"external_user_label_field_separator": "",
13841384
"rejected_code": 403
13851385
}
13861386
}
@@ -1397,7 +1397,7 @@ GET /t
13971397
GET /t
13981398
--- error_code: 400
13991399
--- response_body
1400-
{"error_msg":"failed to check the configuration of plugin acl err: property \"external_user_label_field_key\" validation failed: string too short, expected at least 1, got 0"}
1400+
{"error_msg":"failed to check the configuration of plugin acl err: property \"external_user_label_field_separator\" validation failed: string too short, expected at least 1, got 0"}
14011401
14021402
14031403
@@ -1424,7 +1424,7 @@ GET /t
14241424
},
14251425
"external_user_label_field": "team",
14261426
"external_user_label_field_parser": "segmented_text",
1427-
"external_user_label_field_key": {},
1427+
"external_user_label_field_separator": {},
14281428
"rejected_code": 403
14291429
}
14301430
}
@@ -1441,7 +1441,7 @@ GET /t
14411441
GET /t
14421442
--- error_code: 400
14431443
--- response_body
1444-
{"error_msg":"failed to check the configuration of plugin acl err: property \"external_user_label_field_key\" validation failed: wrong type: expected string, got table"}
1444+
{"error_msg":"failed to check the configuration of plugin acl err: property \"external_user_label_field_separator\" validation failed: wrong type: expected string, got table"}
14451445
14461446
14471447

0 commit comments

Comments
 (0)