Merge pull request #886 from sonalkr132/auth-registration

Fix authorization of new and create in ConferenceRegistrationsController

app/controllers/conference_registrations_controller.rb

@@ -1,10 +1,13 @@
 class ConferenceRegistrationsController < ApplicationController
   before_filter :authenticate_user!, except: [:new, :create]
   load_resource :conference, find_by: :short_title
-  authorize_resource :conference_registrations, class: Registration
+  authorize_resource :conference_registrations, class: Registration, except: [:new, :create]
   before_action :set_registration, only: [:edit, :update, :destroy, :show]
 
   def new
+    @registration = Registration.new(conference_id: @conference.id)
+    authorize! :new, @registration
+
     # Redirect to registration edit when user is already registered
     if @conference.user_registered?(current_user)
       redirect_to edit_conference_conference_registrations_path(@conference.short_title)
@@ -25,8 +28,6 @@ class ConferenceRegistrationsController < ApplicationController
       return
     end
 
-    @registration = Registration.new
-
     # @user variable needs to be set so that _sign_up_form_embedded works properly
     @user = @registration.build_user
   end
@@ -50,6 +51,7 @@ class ConferenceRegistrationsController < ApplicationController
     end
 
     @registration.user = @user
+    authorize! :create, @registration
 
     if @registration.save
       # Trigger ahoy event

app/models/ability.rb

@@ -46,9 +46,14 @@ class Ability
     can :show, Commercial, commercialable_type: 'Event', commercialable_id: Event.where(state: 'confirmed').pluck(:id)
     can :show, User
     unless CONFIG['authentication']['ichain']['enabled']
-      can [:show, :create], Registration do |registration|
+      can :show, Registration do |registration|
         registration.new_record?
       end
+
+      can [:new, :create], Registration do |registration|
+        registration.conference.registration_open? && registration.new_record?
+      end
+
       can :show, Event do |event|
         event.new_record?
       end
@@ -68,6 +73,10 @@ class Ability
 
     can :manage, Registration, user_id: user.id
 
+    can [:new, :create], Registration do |registration|
+      registration.conference.registration_open?
+    end
+
     can :index, Ticket
     can :manage, TicketPurchase, user_id: user.id
 

spec/features/proposal_spec.rb

@@ -2,6 +2,7 @@ require 'spec_helper'
 
 feature Event do
   let!(:conference) { create(:conference) }
+  let!(:registration_period) { create(:registration_period, conference: conference, start_date: Date.current) }
   let!(:cfp) { create(:cfp, program_id: conference.program.id) }
   let!(:organizer_role) { Role.find_by(name: 'organizer', resource: conference) }
   let!(:organizer) { create(:user, email: 'admin@example.com', role_ids: [organizer_role.id]) }
@@ -134,6 +135,7 @@ feature Event do
       click_link "confirm_proposal_#{@event.id}"
       expect(flash).
         to eq('The proposal was confirmed. Please register to attend the conference.')
+      expect(current_path).to eq(new_conference_conference_registrations_path(conference.short_title))
       @event.reload
       expect(@event.state).to eq('confirmed')
     end

spec/models/ability_spec.rb

@@ -34,6 +34,10 @@ describe 'User' do
 
     let(:program_with_cfp) { create(:program, cfp: create(:cfp)) }
     let(:program_without_cfp) { create(:program) }
+    let(:conference_with_open_registration) { create(:conference) }
+    let!(:open_registration_period) { create(:registration_period, conference: conference_with_open_registration, start_date: Date.current - 6.days) }
+    let(:conference_with_closed_registration) { create(:conference) }
+    let!(:closed_registration_period) { create(:registration_period, conference: conference_with_closed_registration, start_date: Date.current - 6.days, end_date: Date.current - 6.days) }
 
     # Test abilities for not signed in users
     context 'when user is not signed in' do
@@ -57,8 +61,11 @@ describe 'User' do
 
       it{ should be_able_to(:show, User)}
 
-      it{ should be_able_to(:create, Registration)}
       it{ should be_able_to(:show, Registration.new)}
+      it{ should be_able_to(:create, Registration.new(conference_id: conference_with_open_registration.id))}
+      it{ should be_able_to(:new, Registration.new(conference_id: conference_with_open_registration.id))}
+      it{ should_not be_able_to(:new, Registration.new(conference_id: conference_with_closed_registration.id))}
+      it{ should_not be_able_to(:create, Registration.new(conference_id: conference_with_closed_registration.id))}
       it{ should_not be_able_to(:manage, registration)}
 
       it{ should be_able_to(:new, Event.new(program: program_with_cfp)) }
@@ -86,6 +93,8 @@ describe 'User' do
 
       it{ should be_able_to(:manage, registration_public) }
       it{ should be_able_to(:manage, registration_not_public) }
+      it{ should_not be_able_to(:new, Registration.new(conference_id: conference_with_closed_registration.id))}
+      it{ should_not be_able_to(:create, Registration.new(conference_id: conference_with_closed_registration.id))}
 
       it{ should be_able_to(:index, Ticket) }
       it{ should be_able_to(:manage, TicketPurchase.new(user_id: user.id)) }