Merge pull request #109 from kortirso/issue_106

IS-106 Users sessions expiration

CHANGELOG.md

@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## Unreleased
 ### Added
 - login through Gitlab
+- expiration time to auth tokens
+- removing expired users sessions
 
 ### Modified
 - default insight fields

app/controllers/users/omniauth_callbacks_controller.rb

@@ -3,7 +3,7 @@
 module Users
   class OmniauthCallbacksController < ApplicationController
     skip_before_action :verify_authenticity_token
-    skip_before_action :authenticate
+    skip_before_action :authenticate, only: %i[create]
     before_action :validate_provider, only: %i[create]
     before_action :validate_auth, only: %i[create]
 
@@ -22,7 +22,7 @@ def create
     end
 
     def destroy
-      # TODO: Here can be destroying token from database
+      Auth::FetchUserService.call(token: session[:pullmetry_token]).session&.destroy
       session[:pullmetry_token] = nil
       redirect_to root_path, notice: t('controllers.users.sessions.success_destroy')
     end

app/controllers/vacations_controller.rb

@@ -24,7 +24,8 @@ def destroy
   private
 
   def find_vacation
-    @vacation = current_user.vacations.find(params[:id])
+    @vacation = current_user.vacations.find_by(id: params[:id])
+    redirect_to profile_path, alert: 'Vacation is not found' if @vacation.nil?
   end
 
   def vacation_params

app/jobs/users/sessions/remove_expired_job.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module Users
+  module Sessions
+    class RemoveExpiredJob < ApplicationJob
+      queue_as :default
+
+      def perform
+        Users::Session
+          .where('created_at < ?', DateTime.now - JwtEncoder::EXPIRATION_SECONDS.seconds)
+          .destroy_all
+      end
+    end
+  end
+end

app/lib/jwt_encoder.rb

@@ -4,12 +4,21 @@ module JwtEncoder
   module_function
 
   HMAC_SECRET = Rails.application.secret_key_base
+  EXPIRATION_SECONDS = 86_400
 
   def encode(payload)
-    JWT.encode(payload, HMAC_SECRET)
+    JWT.encode(modify_payload(payload), HMAC_SECRET)
   end
 
   def decode(token)
     JWT.decode(token, HMAC_SECRET).first
+  rescue JWT::DecodeError
+    {}
+  end
+
+  def modify_payload(payload)
+    payload.merge!(
+      exp: DateTime.now.to_i + EXPIRATION_SECONDS
+    )
   end
 end

app/models/user.rb

@@ -7,7 +7,7 @@ class User < ApplicationRecord
   REGULAR = 'regular'
   ADMIN = 'admin'
 
-  has_one :users_session, class_name: 'Users::Session', dependent: :destroy
+  has_many :users_sessions, class_name: 'Users::Session', dependent: :destroy
 
   has_many :companies, dependent: :destroy
   has_many :repositories, through: :companies

app/services/auth/fetch_user_service.rb

@@ -4,24 +4,25 @@ module Auth
   class FetchUserService
     prepend ApplicationService
 
+    attr_reader :session
+
     def call(token:)
       @payload = extract_uuid(token)
+      @session = find_session
 
-      fail!('Forbidden') if @payload.blank? || session.blank?
+      fail!('Forbidden') if @payload.blank? || @session.blank?
 
-      @result = session&.user
+      @result = @session&.user
     end
 
     private
 
     def extract_uuid(token)
       JwtEncoder.decode(token)
-    rescue JWT::DecodeError
-      {}
     end
 
-    def session
-      @session ||= Users::Session.where(uuid: @payload.fetch('uuid', '')).first
+    def find_session
+      Users::Session.where(uuid: @payload.fetch('uuid', '')).first
     end
   end
 end

app/services/auth/generate_token_service.rb

@@ -5,10 +5,7 @@ class GenerateTokenService
     prepend ApplicationService
 
     def call(user:)
-      ActiveRecord::Base.transaction do
-        Users::Session.where(user: user).destroy_all
-        @result = JwtEncoder.encode(uuid: Users::Session.create!(user: user).uuid)
-      end
+      @result = JwtEncoder.encode(uuid: Users::Session.create!(user: user).uuid)
     end
   end
 end

