You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/01/06 18:44:26 UTC
[GitHub] [apisix] bzp2010 opened a new pull request #6037: feat: add forward-auth plugin
bzp2010 opened a new pull request #6037:
URL: https://github.com/apache/apisix/pull/6037
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
Add the `forward-auth` plugin. Implement #6007
### Pre-submission checklist:
<!--
Please follow the PR manners:
1. Use Draft if the PR is not ready to be reviewed
2. Test is required for the feat/fix PR, unless you have a good reason
3. Doc is required for the feat PR
4. Use a new commit to resolve review instead of `push -f`
5. If you need to resolve merge conflicts after the PR is reviewed, please merge master but do not rebase
6. Use "request review" to notify the reviewer once you have resolved the review
7. Only reviewer can click "Resolve conversation" to mark the reviewer's review resolved
-->
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r779990972
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,143 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
Review comment:
changed
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780998115
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,246 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/upstreams/u1",
+ data = [[{
+ "nodes": {
+ "127.0.0.1:1984": 1
+ },
+ "type": "roundrobin"
+ }]],
+ },
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function(conf, ctx)
Review comment:
I've merged it with the checkcase for overriding user input requests, which will check all APISIX generated headers at once.
https://github.com/apache/apisix/blob/beea582ddff8879e94484f84202307c488f30a70/t/plugin/forward-auth.t#L221-L225
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,130 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implements a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request, and replaces the result when the authentication service responds with a non-2xx status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service. When it is not set, all `client` request headers are sent to the `authorization` service, except for those provided by APISIX (X-Forwarded-XXX). |
Review comment:
updated
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r779983234
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
Review comment:
I don't think it's needed unless we provide other special features for this, the implementation in other software is the same.
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780005138
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,135 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implement a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request and replaces the result when the authentication service responds with a non-20x status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service |
+| upstream_headers | array[string] | optional | | | `authorization` service response header that will be sent to the `upstream` |
+| client_headers | array[string] | optional | | | `authorization` response header that will be sent to the `client` when authorize failure |
+| timeout | integer | optional | 3000ms | [1, 60000]ms | Authorization service HTTP call timeout |
+| keepalive | boolean | optional | true | | HTTP keepalive |
+| keepalive_timeout | integer | optional | 60000ms | [1000, ...]ms | keepalive idle timeout |
+| keepalive_pool | integer | optional | 5 | [1, ...]ms | Connection pool limit |
+
+## Example
+
+First, you need to setup an external authorization service. Here is an example of using Apache APISIX's serverless plugin to mock.
+
+```shell
+$ curl -X PUT 'http://127.0.0.1:9080/apisix/admin/routes/auth' \
+ -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' \
+ -H 'Content-Type: application/json' \
+ -d '{
+ "uri": "/auth",
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); local authorization = core.request.header(ctx, \"Authorization\"); if authorization == \"123\" then core.response.exit(200); elseif authorization == \"321\" then core.response.set_header(\"X-User-ID\", \"i-am-user\"); core.response.exit(200); else core.response.set_header(\"Location\", \"http://example.com/auth\"); core.response.exit(403); end end"
+ ]
+ }
+ },
+ "upstream": {
+ "nodes": {},
+ "scheme": "https",
Review comment:
removed
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780282076
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
Then, referring to the way it is usually used, I will check if `err` is `not nil`.
```lua
if err then
xxx
end
```
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780282076
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
Referring to the way it is usually used, I will check if `err` is `not nil`.
```lua
if err then
xxx
end
```
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780278449
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
removed
--
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
[GitHub] [apisix] spacewander commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780160633
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
Don't make thing complex!
The name of request_uri and uri already show the difference. There is no need to wrap them into a function which is rarely used. Premature optimization is the source of evil.
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780282224
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
changed
--
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
[GitHub] [apisix] spacewander merged pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #6037:
URL: https://github.com/apache/apisix/pull/6037
--
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
[GitHub] [apisix] shuaijinchao commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780794501
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
I think it should be used
````lua
if res then
err
end
````
instead of
```lua
if err then
err
end
```
this way of writing may be more suitable for languages such as `Go`
the common exception function return in OpenResty is `return nil, err`, the first value is enough to know whether to handle `err`, and you can refer to more code and test cases in `lua-resty-core`, such as: https://github.com/openresty/lua-resty-core/blob/e5217414669c100b334940b250d5340911a02dd0/t/ctx.t#L133-L143
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780278449
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
removed, I will remove `get_path` in a later PR.
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780058183
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,157 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": ["return function (conf, ctx) local core = require(\"apisix.core\"); local authorization = core.request.header(ctx, \"Authorization\"); if authorization == \"123\" then core.response.exit(200); elseif authorization == \"321\" then core.response.set_header(\"X-User-ID\", \"i-am-an-user\"); core.response.exit(200); else core.response.set_header(\"Location\", \"http://example.com/auth\"); core.response.exit(403); end end"]
+ }
+ },
+ "uri": "/auth"
+ }]],
+ },
+ {
+ url = "/apisix/admin/routes/echo",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": ["return function (conf, ctx) local core = require(\"apisix.core\"); core.response.exit(200, core.request.headers(ctx)); end"]
+ }
+ },
+ "uri": "/echo"
+ }]],
+ },
+ {
+ url = "/apisix/admin/routes/1",
+ data = [[{
+ "plugins": {
+ "forward-auth": {
+ "host": "http://127.0.0.1:1984/auth",
+ "request_headers": ["Authorization"],
+ "upstream_headers": ["X-User-ID"],
+ "client_headers": ["Location"]
+ },
+ "proxy-rewrite": {
+ "uri": "/echo"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1984": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]],
+ },
+ }
+
+ local t = require("lib.test_admin").test
+
+ for _, data in ipairs(datas) do
+ local code, body = t(data.url, ngx.HTTP_PUT, data.data)
+ ngx.say(code..body)
+ end
+ }
+ }
+--- response_body eval
+"201passed\n" x 3
+
+
+
+=== TEST 3: hit route (test request_headers)
+--- request
+GET /hello
+--- more_headers
+Authorization: 123
+--- response_body_like eval
+qr/\"authorization\":\"123\"/
+
+
+
+=== TEST 4: hit route (test upstream_headers)
+--- request
+GET /hello
+--- more_headers
+Authorization: 321
+--- response_body_like eval
+qr/\"x-user-id\":\"i-am-an-user\"/
+
+
+
+=== TEST 5: hit route (test client_headers)
+--- request
+GET /hello
+--- error_code: 403
+--- response_headers
+Location: http://example.com/auth
Review comment:
added, I added a test case to demonstrate that these client-side incoming key headers are indeed dropped in APISIX.
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
So I should just use ctx.var.request_uri instead of wrapping it? Can I merge the functionality of `get_path` into `get_uri` and add a `with_args` parameter indicating whether or not I need to carry parameters.
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
So I should just use ctx.var.request_uri instead of wrapping it? Can I merge the functionality of `get_path` into `get_uri` and add a `with_args` parameter indicating whether or not I need to carry queries.
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
removed
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
Then, referring to the way it is usually used, I will determine if `err` is `not nil`.
```lua
if err then
xxx
end
```
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
changed
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
Then, referring to the way it is usually used, I will check if `err` is `not nil`.
```lua
if err then
xxx
end
```
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
Referring to the way it is usually used, I will check if `err` is `not nil`.
```lua
if err then
xxx
end
```
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
Also, I think there needs to be a specification of what rules developers should follow to handle error checking (at least on the http client), the mix of different usages is confusing.
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
removed, I will remove `get_path` in a later PR.
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
Also, I think there needs to make a rules that developers should follow to handle error checking (at least on the http client), the mix of different usages is confusing.
--
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
[GitHub] [apisix] spacewander commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r781689556
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,130 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implements a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request, and replaces the result when the authentication service responds with a non-2xx status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service. When it is not set, no `client` request headers are sent to the `authorization` service, except for those provided by APISIX (X-Forwarded-XXX). |
Review comment:
Let's provide a list of the `X-Forwarded-XXX` headers
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r781695624
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,130 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implements a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request, and replaces the result when the authentication service responds with a non-2xx status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service. When it is not set, no `client` request headers are sent to the `authorization` service, except for those provided by APISIX (X-Forwarded-XXX). |
Review comment:
the `Data Definition` section added
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r781695624
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,130 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implements a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request, and replaces the result when the authentication service responds with a non-2xx status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service. When it is not set, no `client` request headers are sent to the `authorization` service, except for those provided by APISIX (X-Forwarded-XXX). |
Review comment:
the `Data Definition` added
--
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
[GitHub] [apisix] spacewander commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780775949
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
Currently, we have to enforce it with code review. Maybe we can invest in linter in the future.
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r779989086
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
Review comment:
In our or other forward-auth plugins, authentication-related data is sent via fixed requests, not POST data, so I think that's enough too.
ping @shuaijinchao
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780868113
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,178 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"111\" then core.response.exit(200); end end",
Review comment:
As far as I know, JSON does not support direct line feeds. And according to the code of serverless plugin, each item in functions is a function, not a combination of code execution. Perhaps it can only be written in one line. 🤔
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780880736
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,178 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"111\" then core.response.exit(200); end end",
Review comment:
Splitting the single line long code is done, which uses `]]..[[` truncates the data code, the code is so long that it cannot be parsed, so I modified it by referring to the explanation in this issue.
[https://github.com/openresty/lua-nginx-module/issues/1622](https://github.com/openresty/lua-nginx-module/issues/1622)
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780889247
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization service"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorizing failed"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = ctx.var.request_uri,
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ if not auth_headers[header] then
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
+
+ if res.status >= 300 then
+ local client_headers = {}
+
+ if #conf.client_headers > 0 then
+ for _, header in ipairs(conf.client_headers) do
+ client_headers[header] = res.headers[header]
+ end
+ else
+ client_headers = res.headers
Review comment:
Ditto, changed
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780923002
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,178 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"111\" then core.response.exit(200); end end",
Review comment:
> ~~Log check for `X-Forwarded-XX`, I will add it later.~~
In the session of adding more cases, I found out that we won't actually use ta, and that the function to print the request header is already implemented using `echo`.
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,178 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"111\" then core.response.exit(200); end end",
Review comment:
> ~~Log check for `X-Forwarded-XX`, I will add it later.~~
In the session of adding more cases, I found out that we won't actually use ta, and that the function to print the request header is already implemented using `/echo`.
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r779991014
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,135 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implement a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
Review comment:
changed
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,135 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implement a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request and replaces the result when the authentication service responds with a non-20x status.
Review comment:
changed
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,135 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implement a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request and replaces the result when the authentication service responds with a non-20x status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service |
+| upstream_headers | array[string] | optional | | | `authorization` service response header that will be sent to the `upstream` |
+| client_headers | array[string] | optional | | | `authorization` response header that will be sent to the `client` when authorize failure |
+| timeout | integer | optional | 3000ms | [1, 60000]ms | Authorization service HTTP call timeout |
+| keepalive | boolean | optional | true | | HTTP keepalive |
+| keepalive_timeout | integer | optional | 60000ms | [1000, ...]ms | keepalive idle timeout |
+| keepalive_pool | integer | optional | 5 | [1, ...]ms | Connection pool limit |
+
+## Example
+
+First, you need to setup an external authorization service. Here is an example of using Apache APISIX's serverless plugin to mock.
+
+```shell
+$ curl -X PUT 'http://127.0.0.1:9080/apisix/admin/routes/auth' \
+ -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' \
+ -H 'Content-Type: application/json' \
+ -d '{
+ "uri": "/auth",
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); local authorization = core.request.header(ctx, \"Authorization\"); if authorization == \"123\" then core.response.exit(200); elseif authorization == \"321\" then core.response.set_header(\"X-User-ID\", \"i-am-user\"); core.response.exit(200); else core.response.set_header(\"Location\", \"http://example.com/auth\"); core.response.exit(403); end end"
+ ]
+ }
+ },
+ "upstream": {
+ "nodes": {},
+ "scheme": "https",
+ "type": "roundrobin"
+ }
+}'
+```
+
+Next, we create a route for testing.
+
+```shell
+$ curl -X PUT http://127.0.0.1:9080/apisix/admin/routes/1
+ -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1'
+ -d '{
+ "uri": "/headers",
+ "plugins": {
+ "forward-auth": {
+ "host": "http://127.0.0.1:9080/auth",
+ "request_headers": ["Authorization"],
+ "upstream_headers": ["X-User-ID"],
+ "client_headers": ["Location"]
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "httpbin.org:80": 1
+ },
+ "type": "roundrobin"
+ }
+}'
+```
+
+We can perform the following three tests.
+
+1. **request_headers** Send Authorization header from `client` to `authorization` service
+
+```shell
+$ curl http://127.0.0.1:9080/headers -H 'Authorization: 123'
+{
+ "headers": {
+ "Authorization": "123",
+ "Next": "More-headers"
+ }
+}
+```
+
+2. **upstream_headers** Send `authorization` service response header to the `upstream`
+
+```shell
+$ curl http://127.0.0.1:9080/headers -H 'Authorization: 321'
+{
+ "headers": {
+ "Authorization": "321",
+ "X-User-ID": "i-am-user",
+ "Next": "More-headers"
+ }
+}
+```
+
+3. **client_headers** Send `authorization` service response header to `client` when authorize failure
Review comment:
changed
--
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
[GitHub] [apisix] shuaijinchao commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r779975193
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
Review comment:
Should the request method be mapped to the client request method?
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
```suggestion
if not res then
core.log.error("failed to process forward auth, err: ", err)
return 403
end
```
Would it be better to judge this way?
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r779995455
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,143 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
Review comment:
fixed, because `auth_headers` has been set in our concern headers, after the judgment, the request header from the client can not override them, that's not trust the client header.
https://github.com/apache/apisix/blob/8a853d345436e842845ae5bd57de8bb60ac8aee7/apisix/plugins/forward-auth.lua#L78-L93
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r779995970
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,143 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
Review comment:
Because, merge the list of defined headers into the client request header, any key headers passed in by the client are overwritten, so I implement the untrusted client header.
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r779988474
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
I don't understand how to do it really right, I noticed that in other plugin implementations there are those that use `not res` alone, those that use `err` alone and those that use `not res or err`, which one should we take?
https://github.com/apache/apisix/blob/58fec8f5eb599c93c3491e200a4ab66907a3dd24/apisix/plugins/authz-keycloak.lua#L251-L254
https://github.com/apache/apisix/blob/58fec8f5eb599c93c3491e200a4ab66907a3dd24/apisix/plugins/serverless/generic-upstream.lua#L110-L113
https://github.com/apache/apisix/blob/58fec8f5eb599c93c3491e200a4ab66907a3dd24/apisix/plugins/wolf-rbac.lua#L145-L147
I hope to get your guidance.
cc @spacewander
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780282076
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
Then, referring to the way it is usually used, I will determine if `err` is `not nil`.
```lua
if err then
xxx
end
```
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780868256
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,178 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"111\" then core.response.exit(200); end end",
Review comment:
Log check for `X-Forwarded-XX`, I will add it later.
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780792258
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
Review comment:
> OK, use `GET` as a fixed request method you should remove it because the default request method is `GET`
Oh, I get it. Modify will be made later.
--
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
[GitHub] [apisix] shuaijinchao commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780791714
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
Review comment:
OK, use `GET` as a fixed request method you should remove it because the default request method is `GET`
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780923002
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,178 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"111\" then core.response.exit(200); end end",
Review comment:
> ~~Log check for `X-Forwarded-XX`, I will add it later.~~
In the session of adding more cases, I found out that we won't actually use it, and that the function to print the request header is already implemented using `/echo`.
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r779995455
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,143 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
Review comment:
fixed, because `auth_headers` has been set in our concern headers, after the judgment, the request header from the client can not override them, that's not trust the client header. L89-L91
https://github.com/apache/apisix/blob/8a853d345436e842845ae5bd57de8bb60ac8aee7/apisix/plugins/forward-auth.lua#L78-L93
--
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
[GitHub] [apisix] spacewander commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r779976644
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
We should not add a common method that is only used by a place. Actually, I think we should remove the `get_path` too.
It is strange that `get_path` fetches `$uri` but `get_uri` fetches `$request_uri`. Look like it is a premature optimization that brings inconsistent names.
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,143 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
Review comment:
Ditto
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,135 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implement a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request and replaces the result when the authentication service responds with a non-20x status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service |
+| upstream_headers | array[string] | optional | | | `authorization` service response header that will be sent to the `upstream` |
+| client_headers | array[string] | optional | | | `authorization` response header that will be sent to the `client` when authorize failure |
+| timeout | integer | optional | 3000ms | [1, 60000]ms | Authorization service HTTP call timeout |
+| keepalive | boolean | optional | true | | HTTP keepalive |
+| keepalive_timeout | integer | optional | 60000ms | [1000, ...]ms | keepalive idle timeout |
+| keepalive_pool | integer | optional | 5 | [1, ...]ms | Connection pool limit |
+
+## Example
+
+First, you need to setup an external authorization service. Here is an example of using Apache APISIX's serverless plugin to mock.
+
+```shell
+$ curl -X PUT 'http://127.0.0.1:9080/apisix/admin/routes/auth' \
+ -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' \
+ -H 'Content-Type: application/json' \
+ -d '{
+ "uri": "/auth",
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); local authorization = core.request.header(ctx, \"Authorization\"); if authorization == \"123\" then core.response.exit(200); elseif authorization == \"321\" then core.response.set_header(\"X-User-ID\", \"i-am-user\"); core.response.exit(200); else core.response.set_header(\"Location\", \"http://example.com/auth\"); core.response.exit(403); end end"
+ ]
+ }
+ },
+ "upstream": {
+ "nodes": {},
+ "scheme": "https",
+ "type": "roundrobin"
+ }
+}'
+```
+
+Next, we create a route for testing.
+
+```shell
+$ curl -X PUT http://127.0.0.1:9080/apisix/admin/routes/1
+ -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1'
+ -d '{
+ "uri": "/headers",
+ "plugins": {
+ "forward-auth": {
+ "host": "http://127.0.0.1:9080/auth",
+ "request_headers": ["Authorization"],
+ "upstream_headers": ["X-User-ID"],
+ "client_headers": ["Location"]
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "httpbin.org:80": 1
+ },
+ "type": "roundrobin"
+ }
+}'
+```
+
+We can perform the following three tests.
+
+1. **request_headers** Send Authorization header from `client` to `authorization` service
+
+```shell
+$ curl http://127.0.0.1:9080/headers -H 'Authorization: 123'
+{
+ "headers": {
+ "Authorization": "123",
+ "Next": "More-headers"
+ }
+}
+```
+
+2. **upstream_headers** Send `authorization` service response header to the `upstream`
+
+```shell
+$ curl http://127.0.0.1:9080/headers -H 'Authorization: 321'
+{
+ "headers": {
+ "Authorization": "321",
+ "X-User-ID": "i-am-user",
+ "Next": "More-headers"
+ }
+}
+```
+
+3. **client_headers** Send `authorization` service response header to `client` when authorize failure
Review comment:
```suggestion
3. **client_headers** Send `authorization` service response header to `client` when authorizing failed
```
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,135 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implement a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request and replaces the result when the authentication service responds with a non-20x status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service |
+| upstream_headers | array[string] | optional | | | `authorization` service response header that will be sent to the `upstream` |
Review comment:
Ditto
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,135 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implement a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request and replaces the result when the authentication service responds with a non-20x status.
Review comment:
```suggestion
Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request, and replaces the result when the authentication service responds with a non-2xx status.
```
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,143 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
Review comment:
```suggestion
description = "client request header that will be sent to the authorization service"
```
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,143 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
Review comment:
```suggestion
.. "the client when authorizing failed"
```
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,143 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
Review comment:
Better not trust the client's `X-Forwarded-XX` by default
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,157 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": ["return function (conf, ctx) local core = require(\"apisix.core\"); local authorization = core.request.header(ctx, \"Authorization\"); if authorization == \"123\" then core.response.exit(200); elseif authorization == \"321\" then core.response.set_header(\"X-User-ID\", \"i-am-an-user\"); core.response.exit(200); else core.response.set_header(\"Location\", \"http://example.com/auth\"); core.response.exit(403); end end"]
+ }
+ },
+ "uri": "/auth"
+ }]],
+ },
+ {
+ url = "/apisix/admin/routes/echo",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": ["return function (conf, ctx) local core = require(\"apisix.core\"); core.response.exit(200, core.request.headers(ctx)); end"]
+ }
+ },
+ "uri": "/echo"
+ }]],
+ },
+ {
+ url = "/apisix/admin/routes/1",
+ data = [[{
+ "plugins": {
+ "forward-auth": {
+ "host": "http://127.0.0.1:1984/auth",
+ "request_headers": ["Authorization"],
+ "upstream_headers": ["X-User-ID"],
+ "client_headers": ["Location"]
+ },
+ "proxy-rewrite": {
+ "uri": "/echo"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1984": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]],
+ },
+ }
+
+ local t = require("lib.test_admin").test
+
+ for _, data in ipairs(datas) do
+ local code, body = t(data.url, ngx.HTTP_PUT, data.data)
+ ngx.say(code..body)
+ end
+ }
+ }
+--- response_body eval
+"201passed\n" x 3
+
+
+
+=== TEST 3: hit route (test request_headers)
+--- request
+GET /hello
+--- more_headers
+Authorization: 123
+--- response_body_like eval
+qr/\"authorization\":\"123\"/
+
+
+
+=== TEST 4: hit route (test upstream_headers)
+--- request
+GET /hello
+--- more_headers
+Authorization: 321
+--- response_body_like eval
+qr/\"x-user-id\":\"i-am-an-user\"/
+
+
+
+=== TEST 5: hit route (test client_headers)
+--- request
+GET /hello
+--- error_code: 403
+--- response_headers
+Location: http://example.com/auth
Review comment:
Let's add a test that the client passes X-Forwarded-XX by itself
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,157 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": ["return function (conf, ctx) local core = require(\"apisix.core\"); local authorization = core.request.header(ctx, \"Authorization\"); if authorization == \"123\" then core.response.exit(200); elseif authorization == \"321\" then core.response.set_header(\"X-User-ID\", \"i-am-an-user\"); core.response.exit(200); else core.response.set_header(\"Location\", \"http://example.com/auth\"); core.response.exit(403); end end"]
Review comment:
Let's log down the X-Forwarded-XX received by the service. And better to break down the very long code.
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,135 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implement a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request and replaces the result when the authentication service responds with a non-20x status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service |
+| upstream_headers | array[string] | optional | | | `authorization` service response header that will be sent to the `upstream` |
+| client_headers | array[string] | optional | | | `authorization` response header that will be sent to the `client` when authorize failure |
+| timeout | integer | optional | 3000ms | [1, 60000]ms | Authorization service HTTP call timeout |
+| keepalive | boolean | optional | true | | HTTP keepalive |
+| keepalive_timeout | integer | optional | 60000ms | [1000, ...]ms | keepalive idle timeout |
+| keepalive_pool | integer | optional | 5 | [1, ...]ms | Connection pool limit |
+
+## Example
+
+First, you need to setup an external authorization service. Here is an example of using Apache APISIX's serverless plugin to mock.
+
+```shell
+$ curl -X PUT 'http://127.0.0.1:9080/apisix/admin/routes/auth' \
+ -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' \
+ -H 'Content-Type: application/json' \
+ -d '{
+ "uri": "/auth",
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); local authorization = core.request.header(ctx, \"Authorization\"); if authorization == \"123\" then core.response.exit(200); elseif authorization == \"321\" then core.response.set_header(\"X-User-ID\", \"i-am-user\"); core.response.exit(200); else core.response.set_header(\"Location\", \"http://example.com/auth\"); core.response.exit(403); end end"
+ ]
+ }
+ },
+ "upstream": {
+ "nodes": {},
+ "scheme": "https",
Review comment:
Let's remove useless field
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,135 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implement a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
Review comment:
```suggestion
The `forward-auth` plugin implements a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
```
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,135 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implement a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request and replaces the result when the authentication service responds with a non-20x status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service |
Review comment:
Let's doc the behavior when this field is empty
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780889234
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
Review comment:
I think it's rightfully so, and that's an oversight on my part. I have reversed the meaning of `request_headers` and `client_headers`, if the user does not set this parameter, APISIX will send nothing.
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization service"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorizing failed"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = ctx.var.request_uri,
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ if not auth_headers[header] then
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
+
+ if res.status >= 300 then
+ local client_headers = {}
+
+ if #conf.client_headers > 0 then
+ for _, header in ipairs(conf.client_headers) do
+ client_headers[header] = res.headers[header]
+ end
+ else
+ client_headers = res.headers
Review comment:
Ditto
--
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
[GitHub] [apisix] spacewander commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780775740
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization service"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorizing failed"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = ctx.var.request_uri,
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ if not auth_headers[header] then
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
+
+ if res.status >= 300 then
+ local client_headers = {}
+
+ if #conf.client_headers > 0 then
+ for _, header in ipairs(conf.client_headers) do
+ client_headers[header] = res.headers[header]
+ end
+ else
+ client_headers = res.headers
Review comment:
IMHO, would be better to use allowlist? If no client_headers, no headers will be returned to the client. Some headers are not expected to be overridden, like Server / Date.
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,178 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"111\" then core.response.exit(200); end end",
Review comment:
Would be better to log down the "X-Forwarded-XX" headers and check it
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
Review comment:
Look like we can't support forward no headers from the client?
If this field is empty, all headers are forwarded. What about don't provide a default value? (Use empty array if no headers are need)
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,178 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"111\" then core.response.exit(200); end end",
Review comment:
Let's indent the code. Like:
```
diff --git t/plugin/forward-auth.t t/plugin/forward-auth.t
index dca35b76..285e60fb 100644
--- t/plugin/forward-auth.t
+++ t/plugin/forward-auth.t
@@ -74,7 +74,12 @@ property "request_headers" validation failed: wrong type: expected array, got st
"serverless-pre-function": {
"phase": "rewrite",
"functions": [
- "return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"111\" then core.response.exit(200); end end",
+ "return function (conf, ctx)
+ local core = require(\"apisix.core\");
+ if core.request.header(ctx, \"Authorization\") == \"111\" then
+ core.response.exit(200)
+ end
+ end",
"return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"222\" then core.response.set_header(\"X-User-ID\", \"i-am-an-user\"); core.response.exit(200); end end",
"return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"333\" then core.response.set_header(\"Location\", \"http://example.com/auth\"); core.response.exit(403); end end",
"return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"444\" then core.response.exit(403, core.request.headers(ctx)); end end"
```
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r779990931
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,143 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
Review comment:
changed
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780868113
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,178 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"111\" then core.response.exit(200); end end",
Review comment:
As far as I know, JSON does not support direct line feeds. And according to the code of serverless plugin, each item in functions is a function, not a combination of code execution. Perhaps it can only be written in one line. 🤔
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780998115
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,246 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/upstreams/u1",
+ data = [[{
+ "nodes": {
+ "127.0.0.1:1984": 1
+ },
+ "type": "roundrobin"
+ }]],
+ },
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function(conf, ctx)
Review comment:
I've merged it with the checkcase for overriding user input requests, which will check all APISIX generated headers at once. `TEST 6: hit route (check APISIX generated headers and ignore client headers)`, It is provided by our echo function.
https://github.com/apache/apisix/blob/beea582ddff8879e94484f84202307c488f30a70/t/plugin/forward-auth.t#L221-L225
--
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
[GitHub] [apisix] spacewander commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780160633
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
Don't make thing complex!
The name of request_uri and uri already show the difference. There is no need to wrap them into a function which is rarely used. Premature optimization is the source of evil.
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
We can remove the `get_path` in the next PR. Better to get rid of it before the release, so that people won't ask why we use `get_path` in one place but use `ctx.var.uri` in the most elsewhere.
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
I read through each branch of https://github.com/api7/lua-resty-http/blob/b63a9bbf2a3361836b2322e3c689d6d7fd83ec55/lib/resty/http.lua#L900, look like there are only two cases:
1. `nil, err`
2. `res, nil`, while res is a table
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780059069
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,157 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": ["return function (conf, ctx) local core = require(\"apisix.core\"); local authorization = core.request.header(ctx, \"Authorization\"); if authorization == \"123\" then core.response.exit(200); elseif authorization == \"321\" then core.response.set_header(\"X-User-ID\", \"i-am-an-user\"); core.response.exit(200); else core.response.set_header(\"Location\", \"http://example.com/auth\"); core.response.exit(403); end end"]
Review comment:
I used another method to record these heads. I also added a simple `echo` function under `/auth` route.
--
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
[GitHub] [apisix] spacewander commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780163880
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
We can remove the `get_path` in the next PR. Better to get rid of it before the release, so that people won't ask why we use `get_path` in one place but use `ctx.var.uri` in the most elsewhere.
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780292697
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
Also, I think there needs to make a rules that developers should follow to handle error checking (at least on the http client), the mix of different usages is confusing.
--
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
[GitHub] [apisix] spacewander commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780264643
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
I read through each branch of https://github.com/api7/lua-resty-http/blob/b63a9bbf2a3361836b2322e3c689d6d7fd83ec55/lib/resty/http.lua#L900, look like there are only two cases:
1. `nil, err`
2. `res, nil`, while res is a table
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780004879
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,135 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implement a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request and replaces the result when the authentication service responds with a non-20x status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service |
Review comment:
added
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,135 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implement a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request and replaces the result when the authentication service responds with a non-20x status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service |
+| upstream_headers | array[string] | optional | | | `authorization` service response header that will be sent to the `upstream` |
Review comment:
added
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780058183
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,157 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": ["return function (conf, ctx) local core = require(\"apisix.core\"); local authorization = core.request.header(ctx, \"Authorization\"); if authorization == \"123\" then core.response.exit(200); elseif authorization == \"321\" then core.response.set_header(\"X-User-ID\", \"i-am-an-user\"); core.response.exit(200); else core.response.set_header(\"Location\", \"http://example.com/auth\"); core.response.exit(403); end end"]
+ }
+ },
+ "uri": "/auth"
+ }]],
+ },
+ {
+ url = "/apisix/admin/routes/echo",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": ["return function (conf, ctx) local core = require(\"apisix.core\"); core.response.exit(200, core.request.headers(ctx)); end"]
+ }
+ },
+ "uri": "/echo"
+ }]],
+ },
+ {
+ url = "/apisix/admin/routes/1",
+ data = [[{
+ "plugins": {
+ "forward-auth": {
+ "host": "http://127.0.0.1:1984/auth",
+ "request_headers": ["Authorization"],
+ "upstream_headers": ["X-User-ID"],
+ "client_headers": ["Location"]
+ },
+ "proxy-rewrite": {
+ "uri": "/echo"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1984": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]],
+ },
+ }
+
+ local t = require("lib.test_admin").test
+
+ for _, data in ipairs(datas) do
+ local code, body = t(data.url, ngx.HTTP_PUT, data.data)
+ ngx.say(code..body)
+ end
+ }
+ }
+--- response_body eval
+"201passed\n" x 3
+
+
+
+=== TEST 3: hit route (test request_headers)
+--- request
+GET /hello
+--- more_headers
+Authorization: 123
+--- response_body_like eval
+qr/\"authorization\":\"123\"/
+
+
+
+=== TEST 4: hit route (test upstream_headers)
+--- request
+GET /hello
+--- more_headers
+Authorization: 321
+--- response_body_like eval
+qr/\"x-user-id\":\"i-am-an-user\"/
+
+
+
+=== TEST 5: hit route (test client_headers)
+--- request
+GET /hello
+--- error_code: 403
+--- response_headers
+Location: http://example.com/auth
Review comment:
added, I added a test case to demonstrate that these client-side incoming key headers are indeed dropped in APISIX.
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780292697
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
Also, I think there needs to be a specification of what rules developers should follow to handle error checking (at least on the http client), the mix of different usages is confusing.
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780061628
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
So I should just use ctx.var.request_uri instead of wrapping it? Can I merge the functionality of `get_path` into `get_uri` and add a `with_args` parameter indicating whether or not I need to carry parameters.
##########
File path: apisix/core/request.lua
##########
@@ -278,6 +278,15 @@ function _M.get_path(ctx)
end
+function _M.get_uri(ctx)
Review comment:
So I should just use ctx.var.request_uri instead of wrapping it? Can I merge the functionality of `get_path` into `get_uri` and add a `with_args` parameter indicating whether or not I need to carry queries.
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780924096
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
+ headers = auth_headers,
+ keepalive = conf.keepalive,
+ ssl_verify = conf.ssl_verify
+ }
+
+ if conf.keepalive then
+ params.keepalive_timeout = conf.keepalive_timeout
+ params.keepalive_pool = conf.keepalive_pool
+ end
+
+ local httpc = http.new()
+ httpc:set_timeout(conf.timeout)
+
+ local res, err = httpc:request_uri(conf.host, params)
+
+ -- block by default when authorization service is unavailable
+ if not res or err then
+ core.log.error("failed to process forward auth, err: ", err)
+ return 403
+ end
Review comment:
changed
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780923574
##########
File path: apisix/plugins/forward-auth.lua
##########
@@ -0,0 +1,145 @@
+--
+-- 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 ipairs = ipairs
+local core = require("apisix.core")
+local http = require("resty.http")
+
+local schema = {
+ type = "object",
+ properties = {
+ host = {type = "string"},
+ ssl_verify = {
+ type = "boolean",
+ default = true,
+ },
+ request_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "client request header that will be sent to the authorization"
+ },
+ upstream_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to the upstream"
+ },
+ client_headers = {
+ type = "array",
+ default = {},
+ items = {type = "string"},
+ description = "authorization response header that will be sent to"
+ .. "the client when authorize failure"
+ },
+ timeout = {
+ type = "integer",
+ minimum = 1,
+ maximum = 60000,
+ default = 3000,
+ description = "timeout in milliseconds",
+ },
+ keepalive = {type = "boolean", default = true},
+ keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
+ keepalive_pool = {type = "integer", minimum = 1, default = 5},
+ },
+ required = {"host"}
+}
+
+
+local _M = {
+ version = 0.1,
+ priority = 2002,
+ name = "forward-auth",
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local auth_headers = {
+ ["X-Forwarded-Proto"] = core.request.get_scheme(ctx),
+ ["X-Forwarded-Method"] = core.request.get_method(),
+ ["X-Forwarded-Host"] = core.request.get_host(ctx),
+ ["X-Forwarded-Uri"] = core.request.get_uri(ctx),
+ ["X-Forwarded-For"] = core.request.get_remote_client_ip(ctx),
+ }
+
+ -- append headers that need to be get from the client request header
+ if #conf.request_headers > 0 then
+ for _, header in ipairs(conf.request_headers) do
+ auth_headers[header] = core.request.header(ctx, header)
+ end
+ else
+ auth_headers = core.table.merge(core.request.headers(), auth_headers)
+ end
+
+ local params = {
+ method = "GET",
Review comment:
removed
--
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
[GitHub] [apisix] bzp2010 commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780868256
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,178 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx) local core = require(\"apisix.core\"); if core.request.header(ctx, \"Authorization\") == \"111\" then core.response.exit(200); end end",
Review comment:
~~Log check for `X-Forwarded-XX`, I will add it later.~~
--
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
[GitHub] [apisix] spacewander commented on a change in pull request #6037: feat: add forward-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6037:
URL: https://github.com/apache/apisix/pull/6037#discussion_r780968118
##########
File path: docs/en/latest/plugins/forward-auth.md
##########
@@ -0,0 +1,130 @@
+---
+title: forward-auth
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## Summary
+
+- [**Description**](#description)
+- [**Attributes**](#attributes)
+- [**Example**](#example)
+
+## Description
+
+The `forward-auth` plugin implements a classic external authentication model. We can implement a custom error return or user redirection to the authentication page if the authentication fails.
+
+Forward Auth cleverly moves the authentication and authorization logic to a dedicated external service, where the gateway forwards the user's request to the authentication service and blocks the original request, and replaces the result when the authentication service responds with a non-2xx status.
+
+## Attributes
+
+| Name | Type | Requirement | Default | Valid | Description |
+| -- | -- | -- | -- | -- | -- |
+| host | string | required | | | Authorization service host (eg. https://localhost:9188) |
+| ssl_verify | boolean | optional | true | | Whether to verify the certificate |
+| request_headers | array[string] | optional | | | `client` request header that will be sent to the `authorization` service. When it is not set, all `client` request headers are sent to the `authorization` service, except for those provided by APISIX (X-Forwarded-XXX). |
Review comment:
Need to update the doc
##########
File path: t/plugin/forward-auth.t
##########
@@ -0,0 +1,246 @@
+#
+# 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);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local test_cases = {
+ {host = "http://127.0.0.1:8199"},
+ {request_headers = {"test"}},
+ {host = 3233},
+ {host = "http://127.0.0.1:8199", request_headers = "test"}
+ }
+ local plugin = require("apisix.plugins.forward-auth")
+
+ for _, case in ipairs(test_cases) do
+ local ok, err = plugin.check_schema(case)
+ ngx.say(ok and "done" or err)
+ end
+ }
+ }
+--- response_body
+done
+property "host" is required
+property "host" validation failed: wrong type: expected string, got number
+property "request_headers" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: setup route with plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local datas = {
+ {
+ url = "/apisix/admin/upstreams/u1",
+ data = [[{
+ "nodes": {
+ "127.0.0.1:1984": 1
+ },
+ "type": "roundrobin"
+ }]],
+ },
+ {
+ url = "/apisix/admin/routes/auth",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function(conf, ctx)
Review comment:
Would be better to log down the APISIX generated "X-Forwarded-XX" headers and check it
--
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