diff --git a/app/controllers/emails_controller.rb b/app/controllers/emails_controller.rb index 2eb034ecf..6d8eb3f19 100644 --- a/app/controllers/emails_controller.rb +++ b/app/controllers/emails_controller.rb @@ -49,7 +49,7 @@ class EmailsController < ApplicationController redirect_to edit_user_email_path(@user) elsif params[:email_verification_key].present? authorize @email_address - @email_address.update!(is_verified: true) + @email_address.verify! flash[:notice] = "Email address verified" redirect_to @email_address.user else diff --git a/app/controllers/favorites_controller.rb b/app/controllers/favorites_controller.rb index ba293bb95..76f8beb3b 100644 --- a/app/controllers/favorites_controller.rb +++ b/app/controllers/favorites_controller.rb @@ -11,7 +11,7 @@ class FavoritesController < ApplicationController elsif params[:user_id].present? user = User.find(params[:user_id]) 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) else redirect_to posts_path(format: request.format.symbol) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f28cd9390..a53d97ade 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -48,7 +48,7 @@ class UsersController < ApplicationController def profile @user = authorize CurrentUser.user - if @user.is_member? + if !@user.is_anonymous? params[:action] = "show" respond_with(@user, methods: @user.full_attributes, template: "users/show") elsif request.format.html? @@ -59,11 +59,12 @@ class UsersController < ApplicationController end def create - requires_verification = UserVerifier.new(CurrentUser.user, request).requires_verification? + user_verifier = UserVerifier.new(CurrentUser.user, request) @user = authorize User.new( 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], password: params[:user][:password], password_confirmation: params[:user][:password_confirmation] diff --git a/app/javascript/src/styles/base/040_colors.css b/app/javascript/src/styles/base/040_colors.css index 5441c46f4..472052818 100644 --- a/app/javascript/src/styles/base/040_colors.css +++ b/app/javascript/src/styles/base/040_colors.css @@ -205,6 +205,7 @@ --user-platinum-color: gray; --user-gold-color: #00F; --user-member-color: var(--link-color); + --user-restricted-color: var(--link-color); --user-banned-color: black; --user-verified-email-color: #0A0; @@ -294,6 +295,7 @@ body[data-current-user-theme="dark"] { --collection-pool-hover-color: var(--general-tag-hover-color); --user-banned-color: var(--grey-1); + --user-restricted-color: var(--blue-1); --user-member-color: var(--blue-1); --user-gold-color: var(--yellow-1); --user-platinum-color: var(--grey-4); diff --git a/app/javascript/src/styles/common/user_styles.scss b/app/javascript/src/styles/common/user_styles.scss index 14aff9a51..9132bf835 100644 --- a/app/javascript/src/styles/common/user_styles.scss +++ b/app/javascript/src/styles/common/user_styles.scss @@ -26,4 +26,8 @@ body[data-current-user-style-usernames="true"] { a.user-member { color: var(--user-member-color); } + + a.user-restricted { + color: var(--user-restricted-color); + } } diff --git a/app/javascript/src/styles/specific/user_tooltips.scss b/app/javascript/src/styles/specific/user_tooltips.scss index 881dae1d6..8b0fc6be9 100644 --- a/app/javascript/src/styles/specific/user_tooltips.scss +++ b/app/javascript/src/styles/specific/user_tooltips.scss @@ -36,6 +36,7 @@ &.user-tooltip-badge-platinum { background-color: var(--user-platinum-color); } &.user-tooltip-badge-gold { background-color: var(--user-gold-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-positive-feedback { diff --git a/app/logical/user_verifier.rb b/app/logical/user_verifier.rb index d1e5991b9..3b493a51c 100644 --- a/app/logical/user_verifier.rb +++ b/app/logical/user_verifier.rb @@ -1,6 +1,8 @@ # Checks whether a new account seems suspicious and should require email verification. class UserVerifier + extend Memoist + attr_reader :current_user, :request # 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? end + def initial_level + if requires_verification? + User::Levels::RESTRICTED + else + User::Levels::MEMBER + end + end + private def ip_address @@ -48,4 +58,6 @@ class UserVerifier def is_proxy? IpLookup.new(ip_address).is_proxy? end + + memoize :is_ip_banned?, :is_proxy?, :is_recent_signup? end diff --git a/app/models/comment_vote.rb b/app/models/comment_vote.rb index b87642d10..0df3cc952 100644 --- a/app/models/comment_vote.rb +++ b/app/models/comment_vote.rb @@ -11,10 +11,10 @@ class CommentVote < ApplicationRecord def self.visible(user) if user.is_moderator? all - elsif user.is_member? - where(user: user) - else + elsif user.is_anonymous? none + else + where(user: user) end end diff --git a/app/models/email_address.rb b/app/models/email_address.rb index f0420a734..7d437751d 100644 --- a/app/models/email_address.rb +++ b/app/models/email_address.rb @@ -5,7 +5,6 @@ class EmailAddress < ApplicationRecord validates :normalized_address, uniqueness: true validates :user_id, uniqueness: true validate :validate_deliverable, on: :deliverable - after_save :update_user def self.visible(user) if user.is_moderator? @@ -60,8 +59,14 @@ class EmailAddress < ApplicationRecord end end - def update_user - user.update!(is_verified: is_verified? && !is_restricted?) + def verify! + 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 concerning :VerificationMethods do diff --git a/app/models/upload.rb b/app/models/upload.rb index 6c2878076..9bc97dfc5 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -93,10 +93,10 @@ class Upload < ApplicationRecord def self.visible(user) if user.is_admin? all - elsif user.is_member? - completed.or(where(uploader: user)) - else + elsif user.is_anonymous? completed + else + completed.or(where(uploader: user)) end end diff --git a/app/models/user.rb b/app/models/user.rb index 9b47b01d6..c48b5c620 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -6,6 +6,7 @@ class User < ApplicationRecord module Levels ANONYMOUS = 0 + RESTRICTED = 10 MEMBER = 20 GOLD = 30 PLATINUM = 31 @@ -211,6 +212,7 @@ class User < ApplicationRecord def level_hash return { "Member" => Levels::MEMBER, + "Restricted" => Levels::RESTRICTED, "Gold" => Levels::GOLD, "Platinum" => Levels::PLATINUM, "Builder" => Levels::BUILDER, @@ -225,6 +227,9 @@ class User < ApplicationRecord when Levels::ANONYMOUS "Anonymous" + when Levels::RESTRICTED + "Restricted" + when Levels::MEMBER "Member" @@ -278,14 +283,14 @@ class User < ApplicationRecord name.match?(/\Auser_[0-9]+~*\z/) end - def is_restricted? - requires_verification? && !is_verified? - end - def is_anonymous? level == Levels::ANONYMOUS end + def is_restricted? + level == Levels::RESTRICTED + end + def is_member? level >= Levels::MEMBER end diff --git a/app/models/user_name_change_request.rb b/app/models/user_name_change_request.rb index 9b6b69e2b..536863919 100644 --- a/app/models/user_name_change_request.rb +++ b/app/models/user_name_change_request.rb @@ -11,10 +11,10 @@ class UserNameChangeRequest < ApplicationRecord def self.visible(user) if user.is_moderator? all - elsif user.is_member? - where(user: User.undeleted) - else + elsif user.is_anonymous? none + else + where(user: User.undeleted) end end diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index f36487709..fe035e33e 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -39,11 +39,7 @@ class ApplicationPolicy end def unbanned? - user.is_member? && !user.is_banned? && verified? - end - - def verified? - user.is_verified? || user.is_gold? || !user.requires_verification? + user.is_member? && !user.is_banned? && !user.is_restricted? end def policy(object) diff --git a/app/policies/artist_policy.rb b/app/policies/artist_policy.rb index ae0851d38..cd9f4b914 100644 --- a/app/policies/artist_policy.rb +++ b/app/policies/artist_policy.rb @@ -12,7 +12,7 @@ class ArtistPolicy < ApplicationPolicy end def can_view_banned? - user.is_member? + !user.is_anonymous? end def permitted_attributes diff --git a/app/policies/dmail_policy.rb b/app/policies/dmail_policy.rb index 152d0c549..a3a2b8d18 100644 --- a/app/policies/dmail_policy.rb +++ b/app/policies/dmail_policy.rb @@ -4,20 +4,20 @@ class DmailPolicy < ApplicationPolicy end def index? - user.is_member? + !user.is_anonymous? end def mark_all_as_read? - user.is_member? + !user.is_anonymous? end def update? - user.is_member? && record.owner_id == user.id + !user.is_anonymous? && record.owner_id == user.id end def show? 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 def reportable? diff --git a/app/policies/favorite_group_policy.rb b/app/policies/favorite_group_policy.rb index 784f8b33a..ff3e7427d 100644 --- a/app/policies/favorite_group_policy.rb +++ b/app/policies/favorite_group_policy.rb @@ -4,7 +4,7 @@ class FavoriteGroupPolicy < ApplicationPolicy end def create? - user.is_member? + !user.is_anonymous? end def update? diff --git a/app/policies/favorite_policy.rb b/app/policies/favorite_policy.rb index d10be727e..be6630e3f 100644 --- a/app/policies/favorite_policy.rb +++ b/app/policies/favorite_policy.rb @@ -1,9 +1,9 @@ class FavoritePolicy < ApplicationPolicy def create? - user.is_member? + !user.is_anonymous? end def destroy? - user.is_member? + !user.is_anonymous? end end diff --git a/app/policies/forum_topic_policy.rb b/app/policies/forum_topic_policy.rb index de6e2edd3..2f81c0818 100644 --- a/app/policies/forum_topic_policy.rb +++ b/app/policies/forum_topic_policy.rb @@ -20,7 +20,7 @@ class ForumTopicPolicy < ApplicationPolicy end def mark_all_as_read? - user.is_member? + !user.is_anonymous? end def reply? diff --git a/app/policies/saved_search_policy.rb b/app/policies/saved_search_policy.rb index f8671fb37..d43298575 100644 --- a/app/policies/saved_search_policy.rb +++ b/app/policies/saved_search_policy.rb @@ -1,10 +1,10 @@ class SavedSearchPolicy < ApplicationPolicy def index? - user.is_member? + !user.is_anonymous? end def create? - user.is_member? + !user.is_anonymous? end def update? diff --git a/app/policies/user_name_change_request_policy.rb b/app/policies/user_name_change_request_policy.rb index 53d223578..e278018c4 100644 --- a/app/policies/user_name_change_request_policy.rb +++ b/app/policies/user_name_change_request_policy.rb @@ -1,10 +1,10 @@ class UserNameChangeRequestPolicy < ApplicationPolicy def index? - user.is_member? + !user.is_anonymous? end 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 def permitted_attributes diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 1aaaaa84f..58d6b654e 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -16,7 +16,7 @@ class UserPolicy < ApplicationPolicy end def upgrade? - user.is_member? + !user.is_anonymous? end def reportable? @@ -24,7 +24,7 @@ class UserPolicy < ApplicationPolicy end def fix_counts? - user.is_member? + !user.is_anonymous? end def can_see_last_logged_in_at? diff --git a/app/policies/user_upgrade_policy.rb b/app/policies/user_upgrade_policy.rb index d5022a350..a81e80c50 100644 --- a/app/policies/user_upgrade_policy.rb +++ b/app/policies/user_upgrade_policy.rb @@ -1,6 +1,6 @@ class UserUpgradePolicy < ApplicationPolicy def create? - user.is_member? + !user.is_anonymous? end def new? diff --git a/app/views/forum_topics/_secondary_links.html.erb b/app/views/forum_topics/_secondary_links.html.erb index 9ce1011e7..1a5b69fc1 100644 --- a/app/views/forum_topics/_secondary_links.html.erb +++ b/app/views/forum_topics/_secondary_links.html.erb @@ -17,7 +17,7 @@ <%= subnav_link_to "Search", search_forum_posts_path %> <%= 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? %>
  • |
  • <%= subnav_link_to "Reply", new_forum_post_path(:topic_id => @forum_topic.id) %> <% if !@forum_topic.new_record? && policy(@forum_topic).update? %> diff --git a/app/views/posts/partials/index/_posts.html.erb b/app/views/posts/partials/index/_posts.html.erb index 701415e19..cb555b7dc 100644 --- a/app/views/posts/partials/index/_posts.html.erb +++ b/app/views/posts/partials/index/_posts.html.erb @@ -19,7 +19,7 @@ <% 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 %> <% if post_set.pending_bulk_update_requests.present? %>
    diff --git a/script/fixes/069_migrate_restricted_users.rb b/script/fixes/069_migrate_restricted_users.rb new file mode 100755 index 000000000..f974aebc3 --- /dev/null +++ b/script/fixes/069_migrate_restricted_users.rb @@ -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 diff --git a/test/factories/user.rb b/test/factories/user.rb index e06e2c29c..15ae2acb6 100644 --- a/test/factories/user.rb +++ b/test/factories/user.rb @@ -15,6 +15,12 @@ FactoryBot.define do is_banned {true} end + factory(:restricted_user) do + level {10} + requires_verification { true } + is_verified { false } + end + factory(:member_user) do level {20} end diff --git a/test/functional/emails_controller_test.rb b/test/functional/emails_controller_test.rb index 8d646a382..ce6c5ae5c 100644 --- a/test/functional/emails_controller_test.rb +++ b/test/functional/emails_controller_test.rb @@ -7,7 +7,7 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest setup do @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 })) - @restricted_user = create(:user, requires_verification: true, is_verified: false) + @restricted_user = create(:restricted_user, email_address: build(:email_address, { is_verified: false })) end context "#index" do @@ -136,27 +136,47 @@ class EmailsControllerTest < ActionDispatch::IntegrationTest end end - context "with a nondisposable email address" do - should "mark the user as verified" do - Danbooru.config.stubs(:email_domain_verification_list).returns(["gmail.com"]) - @user.email_address.update!(address: "test@gmail.com") - get email_verification_url(@user) + context "for a Restricted user" do + context "with a nondisposable email address" do + should "unrestrict the user's account" do + Danbooru.config.stubs(:email_domain_verification_list).returns(["gmail.com"]) + @restricted_user.email_address.update!(address: "test@gmail.com") - assert_redirected_to @user - assert_equal(true, @user.reload.email_address.is_verified) - assert_equal(true, @user.is_verified) + get email_verification_url(@restricted_user) + + 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 - context "with a disposable email address" do - should "not mark the user as verified" do + context "for a Gold user" 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"]) - @user.email_address.update!(address: "test@mailinator.com") + get email_verification_url(@user) assert_redirected_to @user 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 diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index af4c0b2e2..2c307d156 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -659,8 +659,7 @@ class PostsControllerTest < ActionDispatch::IntegrationTest end should "not allow unverified users to update posts" do - @user.update!(requires_verification: true, is_verified: false) - put_auth post_path(@post), @user, params: { post: { tag_string: "blah" }} + put_auth post_path(@post), create(:restricted_user), params: { post: { tag_string: "blah" }} assert_response 403 assert_not_equal("blah", @post.reload.tag_string) end diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 85adf6a47..0b807e4c3 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -256,6 +256,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_redirected_to User.last assert_equal("xxx", User.last.name) + assert_equal(User::Levels::MEMBER, User.last.level) assert_equal(User.last, User.last.authenticate_password("xxxxx1")) assert_nil(User.last.email_address) assert_no_enqueued_emails @@ -303,66 +304,80 @@ class UsersControllerTest < ActionDispatch::IntegrationTest post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }} 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) 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 post_auth users_path, @user, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }} 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) 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? self.remote_addr = @proxy_ip post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }} 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) 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 @ip_ban = create(:ip_ban, ip_addr: self.remote_addr, category: :partial) post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }} 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(1, @ip_ban.reload.hit_count) assert(@ip_ban.last_hit_at > 1.minute.ago) 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? self.remote_addr = @valid_ip post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }} 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) 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) self.remote_addr = @valid_ip post users_path, params: { user: { name: "dupe", password: "xxxxx1", password_confirmation: "xxxxx1" }} 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) 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" post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" }} 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) end end