From f090586220738a87d1630cca7068a969d9b16e7b Mon Sep 17 00:00:00 2001 From: spacewander Date: Fri, 18 Oct 2024 10:38:59 +0800 Subject: [PATCH] change: allow missing consumer If consumer is required, use consumerRestriction plugin to restrict. This change makes it possible to support limit rate per consumer coexists with limit rate without consumer. Signed-off-by: spacewander --- api/pkg/filtermanager/filtermanager.go | 11 +---- e2e/tests/consumer.yml | 3 ++ e2e/tests/consumer_in_gateway.yml | 3 ++ plugins/plugins/consumerrestriction/config.go | 4 ++ plugins/plugins/consumerrestriction/filter.go | 12 ++--- plugins/tests/integration/hmac_auth_test.go | 19 ++++++-- plugins/tests/integration/key_auth_test.go | 47 ++++++++++++++----- site/content/en/docs/concept/consumer.md | 2 +- .../reference/plugins/consumer_restriction.md | 13 ++--- site/content/zh-hans/docs/concept/consumer.md | 2 +- .../reference/plugins/consumer_restriction.md | 13 ++--- tools/cmd/linter/main.go | 3 +- .../plugins/consumerrestriction/config.pb.go | 30 +++++++++--- .../consumerrestriction/config.pb.validate.go | 13 +++++ .../plugins/consumerrestriction/config.proto | 1 + 15 files changed, 123 insertions(+), 53 deletions(-) diff --git a/api/pkg/filtermanager/filtermanager.go b/api/pkg/filtermanager/filtermanager.go index 17a9134e5..4d3e13fe4 100644 --- a/api/pkg/filtermanager/filtermanager.go +++ b/api/pkg/filtermanager/filtermanager.go @@ -384,16 +384,7 @@ func (m *filterManager) DecodeHeaders(headers capi.RequestHeaderMap, endStream b // we check consumer at the end of authn filters, so we can have multiple authn filters // configured and the consumer will be set by any of them c, ok := m.callbacks.consumer.(*consumer.Consumer) - if !ok { - api.LogInfo("reject for consumer not found") - m.localReply(&api.LocalResponse{ - Code: 401, - Msg: "consumer not found", - }, true) - return - } - - if len(c.FilterConfigs) > 0 { + if ok && len(c.FilterConfigs) > 0 { api.LogDebugf("merge filters from consumer: %s", c.Name()) c.InitOnce.Do(func() { diff --git a/e2e/tests/consumer.yml b/e2e/tests/consumer.yml index 166bf158a..2cd4894eb 100644 --- a/e2e/tests/consumer.yml +++ b/e2e/tests/consumer.yml @@ -68,3 +68,6 @@ spec: config: keys: - name: Authorization + consumerRestriction: + config: + denyIfNoConsumer: true diff --git a/e2e/tests/consumer_in_gateway.yml b/e2e/tests/consumer_in_gateway.yml index ac1721b73..8819e1b02 100644 --- a/e2e/tests/consumer_in_gateway.yml +++ b/e2e/tests/consumer_in_gateway.yml @@ -67,6 +67,9 @@ spec: config: keys: - name: Authorization + consumerRestriction: + config: + denyIfNoConsumer: true --- apiVersion: htnn.mosn.io/v1 kind: FilterPolicy diff --git a/plugins/plugins/consumerrestriction/config.go b/plugins/plugins/consumerrestriction/config.go index 7a06f12de..e0e2a31d6 100644 --- a/plugins/plugins/consumerrestriction/config.go +++ b/plugins/plugins/consumerrestriction/config.go @@ -49,6 +49,10 @@ type Rule struct { } func (conf *config) Init(cb api.ConfigCallbackHandler) error { + if conf.GetDenyIfNoConsumer() { + return nil + } + rules := conf.GetDeny() if rules == nil { rules = conf.GetAllow() diff --git a/plugins/plugins/consumerrestriction/filter.go b/plugins/plugins/consumerrestriction/filter.go index 957c17f8a..d9991274d 100644 --- a/plugins/plugins/consumerrestriction/filter.go +++ b/plugins/plugins/consumerrestriction/filter.go @@ -32,15 +32,15 @@ type filter struct { config *config } -func (f *filter) reject(msg string) api.ResultAction { - return &api.LocalResponse{Code: 403, Msg: msg} -} - func (f *filter) DecodeHeaders(headers api.RequestHeaderMap, endStream bool) api.ResultAction { consumer := f.callbacks.GetConsumer() if consumer == nil { api.LogInfo("consumerRestriction: consumer not found") - return f.reject("consumer not found") + return &api.LocalResponse{Code: 401, Msg: "consumer not found"} + } + + if f.config.GetDenyIfNoConsumer() { + return api.Continue } consumerName := consumer.Name() @@ -49,7 +49,7 @@ func (f *filter) DecodeHeaders(headers api.RequestHeaderMap, endStream bool) api if methodMatched != f.config.allow { api.LogInfof("consumerRestriction: consumer %s not allowed", consumerName) - return f.reject("consumer not allowed") + return &api.LocalResponse{Code: 403, Msg: "consumer not allowed"} } return api.Continue diff --git a/plugins/tests/integration/hmac_auth_test.go b/plugins/tests/integration/hmac_auth_test.go index efb7c85e3..e067b55d0 100644 --- a/plugins/tests/integration/hmac_auth_test.go +++ b/plugins/tests/integration/hmac_auth_test.go @@ -57,10 +57,21 @@ func TestHmacAuth(t *testing.T) { }{ { name: "sanity", - config: controlplane.NewSinglePluginConfig("hmacAuth", map[string]interface{}{ - "signatureHeader": "x-sign-hdr", - "accessKeyHeader": "x-ak", - "dateHeader": "x-date", + config: controlplane.NewPluginConfig([]*model.FilterConfig{ + { + Name: "hmacAuth", + Config: map[string]interface{}{ + "signatureHeader": "x-sign-hdr", + "accessKeyHeader": "x-ak", + "dateHeader": "x-date", + }, + }, + { + Name: "consumerRestriction", + Config: map[string]interface{}{ + "deny_if_no_consumer": true, + }, + }, }), run: func(t *testing.T) { hdr := http.Header{} diff --git a/plugins/tests/integration/key_auth_test.go b/plugins/tests/integration/key_auth_test.go index d0e4ad3e7..e55bd9f9a 100644 --- a/plugins/tests/integration/key_auth_test.go +++ b/plugins/tests/integration/key_auth_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/assert" "mosn.io/htnn/api/pkg/filtermanager" + "mosn.io/htnn/api/pkg/filtermanager/model" "mosn.io/htnn/api/plugins/tests/integration/controlplane" "mosn.io/htnn/api/plugins/tests/integration/dataplane" ) @@ -51,10 +52,21 @@ func TestKeyAuth(t *testing.T) { }{ { name: "key in the header", - config: controlplane.NewSinglePluginConfig("keyAuth", map[string]interface{}{ - "keys": []interface{}{ - map[string]interface{}{ - "name": "Authorization", + config: controlplane.NewPluginConfig([]*model.FilterConfig{ + { + Name: "keyAuth", + Config: map[string]interface{}{ + "keys": []interface{}{ + map[string]interface{}{ + "name": "Authorization", + }, + }, + }, + }, + { + Name: "consumerRestriction", + Config: map[string]interface{}{ + "deny_if_no_consumer": true, }, }, }), @@ -72,15 +84,26 @@ func TestKeyAuth(t *testing.T) { }, { name: "key in the query", - config: controlplane.NewSinglePluginConfig("keyAuth", map[string]interface{}{ - "keys": []interface{}{ - map[string]interface{}{ - "name": "Authorization", - "source": "HEADER", + config: controlplane.NewPluginConfig([]*model.FilterConfig{ + { + Name: "keyAuth", + Config: map[string]interface{}{ + "keys": []interface{}{ + map[string]interface{}{ + "name": "Authorization", + "source": "HEADER", + }, + map[string]interface{}{ + "name": "ak", + "source": "QUERY", + }, + }, }, - map[string]interface{}{ - "name": "ak", - "source": "QUERY", + }, + { + Name: "consumerRestriction", + Config: map[string]interface{}{ + "deny_if_no_consumer": true, }, }, }), diff --git a/site/content/en/docs/concept/consumer.md b/site/content/en/docs/concept/consumer.md index d46379f26..f3011607e 100644 --- a/site/content/en/docs/concept/consumer.md +++ b/site/content/en/docs/concept/consumer.md @@ -92,7 +92,7 @@ Each Consumer plugin configured on the Route will proceed through the following 1. If the match is unsuccessful, return a 401 HTTP status code. 2. If the match is successful, move on to the next plugin. -If no Consumer is matched after all the Consumer plugins have been executed, a 401 HTTP status code will be returned. +Unlike Kong/APISIX, requests that do not match a Consumer are not interrupted. If you want to ensure that only authenticated consumers can access backend services, we need to use it in conjunction with the [consumerRestriction plugin](../reference/plugins/consumer_restriction.md). In addition to that, we can configure additional plugins for consumers under the `filters` field. These plugins are only executed after the consumer has been authenticated. Take the following configuration as an example: diff --git a/site/content/en/docs/reference/plugins/consumer_restriction.md b/site/content/en/docs/reference/plugins/consumer_restriction.md index b3e7c87f4..6418f16b7 100644 --- a/site/content/en/docs/reference/plugins/consumer_restriction.md +++ b/site/content/en/docs/reference/plugins/consumer_restriction.md @@ -4,7 +4,7 @@ title: Consumer Restriction ## Description -The `consumerRestriction` plugin determines whether the current consumer has access permission based on the configuration. If there is no current consumer or the consumer does not have access permission, a 403 HTTP status code is returned. +The `consumerRestriction` plugin determines whether the current consumer has access permission based on the configuration. If there is no current consumer, a 401 HTTP status code is returned. If the consumer does not have access permission, a 403 HTTP status code is returned. ## Attribute @@ -16,12 +16,13 @@ The `consumerRestriction` plugin determines whether the current consumer has acc ## Configuration -| Name | Type | Required | Validation | Description | -|-------|-------|----------|------------|--------------------------------------| -| allow | Rules | False | | List of rules allowing access access | -| deny | Rules | False | | List of rules denying access access | +| Name | Type | Required | Validation | Description | +|------------------|-------|----------|------------|----------------------------------------------| +| allow | Rules | False | | List of rules allowing access access | +| deny | Rules | False | | List of rules denying access access | +| denyIfNoConsumer | bool | False | | Deny request if there is no matched consumer | -Only one of `allow` or `deny` can be configured. +Only one of `allow` or `deny` or `denyIfNoConsumer` can be configured. ### Rules diff --git a/site/content/zh-hans/docs/concept/consumer.md b/site/content/zh-hans/docs/concept/consumer.md index d1f424796..85fa70c7d 100644 --- a/site/content/zh-hans/docs/concept/consumer.md +++ b/site/content/zh-hans/docs/concept/consumer.md @@ -92,7 +92,7 @@ spec: 1. 如果匹配失败,返回 401 HTTP 状态码。 2. 如果匹配成功,则执行下一个插件。 -如果执行完全部消费者插件后,仍然没有匹配到消费者,则返回 401 HTTP 状态码。 +和 Kong/APISIX 不同的是,请求没有匹配到消费者时不会被中断。如果想在保证只有经过认证的消费者才能访问后端服务,我们需要额外配合 [consumerRestriction 插件](../reference/plugins/consumer_restriction.md) 一起使用。 除此之外,我们还可以在 `filters` 字段下给消费者配置额外的插件。这些插件只有在通过认证之后才会执行。以下面的配置为例: diff --git a/site/content/zh-hans/docs/reference/plugins/consumer_restriction.md b/site/content/zh-hans/docs/reference/plugins/consumer_restriction.md index 55c5a23ca..8db8da726 100644 --- a/site/content/zh-hans/docs/reference/plugins/consumer_restriction.md +++ b/site/content/zh-hans/docs/reference/plugins/consumer_restriction.md @@ -4,7 +4,7 @@ title: Consumer Restriction ## 说明 -`consumerRestriction` 插件根据配置,判断当前的消费者是否有访问权限。如果当前不存在消费者或消费者没有访问权限,则返回 403 HTTP 状态码。 +`consumerRestriction` 插件根据配置,判断当前的消费者是否有访问权限。如果当前不存在消费者,则返回 403 HTTP 状态码。如果消费者没有访问权限,则返回 403 HTTP 状态码。 ## 属性 @@ -16,12 +16,13 @@ title: Consumer Restriction ## 配置 -| 名称 | 类型 | 必选 | 校验规则 | 说明 | -|-------|-------|------|----------|--------------------| -| allow | Rules | 否 | | 允许访问的规则列表 | -| deny | Rules | 否 | | 禁止访问的规则列表 | +| 名称 | 类型 | 必选 | 校验规则 | 说明 | +|------------------|-------|------|----------|----------------------------------| +| allow | Rules | 否 | | 允许访问的规则列表 | +| deny | Rules | 否 | | 禁止访问的规则列表 | +| denyIfNoConsumer | bool | 否 | | 如果没有匹配到消费者,则禁止访问 | -`allow` 和 `deny` 之间只能配置一个。 +`allow` 和 `deny`、`denyIfNoConsumer` 之间只能配置一个。 ### Rules diff --git a/tools/cmd/linter/main.go b/tools/cmd/linter/main.go index 06fa7e243..20134c5c3 100644 --- a/tools/cmd/linter/main.go +++ b/tools/cmd/linter/main.go @@ -487,7 +487,8 @@ func lintConfigurationByCategory(category string) error { } name := filepath.Base(path)[:len(filepath.Base(path))-len(ext)] - pb := filepath.Join("types", category, name, "config.proto") + goPkgName := strings.ReplaceAll(name, "_", "") + pb := filepath.Join("types", category, goPkgName, "config.proto") ms, err := readProto(pb) if err != nil { return err diff --git a/types/plugins/consumerrestriction/config.pb.go b/types/plugins/consumerrestriction/config.pb.go index 1037c010c..bec102ac1 100644 --- a/types/plugins/consumerrestriction/config.pb.go +++ b/types/plugins/consumerrestriction/config.pb.go @@ -148,6 +148,7 @@ type Config struct { // // *Config_Allow // *Config_Deny + // *Config_DenyIfNoConsumer ConfigType isConfig_ConfigType `protobuf_oneof:"config_type"` } @@ -204,6 +205,13 @@ func (x *Config) GetDeny() *Rules { return nil } +func (x *Config) GetDenyIfNoConsumer() bool { + if x, ok := x.GetConfigType().(*Config_DenyIfNoConsumer); ok { + return x.DenyIfNoConsumer + } + return false +} + type isConfig_ConfigType interface { isConfig_ConfigType() } @@ -216,10 +224,16 @@ type Config_Deny struct { Deny *Rules `protobuf:"bytes,2,opt,name=deny,proto3,oneof"` } +type Config_DenyIfNoConsumer struct { + DenyIfNoConsumer bool `protobuf:"varint,3,opt,name=deny_if_no_consumer,json=denyIfNoConsumer,proto3,oneof"` +} + func (*Config_Allow) isConfig_ConfigType() {} func (*Config_Deny) isConfig_ConfigType() {} +func (*Config_DenyIfNoConsumer) isConfig_ConfigType() {} + var File_types_plugins_consumerrestriction_config_proto protoreflect.FileDescriptor var file_types_plugins_consumerrestriction_config_proto_rawDesc = []byte{ @@ -240,7 +254,7 @@ var file_types_plugins_consumerrestriction_config_proto_rawDesc = []byte{ 0x73, 0x2e, 0x70, 0x6c, 0x75, 0x67, 0x69, 0x6e, 0x73, 0x2e, 0x63, 0x6f, 0x6e, 0x73, 0x75, 0x6d, 0x65, 0x72, 0x72, 0x65, 0x73, 0x74, 0x72, 0x69, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x2e, 0x52, 0x75, 0x6c, 0x65, 0x42, 0x08, 0xfa, 0x42, 0x05, 0x92, 0x01, 0x02, 0x08, 0x01, 0x52, 0x05, 0x72, 0x75, - 0x6c, 0x65, 0x73, 0x22, 0x9e, 0x01, 0x0a, 0x06, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x12, 0x40, + 0x6c, 0x65, 0x73, 0x22, 0xcf, 0x01, 0x0a, 0x06, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x12, 0x40, 0x0a, 0x05, 0x61, 0x6c, 0x6c, 0x6f, 0x77, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x28, 0x2e, 0x74, 0x79, 0x70, 0x65, 0x73, 0x2e, 0x70, 0x6c, 0x75, 0x67, 0x69, 0x6e, 0x73, 0x2e, 0x63, 0x6f, 0x6e, 0x73, 0x75, 0x6d, 0x65, 0x72, 0x72, 0x65, 0x73, 0x74, 0x72, 0x69, 0x63, 0x74, 0x69, 0x6f, @@ -249,11 +263,14 @@ var file_types_plugins_consumerrestriction_config_proto_rawDesc = []byte{ 0x2e, 0x74, 0x79, 0x70, 0x65, 0x73, 0x2e, 0x70, 0x6c, 0x75, 0x67, 0x69, 0x6e, 0x73, 0x2e, 0x63, 0x6f, 0x6e, 0x73, 0x75, 0x6d, 0x65, 0x72, 0x72, 0x65, 0x73, 0x74, 0x72, 0x69, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x2e, 0x52, 0x75, 0x6c, 0x65, 0x73, 0x48, 0x00, 0x52, 0x04, 0x64, 0x65, 0x6e, 0x79, - 0x42, 0x12, 0x0a, 0x0b, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x5f, 0x74, 0x79, 0x70, 0x65, 0x12, - 0x03, 0xf8, 0x42, 0x01, 0x42, 0x30, 0x5a, 0x2e, 0x6d, 0x6f, 0x73, 0x6e, 0x2e, 0x69, 0x6f, 0x2f, - 0x68, 0x74, 0x6e, 0x6e, 0x2f, 0x74, 0x79, 0x70, 0x65, 0x73, 0x2f, 0x70, 0x6c, 0x75, 0x67, 0x69, - 0x6e, 0x73, 0x2f, 0x63, 0x6f, 0x6e, 0x73, 0x75, 0x6d, 0x65, 0x72, 0x72, 0x65, 0x73, 0x74, 0x72, - 0x69, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x12, 0x2f, 0x0a, 0x13, 0x64, 0x65, 0x6e, 0x79, 0x5f, 0x69, 0x66, 0x5f, 0x6e, 0x6f, 0x5f, 0x63, + 0x6f, 0x6e, 0x73, 0x75, 0x6d, 0x65, 0x72, 0x18, 0x03, 0x20, 0x01, 0x28, 0x08, 0x48, 0x00, 0x52, + 0x10, 0x64, 0x65, 0x6e, 0x79, 0x49, 0x66, 0x4e, 0x6f, 0x43, 0x6f, 0x6e, 0x73, 0x75, 0x6d, 0x65, + 0x72, 0x42, 0x12, 0x0a, 0x0b, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x5f, 0x74, 0x79, 0x70, 0x65, + 0x12, 0x03, 0xf8, 0x42, 0x01, 0x42, 0x30, 0x5a, 0x2e, 0x6d, 0x6f, 0x73, 0x6e, 0x2e, 0x69, 0x6f, + 0x2f, 0x68, 0x74, 0x6e, 0x6e, 0x2f, 0x74, 0x79, 0x70, 0x65, 0x73, 0x2f, 0x70, 0x6c, 0x75, 0x67, + 0x69, 0x6e, 0x73, 0x2f, 0x63, 0x6f, 0x6e, 0x73, 0x75, 0x6d, 0x65, 0x72, 0x72, 0x65, 0x73, 0x74, + 0x72, 0x69, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( @@ -331,6 +348,7 @@ func file_types_plugins_consumerrestriction_config_proto_init() { file_types_plugins_consumerrestriction_config_proto_msgTypes[2].OneofWrappers = []interface{}{ (*Config_Allow)(nil), (*Config_Deny)(nil), + (*Config_DenyIfNoConsumer)(nil), } type x struct{} out := protoimpl.TypeBuilder{ diff --git a/types/plugins/consumerrestriction/config.pb.validate.go b/types/plugins/consumerrestriction/config.pb.validate.go index b907c8ad8..969b81db5 100644 --- a/types/plugins/consumerrestriction/config.pb.validate.go +++ b/types/plugins/consumerrestriction/config.pb.validate.go @@ -412,6 +412,19 @@ func (m *Config) validate(all bool) error { } } + case *Config_DenyIfNoConsumer: + if v == nil { + err := ConfigValidationError{ + field: "ConfigType", + reason: "oneof value cannot be a typed-nil", + } + if !all { + return err + } + errors = append(errors, err) + } + oneofConfigTypePresent = true + // no validation rules for DenyIfNoConsumer default: _ = v // ensures v is used } diff --git a/types/plugins/consumerrestriction/config.proto b/types/plugins/consumerrestriction/config.proto index d2f079bb3..0de1ba2e7 100644 --- a/types/plugins/consumerrestriction/config.proto +++ b/types/plugins/consumerrestriction/config.proto @@ -35,5 +35,6 @@ message Config { option (validate.required) = true; Rules allow = 1; Rules deny = 2; + bool deny_if_no_consumer = 3; } }