Browse Source

Fix oversight allowing cross-group Activity viewing

Cause:
  - :require_membership uses @group to check whether user can view,
  - @group is set by :set_group, from :group_id parameter, _without_
      checking whether (for the methods dealing with an Activity) that
      Group is the Activity's Group
  - if a user is a member of at least one Group (default situation),
      then the User could freely view other activities by guessing at
      the id, but leaving the :group_id set to the User's Group.
Maarten van den Berg 7 years ago
parent
commit
6a813d995c
1 changed files with 5 additions and 4 deletions
  1. 5 4
      app/controllers/activities_controller.rb

+ 5 - 4
app/controllers/activities_controller.rb

@@ -1,8 +1,8 @@
1 1
 class ActivitiesController < ApplicationController
2 2
   include GroupsHelper
3 3
   include ActivitiesHelper
4
-  before_action :set_activity, only: [:show, :edit, :update, :destroy, :presence]
5
-  before_action :set_group
4
+  before_action :set_activity_and_group, only: [:show, :edit, :update, :destroy, :presence]
5
+  before_action :set_group,            except: [:show, :edit, :update, :destroy, :presence]
6 6
   before_action :require_membership!
7 7
   before_action :require_leader!, only: [:mass_new, :mass_create, :new, :create, :destroy]
8 8
   before_action :require_organizer!, only: [:edit, :update, :change_organizer]
@@ -160,9 +160,10 @@ class ActivitiesController < ApplicationController
160 160
   end
161 161
 
162 162
   private
163
-    # Use callbacks to share common setup or constraints between actions.
164
-    def set_activity
163
+    # The Activity's group takes precedence over whatever's in the URL, set_group not required (and can be mislead)
164
+    def set_activity_and_group
165 165
       @activity = Activity.find(params[:id])
166
+      @group = @activity.group
166 167
     end
167 168
 
168 169
     def set_group