From 17eb40f9f78ad0b26a4537cc62cf647bc7ab0415 Mon Sep 17 00:00:00 2001 From: Niklas van Schrick Date: Wed, 13 Dec 2023 21:08:00 +0100 Subject: [PATCH] Allow to log out sessions --- .rubocop.yml | 4 + Gemfile | 2 + Gemfile.lock | 2 + app/controllers/graphql_controller.rb | 6 +- app/graphql/mutations/base_mutation.rb | 4 + app/graphql/mutations/users/logout.rb | 22 ++++++ app/graphql/types/mutation_type.rb | 1 + app/graphql/types/user_session_type.rb | 2 + app/models/ability.rb | 21 ++++++ app/policies/base_policy.rb | 4 + app/policies/user_session_policy.rb | 7 ++ app/services/service_response.rb | 2 +- app/services/user_logout_service.rb | 26 +++++++ spec/graphql/types/user_session_type_spec.rb | 1 + spec/policies/user_session_policy_spec.rb | 31 ++++++++ .../graphql/mutation/users/logout_spec.rb | 74 +++++++++++++++++++ spec/services/user_logout_service_spec.rb | 40 ++++++++++ spec/support/helpers/graphql_helpers.rb | 2 +- 18 files changed, 248 insertions(+), 3 deletions(-) create mode 100644 app/graphql/mutations/users/logout.rb create mode 100644 app/models/ability.rb create mode 100644 app/policies/base_policy.rb create mode 100644 app/policies/user_session_policy.rb create mode 100644 app/services/user_logout_service.rb create mode 100644 spec/policies/user_session_policy_spec.rb create mode 100644 spec/requests/graphql/mutation/users/logout_spec.rb create mode 100644 spec/services/user_logout_service_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index 49e88553..293644d4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -11,6 +11,10 @@ AllCops: GraphQL/ExtractInputType: Enabled: false +Lint/AmbiguousBlockAssociation: + AllowedMethods: [change] + + Metrics/AbcSize: Enabled: false diff --git a/Gemfile b/Gemfile index 77256e72..9f22bec5 100644 --- a/Gemfile +++ b/Gemfile @@ -70,3 +70,5 @@ gem 'seed-fu', '~> 2.3' gem 'sidekiq', '~> 7.1' gem 'lograge', '~> 0.14.0' + +gem 'declarative_policy', '~> 1.1' diff --git a/Gemfile.lock b/Gemfile.lock index 5f0ac124..736c1d7f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -97,6 +97,7 @@ GEM irb (>= 1.5.0) reline (>= 0.3.1) debug_inspector (1.1.0) + declarative_policy (1.1.0) diff-lcs (1.5.0) docile (1.4.0) drb (2.2.0) @@ -319,6 +320,7 @@ DEPENDENCIES bootsnap database_cleaner-active_record (~> 2.1) debug + declarative_policy (~> 1.1) factory_bot_rails (~> 6.2) graphql (~> 2.1) lograge (~> 0.14.0) diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index 8d0c1239..6168549f 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -101,7 +101,11 @@ def mutations_allowed? false end - %i[none invalid session].each do |t| + def invalid? + (authorization.nil? && !none?) || type == :invalid + end + + %i[none session].each do |t| define_method :"#{t}?" do type == t end diff --git a/app/graphql/mutations/base_mutation.rb b/app/graphql/mutations/base_mutation.rb index a74234e4..38dde893 100644 --- a/app/graphql/mutations/base_mutation.rb +++ b/app/graphql/mutations/base_mutation.rb @@ -21,5 +21,9 @@ def self.require_one_of(arguments, context) field :errors, [GraphQL::Types::String], null: false, description: 'Errors encountered during execution of the mutation.' + + def current_user + context[:current_user] + end end end diff --git a/app/graphql/mutations/users/logout.rb b/app/graphql/mutations/users/logout.rb new file mode 100644 index 00000000..8594b6c3 --- /dev/null +++ b/app/graphql/mutations/users/logout.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Mutations + module Users + class Logout < BaseMutation + description 'Logout an existing user session' + + field :user_session, Types::UserSessionType, null: true, description: 'The logged out user session' + + argument :user_session_id, Types::GlobalIdType[::UserSession], required: true, + description: 'ID of the session to logout' + + def resolve(user_session_id:) + user_session = SagittariusSchema.object_from_id(user_session_id) + + return { user_session: nil, errors: ['Invalid user session'] } if user_session.nil? + + UserLogoutService.new(current_user, user_session).execute.to_mutation_response(success_key: :user_session) + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index ecc495ab..4ab0f56a 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -7,6 +7,7 @@ class MutationType < Types::BaseObject include Sagittarius::Graphql::MountMutation mount_mutation Mutations::Users::Login + mount_mutation Mutations::Users::Logout mount_mutation Mutations::Users::Register field :echo, GraphQL::Types::String, null: false, diff --git a/app/graphql/types/user_session_type.rb b/app/graphql/types/user_session_type.rb index 25eb5740..a28c967c 100644 --- a/app/graphql/types/user_session_type.rb +++ b/app/graphql/types/user_session_type.rb @@ -4,6 +4,8 @@ module Types class UserSessionType < Types::BaseObject description 'Represents a user session' + field :active, GraphQL::Types::Boolean, null: false, + description: 'Whether or not the session is active and can be used' field :id, Types::GlobalIdType[::UserSession], null: false, description: 'GlobalID of the user' field :token, String, null: true, description: 'Token belonging to the session, only present on creation' field :user, Types::UserType, null: false, description: 'User that belongs to the session' diff --git a/app/models/ability.rb b/app/models/ability.rb new file mode 100644 index 00000000..a91dbf22 --- /dev/null +++ b/app/models/ability.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Ability + module_function + + def allowed?(user, ability, subject = :global) + policy = policy_for(user, subject) + + policy.allowed?(ability) + end + + def policy_for(user, subject = :global) + Cache.policies ||= {} + + DeclarativePolicy.policy_for(user, subject, cache: Cache.policies) + end + + class Cache < ActiveSupport::CurrentAttributes + attribute :policies + end +end diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb new file mode 100644 index 00000000..b6af3fbf --- /dev/null +++ b/app/policies/base_policy.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class BasePolicy < DeclarativePolicy::Base +end diff --git a/app/policies/user_session_policy.rb b/app/policies/user_session_policy.rb new file mode 100644 index 00000000..22c41400 --- /dev/null +++ b/app/policies/user_session_policy.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class UserSessionPolicy < BasePolicy + condition(:session_owner) { @subject.user_id == @user&.id } + + rule { session_owner }.enable :logout_session +end diff --git a/app/services/service_response.rb b/app/services/service_response.rb index 560ef317..9e4faff7 100644 --- a/app/services/service_response.rb +++ b/app/services/service_response.rb @@ -40,7 +40,7 @@ def to_mutation_response(success_key: :object) if payload.is_a?(ActiveModel::Errors) { success_key => nil, errors: payload.full_messages } else - { success_key => nil, errors: payload } + { success_key => nil, errors: Array.wrap(payload) } end end end diff --git a/app/services/user_logout_service.rb b/app/services/user_logout_service.rb new file mode 100644 index 00000000..febe2138 --- /dev/null +++ b/app/services/user_logout_service.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class UserLogoutService + include Sagittarius::Loggable + + def initialize(current_user, user_session) + @current_user = current_user + @user_session = user_session + end + + def execute + unless Ability.allowed?(@current_user, :logout_session, @user_session) + return ServiceResponse.error(payload: "You can't log out this session") + end + + @user_session.active = false + + if @user_session.save + logger.info(message: 'Logged out session', session_id: @user_session.id, user_id: @user_session.user_id) + ServiceResponse.success(message: 'Logged out session', payload: @user_session) + else + logger.warn(message: 'Failed to log out session', session_id: @user_session.id, user_id: @user_session.user_id) + ServiceResponse.error(payload: 'Failed to log out session') + end + end +end diff --git a/spec/graphql/types/user_session_type_spec.rb b/spec/graphql/types/user_session_type_spec.rb index 49ecbe43..146872b9 100644 --- a/spec/graphql/types/user_session_type_spec.rb +++ b/spec/graphql/types/user_session_type_spec.rb @@ -8,6 +8,7 @@ id user token + active ] end diff --git a/spec/policies/user_session_policy_spec.rb b/spec/policies/user_session_policy_spec.rb new file mode 100644 index 00000000..36278e55 --- /dev/null +++ b/spec/policies/user_session_policy_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe UserSessionPolicy do + subject { described_class.new(current_user, user_session) } + + let(:current_user) { nil } + let(:user_session) { nil } + + context 'when user is owner of the session' do + let(:current_user) { create(:user) } + let(:user_session) { create(:user_session, user: current_user) } + + it { is_expected.to be_allowed(:logout_session) } + end + + context 'when user is not owner of the session' do + let(:current_user) { create(:user) } + let(:user_session) { create(:user_session) } + + it { is_expected.not_to be_allowed(:logout_session) } + end + + context 'when user is nil' do + let(:current_user) { nil } + let(:user_session) { create(:user_session) } + + it { is_expected.not_to be_allowed(:logout_session) } + end +end diff --git a/spec/requests/graphql/mutation/users/logout_spec.rb b/spec/requests/graphql/mutation/users/logout_spec.rb new file mode 100644 index 00000000..a685815e --- /dev/null +++ b/spec/requests/graphql/mutation/users/logout_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'usersLogout Mutation' do + include GraphqlHelpers + + let(:mutation) do + <<~QUERY + mutation($input: UsersLogoutInput!) { + usersLogout(input: $input) { + errors + userSession { + id + active + } + } + } + QUERY + end + + let(:user_session_id) { nil } + let(:current_user) { nil } + let(:variables) { { input: { userSessionId: user_session_id } } } + + before { post_graphql mutation, variables: variables, current_user: current_user } + + context 'when input is valid' do + let(:user_session) { create(:user_session) } + let(:user_session_id) { user_session.to_global_id.to_s } + + context 'when logging out a session of the same user' do + let(:current_user) { user_session.user } + + it 'logs out the session', :aggregate_failures do + expect(graphql_data_at(:users_logout, :user_session, :id)).to eq(user_session_id) + expect(graphql_data_at(:users_logout, :user_session, :active)).to be(false) + end + end + + context 'when logging out a session of another user' do + let(:current_user) { create(:user) } + + it 'does not log out the session', :aggregate_failures do + expect(graphql_data_at(:users_logout, :errors)).to include("You can't log out this session") + expect(graphql_data_at(:users_logout, :user_session)).to be_nil + end + end + end + + context 'when input is invalid' do + let(:current_user) { create(:user) } + + context 'when session id is invalid' do + let(:user_session_id) { 'some random string' } + + it 'raises validation error' do + expect(graphql_errors).to include( + a_hash_including( + 'message' => a_string_including("Could not coerce value \"#{user_session_id}\" to UserSessionID") + ) + ) + end + end + + context 'when session id is does not exist' do + let(:user_session_id) { 'gid://Sagittarius/UserSession/0' } + + it 'raises validation error' do + expect(graphql_data_at(:users_logout, :errors)).to include('Invalid user session') + end + end + end +end diff --git a/spec/services/user_logout_service_spec.rb b/spec/services/user_logout_service_spec.rb new file mode 100644 index 00000000..b22f0165 --- /dev/null +++ b/spec/services/user_logout_service_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe UserLogoutService do + subject(:service_response) { described_class.new(current_user, user_session).execute } + + context 'when current_user can log out user_session' do + let(:current_user) { create(:user) } + let(:user_session) { create(:user_session, user: current_user) } + + it { is_expected.to be_success } + + it 'changes the session to inactive' do + expect { service_response }.to change { user_session.reload.active }.from(true).to(false) + end + end + + context 'when current_user can not log out user_session' do + let(:current_user) { create(:user) } + let(:user_session) { create(:user_session) } + + it { is_expected.not_to be_success } + + it 'does not change the session to inactive' do + expect { service_response }.not_to change { user_session.reload.active } + end + end + + context 'when current_user is nil' do + let(:current_user) { nil } + let(:user_session) { create(:user_session) } + + it { is_expected.not_to be_success } + + it 'does not change the session to inactive' do + expect { service_response }.not_to change { user_session.reload.active } + end + end +end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 06578cb4..ccb78374 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -46,7 +46,7 @@ def graphql_dig_at(data, *path) keys = path.map { |segment| segment.is_a?(Integer) ? segment : GraphqlHelpers.graphql_field_name(segment) } keys.reduce(data) do |acc, cur| - if acc.is_a?(Array) && key.is_a?(Integer) + if acc.is_a?(Array) && cur.is_a?(Integer) acc[cur] elsif acc.is_a?(Array) acc.compact.pluck(cur)