Skip to content

Commit aad96fe

Browse files
committed
Implement auth token cache
This adds the necessary logic to deal with caching auth tokens. As this is intended to be a Rails plugin we are leveraging the `Rails.cache` instead of injecting it. We already directly utilize the Rails cache in other parts of the code so this is consistent. We store the key as the auth token and the value as a hash containing the authenticated user and team ids. This will allow clients to by-pass the expensive network call while still being able to resolve the critical information about user and team related ids. This could also potentially save on a potential DB lookup if the endpoints using this no longer need to perform the full user and/or team lookups and instance creations. However, we still provide the common `current_user` helper as it is likely many existing endpoints still rely on it. But since we are mainly caching ids this helper is lazy loaded for those endpoints which do not require the user lookup and instantiation. Since we are now caching the team ids as well, we expose an additional helper to access them. For symmetry, we expose a helper for the user id. This is also the first step in supporting team level tokens. With this hash format, such a token would simply have a `nil` user id and a single team id (still in an array). Lastly, we need to be aware that different caching strategies may treat hashes differently. Mainly, an in-memory cache has some odd quirks. The in-memory cache returns **the same hash object** (the object ids match) when you read it. However, if the hash was frozen before being cached, when it is read it will **not** be frozen. And since reading the cache gives a new pointer to the same object (just with different frozen state) it can be mutated. Resulting in additional reads seeing the modified state. To prevent this, and encourage clients to be defensive, we actively re-freeze the cache when we read it. This also must include freezing the internal team id array. On a cache miss, this means we also need to be sure we freeze it before setting our internal state. In the interest of reconfigurability, without needing to make code deploys, we read the cache lifetime from the environment. Since these are meant to be API auth tokens, which tend to have relatively long lifetimes, but are still sensitive enough to that we may need to kill them relatively quickly, we default to a shorter lifetime: 1 minutes. This was arbitrarily chosen and not based on any usage patterns. Since this gem may be used on a variety of different servers (i.e. threaded and multi-process) we need to be aware of potential dog pile effects. To try to allow for faster network requests to handle the cache we keep the `race_condition_ttl` to the minimum: 1 second. From what we've seen in the vast majority of cases this will be more than enough time for any request to return. We are not caching invalid tokens. The problem here is malicious attack vectors. The API endpoints are vulnerable to a both brute force and DOS attack by sending lots of requests with invalid tokens. To be fair a valid token could also result in a DOS - but we generally expect large traffic volumes with valid tokens. So why prevent invalid tokens from being cached? The token auth can be likened to login operations. It is generally wise to slow this process down as much as possible. This way malicious requests take longer to process. This makes brute force attacks prohibitive. It also may (and I heavily stress the maybe) slow down DOS vectors - but it's very likely attackers run more than one process / thread and this slow down won't hinder them, only make the DOS worse by greatly increasing the request times. Still, security wisdom states login/auth processes should be as slow as your system can reasonably allow. If we allowed the invalid tokens to be cached, a DOS with the same token would get processed a lot faster. This may allow more valid requests through, but this is unlikely to be a significant portion of the traffic. Caching the invalid tokens would have no affect on the time for a brute force attempt, as by design, a BF attack will change the token on every request. However, a BF attack would destroy any viable valid cache. Since it's extremely likely the invalid tokens would consume all of the available cache space. This means valid cache hits will drop to roughly zero. Additionally, once the cache is maxed out, every request would result in a pruning/clean up. But since it's likely that all the invalid cached tokens will still have long expire times we resort to the alternative algorithm. So by not caching the invalid tokens we greatly reduce our attack surface. Now we'd simply have to deal with the standard DOS attack responses which for us would mostly be handled by Heroku and dealt with at the router level - not the app level. Finally, we've dropped the `current_user` check. This is mainly meant to be a shim for the test plugin to override who the currently authenticated user/token is. We will replace this logic with simply setting the internal ivar state/cache value appropriately.
1 parent a42963a commit aad96fe

File tree

3 files changed

+195
-33
lines changed

3 files changed

