AlinsRan commented on code in PR #13381: URL: https://github.com/apache/apisix/pull/13381#discussion_r3269291359
########## 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: Not adopting. The plugin follows the same pattern as `cas-auth`: `redirect_uri` is fully operator-configured (typically the DingTalk OAuth login URL that already encodes the state on the DingTalk side). Unlike `authz-casdoor`, this plugin does not own a callback URL inside APISIX, so there is nowhere to round-trip a server-generated state value. CSRF protection is the responsibility of the operator when configuring the DingTalk app. ########## 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: Not adopting. The two defaults are compatible in practice: the OAuth2 user token obtained from `api.dingtalk.com/v1.0/oauth2/accessToken` is accepted as the `access_token` query parameter by `oapi.dingtalk.com/topapi/v2/user/getuserinfo`. Both URLs are fully configurable so operators can override them for any DingTalk environment. ########## 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"}, Review Comment: Acknowledged. APISIX's `encrypt_fields` does not traverse arrays, so encrypting individual items in `secret_fallbacks` is not supported by the framework. Added a comment in the schema to document this limitation and remind operators to rotate fallback secrets promptly. Encrypting array items would require a framework-level change and is out of scope for this PR. ########## 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, +}) Review Comment: Not adopting in this PR. APISIX's module-level `lrucache` does not support per-item TTL without a significant refactor. The 7000-second TTL is a conservative value safely under DingTalk's standard 7200-second token lifetime. Both endpoints are configurable for non-standard environments. Dynamic TTL from `expireIn` is a good future enhancement. ########## 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" Review Comment: Not adopting. The mock uses errcode `403` purely to produce a non-zero errcode to exercise the plugin's error-handling path. The test validates plugin behavior (correct HTTP 401 for any non-zero errcode), not DingTalk-specific error code semantics. -- 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]
