Download as
Browse Code »

Commit 67d9573015d96cada75a6595e6c873a1b5a90c09

Authored by Rodrigo Souto
1 parent be50abba
Exists in master and in 29 other branches add_member_task_reject_details, admin_visible_profile, article-list-template, community_notifications, contact_admin_translation, event_fixes, fix_comments_pagination, fix_rails4_organization_ratings, fix_sign_up_form, follow_step_fix, forum_topic_creation, mirror_block_improvements, multi_env_on_remote_user, new_security, noosfero_spb_ci, organization_ratings_improvements, organization_ratings_link_to_profile_stable-spb-1.3, organization_ratings_translations_fix, profile_api_improvements, ratings_minor_fixes, remote_user_fix, send_email_to_admins, stable-spb-1.3, stable-spb-1.3-fixes, stable-spb-1.4, stable-spb-1.5, suggest_rejected_value, web_steps_improvements, xss_terminate_custom_options

permissions: bypass permissions check if the user is admin 

If the user is profile/environment admin, bypasses the permissions check
so that he/she always has access to everything related to the profile.

The absence of this feature caused many strange behaviors like env
admins unable to access a community control panel section.

This also caused many complex permission checking codes where we were
forced to test lots of combinations instead of a straight check (e.g.
user is env admin or user is profile admin or user has permission x or
...).

After this change, there might still be redundant testing around the
code, but that might be reviewed slowly.
Showing 2 changed files with 23 additions and 0 deletions   Show diff stats
app/models/person.rb
  Diff comments   View file @67d9573
@@ -28,6 +28,12 @@ class Person < Profile @@ -28,6 +28,12 @@ class Person < Profile
28 { :select => 'DISTINCT profiles.*', :conditions => ['"profiles"."id" NOT IN (SELECT DISTINCT profiles.id FROM "profiles" INNER JOIN "role_assignments" ON "role_assignments"."accessor_id" = "profiles"."id" AND "role_assignments"."accessor_type" = (\'Profile\') WHERE "profiles"."type" IN (\'Person\') AND (%s))' % conditions] } 28 { :select => 'DISTINCT profiles.*', :conditions => ['"profiles"."id" NOT IN (SELECT DISTINCT profiles.id FROM "profiles" INNER JOIN "role_assignments" ON "role_assignments"."accessor_id" = "profiles"."id" AND "role_assignments"."accessor_type" = (\'Profile\') WHERE "profiles"."type" IN (\'Person\') AND (%s))' % conditions] }
29 } 29 }
30 30
  31 + def has_permission_with_admin?(permission, profile)
  32 + return true if profile.admins.include?(self) || profile.environment.admins.include?(self)
  33 + has_permission_without_admin?(permission, profile)
  34 + end
  35 + alias_method_chain :has_permission?, :admin
  36 +
31 def has_permission_with_plugins?(permission, profile) 37 def has_permission_with_plugins?(permission, profile)
32 permissions = [has_permission_without_plugins?(permission, profile)] 38 permissions = [has_permission_without_plugins?(permission, profile)]
33 permissions += plugins.map do |plugin| 39 permissions += plugins.map do |plugin|
test/unit/person_test.rb
  Diff comments   View file @67d9573
@@ -1263,6 +1263,23 @@ class PersonTest < ActiveSupport::TestCase @@ -1263,6 +1263,23 @@ class PersonTest < ActiveSupport::TestCase
1263 assert_equivalent [person_scrap,person_activity], person.activities.map { |a| a.klass.constantize.find(a.id) } 1263 assert_equivalent [person_scrap,person_activity], person.activities.map { |a| a.klass.constantize.find(a.id) }
1264 end 1264 end
1265 1265
  1266 + should 'grant every permission over profile for its admin' do
  1267 + admin = create_user('some-user').person
  1268 + profile = fast_create(Profile)
  1269 + profile.add_admin(admin)
  1270 +
  1271 + assert admin.has_permission?('anything', profile), 'Admin does not have every permission!'
  1272 + end
  1273 +
  1274 + should 'grant every permission over profile for environment admin' do
  1275 + admin = create_user('some-user').person
  1276 + profile = fast_create(Profile)
  1277 + environment = profile.environment
  1278 + environment.add_admin(admin)
  1279 +
  1280 + assert admin.has_permission?('anything', profile), 'Environment admin does not have every permission!'
  1281 + end
  1282 +
1266 should 'allow plugins to extend person\'s permission access' do 1283 should 'allow plugins to extend person\'s permission access' do
1267 person = create_user('some-user').person 1284 person = create_user('some-user').person
1268 class Plugin1 < Noosfero::Plugin 1285 class Plugin1 < Noosfero::Plugin