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

Add caching to the token introspection policy #656

Merged
merged 8 commits into from
Jun 4, 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/).
- 3scale referrer policy [PR #728](https://github.com/3scale/apicast/pull/728)
- Liquid templating support in the rate-limit policy [PR #719](https://github.com/3scale/apicast/pull/719)
- Default credentials policy [PR #741](https://github.com/3scale/apicast/pull/741), [THREESCALE-586](https://issues.jboss.org/browse/THREESCALE-586)
- Configurable caching for the token introspection policy [PR #656](https://github.com/3scale/apicast/pull/656)

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@
"client_secret": {
"description": "Client Secret for the Token Introspection Endpoint",
"type": "string"
},
"max_ttl_tokens": {
"description": "Max TTL for cached tokens",
"type": "integer",
"minimum": 1,
"maximum": 3600
},
"max_cached_tokens": {
"description": "Max number of tokens to cache",
"type": "integer",
"minimum": 0,
"maximum": 10000
Copy link
Contributor

@mikz mikz Jun 4, 2018

Choose a reason for hiding this comment

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

Probably should have "default" 0 ?

edit: maybe not needed when it is not required property

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. It already defaults to 0 in the code but should be here too.

Copy link
Contributor

Choose a reason for hiding this comment

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

@davidor probably not needed. Only if we want to populate it in the UI.

}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,15 @@ local http_ng = require 'resty.http_ng'
local user_agent = require 'apicast.user_agent'
local resty_env = require('resty.env')

local tokens_cache = require('tokens_cache')

local tonumber = tonumber

local new = _M.new

local noop = function() end
local noop_cache = { get = noop, set = noop }

function _M.new(config)
local self = new(config)
self.config = config or {}
Expand All @@ -26,12 +33,26 @@ function _M.new(config)
ssl = { verify = resty_env.enabled('OPENSSL_VERIFY') }
}
}

local max_cached_tokens = tonumber(config.max_cached_tokens) or 0
self.caching_enabled = max_cached_tokens > 0

if self.caching_enabled then
self.tokens_cache = tokens_cache.new(
config.max_ttl_tokens, config.max_cached_tokens)
else
self.tokens_cache = noop_cache
end

return self
end

--- OAuth 2.0 Token Introspection defined in RFC7662.
-- https://tools.ietf.org/html/rfc7662
local function introspect_token(self, token)
local cached_token_info = self.tokens_cache:get(token)
if cached_token_info then return cached_token_info end

--- Parameters for the token introspection endpoint.
-- https://tools.ietf.org/html/rfc7662#section-2.1
local res, err = self.http_client.post(self.introspection_url , { token = token, token_type_hint = 'access_token'})
Expand All @@ -43,6 +64,7 @@ local function introspect_token(self, token)
if res.status == 200 then
local token_info, decode_err = cjson.decode(res.body)
if type(token_info) == 'table' then
self.tokens_cache:set(token, token_info)
return token_info
else
ngx.log(ngx.ERR, 'failed to parse token introspection response:', decode_err)
Expand Down
43 changes: 43 additions & 0 deletions gateway/src/apicast/policy/token_introspection/tokens_cache.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
--- TokensCache
-- Tokens cache for the token introspection policy.

local setmetatable = setmetatable
local tonumber = tonumber

local lrucache = require('resty.lrucache')

local _M = { }

local mt = { __index = _M }

local function ttl_from_introspection(introspection_info)
local token_exp = introspection_info.exp
return token_exp and (tonumber(token_exp) - ngx.time())
end

function _M.new(max_ttl, max_cached_tokens)
local self = setmetatable({}, mt)
self.max_ttl = max_ttl
self.storage = lrucache.new(max_cached_tokens or 10000)
return self
end

function _M:get(token)
return self.storage:get(token)
end

function _M:set(token, introspection_info)
local ttl = ttl_from_introspection(introspection_info)

if self.max_ttl and (not ttl or self.max_ttl < ttl) then
ttl = self.max_ttl
end

-- If the config does not contain a max ttl and the token instrospection did
-- not return one, we cannot cache the token.
if ttl and ttl > 0 then
self.storage:set(token, introspection_info, ttl)
end
end

return _M
70 changes: 65 additions & 5 deletions spec/policy/token_introspection/token_introspection_spec.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
local TokenIntrospection = require('apicast.policy.token_introspection')
local TokensCache = require('apicast.policy.token_introspection.tokens_cache')

local test_backend_client = require('resty.http_ng.backend.test')
local cjson = require('cjson')
describe("token introspection policy", function()
Expand Down Expand Up @@ -50,7 +53,7 @@ describe("token introspection policy", function()
active = true
})
}
local token_policy = require('apicast.policy.token_introspection').new(policy_config)
local token_policy = TokenIntrospection.new(policy_config)
token_policy.http_client.backend = test_backend
token_policy:access(context)
end)
Expand Down Expand Up @@ -81,7 +84,7 @@ describe("token introspection policy", function()
stub(ngx, 'say')
stub(ngx, 'exit')

local token_policy = require('apicast.policy.token_introspection').new(policy_config)
local token_policy = TokenIntrospection.new(policy_config)
token_policy.http_client.backend = test_backend
token_policy:access(context)
assert_authentication_failed()
Expand Down Expand Up @@ -110,7 +113,7 @@ describe("token introspection policy", function()
stub(ngx, 'say')
stub(ngx, 'exit')

local token_policy = require('apicast.policy.token_introspection').new(policy_config)
local token_policy = TokenIntrospection.new(policy_config)
token_policy.http_client.backend = test_backend
token_policy:access(context)
assert_authentication_failed()
Expand Down Expand Up @@ -140,7 +143,7 @@ describe("token introspection policy", function()
stub(ngx, 'say')
stub(ngx, 'exit')

local token_policy = require('apicast.policy.token_introspection').new(policy_config)
local token_policy = TokenIntrospection.new(policy_config)
token_policy.http_client.backend = test_backend
token_policy:access(context)
assert_authentication_failed()
Expand Down Expand Up @@ -170,12 +173,69 @@ describe("token introspection policy", function()
stub(ngx, 'say')
stub(ngx, 'exit')

local token_policy = require('apicast.policy.token_introspection').new(policy_config)
local token_policy = TokenIntrospection.new(policy_config)
token_policy.http_client.backend = test_backend
token_policy:access(context)
assert_authentication_failed()
end)

describe('when caching is enabled', function()
local introspection_url = "http://example/token/introspection"
local policy_config = {
introspection_url = introspection_url,
client_id = test_client_id,
client_secret = test_client_secret,
max_ttl_tokens = 120,
max_cached_tokens = 10
}

local test_token_info = { active = true }
local test_tokens_cache

local token_policy = TokenIntrospection.new(policy_config)

describe('and the token is cached', function()
setup(function()
test_tokens_cache = TokensCache.new(60)
test_tokens_cache:set(test_access_token, test_token_info)
end)

it('does not call the introspection endpoint', function()
token_policy.tokens_cache = test_tokens_cache
token_policy.http_client.backend = { post = function () end }
local http_client_spy = spy.on(token_policy.http_client.backend, 'post')

token_policy:access(context)

assert.spy(http_client_spy).was_not_called()
end)
end)

describe('and the token is not cached', function()
setup(function()
test_tokens_cache = TokensCache.new(60)
end)

it('calls the introspection endpoint and caches the result', function()
test_backend
.expect{
url = introspection_url,
method = 'POST',
body = 'token='..test_access_token..'&token_type_hint=access_token',
headers = { ['Authorization'] = test_basic_auth }
}
.respond_with{ status = 200, body = cjson.encode(test_token_info) }

token_policy.tokens_cache = test_tokens_cache
token_policy.http_client.backend = test_backend

token_policy:access(context)

assert.same(test_token_info, test_tokens_cache:get(test_access_token))
end)
end)
end)

after_each(function()
test_backend.verify_no_outstanding_expectations()
end)
Expand Down
116 changes: 116 additions & 0 deletions spec/policy/token_introspection/tokens_cache_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
local lrucache = require('resty.lrucache')

local TokenCache = require('apicast.policy.token_introspection.tokens_cache')

local function assert_cache_spy_called_with_ttl(cache_spy, ttl)
-- We only care about the 4th arg (ttl) of the first set() call
assert.equals(ttl, cache_spy.calls[1].vals[4])
end

describe('token_cache', function()
local current_time = 1521054560
local test_token = 'a_token'

local max_ttl = 10
local ttl_longer_than_max = max_ttl + 1
local ttl_shorter_than_max = max_ttl - 1

-- Storage to inject in the cache so we can spy on it
local cache_storage
local cache_storage_spy

before_each(function()
stub(ngx, 'time').returns(current_time)

cache_storage = lrucache.new(10)
cache_storage_spy = spy.on(cache_storage, 'set')
end)

describe('when max TTL is set to 0', function()
it('does not cache anything', function()
local cache = TokenCache.new(0)
cache:set(test_token, { exp = current_time + 60 })
assert.is_falsy(cache:get(test_token))
end)
end)

describe('when the token info contains an "exp" field', function()
describe('and max TTL > "exp" TTL ', function()
it('caches the token with the TTL from "exp"', function()
local cache = TokenCache.new(max_ttl)
cache.storage = cache_storage
local introspection_info = {
active = true,
exp = current_time + ttl_shorter_than_max
}

cache:set(test_token, introspection_info)

assert.same(introspection_info, cache:get(test_token))
assert_cache_spy_called_with_ttl(
cache_storage_spy, ttl_shorter_than_max)
end)
end)

describe('and max TTL < "exp" TTL', function()
it('caches the token with the max TTL', function()
local cache = TokenCache.new(max_ttl)
cache.storage = cache_storage
local introspection_info = {
active = true,
exp = current_time + ttl_longer_than_max
}

cache:set(test_token, introspection_info)

assert.same(introspection_info, cache:get(test_token))
assert_cache_spy_called_with_ttl(cache_storage_spy, max_ttl)
end)
end)

describe('and max TTL is nil', function()
it('caches the token with the TTL from "exp"', function()
local cache = TokenCache.new()
cache.storage = cache_storage
local introspection_info = {
active = true,
exp = current_time + ttl_longer_than_max
}

cache:set(test_token, introspection_info)

assert.same(introspection_info, cache:get(test_token))
assert_cache_spy_called_with_ttl(
cache_storage_spy, ttl_longer_than_max)
end)
end)
end)

describe('when the token info does not contain an "exp" field', function()
describe('and there is a max TTL configured', function()
it('caches the token with the max TTL', function()
local cache = TokenCache.new(max_ttl)
cache.storage = cache_storage
local introspection_info = { active = true, }

cache:set(test_token, introspection_info)

assert.same(introspection_info, cache:get(test_token))
assert_cache_spy_called_with_ttl(cache_storage_spy, max_ttl)

end)
end)

describe('and there is not a max TTL configured', function()
it('does not cache the token', function()
local cache = TokenCache.new()
cache.storage = cache_storage
local introspection_info = { active = true, }

cache:set(test_token, introspection_info)

assert.is_falsy(cache:get(test_token))
end)
end)
end)
end)