Clean up rest of :manage_admin_users perm

Refs FOO-171
flag=granular_permissions_manage_users

There were still lots of places in the code where only the old
:manage_admin_users permission was getting checked instead of
conditionally looking at the new "grab-bag" permission which
is :allow_course_admin_actions ... hopefully this catches the
rest of them.

Test plan
* tests pass
* at some point we'll need some heavy manual testing of all this

Change-Id: I008e15fee69523c9c33de86e22661ef0639f24c9
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/258858
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
This commit is contained in:
Charley Kline 2021-02-16 15:35:56 -06:00
parent 92e03a64cd
commit c5e5c51cee
8 changed files with 44 additions and 12 deletions

View File

@ -2365,7 +2365,12 @@ class UsersController < ApplicationController
# returns the original list in :invited_users (with ids) if successfully added, or in :errored_users if not
get_context
return unless authorized_action(@context, @current_user, [:manage_students, :manage_admin_users])
manage_perm = if @context.root_account.feature_enabled? :granular_permissions_manage_users
:allow_course_admin_actions
else
:manage_admin_users
end
return unless authorized_action(@context, @current_user, [:manage_students, manage_perm])
root_account = context.root_account
unless root_account.open_registration? || root_account.grants_right?(@current_user, session, :manage_user_logins)

View File

@ -2665,9 +2665,15 @@ class Course < ActiveRecord::Base
end
def enrollment_visibility_level_for(user, visibilities = section_visibilities_for(user), require_message_permission = false)
manage_perm = if self.root_account.feature_enabled? :granular_permissions_manage_users
:allow_course_admin_actions
else
:manage_admin_users
end
permissions = require_message_permission ?
[:send_messages] :
[:manage_grades, :manage_students, :manage_admin_users, :read_roster, :view_all_grades, :read_as_admin]
[:manage_grades, :manage_students, manage_perm, :read_roster, :view_all_grades, :read_as_admin]
granted_permissions = self.granted_rights(user, *permissions)
if granted_permissions.empty?
:restricted # e.g. observer, can only see admins in the course

View File

@ -161,7 +161,14 @@ class CourseSection < ActiveRecord::Base
end
can :read and can :delete
given { |user, session| self.course.grants_any_right?(user, session, :manage_students, :manage_admin_users) }
given do |user, session|
manage_perm = if self.root_account.feature_enabled? :granular_permissions_manage_users
:allow_course_admin_actions
else
:manage_admin_users
end
self.course.grants_any_right?(user, session, :manage_students, manage_perm)
end
can :read
given { |user| self.course.account_membership_allows(user, :read_roster) }

View File

@ -38,7 +38,12 @@
</li>
<% end %>
<% if can_do(@context, @current_user, :manage_admin_users, :manage_students, :read_prior_roster) %>
<% if can_do(@context,
@current_user,
@context.root_account.feature_enabled?(:granular_permissions_manage_users) ? :allow_course_admin_actions : :manage_admin_users,
:manage_students,
:read_prior_roster
) %>
<li role="presentation">
<%= sidebar_button course_prior_users_path(@context),
t('View Prior Enrollments'),

View File

@ -79,8 +79,9 @@
<%= render :partial => 'shared/profile' %>
<% manage_perm = @context.root_account.feature_enabled?(:granular_permissions_manage_users) ? :allow_course_admin_actions : :manage_admin_users %>
<% can_manage_students = can_do(@context, @current_user, :manage_students) %>
<% can_manage_admins = can_do(@context, @current_user, :manage_admin_users) %>
<% can_manage_admins = can_do(@context, @current_user, manage_perm) %>
<% if !@context.is_a?(Group) && (can_manage_students || can_manage_admins) %>
<div class="more_user_information">
@ -134,7 +135,7 @@
<% end %>
</tr>
<% end %>
<% if @context.is_a?(Course) && can_do(@context, @current_user, :manage_admin_users) %>
<% if @context.is_a?(Course) && can_do(@context, @current_user, manage_perm) %>
<tr id="priveleges">
<td scope="row">
<%= before_label('user_privileges', %{Privileges}) %>
@ -152,7 +153,7 @@
</tr>
<% end %>
</table>
<% if @context.is_a?(Course) && can_do(@context, @current_user, :manage_admin_users) %>
<% if @context.is_a?(Course) && can_do(@context, @current_user, manage_perm) %>
<div id="student_last_attended__component"></div>
<% end %>
</fieldset>

View File

@ -119,8 +119,10 @@
<% end %>
<h1><%= @user.name %> <% if @user.pronouns %><i>(<%= @user.pronouns %>)</i><% end %></h1>
<% manage_perm = @context.root_account.feature_enabled?(:granular_permissions_manage_users) ? :allow_course_admin_actions : :manage_admin_users %>
<% can_manage_students = can_do(@context, @current_user, :manage_students) %>
<% can_manage_admins = can_do(@context, @current_user, :manage_admin_users) %>
<% can_manage_admins = can_do(@context, @current_user, manage_perm) %>
<% if !@context.is_a?(Group) && (can_manage_students || can_manage_admins) %>
<%= render :partial => 'users/name' %>
@ -173,7 +175,7 @@
<% end %>
</tr>
<% end %>
<% if @context.is_a?(Course) && can_do(@context, @current_user, :manage_admin_users) %>
<% if @context.is_a?(Course) && can_do(@context, @current_user, manage_perm) %>
<tr>
<td>
<%= before_label('user_privileges', %{Privileges}) %>
@ -195,7 +197,7 @@
</tr>
<% end %>
</table>
<% if @context.is_a?(Course) && can_do(@context, @current_user, :manage_admin_users) %>
<% if @context.is_a?(Course) && can_do(@context, @current_user, manage_perm) %>
<div id="student_last_attended__component"></div>
<% end %>
</fieldset>

View File

@ -243,7 +243,8 @@ h3 .tally {
</div>
<% end %>
</div>
<% if can_do @context, @current_user, :read_roster, :manage_students, :manage_admin_users %>
<% manage_perm = @context.root_account.feature_enabled?(:granular_permissions_manage_users) ? :allow_course_admin_actions : :manage_admin_users %>
<% if can_do @context, @current_user, :read_roster, :manage_students, manage_perm %>
<div class="enrollments-lists">
<h3><%= t('titles.current_enrollments', 'Current Enrollments') %></h3>
<div class="user-list-wrapper" id="current-enrollment-list">

View File

@ -18,7 +18,12 @@
<% unless @read_only
account = @context && (@context.is_a?(Account) ? @context : @context.account)
managed_accounts = @user.associated_root_accounts.active.select{|a| can_do(a, @current_user, :manage_admin_users) }
# Note: we'd like to pick :allow_course_admin_actions or :manage_admin_users depending on
# whether or not the :granular_permissions_manage_users FF is on for the root account, but
# we need the permission in order to even figure out what the appropriate root account is.
# So in this case, we'll take either; this appears to be safe because the permission check
# is not used anywhere else in this view.
managed_accounts = @user.associated_root_accounts.active.select{|a| can_do(a, @current_user, :allow_course_admin_actions, :manage_admin_users) }
account ||= managed_accounts.first
root_account = account.root_account if account
end