[GitHub] [apisix] moonming commented on a change in pull request #2092: bug: removed default access token for Admin API.

2020-09-14 Thread GitBox 


moonming commented on a change in pull request #2092:
URL: https://github.com/apache/apisix/pull/2092#discussion_r487758643



##
File path: bin/apisix
##
@@ -637,6 +637,45 @@ local function read_yaml_conf()
 merge_conf(default_conf, user_conf)
 end
 
+-- check the Admin API token
+if default_conf.apisix.enable_admin then
+local help = [[
+ERROR: missing valid apisix.admin_key
+
+Needs to set Admin API key in file `conf/config.yaml` . Here is an example:
+
+#
+apisix:
+admin_key:
+-
+name: "admin"
+key:  # <-- replace with your Admin Key
+role: admin
+#
+
+Then you can use it to access Admin API.
+eg: $ curl -i http://127.0.0.1:]] .. default_conf.apisix.node_listen .. 
[[/apisix/admin/routes/1 -H 'X-API-KEY: YOUR-KEY'
+]]
+if type(default_conf.apisix.admin_key) ~= "table" or
+   #default_conf.apisix.admin_key == 0
+then
+io.stderr:write(help, "\n")
+os.exit(1)
+end
+
+for _, admin in ipairs(default_conf.apisix.admin_key) do
+if type(admin.key) == "table" then
+admin.key = ""
+else
+admin.key = tostring(admin.key)
+end
+
+if admin.key == "" or admin.key:gsub("*", "") == "" then

Review comment:
   can we add `gen_admin_key` to the `start` function? 





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.

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

[GitHub] [apisix] moonming commented on a change in pull request #2092: bug: removed default access token for Admin API.

2020-09-10 Thread GitBox 


moonming commented on a change in pull request #2092:
URL: https://github.com/apache/apisix/pull/2092#discussion_r486140991



##
File path: FAQ.md
##
@@ -80,7 +80,7 @@ An example, `foo.com/product/index.html?id=204=2`, gray 
release based on `i
 
 here is the way:
 ```shell
-curl -i http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: 
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+curl -i http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: **' -X 
PUT -d '

Review comment:
   Or just `YOUR_API_TOKEN in conf/config.yaml` instead of `**` 





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.

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

[GitHub] [apisix] moonming commented on a change in pull request #2092: bug: removed default access token for Admin API.

2020-09-10 Thread GitBox 


moonming commented on a change in pull request #2092:
URL: https://github.com/apache/apisix/pull/2092#discussion_r486140004



##
File path: FAQ.md
##
@@ -80,7 +80,7 @@ An example, `foo.com/product/index.html?id=204=2`, gray 
release based on `i
 
 here is the way:
 ```shell
-curl -i http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: 
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+curl -i http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: **' -X 
PUT -d '

Review comment:
   `**` should be the key in `conf/config.yaml`. Is it possible to do 
this through a shell script?





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.

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

[GitHub] [apisix] moonming commented on a change in pull request #2092: bug: removed default access token for Admin API.

2020-09-10 Thread GitBox 


moonming commented on a change in pull request #2092:
URL: https://github.com/apache/apisix/pull/2092#discussion_r486136441



##
File path: conf/config.yaml
##
@@ -14,10 +14,23 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
+#
+# If you want specify the Admin API token. It is highly recommended to set a
+# high-complexity key to protect APISIX. This is an example:
+#
+# apisix:
+# admin_key:
+# -
+# name: "admin"
+# key:# <-- replace with your Admin Key
+# role: admin

Review comment:
   Why comment these codes?

##
File path: bin/apisix
##
@@ -637,6 +637,45 @@ local function read_yaml_conf()
 merge_conf(default_conf, user_conf)
 end
 
+-- check the Admin API token
+if default_conf.apisix.enable_admin then
+local help = [[
+ERROR: missing valid apisix.admin_key
+
+Needs to set Admin API key in file `conf/config.yaml` . Here is an example:
+
+#
+apisix:
+admin_key:
+-
+name: "admin"
+key:  # <-- replace with your Admin Key
+role: admin
+#
+
+Then you can use it to access Admin API.
+eg: $ curl -i http://127.0.0.1:]] .. default_conf.apisix.node_listen .. 
[[/apisix/admin/routes/1 -H 'X-API-KEY: YOUR-KEY'
+]]
+if type(default_conf.apisix.admin_key) ~= "table" or
+   #default_conf.apisix.admin_key == 0
+then
+io.stderr:write(help, "\n")
+os.exit(1)
+end
+
+for _, admin in ipairs(default_conf.apisix.admin_key) do
+if type(admin.key) == "table" then
+admin.key = ""
+else
+admin.key = tostring(admin.key)
+end
+
+if admin.key == "" or admin.key:gsub("*", "") == "" then

Review comment:
   After the user installs Apache APISIX, it will definitely fail at start, 
which is a very bad experience. 
   As discussed in the mailing list, we need to automatically generate a random 
key when apisix starts





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.

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

[GitHub] [apisix] moonming commented on a change in pull request #2092: bug: removed default access token for Admin API.

2020-08-31 Thread GitBox 


moonming commented on a change in pull request #2092:
URL: https://github.com/apache/apisix/pull/2092#discussion_r47769



##
File path: .travis/linux_apisix_master_luarocks_runner.sh
##
@@ -63,6 +63,14 @@ script() {
 sudo mkdir -p /usr/local/apisix/deps
 sudo PATH=$PATH ./utils/install-apisix.sh install > build.log 2>&1 || (cat 
build.log && exit 1)
 
+cat > /usr/local/apisix/conf/config.yaml  /usr/local/apisix/conf/config.yaml

[GitHub] [apisix] moonming commented on a change in pull request #2092: bug: removed default access token for Admin API.

2020-08-26 Thread GitBox 


moonming commented on a change in pull request #2092:
URL: https://github.com/apache/apisix/pull/2092#discussion_r477824853



##
File path: .travis/apisix_cli_test.sh
##
@@ -23,7 +23,14 @@
 
 set -ex
 
-git checkout conf/config.yaml
+cat > conf/config.yaml < conf/config.yaml <

[GitHub] [apisix] moonming commented on a change in pull request #2092: bug: removed default access token for Admin API.

2020-08-26 Thread GitBox 


moonming commented on a change in pull request #2092:
URL: https://github.com/apache/apisix/pull/2092#discussion_r477822484



##
File path: conf/config.yaml
##
@@ -21,3 +21,13 @@
 # host:
 #   - "http://127.0.0.1:2379;
 #
+#
+# If you want specify the Admin API token, this is an example:
+#
+# apisix:
+# admin_key:
+# -
+# name: "admin"
+# key:  **# <-- replace with a random key

Review comment:
   how about empty? user don't know `**` is invalid key.





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.

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

[GitHub] [apisix] moonming commented on a change in pull request #2092: bug: removed default access token for Admin API.

2020-08-25 Thread GitBox 


moonming commented on a change in pull request #2092:
URL: https://github.com/apache/apisix/pull/2092#discussion_r476292239



##
File path: conf/config.yaml
##
@@ -21,3 +21,13 @@
 # host:
 #   - "http://127.0.0.1:2379;
 #
+#
+# If you want specify the Admin API token, this is an example:
+#
+# apisix:
+# admin_key:
+# -
+# name: "admin"
+# key:  **# <-- replace with a random key

Review comment:
   please keep the same style as 
https://github.com/apache/apisix/pull/2092/files#diff-4c362b2e3d4cc07f3af3b469be2a913cR75

##
File path: FAQ.md
##
@@ -80,7 +80,7 @@ An example, `foo.com/product/index.html?id=204=2`, gray 
release based on `i
 
 here is the way:
 ```shell
-curl -i http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: 
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+curl -i http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: **' -X 
PUT -d '

Review comment:
   what is `**`? users will copy-paste this cmd directly.

##
File path: t/admin/token.t
##
@@ -155,7 +164,7 @@ PUT /apisix/admin/plugins/reload
 
 === TEST 6: reload plugins with api key(arguments)
 --- request
-PUT /apisix/admin/plugins/reload?api_key=edd1c9f034335f136f87ad84b625c8f1
+PUT /apisix/admin/plugins/reload?api_key=**

Review comment:
   `**` as the key is unacceptable

##
File path: t/APISIX.pm
##
@@ -81,7 +81,11 @@ apisix:
   stream_proxy:
 tcp:
   - 9100
-  admin_key: null
+  admin_key:
+-
+  name: "admin"
+  key: YOUR_API_KEY

Review comment:
   `YOUR_API_KEY` can not be the key





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.

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