FIX: store last_refresh and aal in database per session

mozilla/discourse#188 (comment)

app/models/mozilla_iam/session_data.rb

@@ -0,0 +1,24 @@
+module MozillaIAM
+  class SessionData < ActiveRecord::Base
+
+    TOKEN_COOKIE = Auth::DefaultCurrentUserProvider::TOKEN_COOKIE
+
+    def self.find_or_create(session, cookies)
+      auth_token = cookies[TOKEN_COOKIE]
+      user_token = UserAuthToken.lookup(auth_token)
+
+      if session[:mozilla_iam]
+        session_data = create!(
+          user_auth_token_id: user_token.id,
+          last_refresh: session[:mozilla_iam][:last_refresh],
+          aal: session[:mozilla_iam][:aal]
+        )
+
+        session.delete(:mozilla_iam)
+        session_data
+      else
+        user_token.mozilla_iam_session_data
+      end
+    end
+  end
+end

config/initializers/patch_user_auth_token.rb

@@ -0,0 +1,5 @@
+UserAuthToken.class_eval do
+  has_one :mozilla_iam_session_data,
+    class_name: "MozillaIAM::SessionData",
+    dependent: :destroy
+end

db/migrate/20190416121717_create_session_data.rb

@@ -0,0 +1,9 @@
+class CreateSessionData < ActiveRecord::Migration[5.0]
+  def change
+    create_table :mozilla_iam_session_data do |t|
+      t.integer :user_auth_token_id, null: false
+      t.datetime :last_refresh
+      t.string :aal
+    end
+  end
+end

lib/mozilla_iam/application_extensions.rb

@@ -5,23 +5,9 @@ def check_iam_session
         return unless current_user
         return if current_user.id < 0
 
-        last_refresh = session[:mozilla_iam].try(:[], :last_refresh)
-        no_refresh = session[:mozilla_iam].try(:[], :no_refresh)
-
-        return if no_refresh && !last_refresh
-
-        unless last_refresh
-          current_user.clear_custom_fields
-          last_refresh = Profile.for(current_user)&.last_refresh
-          session[:mozilla_iam] = {} if session[:mozilla_iam].nil?
-          if last_refresh
-            session[:mozilla_iam][:last_refresh] = last_refresh
-          else
-            session[:mozilla_iam][:no_refresh] = true
-            return
-          end
-        end
+        mozilla_session_data = SessionData.find_or_create(session, request.cookies)
 
+        last_refresh = mozilla_session_data.last_refresh
         logout_delay =
           Rails.cache.fetch('mozilla-iam/logout_delay') do
             ::PluginStore.get('mozilla-iam', 'logout_delay')
@@ -33,8 +19,8 @@ def check_iam_session
             user_id: #{current_user.id}, last_refresh: #{last_refresh}, logout_delay: #{logout_delay}
           EOF
         else
-          refresh_iam_session
-          aal = session[:mozilla_iam].try(:[], :aal)
+          mozilla_session_data.update!(last_refresh: Profile.refresh(current_user))
+          aal = mozilla_session_data.aal
           unless Profile.for(current_user).is_aal_enough?(aal)
             raise <<~EOF
               Mozilla IAM: AAL not enough, user logged out
@@ -49,9 +35,5 @@ def check_iam_session
         log_off_user
       end
     end
-
-    def refresh_iam_session
-      session[:mozilla_iam][:last_refresh] = Profile.refresh(current_user)
-    end
   end
 end

plugin.rb

@@ -1,6 +1,6 @@
 # name: mozilla-iam
 # about: A plugin to integrate Discourse with Mozilla's Identity and Access Management (IAM) system
-# version: 1.1.5
+# version: 1.1.6
 # authors: Leo McArdle
 # url: https://github.com/mozilla/discourse-mozilla-iam
 

spec/controllers/application_controller_spec.rb

@@ -14,7 +14,8 @@
 
       get :show, params: { id: 666 }, format: :json
 
-      expect(session[:mozilla_iam][:last_refresh]).to be_within(5.seconds).of last_refresh
+      session_data = MozillaIAM::SessionData.find_or_create({}, request.cookies)
+      expect(session_data.last_refresh).to be_within(5.seconds).of last_refresh
     end
 
     it 'refreshes the session after 15 minutes' do
@@ -28,7 +29,9 @@
       session[:mozilla_iam] = { last_refresh: last_refresh }
 
       get :show, params: { id: 666 }, format: :json
-      expect(session[:mozilla_iam][:last_refresh]).to be_within(5.seconds).of Time.now
+
+      session_data = MozillaIAM::SessionData.find_or_create({}, request.cookies)
+      expect(session_data.last_refresh).to be_within(5.seconds).of Time.now
     end
 
     it 'logs off the user after 7 days' do
@@ -58,94 +61,15 @@
       expect(session['current_user_id']).to be_nil
     end
 
