-
-
Notifications
You must be signed in to change notification settings - Fork 157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Draft] Allow super admin to reset passwords for community admin users #904
Changes from all commits
6aaa18c
119bab0
89bf400
3493e2c
60e7b65
cb2a6cd
b0577fa
6ba8fbc
76d5486
b2c294f
2fb9a5e
71af900
97063f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
module SuperAdmin | ||
class UsersController < ApplicationController | ||
def edit | ||
@user = User.find(params[:id]) | ||
|
||
if @user.role == 'admin' | ||
@user | ||
else | ||
flash[:alert] = 'You are not authorized to perform this action.' | ||
redirect_to(root_path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if it would be better for us to redirect to the community path rather than to the root path which shows the metrics for all of the communities for the super admin. Since it seems the current path is: Metrics root page (after login) -> Communities -> Community -> Find the user in the tiny list -> edit. On save, going all the way back feels jarring. Alternatively, it may also be worth creating a new list view under the Super Admin dashboard that lists all of the admin users in the system that we can use to redirect to and make it easier for super admins to reset admin passwords, regardless of the community. @rudokemper thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I missed this back in May. I think the first solution makes the most sense. |
||
end | ||
end | ||
|
||
def update | ||
@user = User.find(params[:id]) | ||
if @user.role != 'admin' | ||
redirect_to(root_path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's add a flash alert here to denote why it failed/they are being redirected. |
||
elsif @user.update(user_params) | ||
redirect_to edit_member_path(@user), notice: 'User successfully updated' | ||
else | ||
render :edit | ||
end | ||
end | ||
|
||
private | ||
|
||
def user_params | ||
params.require(:user).permit( | ||
:password, | ||
) | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
<% content_for(:title) do %> | ||
<%= @user.display_name %> | <%= t("user") %> | ||
<% end %> | ||
|
||
<% content_for(:main_heading) do %> | ||
<div class="avatar-card"> | ||
<span class="media-with-hover-controls rounded" data-replace-on-delete="<%= asset_path("speaker.png") %>"> | ||
<% if @user.photo.attached? %> | ||
<% if @user.photo.variable? %> | ||
<%= image_tag @user.photo.variant(resize_to_fill: [300, 300]), alt: @user.display_name, title: @user.display_name %> | ||
<% else %> | ||
<%= image_tag @user.photo, alt: @user.display_name, title: @user.display_name %> | ||
<% end %> | ||
<%= link_to user_photo_path(@user), class: "overlay delete-link", method: :delete, data: {confirm: t("dashboard.actions.confirm")}, remote: true do %> | ||
<span><%= t("dashboard.actions.destroy") %></span> | ||
<% end %> | ||
<% else %> | ||
<%= image_tag asset_path("speaker.png"), alt: @user.display_name, title: @user.display_name %> | ||
<% end %> | ||
</span> | ||
<h2> | ||
<div class="small"> | ||
<% if @user.name.present? %> | ||
<%= @user.username %> // | ||
<% end %> | ||
<%= @user.email %> | ||
</div> | ||
<%= @user.display_name %> | ||
<div class="small"> | ||
<span class="badge"><%= User.human_attribute_name("role.#{@user.role}") %></span> | ||
</div> | ||
</h2> | ||
</div> | ||
<% end %> | ||
|
||
<%= form_with url: member_path(@user), model: @user, multipart: true, class: "form aligned", data: {remote: false}, id: "userForm", locale: true do |f| %> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can remove |
||
<%= render partial: "shared/form_errors", locals: { resource: @user } %> | ||
|
||
<fieldset> | ||
<%= f.label :reset_password, for: :user_password %> | ||
<%= f.password_field :password %> | ||
</fieldset> | ||
|
||
<%= f.submit %> | ||
<% end %> | ||
|
||
|
||
<%= render partial: "shared/flash_messages" %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
require 'rails_helper' | ||
|
||
RSpec.describe "SuperAdmin user request", type: :request do | ||
describe "GET edit" do | ||
context "with a super admin privileges" do | ||
let(:super_admin) { FactoryBot.create(:user, role: 100, super_admin: true) } | ||
let(:user) { FactoryBot.create(:user, email: "[email protected]", username: "user_name", role: 2) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case you were unaware, you can reference the roles by their symbols. So, rather than having to integer map |
||
|
||
before do | ||
login_as super_admin | ||
end | ||
|
||
it "should return success if the user is an admin" do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's also add a spec for when the logged-in user is a super admin, but the user itself is not an admin. The controller accounts for this with a redirect, but that is not encapsulated in a spec here. |
||
get "/admin/users/#{user.id}/edit" | ||
expect(response).to have_http_status(200) | ||
end | ||
end | ||
|
||
context "without a super admin privileges" do | ||
let(:other_user) { FactoryBot.create(:user, role: 1) } | ||
let(:user) { FactoryBot.create(:user, email: "[email protected]", username: "user_name", role: 1) } | ||
|
||
before do | ||
login_as other_user | ||
end | ||
|
||
it "should raise an error when the user tries to access super admin" do | ||
assert_raises ActionController::RoutingError do | ||
get "/admin/users/#{user.id}/edit" | ||
end | ||
end | ||
|
||
end | ||
end | ||
|
||
describe "PUT update" do | ||
context "with a super admin privileges" do | ||
let(:super_admin) { FactoryBot.create(:user, role: 100, super_admin: true) } | ||
let(:user) { FactoryBot.create(:user, email: "[email protected]", username: "user_name", role: 2) } | ||
|
||
before do | ||
login_as super_admin | ||
end | ||
it "should return success if the user is an admin" do | ||
put "/admin/users/#{user.id}", :params => {:user => { password: "securePassword" } } | ||
|
||
expect(response).to have_http_status(302) | ||
expect(flash[:notice]).to match('User successfully updated') | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's also add a failure spec here for when the user being updated is not an admin here. |
||
end | ||
|
||
context "without a super admin privileges" do | ||
let(:other_user) { FactoryBot.create(:user, role: 1) } | ||
let(:user) { FactoryBot.create(:user, email: "[email protected]", username: "user_name", role: 1) } | ||
|
||
before do | ||
login_as other_user | ||
end | ||
|
||
it "should raise an error when the user tries to access super admin" do | ||
assert_raises ActionController::RoutingError do | ||
put "/admin/users/#{user.id}/edit" | ||
end | ||
end | ||
|
||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the roles are handled by an enum, we can use the built-in helper methods to check the role: