Commit 85f8b9e

mo <mo@mokhan.ca>
2018-12-16 02:51:52
do not update user session key for every request.
1 parent 1300509
app/controllers/mfas_controller.rb
@@ -7,6 +7,8 @@ class MfasController < ApplicationController
 
   def create
     if current_user.mfa.authenticate(secure_params[:code])
+      reset_session
+      session[:user_session_key] = Current.user_session.key
       session[:mfa] = { issued_at: Time.now.utc.to_i }
       redirect_to response_path
     else
app/controllers/sessions_controller.rb
@@ -11,6 +11,7 @@ class SessionsController < ApplicationController
   ].freeze
   skip_before_action :verify_authenticity_token, only: [:new, :destroy]
   skip_before_action :authenticate!, only: [:new, :show, :create, :destroy]
+  skip_before_action :authenticate_mfa!, only: [:show]
 
   def new
     binding = binding_for(
app/models/current.rb
@@ -19,6 +19,6 @@ class Current < ActiveSupport::CurrentAttributes
     self.request = request
     self.user_session = UserSession.authenticate(session[:user_session_key])
     self.user = user_session&.user
-    session[:user_session_key] = user_session&.access(request)
+    user_session&.access(request)
   end
 end
app/models/user_session.rb
@@ -2,10 +2,8 @@
 
 class UserSession < ApplicationRecord
   audited associated_with: :user, except: [:key, :accessed_at]
+  has_secure_token :key
   belongs_to :user
-  before_validation do |model|
-    model.key = SecureRandom.urlsafe_base64(32)
-  end
 
   scope :active, -> { where.not(id: revoked).where.not(id: expired) }
   scope :revoked, -> { where.not(revoked_at: nil) }
app/views/my/user_sessions/_user_session.html.erb
@@ -2,14 +2,14 @@
   <article class="media">
     <div class="media-left">
       <figure class="image is-64x64">
-        <img src="https://cdnjs.cloudflare.com/ajax/libs/browser-logos/46.1.0/<%= user_session.browser.name.downcase %>/<%= user_session.browser.name.downcase %>_128x128.png" alt="">
+        <img src="https://cdnjs.cloudflare.com/ajax/libs/browser-logos/46.1.0/<%= user_session.browser.name.downcase %>/<%= user_session.browser.name.downcase %>_128x128.png" alt="<%= user_session.browser.name %>" />
       </figure>
     </div>
     <div class="media-content">
       <div class="content">
         <p>
         <strong><%= user_session.ip %></strong> <small><%= user_session.browser.name %></small> <small><%= local_time_ago(user_session.accessed_at) %></small>
-        <br>
+        <br />
         <%= user_session.user_agent %>
         </p>
       </div>
config/locales/en.yml
@@ -73,10 +73,7 @@ en:
       destroy:
         success: Success
       index:
-        ip_address: Ip address
-        last_used: Last used
         title: Sessions
-        user_agent: User agent
   oauth:
     authorizations:
       show:
spec/helpers/application_helper_spec.rb
@@ -24,7 +24,7 @@ RSpec.describe ApplicationHelper do
       end
 
       context "when the item is an instance of ActiveModel::Errors" do
-        class User
+        class TestUser
           extend ActiveModel::Naming
           attr_reader :email, :password
 
@@ -40,7 +40,7 @@ RSpec.describe ApplicationHelper do
             [self]
           end
         end
-        let(:user) { User.new }
+        let(:user) { TestUser.new }
         let(:errors) do
           errors = ActiveModel::Errors.new(user)
           errors.add(:email, 'has already been taken.')
spec/models/user_session_spec.rb
@@ -14,7 +14,6 @@ RSpec.describe UserSession do
   describe "#access" do
     subject { create(:user_session) }
 
-    let!(:original_key) { subject.key }
     let(:request) { instance_double(ActionDispatch::Request, ip: "192.168.1.1", user_agent: "blah") }
     let(:result) { subject.access(request) }
 
@@ -27,7 +26,6 @@ RSpec.describe UserSession do
     specify { expect(subject.ip).to eql(request.ip) }
     specify { expect(subject.user_agent).to eql(request.user_agent) }
     specify { expect(subject).to be_persisted }
-    specify { expect(subject.key).not_to eql(original_key) }
     specify { expect(result).to eql(subject.key) }
   end