Skip to content
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

AO3-6672 Allow some admins to set a generic username #4994

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .erb-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ linters:
rule_set:
- deprecated: ["bookmarks", "collections", "readings", "works"]
suggestion: "Avoid the plural form of these classes."
RequireInputAutocomplete:
enabled: false
Rubocop:
enabled: true
rubocop_config:
Expand Down
1 change: 1 addition & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Lint/AmbiguousBlockAssociation:
Exclude:
# Exception for specs where we use change matchers:
# https://github.com/rubocop-hq/rubocop/issues/4222
- 'features/step_definitions/**/*.rb'
- 'spec/**/*.rb'

Lint/AmbiguousRegexpLiteral:
Expand Down
65 changes: 29 additions & 36 deletions app/controllers/pseuds_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,44 +26,40 @@ def index

# GET /users/:user_id/pseuds/:id
def show
if @user.blank?
raise ActiveRecord::RecordNotFound, "Couldn't find user '#{params[:user_id]}'"
end
@pseud = @user.pseuds.find_by(name: params[:id])
unless @pseud
raise ActiveRecord::RecordNotFound, "Couldn't find pseud '#{params[:id]}'"
end
raise ActiveRecord::RecordNotFound, t(".could_not_find_user", username: params[:user_id]) if @user.blank?
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved

@pseud = @user.pseuds.find_by!(name: params[:id])
@page_subtitle = @pseud.name

# very similar to show under users - if you change something here, change it there too
if !(logged_in? || logged_in_as_admin?)
visible_works = @pseud.works.visible_to_all
visible_series = @pseud.series.visible_to_all
visible_bookmarks = @pseud.bookmarks.visible_to_all
else
if logged_in? || logged_in_as_admin?
visible_works = @pseud.works.visible_to_registered_user
visible_series = @pseud.series.visible_to_registered_user
visible_bookmarks = @pseud.bookmarks.visible_to_registered_user
else
visible_works = @pseud.works.visible_to_all
visible_series = @pseud.series.visible_to_all
visible_bookmarks = @pseud.bookmarks.visible_to_all
end

visible_works = visible_works.revealed.non_anon
visible_series = visible_series.exclude_anonymous

@fandoms = \
Fandom.select("tags.*, count(DISTINCT works.id) as work_count").
joins(:filtered_works).group("tags.id").merge(visible_works).
where(filter_taggings: { inherited: false }).
order('work_count DESC').load
Fandom.select("tags.*, count(DISTINCT works.id) as work_count")
.joins(:filtered_works).group("tags.id").merge(visible_works)
.where(filter_taggings: { inherited: false })
.order("work_count DESC").load

@works = visible_works.order("revised_at DESC").limit(ArchiveConfig.NUMBER_OF_ITEMS_VISIBLE_IN_DASHBOARD)
@series = visible_series.order("updated_at DESC").limit(ArchiveConfig.NUMBER_OF_ITEMS_VISIBLE_IN_DASHBOARD)
@bookmarks = visible_bookmarks.order("updated_at DESC").limit(ArchiveConfig.NUMBER_OF_ITEMS_VISIBLE_IN_DASHBOARD)

if current_user.respond_to?(:subscriptions)
@subscription = current_user.subscriptions.where(subscribable_id: @user.id,
subscribable_type: 'User').first ||
current_user.subscriptions.build(subscribable: @user)
end
return unless current_user.respond_to?(:subscriptions)

@subscription = current_user.subscriptions.where(subscribable_id: @user.id,
subscribable_type: "User").first ||
current_user.subscriptions.build(subscribable: @user)
end

