Merge pull request #10278 from intrip/refactor_require_login_user_not…

…ifications

Replace `require_login` with Pundit in Webui::Users::NotificationsCon…

src/api/app/controllers/webui/users/notifications_controller.rb

@@ -2,10 +2,13 @@ class Webui::Users::NotificationsController < Webui::WebuiController
   MAX_PER_PAGE = 300
   VALID_NOTIFICATION_TYPES = ['read', 'reviews', 'comments', 'requests', 'unread'].freeze
 
-  before_action :require_login
+  # TODO: Remove this when we'll refactor kerberos_auth
+  before_action :kerberos_auth
   before_action :check_param_type, :check_param_project, only: :index
   before_action :check_feature_toggle
 
+  after_action :verify_policy_scoped
+
   def index
     @notifications = paginated_notifications
     @filtered_project = Project.find_by(name: params[:project])

src/api/app/policies/notification_policy.rb

@@ -1,5 +1,11 @@
 class NotificationPolicy < ApplicationPolicy
   class Scope < Scope
+    def initialize(user, scope)
+      raise Pundit::NotAuthorizedError, reason: ApplicationPolicy::ANONYMOUS_USER if user.nil? || user.is_nobody?
+
+      super(user, scope)
+    end
+
     def resolve
       NotificationsFinder.new(scope).for_subscribed_user(user)
     end

src/api/spec/controllers/webui/users/notifications_controller_spec.rb

@@ -21,20 +21,29 @@
 
   before do
     Flipper[:notifications_redesign].enable
-    login user_to_log_in
   end
 
   describe 'GET #index' do
     let(:user_to_log_in) { user }
     let(:default_params) { { user_login: username } }
 
-    subject! do
+    it_behaves_like 'require logged in user' do
+      let(:method) { :get }
+      let(:action) { :index }
+    end
+
+    subject do
+      login user_to_log_in
       get :index, params: params
     end
 
     context 'when no param type is provided' do
       let(:params) { default_params }
 
+      before do
+        subject
+      end
+
       it_behaves_like 'returning success'
 
       it 'assigns notifications with all notifications' do
@@ -54,6 +63,10 @@
     context "when param type is 'read'" do
       let(:params) { default_params.merge(type: 'read') }
 
+      before do
+        subject
+      end
+
       it_behaves_like 'returning success'
 
       it 'sets @notifications to all delivered notifications regardless of type' do
@@ -64,6 +77,10 @@
     context "when param type is 'comments'" do
       let(:params) { default_params.merge(type: 'comments') }
 
+      before do
+        subject
+      end
+
       it_behaves_like 'returning success'
 
       it "sets @notifications to all undelivered notifications of 'comments' type" do
@@ -76,6 +93,10 @@
     context "when param type is 'requests'" do
       let(:params) { default_params.merge(type: 'requests') }
 
+      before do
+        subject
+      end
+
       it_behaves_like 'returning success'
 
       it "sets @notifications to all undelivered notifications of 'requests' type" do
@@ -86,8 +107,14 @@
   end
 
   describe 'PUT #update' do
+    it_behaves_like 'require logged in user' do
+      let(:method) { :get }
+      let(:action) { :index }
+    end
+
     context 'when a user marks one of his unread notifications as read' do
       subject! do
+        login user_to_log_in
         put :update, params: { notification_ids: [state_change_notification.id], user_login: user_to_log_in.login }, xhr: true
       end
 
@@ -102,8 +129,22 @@
       end
     end
 
+    context 'when a user tries to mark other user notifications as read' do
+      subject! do
+        login user_to_log_in
+        put :update, params: { notification_ids: [state_change_notification.id], user_login: user_to_log_in.login }, xhr: true
+      end
+
+      let(:user_to_log_in) { other_user }
+
+      it "doesn't set the notification as read" do
+        expect(state_change_notification.reload.delivered).to be false
+      end
+    end
+
     context 'when a user marks one of his read notifications as unread' do
       subject! do
+        login user_to_log_in
         put :update, params: { notification_ids: [read_notification.id], type: 'read', user_login: user_to_log_in.login }, xhr: true
       end
 

src/api/spec/policies/notification_policy_spec.rb

@@ -0,0 +1,12 @@
+require 'rails_helper'
+
+RSpec.describe NotificationPolicy do
+  describe NotificationPolicy::Scope do
+    let(:user_nobody) { build(:user_nobody) }
+
+    it "doesn't permit anonymous user" do
+      expect { described_class.new(user_nobody, Notification.none) }
+        .to raise_error(an_instance_of(Pundit::NotAuthorizedError).and(having_attributes(reason: :anonymous_user)))
+    end
+  end
+end