Skip to content

Commit

Permalink
Merge pull request #14947 from imtayadeway/api/add-sql-store-to-token…
Browse files Browse the repository at this point in the history
…-store

Add SQL store option to token store
(cherry picked from commit ffd8972)

https://bugzilla.redhat.com/show_bug.cgi?id=1460348
  • Loading branch information
gtanzillo authored and simaishi committed Jun 9, 2017
1 parent 0ec4e76 commit 9503a9a
Show file tree
Hide file tree
Showing 8 changed files with 285 additions and 84 deletions.
4 changes: 4 additions & 0 deletions app/models/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ def self.purge(ttl, batch_size = 100)
def self.purge_one_batch(ttl, batch_size)
sessions = where("updated_at <= ?", ttl.seconds.ago.utc).limit(batch_size)
return 0 if sessions.size.zero?
sessions = sessions.reject do |s|
expires_on = Marshal.load(Base64.decode64(s.data.split("\n").join))[:expires_on] rescue nil
expires_on && expires_on >= Time.zone.now
end

log_off_user_sessions(sessions)
where(:id => sessions.collect(&:id)).destroy_all.size
Expand Down
24 changes: 22 additions & 2 deletions config/brakeman.ignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,25 @@
{
"ignored_warnings": [
{
"warning_type": "Remote Code Execution",
"warning_code": 25,
"fingerprint": "0051a4e9b0e19666f13befb5a9522f07d8de4d6f05ccc6f594f15bf1b9578dc6",
"check_name": "Deserialize",
"message": "Marshal.load called with model attribute",
"file": "lib/token_store/sql_store.rb",
"line": 16,
"link": "http://brakemanscanner.org/docs/warning_types/unsafe_deserialization",
"code": "Marshal.load(Base64.decode64(Session.find_by(:session_id => session_key(token)).data))",
"render_path": null,
"location": {
"type": "method",
"class": "TokenStore::SqlStore",
"method": "read"
},
"user_input": "Session.find_by(:session_id => session_key(token)).data",
"confidence": "Medium",
"note": ""
},
{
"warning_type": "File Access",
"warning_code": 16,
Expand Down Expand Up @@ -101,6 +121,6 @@
"note": "Temporarily skipped, found in new brakeman version"
}
],
"updated": "2017-02-01 08:54:32 -0500",
"brakeman_version": "3.5.0"
"updated": "2017-05-30 13:45:37 -0700",
"brakeman_version": "3.6.2"
}
2 changes: 2 additions & 0 deletions config/settings/test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
:server:
:session_store: memory
14 changes: 7 additions & 7 deletions lib/token_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@ class TokenStore

def self.acquire(namespace, token_ttl)
@token_caches[namespace] ||= begin
if test_environment?
case ::Settings.server.session_store
when "sql"
SqlStore.new(cache_store_options(namespace, token_ttl))
when "memory"
require 'active_support/cache/memory_store'
ActiveSupport::Cache::MemoryStore.new(cache_store_options(namespace, token_ttl))
else
when "cache"
require 'active_support/cache/dalli_store'
memcache_server = ::Settings.session.memcache_server
ActiveSupport::Cache::DalliStore.new(memcache_server, cache_store_options(namespace, token_ttl))
else
raise "unsupported session store type: #{::Settings.server.session_store}"
end
end
end
Expand All @@ -22,9 +27,4 @@ def self.cache_store_options(namespace, token_ttl)
}
end
private_class_method :cache_store_options

def self.test_environment?
!Rails.env.development? && !Rails.env.production?
end
private_class_method :test_environment?
end
39 changes: 39 additions & 0 deletions lib/token_store/sql_store.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
class TokenStore
class SqlStore
def initialize(options)
@namespace = options.fetch(:namespace)
end

def write(token, data, _options = nil)
record = Session.find_or_create_by(:session_id => session_key(token))
record.data = Base64.encode64(Marshal.dump(data))
record.save!
end

def read(token, _options = nil)
record = Session.find_by(:session_id => session_key(token))
return nil unless record
data = Marshal.load(Base64.decode64(record.data))
if data[:expires_on] > Time.zone.now
data
else
record.destroy
nil
end
end

def delete(token)
record = Session.find_by(:session_id => session_key(token))
return nil unless record
record.destroy!
end

private

attr_reader :namespace

def session_key(token)
"#{namespace}:#{token}"
end
end
end
96 changes: 96 additions & 0 deletions spec/lib/token_store/sql_store_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
RSpec.describe TokenStore::SqlStore do
around { |example| Timecop.freeze { example.run } }

describe "#write" do
it "creates a session" do
store = build_sql_store
token = SecureRandom.hex
data = {
:expires_on => 1.hour.from_now,
:token_ttl => 1.hour,
:userid => "alice"
}

expect { store.write(token, data) }.to change(Session, :count).by(1)
end

it "updates a session if it exists" do
store = build_sql_store("TEST")
token = SecureRandom.hex
session = FactoryGirl.create(
:session,
:session_id => "TEST:#{token}",
:data => serialize_data(:expires_on => 1.second.from_now)
)

store.write(token, :expires_on => 1.hour.from_now)

expect(deserialize_data(session.reload.data)[:expires_on]).to eq(1.hour.from_now)
end
end

describe "#read" do
it "reads a valid token" do
store = build_sql_store("TEST")
token = SecureRandom.hex
FactoryGirl.create(
:session,
:session_id => "TEST:#{token}",
:data => serialize_data(:userid => "alice", :expires_on => 1.second.from_now)
)

data = store.read(token)

expect(data[:userid]).to eq("alice")
end

it "returns nil for an expired token" do
store = build_sql_store("TEST")
token = SecureRandom.hex
FactoryGirl.create(
:session,
:session_id => "TEST:#{token}",
:data => serialize_data(:userid => "alice", :expires_on => 1.second.ago)
)

data = store.read(token)

expect(data).to be_nil
end

it "returns nil if no token can be found" do
store = build_sql_store("TEST")
token = SecureRandom.hex

data = store.read(token)

expect(data).to be_nil
end
end

describe "#delete" do
it "deletes a token" do
store = build_sql_store("TEST")
token = SecureRandom.hex
FactoryGirl.create(
:session,
:session_id => "TEST:#{token}",
:data => serialize_data(:userid => "alice", :expires_on => 1.hour.from_now)
)

expect { store.delete(token) }.to change(Session, :count).by(-1)
end
end

def serialize_data(data)
Base64.encode64(Marshal.dump(data))
end

def deserialize_data(data)
Marshal.load(Base64.decode64(data))
end

def build_sql_store(namespace = "FOO")
described_class.new(:namespace => namespace)
end
end
34 changes: 34 additions & 0 deletions spec/models/session_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,39 @@

expect(described_class.count).to eq 0
end

context "given some token store data" do
around { |example| Timecop.freeze { example.run } }

it "will purge an expired token" do
FactoryGirl.create(
:session,
:data => serialize_data(
:expires_on => 1.second.ago,
)
)

described_class.purge(0)

expect(described_class.count).to eq(0)
end

it "won't purge an unexpired token" do
FactoryGirl.create(
:session,
:data => serialize_data(
:expires_on => 1.second.from_now,
)
)

described_class.purge(0)

expect(described_class.count).to eq(1)
end

def serialize_data(data)
Base64.encode64(Marshal.dump(data))
end
end
end
end
Loading

0 comments on commit 9503a9a

Please sign in to comment.