Copilot commented on code in PR #13381: URL: https://github.com/apache/apisix/pull/13381#discussion_r3255861523
########## apisix/plugins/dingtalk-auth.lua: ########## @@ -0,0 +1,291 @@ +-- +-- 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. +-- +local core = require("apisix.core") +local http = require("resty.http") +local session = require("resty.session") + +local base64_encode = ngx.encode_base64 + +-- the access token from dingtalk has a TTL of 7200 seconds, +-- we set the cache TTL to 7000 seconds to avoid edge cases of token expiration during use. +local access_token_cache = core.lrucache.new({ + ttl = 7000, + invalid_stale = true, +}) + +local DEFAULT_USERINFO_URL = "https://oapi.dingtalk.com/topapi/v2/user/getuserinfo"; +local DEFAULT_TOKEN_URL = "https://api.dingtalk.com/v1.0/oauth2/accessToken"; + +local schema = { + type = "object", + properties = { + app_key = {type = "string", minLength = 1}, + app_secret = {type = "string", minLength = 1}, + code_header = { + type = "string", + description = "HTTP header name to extract dingtalk authorization code from.", + default = "X-DingTalk-Code" + }, + code_query = { + type = "string", + description = "Query parameter name to extract dingtalk authorization code from.", + default = "code" + }, + userinfo_url = { + type = "string", + default = DEFAULT_USERINFO_URL + }, + access_token_url = { + type = "string", + default = DEFAULT_TOKEN_URL + }, + set_userinfo_header = { + type = "boolean", + description = "Whether to set dingtalk user information in request headers", + default = true + }, + redirect_uri = {type = "string"}, + timeout = {type = "integer", default = 6000}, + ssl_verify = {type = "boolean", default = true}, + secret = { + type = "string", + description = "Secret used for key derivation.", + minLength = 8, + maxLength = 32, + }, + secret_fallbacks = { + type = "array", + items = { + type = "string", + minLength = 8, + maxLength = 32, + }, + description = "List of secrets for alternative secrets used when doing key rotation" + }, + cookie_expires_in = { + type = "integer", + description = "Valid duration (in seconds) for the authorization cookie." + .. "This value defines how long the cookie remains valid after creation.", + default = 86400, + }, + }, + encrypt_fields = {"app_secret", "secret"}, + required = {"app_key", "app_secret", "secret", "redirect_uri"}, +} + +local _M = { + version = 0.1, + priority = 2430, + name = "dingtalk-auth", + schema = schema, +} + +function _M.check_schema(conf) + return core.schema.check(schema, conf) +end + + +local function fetch_access_token(conf) + local httpc = http.new() + httpc:set_timeout(conf.timeout) + + local body = { + appKey = conf.app_key, + appSecret = conf.app_secret + } + + local res, err = httpc:request_uri(conf.access_token_url, { + method = "POST", + headers = { + ["Content-Type"] = "application/json" + }, + body = core.json.encode(body), + ssl_verify = conf.ssl_verify + }) + + if not res then + core.log.error("failed to get dingtalk token: ", err) + return nil, err + end + + core.log.debug("request dingtalk access token response status: ", + res.status) + + if res.status ~= 200 then + core.log.error("unexpected http response status from dingtalk: ", + res.status, ", body: ", res.body) + return nil, "unexpected response status: " .. res.status + end + + local data, err = core.json.decode(res.body) + if not data then + core.log.error("failed to decode dingtalk token response: ", err) + return nil, "failed to decode response: " .. (err or "nil") + end + + local access_token = data.accessToken + if not access_token then + core.log.error("dingtalk token response missing accessToken: ", res.body) + return nil, "dingtalk token response missing accessToken" + end + return access_token, nil +end + + +local function fetch_userinfo(conf, access_token, code) + local httpc = http.new() + httpc:set_timeout(conf.timeout) + + local params = { + access_token = access_token, + } + + local body = { + code = code + } + + local res, err = httpc:request_uri(conf.userinfo_url, { + method = "POST", + query = params, + headers = { + ["Content-Type"] = "application/json" + }, + body = core.json.encode(body), + ssl_verify = conf.ssl_verify + }) + + if not res then + core.log.error("failed to verify dingtalk user: ", err) + return nil, err + end + + core.log.debug("request dingtalk userinfo response status: ", res.status, ", body: ", res.body) + + if res.status ~= 200 then + core.log.error("unexpected http response status from dingtalk: ", + res.status, ", body: ", res.body) + return nil, "unexpected http response status: " .. res.status + end + + local data, err = core.json.decode(res.body) + if not data then + core.log.error("failed to decode dingtalk userinfo response: ", err) + return nil, "failed to decode response: " .. err + end + + if data.errcode ~= 0 then + return nil, "unexpected error code: " .. data.errcode + .. ", errmsg: " .. (data.errmsg or "nil") + end + + return data.result, nil +end + + +local function get_code(conf, ctx) + local code = core.request.header(ctx, conf.code_header) + if not code then + local uri_args = core.request.get_uri_args(ctx) or {} + code = uri_args[conf.code_query] + end + + return code +end + + +function _M.rewrite(conf, ctx) + local userinfo, err + + local sess, sess_err = session.open( + { + secret = conf.secret, + secret_fallbacks = conf.secret_fallbacks, + cookie_name = "dingtalk_session", + absolute_timeout = conf.cookie_expires_in, + } + ) + if not sess then + core.log.error("failed to open session: ", sess_err) + return 500, {message = "Failed to open session"} + end + + local raw = sess:get("userinfo") + if raw then + userinfo, err = core.json.decode(raw) + if not userinfo then + sess:destroy() + core.log.error("failed to decode userinfo in session: ", err) + return 500, {message = "Invalid userinfo in session"} + end + else + local code = get_code(conf, ctx) + if not code then + core.response.set_header("Location", conf.redirect_uri) + return 302 + end + + local key = core.table.concat({ + conf.access_token_url, + conf.app_key, + conf.app_secret, + }, "#") + local access_token, err = access_token_cache(key, nil, + fetch_access_token, conf) + if not access_token then + core.log.error("failed to get dingtalk access token: ", err) + return 500, { + message = "Invalid configuration", + } + end + + local new_userinfo, err = fetch_userinfo(conf, access_token, code) + if not new_userinfo then + core.log.warn("failed to get dingtalk userinfo: ", err) + return 401, { + message = "Invalid authorization code", + } + end + userinfo = new_userinfo + local raw, err = core.json.encode(userinfo) + if not raw then + core.log.error("failed to encode userinfo: ", err) + return 500, {message = "Invalid userinfo"} + end + + sess:set("userinfo", raw) + local ok, save_err = sess:save() + if not ok then + core.log.error("failed to save session: ", save_err) + return 500, {message = "Failed to save session"} + end + core.log.info("verified dingtalk user, code: ", code, + ", app_key: ", conf.app_key) + end Review Comment: The OAuth flow lacks a `state` parameter for CSRF protection. The plugin redirects unauthenticated users to `redirect_uri` (a static string) and then accepts any code presented at the callback without verifying that the request originated from the same client/session. A typical OAuth integration should generate a random state value, persist it in the pre-auth session, include it in the authorize URL, and verify it on the callback (this is how `authz-casdoor.lua` handles `state`). Consider documenting that callers must perform state-based CSRF protection externally, or implementing it within the plugin. ########## apisix/plugins/dingtalk-auth.lua: ########## @@ -0,0 +1,291 @@ +-- +-- 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. +-- +local core = require("apisix.core") +local http = require("resty.http") +local session = require("resty.session") + +local base64_encode = ngx.encode_base64 + +-- the access token from dingtalk has a TTL of 7200 seconds, +-- we set the cache TTL to 7000 seconds to avoid edge cases of token expiration during use. +local access_token_cache = core.lrucache.new({ + ttl = 7000, + invalid_stale = true, +}) + +local DEFAULT_USERINFO_URL = "https://oapi.dingtalk.com/topapi/v2/user/getuserinfo"; +local DEFAULT_TOKEN_URL = "https://api.dingtalk.com/v1.0/oauth2/accessToken"; + +local schema = { + type = "object", + properties = { + app_key = {type = "string", minLength = 1}, + app_secret = {type = "string", minLength = 1}, + code_header = { + type = "string", + description = "HTTP header name to extract dingtalk authorization code from.", + default = "X-DingTalk-Code" + }, + code_query = { + type = "string", + description = "Query parameter name to extract dingtalk authorization code from.", + default = "code" + }, + userinfo_url = { + type = "string", + default = DEFAULT_USERINFO_URL + }, + access_token_url = { + type = "string", + default = DEFAULT_TOKEN_URL + }, + set_userinfo_header = { + type = "boolean", + description = "Whether to set dingtalk user information in request headers", + default = true + }, + redirect_uri = {type = "string"}, + timeout = {type = "integer", default = 6000}, + ssl_verify = {type = "boolean", default = true}, + secret = { + type = "string", + description = "Secret used for key derivation.", + minLength = 8, + maxLength = 32, + }, + secret_fallbacks = { + type = "array", + items = { + type = "string", + minLength = 8, + maxLength = 32, + }, + description = "List of secrets for alternative secrets used when doing key rotation" + }, + cookie_expires_in = { + type = "integer", + description = "Valid duration (in seconds) for the authorization cookie." + .. "This value defines how long the cookie remains valid after creation.", + default = 86400, + }, + }, + encrypt_fields = {"app_secret", "secret"}, + required = {"app_key", "app_secret", "secret", "redirect_uri"}, +} + +local _M = { + version = 0.1, + priority = 2430, + name = "dingtalk-auth", + schema = schema, +} + +function _M.check_schema(conf) + return core.schema.check(schema, conf) +end + + +local function fetch_access_token(conf) + local httpc = http.new() + httpc:set_timeout(conf.timeout) + + local body = { + appKey = conf.app_key, + appSecret = conf.app_secret + } + + local res, err = httpc:request_uri(conf.access_token_url, { + method = "POST", + headers = { + ["Content-Type"] = "application/json" + }, + body = core.json.encode(body), + ssl_verify = conf.ssl_verify + }) + + if not res then + core.log.error("failed to get dingtalk token: ", err) + return nil, err + end + + core.log.debug("request dingtalk access token response status: ", + res.status) + + if res.status ~= 200 then + core.log.error("unexpected http response status from dingtalk: ", + res.status, ", body: ", res.body) + return nil, "unexpected response status: " .. res.status + end + + local data, err = core.json.decode(res.body) + if not data then + core.log.error("failed to decode dingtalk token response: ", err) + return nil, "failed to decode response: " .. (err or "nil") + end + + local access_token = data.accessToken + if not access_token then + core.log.error("dingtalk token response missing accessToken: ", res.body) + return nil, "dingtalk token response missing accessToken" + end + return access_token, nil +end + + +local function fetch_userinfo(conf, access_token, code) + local httpc = http.new() + httpc:set_timeout(conf.timeout) + + local params = { + access_token = access_token, + } + + local body = { + code = code + } + + local res, err = httpc:request_uri(conf.userinfo_url, { + method = "POST", + query = params, + headers = { + ["Content-Type"] = "application/json" + }, + body = core.json.encode(body), + ssl_verify = conf.ssl_verify + }) + + if not res then + core.log.error("failed to verify dingtalk user: ", err) + return nil, err + end + + core.log.debug("request dingtalk userinfo response status: ", res.status, ", body: ", res.body) + + if res.status ~= 200 then + core.log.error("unexpected http response status from dingtalk: ", + res.status, ", body: ", res.body) + return nil, "unexpected http response status: " .. res.status + end + + local data, err = core.json.decode(res.body) + if not data then + core.log.error("failed to decode dingtalk userinfo response: ", err) + return nil, "failed to decode response: " .. err + end + + if data.errcode ~= 0 then + return nil, "unexpected error code: " .. data.errcode + .. ", errmsg: " .. (data.errmsg or "nil") + end + + return data.result, nil +end + + +local function get_code(conf, ctx) + local code = core.request.header(ctx, conf.code_header) + if not code then + local uri_args = core.request.get_uri_args(ctx) or {} + code = uri_args[conf.code_query] + end + + return code +end + + +function _M.rewrite(conf, ctx) + local userinfo, err + + local sess, sess_err = session.open( + { + secret = conf.secret, + secret_fallbacks = conf.secret_fallbacks, + cookie_name = "dingtalk_session", + absolute_timeout = conf.cookie_expires_in, + } + ) + if not sess then + core.log.error("failed to open session: ", sess_err) + return 500, {message = "Failed to open session"} + end + + local raw = sess:get("userinfo") + if raw then + userinfo, err = core.json.decode(raw) + if not userinfo then + sess:destroy() + core.log.error("failed to decode userinfo in session: ", err) + return 500, {message = "Invalid userinfo in session"} + end + else + local code = get_code(conf, ctx) + if not code then + core.response.set_header("Location", conf.redirect_uri) + return 302 + end + + local key = core.table.concat({ + conf.access_token_url, + conf.app_key, + conf.app_secret, + }, "#") + local access_token, err = access_token_cache(key, nil, + fetch_access_token, conf) + if not access_token then + core.log.error("failed to get dingtalk access token: ", err) + return 500, { + message = "Invalid configuration", Review Comment: When `access_token_cache` fails to fetch a token (e.g., transient network error, DingTalk outage, or wrong credentials), the response is `500 Invalid configuration`. This message conflates configuration errors with runtime/network failures and will mislead operators when diagnosing issues — the actual error from `err` is only logged, not surfaced. Consider returning a more accurate message such as "Failed to obtain access token" so the response distinguishes upstream failures from misconfiguration. ########## apisix/plugins/dingtalk-auth.lua: ########## @@ -0,0 +1,291 @@ +-- +-- 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. +-- +local core = require("apisix.core") +local http = require("resty.http") +local session = require("resty.session") + +local base64_encode = ngx.encode_base64 + +-- the access token from dingtalk has a TTL of 7200 seconds, +-- we set the cache TTL to 7000 seconds to avoid edge cases of token expiration during use. +local access_token_cache = core.lrucache.new({ + ttl = 7000, + invalid_stale = true, +}) + +local DEFAULT_USERINFO_URL = "https://oapi.dingtalk.com/topapi/v2/user/getuserinfo"; +local DEFAULT_TOKEN_URL = "https://api.dingtalk.com/v1.0/oauth2/accessToken"; Review Comment: The default `userinfo_url` (`https://oapi.dingtalk.com/topapi/v2/user/getuserinfo`) is from DingTalk's legacy "old" OpenAPI, whereas the default `access_token_url` (`https://api.dingtalk.com/v1.0/oauth2/accessToken`) is the new OAuth2 API. The access token obtained from the new OAuth2 endpoint is generally not interchangeable with the legacy `topapi/*` endpoints (which expect a token from `/gettoken`). Please verify the defaults are actually compatible end-to-end against a real DingTalk app, or align them to a single API generation (both legacy or both new). This affects users who rely on the defaults. ########## apisix/plugins/dingtalk-auth.lua: ########## @@ -0,0 +1,291 @@ +-- +-- 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. +-- +local core = require("apisix.core") +local http = require("resty.http") +local session = require("resty.session") + +local base64_encode = ngx.encode_base64 + +-- the access token from dingtalk has a TTL of 7200 seconds, +-- we set the cache TTL to 7000 seconds to avoid edge cases of token expiration during use. +local access_token_cache = core.lrucache.new({ + ttl = 7000, + invalid_stale = true, +}) + +local DEFAULT_USERINFO_URL = "https://oapi.dingtalk.com/topapi/v2/user/getuserinfo"; +local DEFAULT_TOKEN_URL = "https://api.dingtalk.com/v1.0/oauth2/accessToken"; + +local schema = { + type = "object", + properties = { + app_key = {type = "string", minLength = 1}, + app_secret = {type = "string", minLength = 1}, + code_header = { + type = "string", + description = "HTTP header name to extract dingtalk authorization code from.", + default = "X-DingTalk-Code" + }, + code_query = { + type = "string", + description = "Query parameter name to extract dingtalk authorization code from.", + default = "code" + }, + userinfo_url = { + type = "string", + default = DEFAULT_USERINFO_URL + }, + access_token_url = { + type = "string", + default = DEFAULT_TOKEN_URL + }, + set_userinfo_header = { + type = "boolean", + description = "Whether to set dingtalk user information in request headers", + default = true + }, + redirect_uri = {type = "string"}, + timeout = {type = "integer", default = 6000}, + ssl_verify = {type = "boolean", default = true}, + secret = { + type = "string", + description = "Secret used for key derivation.", + minLength = 8, + maxLength = 32, + }, + secret_fallbacks = { + type = "array", + items = { + type = "string", + minLength = 8, + maxLength = 32, + }, + description = "List of secrets for alternative secrets used when doing key rotation" + }, + cookie_expires_in = { + type = "integer", + description = "Valid duration (in seconds) for the authorization cookie." + .. "This value defines how long the cookie remains valid after creation.", + default = 86400, + }, + }, + encrypt_fields = {"app_secret", "secret"}, + required = {"app_key", "app_secret", "secret", "redirect_uri"}, +} + +local _M = { + version = 0.1, + priority = 2430, + name = "dingtalk-auth", + schema = schema, +} + +function _M.check_schema(conf) + return core.schema.check(schema, conf) +end + + +local function fetch_access_token(conf) + local httpc = http.new() + httpc:set_timeout(conf.timeout) + + local body = { + appKey = conf.app_key, + appSecret = conf.app_secret + } + + local res, err = httpc:request_uri(conf.access_token_url, { + method = "POST", + headers = { + ["Content-Type"] = "application/json" + }, + body = core.json.encode(body), + ssl_verify = conf.ssl_verify + }) + + if not res then + core.log.error("failed to get dingtalk token: ", err) + return nil, err + end + + core.log.debug("request dingtalk access token response status: ", + res.status) + + if res.status ~= 200 then + core.log.error("unexpected http response status from dingtalk: ", + res.status, ", body: ", res.body) + return nil, "unexpected response status: " .. res.status + end + + local data, err = core.json.decode(res.body) + if not data then + core.log.error("failed to decode dingtalk token response: ", err) + return nil, "failed to decode response: " .. (err or "nil") + end + + local access_token = data.accessToken + if not access_token then + core.log.error("dingtalk token response missing accessToken: ", res.body) + return nil, "dingtalk token response missing accessToken" + end + return access_token, nil +end + + +local function fetch_userinfo(conf, access_token, code) + local httpc = http.new() + httpc:set_timeout(conf.timeout) + + local params = { + access_token = access_token, + } + + local body = { + code = code + } + + local res, err = httpc:request_uri(conf.userinfo_url, { + method = "POST", + query = params, + headers = { + ["Content-Type"] = "application/json" + }, + body = core.json.encode(body), + ssl_verify = conf.ssl_verify + }) + + if not res then + core.log.error("failed to verify dingtalk user: ", err) + return nil, err + end + + core.log.debug("request dingtalk userinfo response status: ", res.status, ", body: ", res.body) + + if res.status ~= 200 then + core.log.error("unexpected http response status from dingtalk: ", + res.status, ", body: ", res.body) + return nil, "unexpected http response status: " .. res.status + end + + local data, err = core.json.decode(res.body) + if not data then + core.log.error("failed to decode dingtalk userinfo response: ", err) + return nil, "failed to decode response: " .. err + end + + if data.errcode ~= 0 then + return nil, "unexpected error code: " .. data.errcode + .. ", errmsg: " .. (data.errmsg or "nil") + end + + return data.result, nil +end + + +local function get_code(conf, ctx) + local code = core.request.header(ctx, conf.code_header) + if not code then + local uri_args = core.request.get_uri_args(ctx) or {} + code = uri_args[conf.code_query] + end + + return code +end + + +function _M.rewrite(conf, ctx) + local userinfo, err + + local sess, sess_err = session.open( + { + secret = conf.secret, + secret_fallbacks = conf.secret_fallbacks, + cookie_name = "dingtalk_session", + absolute_timeout = conf.cookie_expires_in, + } + ) + if not sess then + core.log.error("failed to open session: ", sess_err) + return 500, {message = "Failed to open session"} + end + + local raw = sess:get("userinfo") + if raw then + userinfo, err = core.json.decode(raw) + if not userinfo then + sess:destroy() + core.log.error("failed to decode userinfo in session: ", err) + return 500, {message = "Invalid userinfo in session"} + end + else + local code = get_code(conf, ctx) + if not code then + core.response.set_header("Location", conf.redirect_uri) + return 302 + end + + local key = core.table.concat({ + conf.access_token_url, + conf.app_key, + conf.app_secret, + }, "#") + local access_token, err = access_token_cache(key, nil, + fetch_access_token, conf) + if not access_token then + core.log.error("failed to get dingtalk access token: ", err) + return 500, { + message = "Invalid configuration", + } + end + + local new_userinfo, err = fetch_userinfo(conf, access_token, code) + if not new_userinfo then + core.log.warn("failed to get dingtalk userinfo: ", err) + return 401, { + message = "Invalid authorization code", + } + end + userinfo = new_userinfo + local raw, err = core.json.encode(userinfo) + if not raw then + core.log.error("failed to encode userinfo: ", err) + return 500, {message = "Invalid userinfo"} + end + + sess:set("userinfo", raw) + local ok, save_err = sess:save() + if not ok then + core.log.error("failed to save session: ", save_err) + return 500, {message = "Failed to save session"} + end + core.log.info("verified dingtalk user, code: ", code, + ", app_key: ", conf.app_key) + end + + if userinfo and conf.set_userinfo_header ~= false then + local raw_for_header, encode_err = core.json.encode(userinfo) + if raw_for_header then + core.request.set_header(ctx, "X-Userinfo", base64_encode(raw_for_header)) + else + core.log.warn("failed to encode userinfo for X-Userinfo header: ", encode_err) + end + end + ctx.external_user = userinfo Review Comment: `ctx.external_user = userinfo` is assigned but never read anywhere else in the codebase (no other consumer of this field exists). If the intent is to expose the verified DingTalk user to downstream plugins or logs, please follow an established convention (e.g., `ctx.consumer`/`ctx.consumer_name` or a documented `ctx` field used by other auth plugins) or remove the assignment. ########## t/plugin/dingtalk-auth.t: ########## @@ -0,0 +1,369 @@ +# +# 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'; + +repeat_each(1); +log_level('debug'); +no_long_string(); +no_root_location(); + +add_block_preprocessor(sub { + my ($block) = @_; + + my $http_config = $block->http_config // <<_EOC_; + server { + listen 10421; + + location /v1.0/oauth2/accessToken { + content_by_lua_block { + local json = require("toolkit.json") + ngx.req.read_body() + ngx.status = 200 + ngx.say(json.encode({ + accessToken = "test_access_token_12345", + expireIn = 7200 + })) + } + } + + location /topapi/v2/user/getuserinfo { + content_by_lua_block { + local json = require("toolkit.json") + ngx.req.read_body() + local body = ngx.req.get_body_data() + local data = json.decode(body) + if data.code ~= "valid_code" then + ngx.status = 200 + ngx.say(json.encode({ + errcode = 403, + errmsg = "Unauthorized" + })) + return + end + ngx.status = 200 + ngx.say(json.encode({ + errcode = 0, + errmsg = "ok", + result = { + userid = "user_001", + name = "Test User", + unionid = "union_abc123" + } + })) + } + } + } +_EOC_ + + $block->set_value("http_config", $http_config); + + if (!$block->request) { + $block->set_value("request", "GET /t"); + } + + if ((!defined $block->error_log) && (!defined $block->no_error_log)) { + $block->set_value("no_error_log", "[error]"); + } +}); + +run_tests; + +__DATA__ + +=== TEST 1: schema check - all required fields present +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.dingtalk-auth") + local ok, err = plugin.check_schema({ + app_key = "appkey123", + app_secret = "appsecret456", + secret = "session-secret-key", + redirect_uri = "/login", + }) + if not ok then + ngx.say(err) + return + end + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 2: schema check - missing required field app_key +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.dingtalk-auth") + local ok, err = plugin.check_schema({ + app_secret = "appsecret456", + secret = "session-secret-key", + redirect_uri = "/login", + }) + ngx.say(ok) + } + } +--- response_body +false + + + +=== TEST 3: schema check - secret too short +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.dingtalk-auth") + local ok, err = plugin.check_schema({ + app_key = "appkey123", + app_secret = "appsecret456", + secret = "short", + redirect_uri = "/login", + }) + ngx.say(ok) + } + } +--- response_body +false + Review Comment: Tests 2 and 3 only assert that `ok` is `false`, but do not verify the actual error message returned by `check_schema`. As a result, a regression where the schema rejects for the wrong reason (e.g., a different missing field, or a schema parse failure) would still pass these tests. Please also assert on `err` (e.g., that it mentions `app_key` or the `minLength` constraint). ########## apisix/plugins/dingtalk-auth.lua: ########## @@ -0,0 +1,291 @@ +-- +-- 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. +-- +local core = require("apisix.core") +local http = require("resty.http") +local session = require("resty.session") + +local base64_encode = ngx.encode_base64 + +-- the access token from dingtalk has a TTL of 7200 seconds, +-- we set the cache TTL to 7000 seconds to avoid edge cases of token expiration during use. +local access_token_cache = core.lrucache.new({ + ttl = 7000, + invalid_stale = true, +}) + +local DEFAULT_USERINFO_URL = "https://oapi.dingtalk.com/topapi/v2/user/getuserinfo"; +local DEFAULT_TOKEN_URL = "https://api.dingtalk.com/v1.0/oauth2/accessToken"; + +local schema = { + type = "object", + properties = { + app_key = {type = "string", minLength = 1}, + app_secret = {type = "string", minLength = 1}, + code_header = { + type = "string", + description = "HTTP header name to extract dingtalk authorization code from.", + default = "X-DingTalk-Code" + }, + code_query = { + type = "string", + description = "Query parameter name to extract dingtalk authorization code from.", + default = "code" + }, + userinfo_url = { + type = "string", + default = DEFAULT_USERINFO_URL + }, + access_token_url = { + type = "string", + default = DEFAULT_TOKEN_URL + }, + set_userinfo_header = { + type = "boolean", + description = "Whether to set dingtalk user information in request headers", + default = true + }, + redirect_uri = {type = "string"}, + timeout = {type = "integer", default = 6000}, + ssl_verify = {type = "boolean", default = true}, + secret = { + type = "string", + description = "Secret used for key derivation.", + minLength = 8, + maxLength = 32, + }, + secret_fallbacks = { + type = "array", + items = { + type = "string", + minLength = 8, + maxLength = 32, + }, + description = "List of secrets for alternative secrets used when doing key rotation" + }, + cookie_expires_in = { + type = "integer", + description = "Valid duration (in seconds) for the authorization cookie." + .. "This value defines how long the cookie remains valid after creation.", + default = 86400, + }, Review Comment: `timeout` and `cookie_expires_in` are declared as `integer` without a `minimum`. Negative or zero values would be accepted by the schema but cause undefined behavior (immediately-expired cookies, instant HTTP timeouts). Please add `minimum = 1` (or an appropriate lower bound) to both fields. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