# GET /pseuds/new
Expand All @@ -86,7 +82,7 @@ def create
@pseud.user_id = @user.id
old_default = @user.default_pseud
if @pseud.save
flash[:notice] = ts('Pseud was successfully created.')
flash[:notice] = t(".successfully_created")
if @pseud.is_default
# if setting this one as default, unset the attribute of the current default pseud
old_default.update_attribute(:is_default, false)
Expand All @@ -96,8 +92,8 @@ def create
render action: "new"
end
else
# user tried to add pseud he already has
flash[:error] = ts('You already have a pseud with that name.')
# user tried to add pseud they already have
flash[:error] = t(".already_have_pseud_with_name")
render action: "new"
end
end
Expand All @@ -115,12 +111,9 @@ def update
AdminActivity.log_action(current_admin, @pseud, action: "edit pseud", summary: summary)
end
# if setting this one as default, unset the attribute of the current default pseud
if @pseud.is_default and not(default == @pseud)
# if setting this one as default, unset the attribute of the current active pseud
default.update_attribute(:is_default, false)
end
flash[:notice] = ts('Pseud was successfully updated.')
redirect_to([@user, @pseud])
default.update_attribute(:is_default, false) if @pseud.is_default && default != @pseud
flash[:notice] = t(".successfully_updated")
redirect_to([@user, @pseud])
else
render action: "edit"
end
Expand All @@ -131,24 +124,24 @@ def update
def destroy
@hide_dashboard = true
if params[:cancel_button]
flash[:notice] = ts("The pseud was not deleted.")
flash[:notice] = t(".not_deleted")
redirect_to(user_pseuds_path(@user)) && return
end

@pseud = @user.pseuds.find_by(name: params[:id])
if @pseud.is_default
flash[:error] = ts("You cannot delete your default pseudonym, sorry!")
flash[:error] = t(".cannot_delete_default")
elsif @pseud.name == @user.login
flash[:error] = ts("You cannot delete the pseud matching your user name, sorry!")
flash[:error] = t(".cannot_delete_matching_username")
elsif params[:bookmarks_action] == "transfer_bookmarks"
@pseud.change_bookmarks_ownership
@pseud.replace_me_with_default
flash[:notice] = ts("The pseud was successfully deleted.")
flash[:notice] = t(".successfully_deleted")
elsif params[:bookmarks_action] == "delete_bookmarks" || @pseud.bookmarks.empty?
@pseud.replace_me_with_default
flash[:notice] = ts("The pseud was successfully deleted.")
flash[:notice] = t(".successfully_deleted")
else
render 'delete_preview' and return
render "delete_preview" and return
end

redirect_to(user_pseuds_path(@user))
Expand Down
22 changes: 15 additions & 7 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ class UsersController < ApplicationController

before_action :check_user_status, only: [:edit, :update, :change_username, :changed_username]
before_action :load_user, except: [:activate, :delete_confirmation, :index]
before_action :check_ownership, except: [:activate, :delete_confirmation, :edit, :index, :show, :update]
before_action :check_ownership_or_admin, only: [:edit, :update]
before_action :check_ownership, except: [:activate, :change_username, :changed_username, :delete_confirmation, :edit, :index, :show, :update]
before_action :check_ownership_or_admin, only: [:change_username, :changed_username, :edit, :update]
skip_before_action :store_location, only: [:end_first_login]

def load_user
Expand Down Expand Up @@ -48,6 +48,7 @@ def change_password
end

def change_username
authorize @user if logged_in_as_admin?
@page_subtitle = t(".browser_title")
end

Expand All @@ -70,20 +71,27 @@ def changed_password
end

def changed_username
render(:change_username) && return unless params[:new_login].present?
authorize @user if logged_in_as_admin?
render(:change_username) && return if params[:new_login].blank?

@new_login = params[:new_login]

unless @user.valid_password?(params[:password])
flash[:error] = ts('Your password was incorrect')
unless logged_in_as_admin? || @user.valid_password?(params[:password])
flash[:error] = t(".user.incorrect_password")
render(:change_username) && return
end

@user.login = @new_login
@user.ticket_number = params[:ticket_number]

if @user.save
flash[:notice] = ts('Your user name has been successfully updated.')
redirect_to @user
if logged_in_as_admin?
flash[:notice] = t(".admin.successfully_updated")
redirect_to admin_user_path(@user)
else
flash[:notice] = t(".user.successfully_updated")
redirect_to @user
end
else
@user.reload
render :change_username
Expand Down
6 changes: 3 additions & 3 deletions app/models/concerns/justifiable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ module Justifiable
validates :ticket_number,
presence: true,
numericality: { only_integer: true },
if: :enabled?
if: :justification_enabled?

validate :ticket_number_exists_in_tracker, if: :enabled?
validate :ticket_number_exists_in_tracker, if: :justification_enabled?
end