config/locales/en.yml

@@ -9,7 +9,7 @@ en:
         logout: Logout
   controllers:
     authentication:
-      permission: You need to Sign In
+      permission: You need to sign in
     users:
       sessions:
         success_destroy: You are logged out

config/schedule.rb

@@ -14,3 +14,8 @@
 every 4.hours do
   runner 'Export::SendInsightsNotificationJob.perform_later'
 end
+
+# Clear expired users sessions
+every 1.day do
+  runner 'Users::Sessions::RemoveExpiredJob.perform_later'
+end

db/migrate/20230617083913_change_sessions_index_uniqueness.rb

@@ -0,0 +1,13 @@
+class ChangeSessionsIndexUniqueness < ActiveRecord::Migration[7.0]
+  disable_ddl_transaction!
+
+  def up
+    remove_index :users_sessions, column: :user_id
+    add_index :users_sessions, :user_id, unique: false, algorithm: :concurrently
+  end
+
+  def down
+    remove_index :users_sessions, column: :user_id
+    add_index :users_sessions, :user_id, unique: true, algorithm: :concurrently
+  end
+end

db/structure.sql

@@ -1585,14 +1585,14 @@ CREATE UNIQUE INDEX index_users_on_uuid ON public.users USING btree (uuid);
 -- Name: index_users_sessions_on_user_id; Type: INDEX; Schema: public; Owner: -
 --
 
-CREATE UNIQUE INDEX index_users_sessions_on_user_id ON public.users_sessions USING btree (user_id);
+CREATE INDEX index_users_sessions_on_user_id ON public.users_sessions USING btree (user_id);
 
 
 --
 -- Name: index_users_sessions_on_uuid; Type: INDEX; Schema: public; Owner: -
 --
 
-CREATE UNIQUE INDEX index_users_sessions_on_uuid ON public.users_sessions USING btree (uuid);
+CREATE INDEX index_users_sessions_on_uuid ON public.users_sessions USING btree (uuid);
 
 
 --
@@ -1731,6 +1731,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20230606045228'),
 ('20230606135138'),
 ('20230609173154'),
-('20230612131612');
+('20230612131612'),
+('20230617083913');
 
 

spec/controllers/users/omniauth_callbacks_controller_spec.rb

@@ -0,0 +1,105 @@
+# frozen_string_literal: true
+
+describe Users::OmniauthCallbacksController do
+  describe 'POST#create' do
+    let(:code) { nil }
+    let(:request) { post :create, params: { provider: provider, code: code } }
+
+    context 'for unexisting provider' do
+      let(:provider) { 'unknown' }
+
+      it 'redirects to root path', :aggregate_failures do
+        expect { request }.not_to change(User, :count)
+        expect(response).to redirect_to root_path
+      end
+    end
+
+    context 'for existing provider' do
+      let(:provider) { Providerable::GITHUB }
+
+      context 'for blank code' do
+        it 'redirects to root path', :aggregate_failures do
+          expect { request }.not_to change(User, :count)
+          expect(response).to redirect_to root_path
+        end
+      end
+
+      context 'for present code' do
+        let!(:auth_service) { double }
+        let(:code) { 'code' }
+
+        before do
+          allow(Auth::Providers::Github).to receive(:call).and_return(auth_service)
+        end
+
+        context 'for invalid code' do
+          before do
+            allow(auth_service).to receive(:result).and_return(nil)
+          end
+
+          it 'redirects to root path', :aggregate_failures do
+            expect { request }.not_to change(User, :count)
+            expect(response).to redirect_to root_path
+          end
+        end
+
+        context 'for valid code' do
+          before do
+            allow(auth_service).to receive(:result).and_return(auth_payload)
+          end
+
+          context 'for invalid payload' do
+            let(:auth_payload) do
+              {
+                uid: '123',
+                provider: 'github',
+                login: 'octocat',
+                email: nil
+              }
+            end
+
+            it 'redirects to root path', :aggregate_failures do
+              expect { request }.not_to change(User, :count)
+              expect(response).to redirect_to root_path
+            end
+          end
+
+          context 'for valid payload' do
+            let(:auth_payload) do
+              {
+                uid: '123',
+                provider: 'github',
+                login: 'octocat',
+                email: 'email@gmail.com'
+              }
+            end
+
+            it 'redirects to companies path', :aggregate_failures do
+              expect { request }.to change(User, :count).by(1)
+              expect(response).to redirect_to companies_path
+            end
+          end
+        end
+      end
+    end
+  end
+
+  describe 'DELETE#destroy' do
+    it_behaves_like 'required auth'
+
+    context 'for logged users' do
+      sign_in_user
+
+      before { create :users_session, user: @current_user }
+
+      it 'destroys session and redirects', :aggregate_failures do
+        expect { do_request }.to change(Users::Session, :count).by(-1)
+        expect(response).to redirect_to root_path
+      end
+    end
+
+    def do_request
+      delete :destroy, params: { locale: 'en' }
+    end
+  end
+end