-    context "with no session[:mozilla_iam] set" do
-      let(:user) { Fabricate(:user) }
-      before do
-        authenticate_user(user)
-        log_in_user(user)
-        session[:mozilla_iam] = nil
-      end
-
-      context "and with a user with a last refresh" do
-        it "fetches last refresh from user profile" do
-          last_refresh = Time.now - 5.minutes
-          user.custom_fields['mozilla_iam_last_refresh'] = last_refresh
-          user.save_custom_fields
-
-          get :show, params: { id: 666 }, format: :json
-
-          expect(session[:mozilla_iam][:last_refresh]).to be_within(2.seconds).of last_refresh
-        end
-      end
-
-      context "and with a user with no last refresh" do
-        it "sets session[:mozilla_iam][:no_refresh] to true" do
-          user.custom_fields['mozilla_iam_last_refresh'] = nil
-          user.save_custom_fields
-
-          get :show, params: { id: 666 }, format: :json
-
-          expect(session[:mozilla_iam][:no_refresh]).to eq true
-        end
-      end
-
-      context "and with a user with no profile" do
-        it "sets session[:mozilla_iam][:no_refresh] to true" do
-          user.custom_fields['mozilla_iam_uid'] = nil
-          user.save_custom_fields
-
-          get :show, params: { id: 666 }, format: :json
-
-          expect(session[:mozilla_iam][:no_refresh]).to eq true
-        end
-      end
-
-      context "and when MEDIUM or above AAL required" do
-        it "kills session" do
-          MozillaIAM::Profile.any_instance.expects(:is_aal_enough?).with(nil).returns(true)
-
-          get :show, params: { id: 666 }, format: :json
-          expect(session['current_user_id']).to be
-
-          MozillaIAM::Profile.any_instance.expects(:is_aal_enough?).with(nil).returns(false)
-
-          get :show, params: { id: 666 }, format: :json
-          expect(session['current_user_id']).to be_nil
-        end
-      end
-    end
-
-    context "with session[:mozilla_iam][:no_refresh] set to true" do
-      let(:user) { Fabricate(:user) }
-      before do
+    context "with no last_refresh" do
+      it "kills session" do
+        user = Fabricate(:user)
         authenticate_user(user)
         log_in_user(user)
-        session[:mozilla_iam] = { no_refresh: true }
-      end
-
-      it "doesn't query user profile" do
-        MozillaIAM::Profile.expects(:for).never
+        session[:mozilla_iam] = {}
 
         get :show, params: { id: 666 }, format: :json
-      end
-
-      it "doesn't refresh user profile" do
-        MozillaIAM::Profile.expects(:refresh).never
-
-        get :show, params: { id: 666 }, format: :json
-      end
-
-      context "and with session[:mozilla_iam][:last_refresh] set" do
-        before do
-          last_refresh = Time.now - 5.minutes
-          session[:mozilla_iam][:last_refresh] = last_refresh
-        end
-
-        it "refreshes user profile" do
-          MozillaIAM::Profile.expects(:refresh).once
-
-          get :show, params: { id: 666 }, format: :json
-        end
+        expect(session['current_user_id']).to be_nil
       end
     end
 
@@ -154,7 +78,7 @@
         user = Fabricate(:user)
         authenticate_user(user)
         log_in_user(user)
-        session[:mozilla_iam] = { aal: "LOW" }
+        session[:mozilla_iam] = { last_refresh: Time.now, aal: "LOW" }
 
         MozillaIAM::Profile.any_instance.expects(:is_aal_enough?).with("LOW").returns(true)
 

spec/models/session_data_spec.rb

@@ -0,0 +1,49 @@
+require_relative '../iam_helper'
+
+describe MozillaIAM::SessionData do
+  it "is destroyed when associated auth token is destroyed" do
+
+  end
+
+  describe ".find_or_create" do
+    let!(:last_refresh) { Time.now.round }
+    let(:aal) { "MEDIUM" }
+    let(:cookies) { { described_class::TOKEN_COOKIE => "12345" } }
+    let(:user_auth_token) { UserAuthToken.generate!(user_id: Fabricate(:user).id) }
+    before { UserAuthToken.expects(:lookup).with("12345").returns(user_auth_token) }
+
+    context "with session[:mozilla_iam]" do
+      it "should create SessionData record and remove session[:mozilla_iam]" do
+        session = {
+          mozilla_iam: {
+            last_refresh: last_refresh,
+            aal: aal
+          }
+        }
+
+        session_data = described_class.find_or_create(session, cookies)
+
+        expect(user_auth_token.mozilla_iam_session_data).to eq session_data
+        expect(session_data.last_refresh).to eq last_refresh
+        expect(session_data.aal).to eq aal
+        expect(session[:mozilla_iam]).to be_nil
+      end
+    end
+
+    context "without session[:mozilla_iam]" do
+      it "should find SessionData record" do
+        described_class.create!(
+          user_auth_token_id: user_auth_token.id,
+          last_refresh: last_refresh,
+          aal: aal
+        )
+
+        session_data = described_class.find_or_create({}, cookies)
+
+        expect(user_auth_token.mozilla_iam_session_data).to eq session_data
+        expect(session_data.last_refresh).to eq last_refresh
+        expect(session_data.aal).to eq aal
+      end
+    end
+  end
+end