private

def enabled?
def justification_enabled?
# Only require a ticket if the record has been changed by an admin.
User.current_user.is_a?(Admin) && changed?
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/pseud.rb
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ def self.parse_byline(byline)

# Parse a string of the "pseud.name (user.login)" format into a list of
# pseuds. Usually this will be just one pseud, but if the byline is of the
# form "pseud.name" with no parenthesized user name, it'll look for any pseud
# form "pseud.name" with no parenthesized username, it'll look for any pseud
# with that name.
def self.parse_byline_ambiguous(byline)
pseud_name, login = split_byline(byline)
Expand Down
36 changes: 33 additions & 3 deletions app/models/user.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class User < ApplicationRecord
audited redacted: [:encrypted_password, :password_salt]
include Justifiable
include WorksOwner
include PasswordResetsLimitable
include UserLoggable
Expand Down Expand Up @@ -90,7 +91,7 @@ class User < ApplicationRecord
before_update :add_renamed_at, if: :will_save_change_to_login?
after_update :update_pseud_name
after_update :send_wrangler_username_change_notification, if: :is_tag_wrangler?
after_update :log_change_if_login_was_edited
after_update :log_change_if_login_was_edited, if: :saved_change_to_login?
after_update :log_email_change, if: :saved_change_to_email?

after_commit :reindex_user_creations_after_rename
Expand Down Expand Up @@ -198,6 +199,7 @@ def unread_inbox_comments_count
uniqueness: true,
not_forbidden_name: { if: :will_save_change_to_login? }
validate :username_is_not_recently_changed, if: :will_save_change_to_login?
validate :admin_username_generic, if: :will_save_change_to_login?

# allow nil so can save existing users
validates_length_of :password,
Expand Down Expand Up @@ -525,6 +527,12 @@ def reindex_user_creations

private

# Override the default Justifiable enabled check, because we only need to justify
# username changes at the moment.
def justification_enabled?
User.current_user.is_a?(Admin) && login_changed?
end

# Create and/or return a user account for holding orphaned works
def self.fetch_orphan_account
orphan_account = User.find_or_create_by(login: "orphan_account")
Expand Down Expand Up @@ -564,11 +572,25 @@ def reindex_user_creations_after_rename
end

def add_renamed_at
self.renamed_at = Time.current
if User.current_user == self
self.renamed_at = Time.current
else
self.admin_renamed_at = Time.current
end
end

def log_change_if_login_was_edited
create_log_item(action: ArchiveConfig.ACTION_RENAME, note: "Old Username: #{login_before_last_save}; New Username: #{login}") if saved_change_to_login?
current_admin = User.current_user if User.current_user.is_a?(Admin)
options = {
action: ArchiveConfig.ACTION_RENAME,
admin: current_admin
}
options[:note] = if current_admin
"Old Username: #{login_before_last_save}, New Username: #{login}, Changed by: #{current_admin.login}, Ticket ID: ##{ticket_number}"
else
"Old Username: #{login_before_last_save}; New Username: #{login}"
end
create_log_item(options)
end

def send_wrangler_username_change_notification
Expand All @@ -592,6 +614,8 @@ def remove_stale_from_autocomplete
end

def username_is_not_recently_changed
return if User.current_user.is_a?(Admin)

change_interval_days = ArchiveConfig.USER_RENAME_LIMIT_DAYS
return unless renamed_at && change_interval_days.days.ago <= renamed_at

Expand All @@ -601,6 +625,12 @@ def username_is_not_recently_changed
renamed_at: I18n.l(renamed_at, format: :long))
end

def admin_username_generic
return unless User.current_user.is_a?(Admin)

errors.add(:login, :admin_must_use_default) unless login == "user#{id}"
end

# Extra callback to make sure readings are deleted in an order consistent
# with the ReadingsJob.
#
Expand Down
12 changes: 10 additions & 2 deletions app/policies/user_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ class UserPolicy < ApplicationPolicy
# This is further restricted using ALLOWED_ATTRIBUTES_BY_ROLES.
MANAGE_ROLES = %w[superadmin legal policy_and_abuse open_doors support tag_wrangling].freeze