+195
-33
lines changed

lib/kracken/controllers/token_authenticatable.rb

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ def self.included(base)
1313
end
1414
end
1515

16-
attr_reader :current_user
17-
1816
# NOTE: Monkey-patch until this is merged into the gem
1917
def request_http_token_authentication(realm = 'Application')
2018
headers["WWW-Authenticate"] = %(Token realm="#{realm.gsub(/"/, "")}")
@@ -23,18 +21,47 @@ def request_http_token_authentication(realm = 'Application')
2321

2422
private
2523

24+
CACHE_TTL_OPTS = {
25+
expires_in: ENV.fetch("KRACKEN_TOKEN_TTL", 1.minute).to_i,
26+
race_condition_ttl: 1.second,
27+
}.freeze
28+
2629
# `authenticate_or_request_with_http_token` is a nice Rails helper:
2730
# http://api.rubyonrails.org/classes/ActionController/HttpAuthentication/Token/ControllerMethods.html#method-i-authenticate_or_request_with_http_token
2831
def authenticate_user_with_token!
29-
unless current_user
30-
munge_header_auth_token!
32+
munge_header_auth_token!
3133

32-
authenticate_or_request_with_http_token(realm) { |token, _options|
33-
# Attempt to reduce namespace conflicts with controllers which may access
34-
# an team instance for display.
35-
@current_user = Authenticator.user_with_token(token)
34+
authenticate_or_request_with_http_token(realm) { |token, _options|
35+
# Attempt to reduce ivar namespace conflicts with controllers
36+
@_auth_info = cache_valid_auth(token) {
37+
if @current_user = Authenticator.user_with_token(token)
38+
{ id: @current_user.id, team_ids: @current_user.team_ids }
39+
end
3640
}
37-
end
41+
}
42+
end
43+
44+
def cache_valid_auth(token, &generate_cache)
45+
cache_key = "auth/token/#{token}"
46+
val = Rails.cache.read(cache_key)
47+
val ||= store_valid_auth(cache_key, &generate_cache)
48+
val.transform_values!(&:freeze).freeze if val
49+
end
50+
51+
def current_auth_info
52+
@_auth_info ||= {}
53+
end
54+
55+
def current_team_ids
56+
current_auth_info[:team_ids]
57+
end
58+
59+
def current_user
60+
@current_user ||= Kracken.config.user_class.find(current_auth_info[:id])
61+
end
62+
63+
def current_user_id
64+
current_auth_info[:id]
3865
end
3966

4067
# Make it **explicit** that we are munging the `token` param with the
@@ -53,6 +80,12 @@ def munge_header_auth_token!
5380
def realm
5481
self.class.realm
5582
end
83+
84+
def store_valid_auth(cache_key)
85+
val = yield
86+
Rails.cache.write(cache_key, val, CACHE_TTL_OPTS) if val
87+
val
88+
end
5689
end
5790

5891
end

spec/dummy/app/models/user.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,10 @@ def id
2222
def uid
2323
@hash["uid"]
2424
end
25+
26+
def team_ids
27+
@hash["info"] ||= {}
28+
@hash["info"]["teams"] ||= []
29+
@hash["info"]["teams"].map { |t| t["uid"] }
30+
end
2531
end

spec/kracken/controllers/token_authenticatable_spec.rb

Lines changed: 147 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,21 @@ def authenticate_or_request_with_http_token(realm = nil)
2121

2222
RSpec.describe Controllers::TokenAuthenticatable do
2323
describe "authenticating via a token", :using_cache do
24+
subject(:a_controller) { TokenAuthController.new }
25+
2426
shared_examples "the authorization request headers" do |token_helper|
2527
let(:expected_token) { public_send token_helper }
2628

2729
specify "are munged to include a provided parameterized token" do
28-
controller = TokenAuthController.new
29-
controller.request.env = {
30+
a_controller.request.env = {
3031
'HTTP_AUTHORIZATION' => 'Token token="header token"'
3132
}
32-
controller.params = { token: expected_token }
33+
a_controller.params = { token: expected_token }
3334

3435
expect {
35-
controller.authenticate_user_with_token!
36+
a_controller.authenticate_user_with_token!
3637
}.to change {
37-
controller.request.env
38+
a_controller.request.env
3839
}.from(
3940
'HTTP_AUTHORIZATION' => 'Token token="header token"'
4041
).to(
@@ -43,48 +44,117 @@ def authenticate_or_request_with_http_token(realm = nil)
4344
end
4445

4546
specify "are not modified when no parameterized token provided" do
46-
controller = TokenAuthController.new
47-
controller.request.env = {
47+
a_controller.request.env = {
4848
'HTTP_AUTHORIZATION' => "Token token=\"#{expected_token}\""
4949
}
5050

5151
expect {
52-
controller.authenticate_user_with_token!
53-
}.not_to change { controller.request.env }.from(
52+
a_controller.authenticate_user_with_token!
53+
}.not_to change { a_controller.request.env }.from(
5454
'HTTP_AUTHORIZATION' => "Token token=\"#{expected_token}\""
5555
)
5656
end
5757
end
5858

5959
context "on a cache hit" do
60+
let(:auth_info) {
61+
{
62+
id: :any_id,
63+
team_ids: [:some, :team, :ids],
64+
}
65+
}
6066
let(:cached_token) { "any token" }
6167
let(:cache_key) { "auth/token/any token" }
6268

6369
before do
64-
Rails.cache.write(cache_key, "auth info")
70+
a_controller.request.env = {
71+
'HTTP_AUTHORIZATION' => "Token token=\"#{cached_token}\""
72+
}
73+
74+
Rails.cache.write cache_key, auth_info
75+
stub_const "Kracken::Authenticator", spy("Kracken::Authenticator")
6576
end
6677

6778
include_examples "the authorization request headers", :cached_token
6879

69-
it "uses the exising cache to bypass the authentication process"
70-
it "returns the auth info"
71-
it "exposes the auth info via the `current_` helpers"
72-
it "lazy loads the current user"
80+
it "uses the exising cache to bypass the authentication process" do
81+
a_controller.authenticate_user_with_token!
82+
expect(Authenticator).not_to have_received(:user_with_token)
83+
end
84+
85+
it "returns the auth info" do
86+
expect(a_controller.authenticate_user_with_token!).to eq(
87+
id: :any_id,
88+
team_ids: [:some, :team, :ids],
89+
).and be_frozen
90+
end
91+
92+
it "exposes the auth info via the `current_` helpers", :aggregate_failures do
93+
expect {
94+
a_controller.authenticate_user_with_token!
95+
}.to(
96+
change { a_controller.current_auth_info }.from({}).to(auth_info)
97+
.and change { a_controller.current_user_id }.from(nil).to(:any_id)
98+
.and change { a_controller.current_team_ids }.from(nil).to(
99+
[:some, :team, :ids]
100+
)
101+
)
102+
103+
expect(a_controller.current_auth_info).to be_frozen
104+
expect(a_controller.current_team_ids).to be_frozen
105+
end
106+
107+
it "lazy loads the current user" do
108+
begin
109+
# Ensure we cannot lookup a user - doing so would raise an error
110+
org_user_class = Kracken.config.user_class
111+
user_class = double("AnyUserClass")
112+
Kracken.config.user_class = user_class
113+
114+
# Action under test
115+
a_controller.authenticate_user_with_token!
116+
117+
# Make sure we perform the lookup as expected now
118+
expect(user_class).to receive(:find).with(:any_id).and_return(:user)
119+
120+
expect(a_controller.current_user).to be :user
121+
ensure
122+
Kracken.config.user_class = org_user_class
123+
end
124+
end
73125
end
74126

75127
context "on a cache miss with an invalid token" do
76128
let(:invalid_token) { "any token" }
77129

78130
before do
131+
a_controller.request.env = {
132+
'HTTP_AUTHORIZATION' => "Token token=\"#{invalid_token}\""
133+
}
134+
79135
allow(Authenticator).to receive(:user_with_token).with(invalid_token)
80136
.and_return(nil)
81137
end
82138

83139
include_examples "the authorization request headers", :invalid_token
84140

85-
it "follows the token authentication process"
86-
it "returns nil"
87-
it "doesn't cache invalid tokens"
141+
it "follows the token authentication process" do
142+
a_controller.authenticate_user_with_token!
143+
expect(Authenticator).to have_received(:user_with_token)
144+
.with(invalid_token)
145+
end
146+
147+
it "returns nil" do
148+
expect(a_controller.authenticate_user_with_token!).to be nil
149+
end
150+
151+
it "doesn't cache invalid tokens" do
152+
expect {
153+
a_controller.authenticate_user_with_token!
154+
}.not_to change {
155+
Rails.cache.exist?("auth/token/#{invalid_token}")
156+
}.from false
157+
end
88158
end
89159

90160
context "on a cache miss with a valid token" do
@@ -96,20 +166,73 @@ def authenticate_or_request_with_http_token(realm = nil)
96166
let(:valid_token) { "any token" }
97167

98168
before do
169+
a_controller.request.env = {
170+
'HTTP_AUTHORIZATION' => "Token token=\"#{valid_token}\""
171+
}
172+
99173
allow(Authenticator).to receive(:user_with_token).with(valid_token)
100174
.and_return(a_user)
101175
end
102176

103177
include_examples "the authorization request headers", :valid_token
104178

105-
it "follows the token authentication process"
106-
it "returns the auth info"
107-
it "exposes the auth info via the `current_` helpers"
108-
it "sets the auth info as the cache value"
109-
it "sets the cache expiration to one minute by default"
110-
it "sets the cache expiration to the environment setting `KRACKEN_TOKEN_TTL` when available"
111-
it "eager loads the current user"
179+
it "follows the token authentication process" do
180+
a_controller.authenticate_user_with_token!
181+
expect(Authenticator).to have_received(:user_with_token)
182+
.with(valid_token)
183+
end
184+
185+
it "returns the auth info in a frozen state" do
186+
expect(a_controller.authenticate_user_with_token!).to eq(
187+
id: :any_id,
188+
team_ids: [:some, :team, :ids],
189+
).and be_frozen
190+
end
191+
192+
it "exposes the auth info via the `current_` helpers", :aggregate_failures do
193+
expect {
194+
a_controller.authenticate_user_with_token!
195+
}.to(
196+
change { a_controller.current_auth_info }.from({}).to(
197+
id: :any_id,
198+
team_ids: [:some, :team, :ids],
199+
)
200+
.and change { a_controller.current_user_id }.from(nil).to(:any_id)
201+
.and change { a_controller.current_team_ids }.from(nil).to(
202+
[:some, :team, :ids]
203+
)
204+
)
205+
206+
expect(a_controller.current_auth_info).to be_frozen
207+
expect(a_controller.current_team_ids).to be_frozen
208+
end
209+
210+
it "sets the auth info as the cache value" do
211+
expect {
212+
a_controller.authenticate_user_with_token!
213+
}.to change { Rails.cache.read("auth/token/any token") }.from(nil).to(
214+
id: :any_id,
215+
team_ids: [:some, :team, :ids],
216+
)
217+
end
218+
219+
it "sets the cache expiration to one minute by default" do
220+
expect(Rails.cache).to receive(:write).with(
221+
"auth/token/any token",
222+
anything,
223+
include(expires_in: 1.minute),
224+
)
225+
a_controller.authenticate_user_with_token!
226+
end
227+
228+
it "eager loads the current user" do
229+
expect(Kracken.config.user_class).not_to receive(:find)
230+
a_controller.authenticate_user_with_token!
231+
expect(a_controller.current_user).to be a_user
232+
end
112233

234+
# TODO: Delete this test after implementation is complete
235+
# This is only to ensure we maintain general backwards compatibility
113236
it "authenticates the current user via the token" do
114237
controller = TokenAuthController.new
115238
controller.request.env = {

0 commit comments

Comments
 (0)