users: add Restricted user level.

Add a Restricted user level. Restricted users are level 10, below
Members. New users start out as Restricted if they sign up from a proxy
or an IP recently used by another user.

Restricted users can't update or edit any public content on the site
until they verify their email address, at which point they're promoted
to Member. Restricted users are only allowed to do personal actions
like keep favorites, keep favgroups and saved searches, mark dmails as
read or deleted, or mark forum posts as read.

The restricted state already existed before, the only change here is
that now it's an actual user level instead of a hidden state. Before it
was based on two hidden flags on the user, the `requires_verification`
flag (set when a user signs up from a proxy, etc), and the `is_verified`
flag (set after the user verifies their email). Making it a user level
means that now the Restricted status will be shown publicly.

Introducing a new level below Member means that we have to change every
`is_member?` check to `!is_anonymous` for every place where we used
`is_member?` to check that the current user is logged in.
This commit is contained in:
evazion
2021-01-06 21:56:57 -06:00
parent da3e8e4726
commit 94e125709c
29 changed files with 140 additions and 65 deletions

View File

@@ -49,7 +49,7 @@ class EmailsController < ApplicationController
redirect_to edit_user_email_path(@user) redirect_to edit_user_email_path(@user)
elsif params[:email_verification_key].present? elsif params[:email_verification_key].present?
authorize @email_address authorize @email_address
@email_address.update!(is_verified: true) @email_address.verify!
flash[:notice] = "Email address verified" flash[:notice] = "Email address verified"
redirect_to @email_address.user redirect_to @email_address.user
else else

View File

@@ -11,7 +11,7 @@ class FavoritesController < ApplicationController
elsif params[:user_id].present? elsif params[:user_id].present?
user = User.find(params[:user_id]) user = User.find(params[:user_id])
redirect_to posts_path(tags: "ordfav:#{user.name}", format: request.format.symbol) redirect_to posts_path(tags: "ordfav:#{user.name}", format: request.format.symbol)
elsif CurrentUser.is_member? elsif !CurrentUser.is_anonymous?
redirect_to posts_path(tags: "ordfav:#{CurrentUser.name}", format: request.format.symbol) redirect_to posts_path(tags: "ordfav:#{CurrentUser.name}", format: request.format.symbol)
else else
redirect_to posts_path(format: request.format.symbol) redirect_to posts_path(format: request.format.symbol)

View File