# Roles that are allowed to set a generic username for users.
CHANGE_USERNAME_ROLES = %w[superadmin policy_and_abuse].freeze

# Roles that allow updating the Fannish Next Of Kin of a user.
MANAGE_NEXT_OF_KIN_ROLES = %w[superadmin policy_and_abuse support].freeze

Expand All @@ -18,8 +21,8 @@ class UserPolicy < ApplicationPolicy
# Define which roles can update which attributes.
ALLOWED_ATTRIBUTES_BY_ROLES = {
"open_doors" => [roles: []],
"policy_and_abuse" => [:email, roles: []],
"superadmin" => [:email, roles: []],
"policy_and_abuse" => [:email, { roles: [] }],
"superadmin" => [:email, { roles: [] }],
"support" => %i[email],
"tag_wrangling" => [roles: []]
}.freeze
Expand Down Expand Up @@ -48,6 +51,10 @@ def can_access_creation_summary?
user_has_roles?(REVIEW_CREATIONS_ROLES)
end

def change_username?
user_has_roles?(CHANGE_USERNAME_ROLES)
end

def permitted_attributes
ALLOWED_ATTRIBUTES_BY_ROLES.values_at(*user.roles).compact.flatten
end
Expand All @@ -60,6 +67,7 @@ def can_edit_user_role?(role)
alias bulk_search? can_manage_users?
alias show? can_manage_users?
alias update? can_manage_users?
alias changed_username? change_username?

alias update_next_of_kin? can_manage_next_of_kin?

Expand Down
22 changes: 11 additions & 11 deletions app/views/admin/admin_invitations/find.html.erb
Original file line number Diff line number Diff line change
@@ -1,30 +1,30 @@
<!--Descriptive page name, messages and instructions-->
<h2 class="heading"><%h= 'Track invitations' %></h2>
<h2 class="heading"><%= t(".page_heading") %></h2>
<!--/descriptions-->

<!--subnav-->
<!--/subnav-->

<!--main content-->
<%= form_tag url_for(:controller => 'admin/admin_invitations', :action => 'find'), :method => :get do %>
<%= form_tag url_for(controller: "admin/admin_invitations", action: "find"), autocomplete: "off", method: :get do %>
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
<dl>
<dt><%= label_tag "invitation[user_name]", t(".user_name") %>:</dt>
<dd><%= text_field_tag "invitation[user_name]", params[:invitation][:user_name] %></dd>
<dt><%= label_tag "invitation[token]", t(".token") %>:</dt>
<dd><%= text_field_tag "invitation[token]", params[:invitation][:token] %></dd>
<dt><%= label_tag "invitee_email", t(".email") %>:</dt>
<dt><%= label_tag "invitation[user_name]", t(".username") %></dt>
<dd><%= text_field_tag "invitation[user_name]", params[:invitation][:user_name] %></dd>
<dt><%= label_tag "invitation[token]", t(".token") %></dt>
<dd><%= text_field_tag "invitation[token]", params[:invitation][:token] %></dd>
<dt><%= label_tag "invitee_email", t(".email") %></dt>
<dd><%= text_field_tag "invitation[invitee_email]", params[:invitation][:invitee_email], id: "invitee_email" %></dd>
</dl>
<p class="submit actions"><%= submit_tag "Go" %></p>
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
<% end %>

<% if @user %>
<h3 class="heading"><%h= 'Invitations for' %> <%= link_to @user.login, user_invitations_path(@user) %></h3>
<%= render :partial => 'invitations/user_invitations', :locals => {:invitations => @invitations} %>
<h3 class="heading"><%= t(".invitations_for") %> <%= link_to @user.login, user_invitations_path(@user) %></h3>
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
<%= render partial: "invitations/user_invitations", invitations: @invitations %>
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
<% elsif @invitation %>
<%= render :partial => 'invitations/invitation', :locals => {:invitation => @invitation} %>
<%= render partial: "invitations/invitation", invitation: @invitation %>
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
<% elsif @invitations %>
<%= render :partial => 'invitations/user_invitations', :locals => {:invitations => @invitations} %>
<%= render partial: "invitations/user_invitations", invitations: @invitations %>
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
<% end %>
<!--/content-->

Expand Down
Loading
Loading