[GitHub] [apisix] moonming commented on a diff in pull request #9147: feat(cli): support bypassing Admin API Auth by configuration

["moonming (via GitHub)" <gi...@apache.org>]

moonming commented on code in PR #9147:
URL: https://github.com/apache/apisix/pull/9147#discussion_r1154250154


##########
apisix/admin/init.lua:
##########
@@ -395,6 +401,13 @@ function _M.init_worker()
     events.register(reload_plugins, reload_event, "PUT")
 
     if ngx_worker_id() == 0 then
+        -- check if admin_key is required
+        if local_conf.deployment.admin.admin_key_required == false then
+            core.log.warn("Admin key is bypassed! ",
+                "If you are deploying APISIX in a production environment, ",
+                "please disable it and set a secure password for the admin Key!")

Review Comment:
   admin key is not password



##########
conf/config-default.yaml:
##########
@@ -588,6 +588,10 @@ deployment:
   role_traditional:
     config_provider: etcd
   admin:
+    # admin_key required or not. Default value is true.
+    # Bypass the Admin API authentication by modifying this value to false if needed.

Review Comment:
   ```suggestion
       # admin API authentication is enabled by default. 
       # Set it false in the production environment will cause a serious security issue.
   ```



##########
apisix/cli/ops.lua:
##########
@@ -189,6 +189,13 @@ local function init(env)
        and #allow_admin == 1 and allow_admin[1] == "127.0.0.0/24" then
         checked_admin_key = true
     end
+    -- check if admin_key is required
+    if yaml_conf.deployment.admin.admin_key_required == false then
+        checked_admin_key = true
+        print("Warning! Admin key is bypassed! "
+                .. "If you are deploying APISIX in a production environment, "
+                .. "please disable it and set a secure password for the admin Key!")
+    end

Review Comment:
   duplicate code



##########
apisix/admin/init.lua:
##########
@@ -395,6 +401,13 @@ function _M.init_worker()
     events.register(reload_plugins, reload_event, "PUT")
 
     if ngx_worker_id() == 0 then
+        -- check if admin_key is required
+        if local_conf.deployment.admin.admin_key_required == false then
+            core.log.warn("Admin key is bypassed! ",
+                "If you are deploying APISIX in a production environment, ",
+                "please disable it and set a secure password for the admin Key!")

Review Comment:
   Please write clearly what configuration to disable



##########
t/admin/api.t:
##########
@@ -156,3 +156,40 @@ X-API-VERSION: v2
 GET /t
 --- response_body
 passed
+
+
+
+=== TEST 10: Access with api key, and admin_key_required=true
+--- yaml_config
+deployment:
+  admin:
+    admin_key_required: true
+--- more_headers
+X-API-KEY: edd1c9f034335f136f87ad84b625c8f1
+--- request
+GET /apisix/admin/routes
+--- error_code: 200
+
+
+
+=== TEST 11: Access without api key, but admin_key_required=true
+--- yaml_config
+deployment:
+  admin:
+    admin_key_required: true
+--- request
+GET /apisix/admin/routes
+--- error_code: 401
+
+
+
+=== TEST 12: Access without api key, but admin_key_required=false

Review Comment:
   no test case for `Access with api key, but admin_key_required=false`



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org