@@ -48,7 +48,7 @@ class UsersController < ApplicationController
def profile def profile
@user = authorize CurrentUser.user @user = authorize CurrentUser.user
if @user.is_member? if !@user.is_anonymous?
params[:action] = "show" params[:action] = "show"
respond_with(@user, methods: @user.full_attributes, template: "users/show") respond_with(@user, methods: @user.full_attributes, template: "users/show")
elsif request.format.html? elsif request.format.html?
@@ -59,11 +59,12 @@ class UsersController < ApplicationController
end end
def create def create
requires_verification = UserVerifier.new(CurrentUser.user, request).requires_verification? user_verifier = UserVerifier.new(CurrentUser.user, request)
@user = authorize User.new( @user = authorize User.new(
last_ip_addr: CurrentUser.ip_addr, last_ip_addr: CurrentUser.ip_addr,
requires_verification: requires_verification, requires_verification: user_verifier.requires_verification?,
level: user_verifier.initial_level,
name: params[:user][:name], name: params[:user][:name],
password: params[:user][:password], password: params[:user][:password],
password_confirmation: params[:user][:password_confirmation] password_confirmation: params[:user][:password_confirmation]

View File

@@ -205,6 +205,7 @@
--user-platinum-color: gray; --user-platinum-color: gray;
--user-gold-color: #00F; --user-gold-color: #00F;
--user-member-color: var(--link-color); --user-member-color: var(--link-color);
--user-restricted-color: var(--link-color);
--user-banned-color: black; --user-banned-color: black;
--user-verified-email-color: #0A0; --user-verified-email-color: #0A0;
@@ -294,6 +295,7 @@ body[data-current-user-theme="dark"] {
--collection-pool-hover-color: var(--general-tag-hover-color); --collection-pool-hover-color: var(--general-tag-hover-color);
--user-banned-color: var(--grey-1); --user-banned-color: var(--grey-1);
--user-restricted-color: var(--blue-1);
--user-member-color: var(--blue-1); --user-member-color: var(--blue-1);
--user-gold-color: var(--yellow-1); --user-gold-color: var(--yellow-1);
--user-platinum-color: var(--grey-4); --user-platinum-color: var(--grey-4);

View File

@@ -26,4 +26,8 @@ body[data-current-user-style-usernames="true"] {
a.user-member { a.user-member {
color: var(--user-member-color); color: var(--user-member-color);
} }
a.user-restricted {
color: var(--user-restricted-color);
}
} }

View File

@@ -36,6 +36,7 @@
&.user-tooltip-badge-platinum { background-color: var(--user-platinum-color); } &.user-tooltip-badge-platinum { background-color: var(--user-platinum-color); }
&.user-tooltip-badge-gold { background-color: var(--user-gold-color); } &.user-tooltip-badge-gold { background-color: var(--user-gold-color); }
&.user-tooltip-badge-member { background-color: var(--user-member-color); } &.user-tooltip-badge-member { background-color: var(--user-member-color); }
&.user-tooltip-badge-restricted { background-color: var(--user-restricted-color); }
&.user-tooltip-badge-banned { background-color: var(--user-banned-color); } &.user-tooltip-badge-banned { background-color: var(--user-banned-color); }
&.user-tooltip-badge-positive-feedback { &.user-tooltip-badge-positive-feedback {

View File

@@ -1,6 +1,8 @@
# Checks whether a new account seems suspicious and should require email verification. # Checks whether a new account seems suspicious and should require email verification.
class UserVerifier class UserVerifier
extend Memoist
attr_reader :current_user, :request attr_reader :current_user, :request
# current_user is the user creating the new account, not the new account itself. # current_user is the user creating the new account, not the new account itself.
@@ -16,6 +18,14 @@ class UserVerifier
is_ip_banned? || is_logged_in? || is_recent_signup? || is_proxy? is_ip_banned? || is_logged_in? || is_recent_signup? || is_proxy?
end end
def initial_level
if requires_verification?
User::Levels::RESTRICTED
else
User::Levels::MEMBER
end
end
private private
def ip_address def ip_address
@@ -48,4 +58,6 @@ class UserVerifier
def is_proxy? def is_proxy?
IpLookup.new(ip_address).is_proxy? IpLookup.new(ip_address).is_proxy?
end end
memoize :is_ip_banned?, :is_proxy?, :is_recent_signup?
end end

View File

@@ -11,10 +11,10 @@ class CommentVote < ApplicationRecord
def self.visible(user) def self.visible(user)
if user.is_moderator? if user.is_moderator?
all all
elsif user.is_member? elsif user.is_anonymous?
where(user: user)
else
none none
else
where(user: user)
end end
end end

View File

@@ -5,7 +5,6 @@ class EmailAddress < ApplicationRecord
validates :normalized_address, uniqueness: true validates :normalized_address, uniqueness: true
validates :user_id, uniqueness: true validates :user_id, uniqueness: true
validate :validate_deliverable, on: :deliverable validate :validate_deliverable, on: :deliverable
after_save :update_user
def self.visible(user) def self.visible(user)
if user.is_moderator? if user.is_moderator?
@@ -60,8 +59,14 @@ class EmailAddress < ApplicationRecord
end end
end end
def update_user def verify!
user.update!(is_verified: is_verified? && !is_restricted?) transaction do
update!(is_verified: true)
if user.is_restricted? && !is_restricted?
user.update!(level: User::Levels::MEMBER, is_verified: is_verified?)
end
end
end end
concerning :VerificationMethods do concerning :VerificationMethods do

View File

@@ -93,10 +93,10 @@ class Upload < ApplicationRecord
def self.visible(user) def self.visible(user)
if user.is_admin? if user.is_admin?
all all
elsif user.is_member? elsif user.is_anonymous?
completed.or(where(uploader: user))
else
completed completed
else
completed.or(where(uploader: user))
end end
end end

View File

@@ -6,6 +6,7 @@ class User < ApplicationRecord
module Levels module Levels
ANONYMOUS = 0 ANONYMOUS = 0
RESTRICTED = 10
MEMBER = 20 MEMBER = 20
GOLD = 30 GOLD = 30
PLATINUM = 31 PLATINUM = 31
@@ -211,6 +212,7 @@ class User < ApplicationRecord
def level_hash def level_hash
return { return {
"Member" => Levels::MEMBER, "Member" => Levels::MEMBER,
"Restricted" => Levels::RESTRICTED,
"Gold" => Levels::GOLD, "Gold" => Levels::GOLD,
"Platinum" => Levels::PLATINUM, "Platinum" => Levels::PLATINUM,
"Builder" => Levels::BUILDER, "Builder" => Levels::BUILDER,
@@ -225,6 +227,9 @@ class User < ApplicationRecord
when Levels::ANONYMOUS when Levels::ANONYMOUS
"Anonymous" "Anonymous"
when Levels::RESTRICTED
"Restricted"
when Levels::MEMBER when Levels::MEMBER
"Member" "Member"
@@ -278,14 +283,14 @@ class User < ApplicationRecord
name.match?(/\Auser_[0-9]+~*\z/) name.match?(/\Auser_[0-9]+~*\z/)
end end
def is_restricted?
requires_verification? && !is_verified?
end
def is_anonymous? def is_anonymous?
level == Levels::ANONYMOUS level == Levels::ANONYMOUS
end end
def is_restricted?
level == Levels::RESTRICTED
end
def is_member? def is_member?
level >= Levels::MEMBER level >= Levels::MEMBER
end end

View File

@@ -11,10 +11,10 @@ class UserNameChangeRequest < ApplicationRecord
def self.visible(user) def self.visible(user)
if user.is_moderator? if user.is_moderator?
all all
elsif user.is_member? elsif user.is_anonymous?
where(user: User.undeleted)
else
none none
else
where(user: User.undeleted)
end end
end end

View File

@@ -39,11 +39,7 @@ class ApplicationPolicy
end end
def unbanned? def unbanned?
user.is_member? && !user.is_banned? && verified? user.is_member? && !user.is_banned? && !user.is_restricted?
end
def verified?
user.is_verified? || user.is_gold? || !user.requires_verification?
end end
def policy(object) def policy(object)

View File

@@ -12,7 +12,7 @@ class ArtistPolicy < ApplicationPolicy
end end
def can_view_banned? def can_view_banned?
user.is_member? !user.is_anonymous?
end end
def permitted_attributes def permitted_attributes

View File

@@ -4,20 +4,20 @@ class DmailPolicy < ApplicationPolicy
end end
def index? def index?
user.is_member? !user.is_anonymous?
end end
def mark_all_as_read? def mark_all_as_read?
user.is_member? !user.is_anonymous?
end end
def update? def update?
user.is_member? && record.owner_id == user.id !user.is_anonymous? && record.owner_id == user.id
end end
def show? def show?
return true if user.is_owner? return true if user.is_owner?
user.is_member? && (record.owner_id == user.id || record.valid_key?(request.params[:key])) !user.is_anonymous? && (record.owner_id == user.id || record.valid_key?(request.params[:key]))
end end
def reportable? def reportable?

View File

@@ -4,7 +4,7 @@ class FavoriteGroupPolicy < ApplicationPolicy
end end
def create? def create?
user.is_member? !user.is_anonymous?
end end
def update? def update?

View File

@@ -1,9 +1,9 @@
class FavoritePolicy < ApplicationPolicy class FavoritePolicy < ApplicationPolicy
def create? def create?
user.is_member? !user.is_anonymous?
end end
def destroy? def destroy?
user.is_member? !user.is_anonymous?
end end
end end

View File

@@ -20,7 +20,7 @@ class ForumTopicPolicy < ApplicationPolicy
end end
def mark_all_as_read? def mark_all_as_read?
user.is_member? !user.is_anonymous?
end end
def reply? def reply?

View File

@@ -1,10 +1,10 @@
class SavedSearchPolicy < ApplicationPolicy class SavedSearchPolicy < ApplicationPolicy
def index? def index?
user.is_member? !user.is_anonymous?
end end
def create? def create?
user.is_member? !user.is_anonymous?
end end
def update? def update?

View File

@@ -1,10 +1,10 @@
class UserNameChangeRequestPolicy < ApplicationPolicy class UserNameChangeRequestPolicy < ApplicationPolicy
def index? def index?
user.is_member? !user.is_anonymous?
end end
def show? def show?
user.is_moderator? || (user.is_member? && !record.user.is_deleted?) || (record.user == user) user.is_moderator? || (!user.is_anonymous? && !record.user.is_deleted?) || (record.user == user)
end end
def permitted_attributes def permitted_attributes

View File

@@ -16,7 +16,7 @@ class UserPolicy < ApplicationPolicy
end end
def upgrade? def upgrade?
user.is_member? !user.is_anonymous?
end end
def reportable? def reportable?
@@ -24,7 +24,7 @@ class UserPolicy < ApplicationPolicy
end end
def fix_counts? def fix_counts?
user.is_member? !user.is_anonymous?
end end
def can_see_last_logged_in_at? def can_see_last_logged_in_at?

View File

@@ -1,6 +1,6 @@
class UserUpgradePolicy < ApplicationPolicy class UserUpgradePolicy < ApplicationPolicy
def create? def create?
user.is_member? !user.is_anonymous?
end end
def new? def new?

View File

@@ -17,7 +17,7 @@
<%= subnav_link_to "Search", search_forum_posts_path %> <%= subnav_link_to "Search", search_forum_posts_path %>
<%= subnav_link_to "Help", wiki_page_path("help:forum") %> <%= subnav_link_to "Help", wiki_page_path("help:forum") %>
<% if CurrentUser.is_member? && @forum_topic && !@forum_topic.new_record? %> <% if !CurrentUser.user.is_anonymous? && @forum_topic && !@forum_topic.new_record? %>
<li>|</li> <li>|</li>
<%= subnav_link_to "Reply", new_forum_post_path(:topic_id => @forum_topic.id) %> <%= subnav_link_to "Reply", new_forum_post_path(:topic_id => @forum_topic.id) %>
<% if !@forum_topic.new_record? && policy(@forum_topic).update? %> <% if !@forum_topic.new_record? && policy(@forum_topic).update? %>

View File

@@ -19,7 +19,7 @@
</div> </div>
<% end %> <% end %>
<% if CurrentUser.user.is_member? && post_set.tag.present? && post_set.current_page == 1 %> <% if !CurrentUser.user.is_anonymous? && post_set.tag.present? && post_set.current_page == 1 %>
<% cache("tag-change-notice:#{post_set.tag.name}", expires_in: 4.hours) do %> <% cache("tag-change-notice:#{post_set.tag.name}", expires_in: 4.hours) do %>
<% if post_set.pending_bulk_update_requests.present? %> <% if post_set.pending_bulk_update_requests.present? %>
<div class="fineprint tag-change-notice"> <div class="fineprint tag-change-notice">

View File

@@ -0,0 +1,9 @@
#!/usr/bin/env ruby
require_relative "../../config/environment"
User.transaction do
members = User.where(level: User::Levels::MEMBER)
restricted = members.bit_prefs_match(:requires_verification, true).bit_prefs_match(:is_verified, false)
restricted.update_all(level: User::Levels::RESTRICTED)
end

View File

@@ -15,6 +15,12 @@ FactoryBot.define do
is_banned {true} is_banned {true}
end end
factory(:restricted_user) do
level {10}
requires_verification { true }
is_verified { false }
end
factory(:member_user) do factory(:member_user) do
level {20} level {20}
end end

View File

@@ -7,7 +7,7 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest
setup do setup do
@user = create(:user, email_address: build(:email_address, { address: "bob@ogres.net", is_verified: false })) @user = create(:user, email_address: build(:email_address, { address: "bob@ogres.net", is_verified: false }))
@other_user = create(:user, email_address: build(:email_address, { address: "alice@ogres.net", is_verified: false })) @other_user = create(:user, email_address: build(:email_address, { address: "alice@ogres.net", is_verified: false }))
@restricted_user = create(:user, requires_verification: true, is_verified: false) @restricted_user = create(:restricted_user, email_address: build(:email_address, { is_verified: false }))
end end
context "#index" do context "#index" do
@@ -136,27 +136,47 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest
end end
end end
context "with a nondisposable email address" do context "for a Restricted user" do
should "mark the user as verified" do context "with a nondisposable email address" do
Danbooru.config.stubs(:email_domain_verification_list).returns(["gmail.com"]) should "unrestrict the user's account" do
@user.email_address.update!(address: "test@gmail.com") Danbooru.config.stubs(:email_domain_verification_list).returns(["gmail.com"])
get email_verification_url(@user) @restricted_user.email_address.update!(address: "test@gmail.com")
assert_redirected_to @user get email_verification_url(@restricted_user)
assert_equal(true, @user.reload.email_address.is_verified)
assert_equal(true, @user.is_verified) assert_redirected_to @restricted_user
assert_equal(true, @restricted_user.reload.email_address.is_verified)
assert_equal(false, @restricted_user.is_restricted?)
assert_equal(true, @restricted_user.is_member?)
end
end
context "with a disposable email address" do
should "leave the user's account restricted" do
Danbooru.config.stubs(:email_domain_verification_list).returns(["gmail.com"])
@restricted_user.email_address.update!(address: "test@mailinator.com")
get email_verification_url(@restricted_user)
assert_redirected_to @restricted_user
assert_equal(true, @restricted_user.reload.email_address.is_verified)
assert_equal(true, @restricted_user.is_restricted?)
assert_equal(false, @restricted_user.is_member?)
end
end end
end end
context "with a disposable email address" do context "for a Gold user" do
should "not mark the user as verified" do should "not change the user's level" do
@user = create(:gold_user, email_address: build(:email_address, { address: "test@gmail.com", is_verified: false }))
Danbooru.config.stubs(:email_domain_verification_list).returns(["gmail.com"]) Danbooru.config.stubs(:email_domain_verification_list).returns(["gmail.com"])
@user.email_address.update!(address: "test@mailinator.com")
get email_verification_url(@user) get email_verification_url(@user)
assert_redirected_to @user assert_redirected_to @user
assert_equal(true, @user.reload.email_address.is_verified) assert_equal(true, @user.reload.email_address.is_verified)
assert_equal(false, @user.is_verified) assert_equal(false, @user.is_restricted?)
assert_equal(true, @user.is_gold?)
end end
end end

View File

@@ -659,8 +659,7 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
end end
should "not allow unverified users to update posts" do should "not allow unverified users to update posts" do
@user.update!(requires_verification: true, is_verified: false) put_auth post_path(@post), create(:restricted_user), params: { post: { tag_string: "blah" }}
put_auth post_path(@post), @user, params: { post: { tag_string: "blah" }}
assert_response 403 assert_response 403
assert_not_equal("blah", @post.reload.tag_string) assert_not_equal("blah", @post.reload.tag_string)
end end

View File

@@ -256,6 +256,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to User.last assert_redirected_to User.last
assert_equal("xxx", User.last.name) assert_equal("xxx", User.last.name)
assert_equal(User::Levels::MEMBER, User.last.level)
assert_equal(User.last, User.last.authenticate_password("xxxxx1")) assert_equal(User.last, User.last.authenticate_password("xxxxx1"))
assert_nil(User.last.email_address) assert_nil(User.last.email_address)
assert_no_enqueued_emails assert_no_enqueued_emails
@@ -303,66 +304,80 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }} post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }}
assert_redirected_to User.last assert_redirected_to User.last
assert_equal(true, User.last.is_member?)
assert_equal(false, User.last.is_restricted?)
assert_equal(false, User.last.requires_verification) assert_equal(false, User.last.requires_verification)
end end
should "mark accounts created by already logged in users as requiring verification" do should "mark accounts created by already logged in users as restricted" do
self.remote_addr = @valid_ip self.remote_addr = @valid_ip
post_auth users_path, @user, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }} post_auth users_path, @user, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }}
assert_redirected_to User.last assert_redirected_to User.last
assert_equal(false, User.last.is_member?)
assert_equal(true, User.last.is_restricted?)
assert_equal(true, User.last.requires_verification) assert_equal(true, User.last.requires_verification)
end end
should "mark users signing up from proxies as requiring verification" do should "mark users signing up from proxies as restricted" do
skip unless IpLookup.enabled? skip unless IpLookup.enabled?
self.remote_addr = @proxy_ip self.remote_addr = @proxy_ip
post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }} post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }}
assert_redirected_to User.last assert_redirected_to User.last
assert_equal(false, User.last.is_member?)
assert_equal(true, User.last.is_restricted?)
assert_equal(true, User.last.requires_verification) assert_equal(true, User.last.requires_verification)
end end
should "mark users signing up from a partial banned IP as requiring verification" do should "mark users signing up from a partial banned IP as restricted" do
self.remote_addr = @valid_ip self.remote_addr = @valid_ip
@ip_ban = create(:ip_ban, ip_addr: self.remote_addr, category: :partial) @ip_ban = create(:ip_ban, ip_addr: self.remote_addr, category: :partial)
post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }} post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }}
assert_redirected_to User.last assert_redirected_to User.last
assert_equal(false, User.last.is_member?)
assert_equal(true, User.last.is_restricted?)
assert_equal(true, User.last.requires_verification) assert_equal(true, User.last.requires_verification)
assert_equal(1, @ip_ban.reload.hit_count) assert_equal(1, @ip_ban.reload.hit_count)
assert(@ip_ban.last_hit_at > 1.minute.ago) assert(@ip_ban.last_hit_at > 1.minute.ago)
end end
should "not mark users signing up from non-proxies as requiring verification" do should "not mark users signing up from non-proxies as restricted" do
skip unless IpLookup.enabled? skip unless IpLookup.enabled?
self.remote_addr = @valid_ip self.remote_addr = @valid_ip
post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }} post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }}
assert_redirected_to User.last assert_redirected_to User.last
assert_equal(true, User.last.is_member?)
assert_equal(false, User.last.is_restricted?)
assert_equal(false, User.last.requires_verification) assert_equal(false, User.last.requires_verification)
end end
should "mark accounts registered from an IPv4 address recently used for another account as requiring verification" do should "mark accounts registered from an IPv4 address recently used for another account as restricted" do
@user.update!(last_ip_addr: @valid_ip) @user.update!(last_ip_addr: @valid_ip)
self.remote_addr = @valid_ip self.remote_addr = @valid_ip
post users_path, params: { user: { name: "dupe", password: "xxxxx1", password_confirmation: "xxxxx1" }} post users_path, params: { user: { name: "dupe", password: "xxxxx1", password_confirmation: "xxxxx1" }}
assert_redirected_to User.last assert_redirected_to User.last
assert_equal(false, User.last.is_member?)
assert_equal(true, User.last.is_restricted?)
assert_equal(true, User.last.requires_verification) assert_equal(true, User.last.requires_verification)
end end
should "not mark users signing up from localhost as requiring verification" do should "not mark users signing up from localhost as restricted" do
self.remote_addr = "127.0.0.1" self.remote_addr = "127.0.0.1"
post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }} post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }}
assert_redirected_to User.last assert_redirected_to User.last
assert_equal(true, User.last.is_member?)
assert_equal(false, User.last.is_restricted?)
assert_equal(false, User.last.requires_verification) assert_equal(false, User.last.requires_verification)
end end
end end