Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOAP policy #567

Merged
merged 11 commits into from
Feb 9, 2018
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- New phase: `content` for generating content or getting the upstream response [PR #535](https://github.com/3scale/apicast/pull/535)
- Upstream policy [PR #562](https://github.com/3scale/apicast/pull/562)
- Policy JSON manifest [PR #565](https://github.com/3scale/apicast/pull/565)
- SOAP policy [PR #567](https://github.com/3scale/apicast/pull/567)

## Fixed

Expand Down
46 changes: 46 additions & 0 deletions gateway/src/apicast/policy/soap/apicast-policy.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
{
"$schema": "http://apicast.io/policy-v1/schema#manifest#",
"name": "SOAP policy",
"description":
["This policy adds support for a very small subset of SOAP. \n",
"It expects a SOAP action URI in the SOAPAction header or the Content-Type ",
"header. The SOAPAction header is used in v1.1 of the SOAP standard: ",
"https://www.w3.org/TR/2000/NOTE-SOAP-20000508/#_Toc478383528 , whereas ",
"the Content-Type header is used in v1.2 of the SOAP standard: ",
"https://www.w3.org/TR/soap12-part2/#ActionFeature \n",
"The SOAPAction URI is matched against the mapping rules defined in the ",
"policy and calculates a usage based on that so it can be authorized and ",
"reported against 3scale's backend."],
"version": "0.1",
"configuration": {
"type": "object",
"properties": {
"mapping_rules": {
"description": "Mapping rules.",
"type": "array",
"items": {
"type": "object",
"properties": {
"pattern": {
"description": "Pattern to match against the request.",
"type": "string"
},
"metric_system_name": {
"description": "Metric.",
"type": "string"
},
"delta": {
"description": "Value.",
"type": "integer"
}
},
"required": [
"pattern",
"metric_system_name",
"delta"
]
}
}
}
}
}
1 change: 1 addition & 0 deletions gateway/src/apicast/policy/soap/init.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
return require('soap')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I started to think about this. And I think we can expose some support tooling in different module, so it can be unit tested.

Lets say the main policy code is in soap_policy.lua. Then there can be soap.lua that has stuff like "extracting the soap action" and it can be unit tested in busted.

That would allow us not exposing extra methods on the policy, but still exposing it internally (if the loading works) for tests. And policies should not be able to load other policies (but that is not enforced yet), so we should be fine and the code should be used only from tests.

Just some food for though. I'd like to hear your take.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be a nice improvement.

136 changes: 136 additions & 0 deletions gateway/src/apicast/policy/soap/soap.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
--- SOAP Policy
-- This policy adds support for a very small subset of SOAP.
-- This policy basically expects a SOAPAction URI in the SOAPAction header or
-- the content-type header.
-- The SOAPAction header is used in v1.1 of the SOAP standard:
-- https://www.w3.org/TR/2000/NOTE-SOAP-20000508/#_Toc478383528, whereas the
-- Content-Type header is used in v1.2 of the SOAP standard:
-- https://www.w3.org/TR/soap12-part2/#ActionFeature
-- The SOAPAction URI is matched against the mapping rules defined in the
-- policy and calculates a usage based on that so it can be authorized and
-- reported against 3scale's backend.

local sub = string.sub
local len = string.len
local lower = string.lower
local ipairs = ipairs
local insert = table.insert

local MappingRule = require('apicast.mapping_rule')
local Usage = require('apicast.usage')
local mapping_rules_matcher = require('apicast.mapping_rules_matcher')

local policy = require('apicast.policy')

local _M = policy.new('SOAP policy')

local soap_action_header = 'SOAPAction'
local soap_action_ctype = 'application/soap+xml;'

local new = _M.new

local function starts_with(str, start)
return sub(str, 1, len(start)) == start
end

-- Extracts a SOAP action from the SOAPAction header. Returns nil when not
-- present.
local function soap_action_in_header(headers)
return headers[soap_action_header]
end

local regex_del_leading_spaces = [[^\s*]]
local regex_del_spaces_around_semicolon = [[\s*;\s*]]

-- There can be spaces in the Content-Type, values can be wrapped with '"',
-- etc.
-- See: https://tools.ietf.org/html/rfc7231#section-3.1.1.1
local regex_action_from_ctype = [[action=(?:"(.+)"|([^;"]+))\s*(?:;|$)]]

-- Extracts the SOAP action from a string that contains the parameters of a
-- Content-Type header. The string has this format:
-- a_param=x;action=soap_action;another_param=y
-- This method returns the value of 'action' or nil when it's not present.
local function soap_action_from_ctype_params(ctype_params)
local params = ngx.re.sub(
ctype_params, regex_del_leading_spaces, '', 'oj')

local params_without_blanks = ngx.re.gsub(
params, regex_del_spaces_around_semicolon, ';', 'oj')

local matches = ngx.re.match(
lower(params_without_blanks), regex_action_from_ctype, 'oj')

if not matches then return nil end
return matches[1] or matches[2] -- There are 2 paranthesized captures
end

-- Extracts a SOAP action from the Content-Type header. In SOAP, the
-- type/subtype is application/soap+xml, and the action is specified as a
-- param in that header. When there is no SOAP action, this method returns nil.
local function soap_action_in_ctype(headers)
local ctype = headers['Content-Type']

-- The Content-Type can be a mix of upper and lower-case chars. Convert it to
-- include only lower-case chars to be able to compare it.
if ctype and starts_with(lower(ctype), soap_action_ctype) then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would not work for application/xml+soap ; action="" because of the whitespace.

local header_params = sub(ctype, len(soap_action_ctype) + 1, -1)
return soap_action_from_ctype_params(header_params)
else
return nil
end
end

-- Extracts a SOAP action URI from the SOAP Action and the Content-Type
-- headers. When both contain a SOAP action, the Content-Type one takes
-- precedence.
local function extract_soap_uri()
local headers = ngx.req.get_headers() or {}
return soap_action_in_ctype(headers) or soap_action_in_header(headers)
end

local function usage_from_matching_rules(soap_action_uri, rules)
return mapping_rules_matcher.get_usage_from_matches(
nil, soap_action_uri, {}, rules)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikz Notice that I'm sending http_method = nil here. Not sure if we should take it into account for SOAP actions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. SOAP can be used with GET and POST.
Aha! The GET does not have the Content-Type header. So technically it should be only POST. I guess.

But it looks like the GET is used as some REST hybrid, so I guess we could leave that to the mapping rules.
I'd possibly even go for hardcording POST there.

Copy link
Contributor Author

@davidor davidor Feb 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know about this. The Content-Type is included in the request, not in the SOAP action URI.
Depending on what we decide here we'll need to add it to the JSON schema also. Notice that for now, I only included 'pattern', 'metric', and 'delta', and left out other fields that are present in the proxy rules.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. We are initializing the mapping rules directly from the config. I was thinking that we could just override the http_method attribute with POST there. And then pass the real http method here. So we match it only when the request is POST.
But it is probably not really important.

end

local function mapping_rules_from_config(config)
if not (config and config.mapping_rules) then return {} end

local res = {}

for _, config_rule in ipairs(config.mapping_rules) do
local rule = MappingRule.from_proxy_rule(config_rule)
insert(res, rule)
end

return res
end

--- Initialize a SOAP policy
-- @tparam[opt] table config Configuration
function _M.new(config)
local self = new(config)
self.mapping_rules = mapping_rules_from_config(config)
return self
end

--- Rewrite phase
-- When a SOAP Action is received via the SOAPAction or the Content-Type
-- headers, the policy matches it against the mapping rules defined in the
-- configuration of the policy and calculates the associated usage.
-- This usage is merged with the one received in the shared context.
-- @tparam table context Shared context between policies
function _M:rewrite(context)
local soap_action_uri = extract_soap_uri()

if soap_action_uri then
local soap_usage = usage_from_matching_rules(
soap_action_uri, self.mapping_rules)

context.usage = context.usage or Usage.new()
context.usage:merge(soap_usage)
end
end

return _M
4 changes: 2 additions & 2 deletions gateway/src/apicast/proxy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -296,11 +296,11 @@ function _M:rewrite(service, context)
context.usage = context.usage or Usage.new()
context.usage:merge(usage)

ctx.usage = usage
ctx.usage = context.usage
ctx.credentials = credentials

self.credentials = credentials
self.usage = usage
self.usage = context.usage

var.cached_key = concat(cached_key, ':')

Expand Down
128 changes: 128 additions & 0 deletions spec/policy/soap/policy_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
local Usage = require('apicast.usage')

describe('policy', function()
describe('.rewrite', function()
local context -- Context shared between policies

-- Define a config with 2 rules. One increases hits by 10 and the other by
-- 20. Their patterns have values that allow us to easily associate them
-- with a SOAP action receive via SOAPAction header or via Content-Type.
local policy_config = {
mapping_rules = {
{
pattern = '/soap_action$',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the RFC says these should be full URL we should test also with full URLs. To verify all the escaping works.

metric_system_name = 'hits',
delta = 10
},
{
pattern = '/soap_action_ctype$',
metric_system_name = 'hits',
delta = 20
}
}
}

local soap_policy = require('apicast.policy.soap').new(policy_config)

before_each(function()
-- Initialize a shared context with a usage of hits = 1.
context = { usage = Usage.new() }
context.usage:add('hits', 1)
end)

describe('when the SOAP action is in the SOAPAction header', function()
it('calculates the usage and merges it with the one in the context', function()
ngx.req.get_headers = function()
return { SOAPAction = '/soap_action' }
end

soap_policy:rewrite(context)

assert.equals(11, context.usage.deltas['hits'])
end)
end)

describe('when the SOAP action is in the Content-Type header', function()
describe('and it is the only param', function()
it('calculates the usage and merges it with the one in the context', function()
local header_val = "application/soap+xml;action=/soap_action_ctype"

ngx.req.get_headers = function()
return { ["Content-Type"] = header_val }
end

soap_policy:rewrite(context)

assert.equals(21, context.usage.deltas['hits'])
end)
end)

describe('and there are other params', function()
it('calculates the usage and merges it with the one in the context', function()
local header_val = "application/soap+xml;a_param=x;" ..
"action=/soap_action_ctype;another_param=y"

ngx.req.get_headers = function()
return { ["Content-Type"] = header_val }
end

soap_policy:rewrite(context)

assert.equals(21, context.usage.deltas['hits'])
end)
end)

describe('and the params contain some upper-case chars or spaces', function()
it('calculates the usage and merges it with the one in the context', function()
local header_vals = {
-- Upper-case chars in type/subtype
"Application/SOAP+xml;action=/soap_action_ctype",
-- Upper-case chars in 'Action'
"application/soap+xml;Action=/soap_action_ctype",
-- "" in action value
'application/soap+xml;action="/soap_action_ctype"',
-- Spaces
"application/soap+xml; action=/soap_action_ctype; a_param=x"
}

for _, header_val in ipairs(header_vals) do
ngx.req.get_headers = function()
return { ["Content-Type"] = header_val }
end

context = { usage = Usage.new() }
context.usage:add('hits', 1)
soap_policy:rewrite(context)

assert.equals(21, context.usage.deltas['hits'])
end
end)
end)
end)

describe('when the SOAP action is in the SOAPAction and the Content-Type headers', function()
it('calculates the usage and merges it with the one in the context', function()
ngx.req.get_headers = function()
return {
SOAPAction = '/soap_action',
["Content-Type"] = "application/soap+xml;action=/soap_action_ctype"
}
end

soap_policy:rewrite(context)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. This is so good 🥇 Great we can unit test this.


assert.equals(21, context.usage.deltas['hits'])
end)
end)

describe('when the SOAP action is not specified', function()
it('it does not modify the usage received in the context', function()
ngx.req.get_headers = function() return {} end

soap_policy:rewrite(context)

assert.equals(1, context.usage.deltas['hits'])
end)
end)
end)
end)
Loading