spec/controllers/vacations_controller_spec.rb

@@ -88,4 +88,35 @@ def do_request
       }
     end
   end
+
+  describe 'DELETE#destroy' do
+    it_behaves_like 'required auth'
+
+    context 'for logged users' do
+      sign_in_user
+
+      let!(:vacation) { create :vacation }
+      let(:request) { delete :destroy, params: { id: vacation.id, locale: 'en' } }
+
+      context 'for not user vacation' do
+        it 'destroys vacation and redirects', :aggregate_failures do
+          expect { request }.not_to change(Vacation, :count)
+          expect(response).to redirect_to profile_path
+        end
+      end
+
+      context 'for user vacation' do
+        before { vacation.update!(user: @current_user) }
+
+        it 'destroys vacation and redirects', :aggregate_failures do
+          expect { request }.to change(Vacation, :count).by(-1)
+          expect(response).to redirect_to profile_path
+        end
+      end
+    end
+
+    def do_request
+      delete :destroy, params: { id: 'unexisting', locale: 'en' }
+    end
+  end
 end

spec/helpers/application_helper_spec.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+describe ApplicationHelper do
+  describe '#omniauth_link' do
+    context 'for unexisting provider' do
+      it 'returns nil' do
+        expect(helper.omniauth_link(:unknown)).to be_nil
+      end
+    end
+
+    context 'for github provider' do
+      it 'returns github link' do
+        expect(helper.omniauth_link(:github).include?('https://github.com')).to be_truthy
+      end
+    end
+
+    context 'for gitlab provider' do
+      it 'returns gitlab link' do
+        expect(helper.omniauth_link(:gitlab).include?('https://gitlab.com')).to be_truthy
+      end
+    end
+  end
+end

spec/jobs/users/sessions/remove_expired_job_spec.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+describe Users::Sessions::RemoveExpiredJob, type: :service do
+  subject(:job_call) { described_class.perform_now }
+
+  before do
+    create :users_session, created_at: DateTime.now - JwtEncoder::EXPIRATION_SECONDS.seconds - 10.seconds
+    create :users_session, created_at: DateTime.now
+  end
+
+  it 'removes expired sessions' do
+    expect { job_call }.to change(Users::Session, :count).by(-1)
+  end
+end

spec/models/user_spec.rb

@@ -8,7 +8,7 @@
   end
 
   describe 'associations' do
-    it { is_expected.to have_one(:users_session).class_name('::Users::Session').dependent(:destroy) }
+    it { is_expected.to have_many(:users_sessions).class_name('::Users::Session').dependent(:destroy) }
     it { is_expected.to have_many(:companies).dependent(:destroy) }
     it { is_expected.to have_many(:identities).dependent(:destroy) }
     it { is_expected.to have_many(:subscriptions).dependent(:destroy) }

spec/rails_helper.rb

@@ -27,12 +27,14 @@
 
 # Checks for pending migrations and applies them before tests are run.
 # If you are not using ActiveRecord, you can remove these lines.
+# :skippit:
 begin
   ActiveRecord::Migration.maintain_test_schema!
 rescue ActiveRecord::PendingMigrationError => e
   puts e.to_s.strip
   exit 1
 end
+# :skippit:
 
 RSpec.configure do |config|
   # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures