From b47cdc7ad786427908756f88bf2ae660964f4bd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=BF=E3=81=A6=E3=82=8B=E3=81=9E?= Date: Mon, 4 May 2026 16:22:13 +0900 Subject: [PATCH] =?UTF-8?q?BAN=20=E3=81=AE=E5=AE=9F=E8=A3=85=20(#327)=20(#?= =?UTF-8?q?342)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #327 #327 #327 #327 Merge remote-tracking branch 'origin/main' into feature/327 #327 Co-authored-by: miteruzo Reviewed-on: https://git.miteruzo.com/miteruzo/btrc-hub/pulls/342 --- .../app/controllers/application_controller.rb | 21 +- backend/app/controllers/users_controller.rb | 6 +- backend/app/models/ip_address.rb | 5 +- backend/app/models/user.rb | 6 +- ..._to_banned_at_in_users_and_ip_addresses.rb | 16 ++ backend/db/schema.rb | 8 +- backend/spec/factories/ip_addresses.rb | 10 + backend/spec/factories/users.rb | 15 +- backend/spec/requests/users_spec.rb | 269 ++++++++++++++---- backend/spec/support/test_records.rb | 6 +- 10 files changed, 286 insertions(+), 76 deletions(-) create mode 100644 backend/db/migrate/20260501153900_rename_banned_to_banned_at_in_users_and_ip_addresses.rb create mode 100644 backend/spec/factories/ip_addresses.rb diff --git a/backend/app/controllers/application_controller.rb b/backend/app/controllers/application_controller.rb index 0d412e0..4f3d6ed 100644 --- a/backend/app/controllers/application_controller.rb +++ b/backend/app/controllers/application_controller.rb @@ -1,14 +1,16 @@ class ApplicationController < ActionController::API + before_action :reject_banned_ip_address! before_action :authenticate_user + before_action :reject_banned_user! - def current_user - @current_user - end + def current_user = @current_user private def authenticate_user code = request.headers['X-Transfer-Code'] || request.headers['HTTP_X_TRANSFER_CODE'] + return if code.blank? + @current_user = User.find_by(inheritance_code: code) end @@ -22,4 +24,17 @@ class ApplicationController < ActionController::API s.in?(['', '1', 'true', 'on', 'yes']) end end + + def reject_banned_ip_address! + ip_address = IpAddress.find_by(ip_address: IPAddr.new(request.remote_ip).hton) + return unless ip_address&.banned? + + head :forbidden + end + + def reject_banned_user! + return unless current_user&.banned? + + head :forbidden + end end diff --git a/backend/app/controllers/users_controller.rb b/backend/app/controllers/users_controller.rb index 64aa43c..a144593 100644 --- a/backend/app/controllers/users_controller.rb +++ b/backend/app/controllers/users_controller.rb @@ -1,9 +1,6 @@ class UsersController < ApplicationController def create - return head :unprocessable_entity if request.remote_ip.blank? - user = nil - User.transaction do user = User.create!(inheritance_code: SecureRandom.uuid, role: :guest) attach_ip_address!(user) @@ -17,8 +14,7 @@ class UsersController < ApplicationController def verify user = User.find_by(inheritance_code: params[:code]) return render json: { valid: false } unless user - - return head :unprocessable_entity if request.remote_ip.blank? + return head :forbidden if user.banned? attach_ip_address!(user) diff --git a/backend/app/models/ip_address.rb b/backend/app/models/ip_address.rb index bf73658..e51e3ac 100644 --- a/backend/app/models/ip_address.rb +++ b/backend/app/models/ip_address.rb @@ -1,7 +1,10 @@ class IpAddress < ApplicationRecord validates :ip_address, presence: true, length: { maximum: 16 } - validates :banned, inclusion: { in: [true, false] } has_many :user_ips, dependent: :destroy has_many :users, through: :user_ips + + def banned? = banned_at.present? + def ban! = banned? || update!(banned_at: Time.current) + def unban! = update!(banned_at: nil) end diff --git a/backend/app/models/user.rb b/backend/app/models/user.rb index 7e07642..f974adf 100644 --- a/backend/app/models/user.rb +++ b/backend/app/models/user.rb @@ -4,7 +4,6 @@ class User < ApplicationRecord validates :name, length: { maximum: 255 } validates :inheritance_code, presence: true, length: { maximum: 64 } validates :role, presence: true, inclusion: { in: roles.keys } - validates :banned, inclusion: { in: [true, false] } has_many :created_posts, class_name: 'Post', foreign_key: :uploaded_user_id, dependent: :nullify @@ -19,5 +18,10 @@ class User < ApplicationRecord class_name: 'WikiPage', foreign_key: :updated_user_id, dependent: :nullify def viewed?(post) = user_post_views.exists?(post_id: post.id) + def gte_member? = member? || admin? + + def banned? = banned_at.present? + def ban! = banned? || update!(banned_at: Time.current) + def unban! = update!(banned_at: nil) end diff --git a/backend/db/migrate/20260501153900_rename_banned_to_banned_at_in_users_and_ip_addresses.rb b/backend/db/migrate/20260501153900_rename_banned_to_banned_at_in_users_and_ip_addresses.rb new file mode 100644 index 0000000..71d2c63 --- /dev/null +++ b/backend/db/migrate/20260501153900_rename_banned_to_banned_at_in_users_and_ip_addresses.rb @@ -0,0 +1,16 @@ +class RenameBannedToBannedAtInUsersAndIpAddresses < ActiveRecord::Migration[8.0] + def up + [:users, :ip_addresses].each do + add_column _1, :banned_at, :datetime, after: :banned + add_index _1, :banned_at + remove_column _1, :banned + end + end + + def down + [:ip_addresses, :users].each do + add_column _1, :banned, :boolean, null: false, default: false, after: :banned_at + remove_column _1, :banned_at + end + end +end diff --git a/backend/db/schema.rb b/backend/db/schema.rb index 48ee45f..042d227 100644 --- a/backend/db/schema.rb +++ b/backend/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_04_27_214800) do +ActiveRecord::Schema[8.0].define(version: 2026_05_01_153900) do create_table "active_storage_attachments", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false @@ -50,9 +50,10 @@ ActiveRecord::Schema[8.0].define(version: 2026_04_27_214800) do create_table "ip_addresses", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.binary "ip_address", limit: 16, null: false - t.boolean "banned", default: false, null: false + t.datetime "banned_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.index ["banned_at"], name: "index_ip_addresses_on_banned_at" t.index ["ip_address"], name: "index_ip_addresses_on_ip_address", unique: true end @@ -332,9 +333,10 @@ ActiveRecord::Schema[8.0].define(version: 2026_04_27_214800) do t.string "name" t.string "inheritance_code", limit: 64, null: false t.string "role", null: false - t.boolean "banned", default: false, null: false + t.datetime "banned_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.index ["banned_at"], name: "index_users_on_banned_at" end create_table "wiki_assets", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| diff --git a/backend/spec/factories/ip_addresses.rb b/backend/spec/factories/ip_addresses.rb new file mode 100644 index 0000000..6ffc363 --- /dev/null +++ b/backend/spec/factories/ip_addresses.rb @@ -0,0 +1,10 @@ +FactoryBot.define do + factory :ip_address do + ip_address { IPAddr.new('203.0.113.10').hton } + banned_at { nil } + + trait :banned do + banned_at { Time.current } + end + end +end diff --git a/backend/spec/factories/users.rb b/backend/spec/factories/users.rb index f7db70a..9225a23 100644 --- a/backend/spec/factories/users.rb +++ b/backend/spec/factories/users.rb @@ -1,15 +1,24 @@ FactoryBot.define do factory :user do - name { "test-user" } + name { nil } inheritance_code { SecureRandom.uuid } - role { "guest" } + role { 'guest' } + banned_at { nil } + + trait :guest do + role { 'guest' } + end trait :member do - role { "member" } + role { 'member' } end trait :admin do role { 'admin' } end + + trait :banned do + banned_at { Time.current } + end end end diff --git a/backend/spec/requests/users_spec.rb b/backend/spec/requests/users_spec.rb index 1f28e95..67c556f 100644 --- a/backend/spec/requests/users_spec.rb +++ b/backend/spec/requests/users_spec.rb @@ -1,109 +1,266 @@ -require "rails_helper" +require 'rails_helper' + +RSpec.describe 'Users', type: :request do + let(:remote_ip) { '203.0.113.10' } + + before do + allow_any_instance_of(ActionDispatch::Request) + .to receive(:remote_ip) + .and_return(remote_ip) + end + + def auth_headers(user) + { 'X-Transfer-Code' => user.inheritance_code } + end + + describe 'POST /users' do + it 'creates guest user, IpAddress and UserIp, and returns code' do + expect { + post '/users' + }.to change(User, :count).by(1) + .and change(IpAddress, :count).by(1) + .and change(UserIp, :count).by(1) -RSpec.describe "Users", type: :request do - describe "POST /users" do - it "creates guest user and returns code" do - post "/users" expect(response).to have_http_status(:created) - expect(json["code"]).to be_present - expect(json["user"]["role"]).to eq("guest") + expect(json['code']).to be_present + expect(json['user']['role']).to eq('guest') + + user = User.last + ip_address = IpAddress.find_by(ip_address: IPAddr.new(remote_ip).hton) + + expect(user.role).to eq('guest') + expect(ip_address).to be_present + expect(UserIp.exists?(user:, ip_address:)).to eq(true) + end + + it 'returns 403 and does not create user when current IP address is banned' do + IpAddress.create!( + ip_address: IPAddr.new(remote_ip).hton, + banned_at: Time.current + ) + + expect { + post '/users' + }.not_to change(User, :count) + + expect(response).to have_http_status(:forbidden) + expect(UserIp.count).to eq(0) end end - describe "POST /users/code/renew" do - it "returns 401 when not logged in" do - sign_out - post "/users/code/renew" + describe 'POST /users/code/renew' do + it 'returns 401 when not logged in' do + post '/users/code/renew' + expect(response).to have_http_status(:unauthorized) end + + it 'returns 403 when current user is banned' do + user = create(:user, :banned) + + post '/users/code/renew', headers: auth_headers(user) + + expect(response).to have_http_status(:forbidden) + end + + it 'returns 403 when current IP address is banned' do + user = create(:user) + + IpAddress.create!( + ip_address: IPAddr.new(remote_ip).hton, + banned_at: Time.current + ) + + post '/users/code/renew', headers: auth_headers(user) + + expect(response).to have_http_status(:forbidden) + end end - describe "PUT /users/:id" do - let(:user) { create(:user, name: "old-name", role: "guest") } + describe 'PUT /users/:id' do + let(:user) { create(:user, name: 'old-name', role: 'guest') } + + it 'returns 401 when current_user id mismatch' do + other_user = create(:user) + + put "/users/#{user.id}", + params: { name: 'new-name' }, + headers: auth_headers(other_user) - it "returns 401 when current_user id mismatch" do - sign_in_as(create(:user)) - put "/users/#{user.id}", params: { name: "new-name" } expect(response).to have_http_status(:unauthorized) end - it "returns 400 when name is blank" do - sign_in_as(user) - put "/users/#{user.id}", params: { name: " " } + it 'returns 400 when name is blank' do + put "/users/#{user.id}", + params: { name: ' ' }, + headers: auth_headers(user) + expect(response).to have_http_status(:bad_request) end - it "updates name and returns 201 with user slice" do - sign_in_as(user) - put "/users/#{user.id}", params: { name: "new-name" } + it 'updates name and returns user slice' do + put "/users/#{user.id}", + params: { name: 'new-name' }, + headers: auth_headers(user) expect(response).to have_http_status(:ok) - expect(json["id"]).to eq(user.id) - expect(json["name"]).to eq("new-name") + expect(json['id']).to eq(user.id) + expect(json['name']).to eq('new-name') user.reload - expect(user.name).to eq("new-name") + expect(user.name).to eq('new-name') + end + + it 'returns 403 when current user is banned' do + user.update!(banned_at: Time.current) + + put "/users/#{user.id}", + params: { name: 'new-name' }, + headers: auth_headers(user) + + expect(response).to have_http_status(:forbidden) + + user.reload + expect(user.name).to eq('old-name') + end + + it 'returns 403 when current IP address is banned' do + IpAddress.create!( + ip_address: IPAddr.new(remote_ip).hton, + banned_at: Time.current + ) + + put "/users/#{user.id}", + params: { name: 'new-name' }, + headers: auth_headers(user) + + expect(response).to have_http_status(:forbidden) + + user.reload + expect(user.name).to eq('old-name') end end - describe "POST /users/verify" do - it "returns valid:false when code not found" do - post "/users/verify", params: { code: "nope" } + describe 'POST /users/verify' do + it 'returns valid:false when code not found' do + post '/users/verify', params: { code: 'nope' } + expect(response).to have_http_status(:ok) - expect(json["valid"]).to eq(false) + expect(json['valid']).to eq(false) end - it "creates IpAddress and UserIp, and returns valid:true with user slice" do - user = create(:user, inheritance_code: SecureRandom.uuid, role: "guest") + it 'returns 403 when current IP address is banned' do + user = create(:user, inheritance_code: SecureRandom.uuid, role: 'guest') + + IpAddress.create!( + ip_address: IPAddr.new(remote_ip).hton, + banned_at: Time.current + ) + + expect { + post '/users/verify', params: { code: user.inheritance_code } + }.not_to change(UserIp, :count) + + expect(response).to have_http_status(:forbidden) + end + + it 'returns 403 when verified user is banned' do + user = create( + :user, + :banned, + inheritance_code: SecureRandom.uuid, + role: 'guest' + ) + + expect { + post '/users/verify', params: { code: user.inheritance_code } + }.not_to change(UserIp, :count) + + expect(response).to have_http_status(:forbidden) + end - # request.remote_ip を固定 - allow_any_instance_of(ActionDispatch::Request).to receive(:remote_ip).and_return("203.0.113.10") + it 'creates IpAddress and UserIp, and returns valid:true with user slice' do + user = create(:user, inheritance_code: SecureRandom.uuid, role: 'guest') expect { - post "/users/verify", params: { code: user.inheritance_code } + post '/users/verify', params: { code: user.inheritance_code } }.to change(UserIp, :count).by(1) + .and change(IpAddress, :count).by(1) expect(response).to have_http_status(:ok) - expect(json["valid"]).to eq(true) - expect(json["user"]["id"]).to eq(user.id) - expect(json["user"]["inheritance_code"]).to eq(user.inheritance_code) - expect(json["user"]["role"]).to eq("guest") + expect(json['valid']).to eq(true) + expect(json['user']['id']).to eq(user.id) + expect(json['user']['inheritance_code']).to eq(user.inheritance_code) + expect(json['user']['role']).to eq('guest') - # ついでに IpAddress もできてることを確認（ipの保存形式がバイナリでも count で見れる） - expect(IpAddress.count).to be >= 1 + ip_address = IpAddress.find_by(ip_address: IPAddr.new(remote_ip).hton) + expect(ip_address).to be_present + expect(UserIp.exists?(user:, ip_address:)).to eq(true) end - it "is idempotent for same user+ip (does not create duplicate UserIp)" do - user = create(:user, inheritance_code: SecureRandom.uuid, role: "guest") - allow_any_instance_of(ActionDispatch::Request).to receive(:remote_ip).and_return("203.0.113.10") + it 'is idempotent for same user and same IP address' do + user = create(:user, inheritance_code: SecureRandom.uuid, role: 'guest') - post "/users/verify", params: { code: user.inheritance_code } + post '/users/verify', params: { code: user.inheritance_code } expect(response).to have_http_status(:ok) expect { - post "/users/verify", params: { code: user.inheritance_code } + post '/users/verify', params: { code: user.inheritance_code } }.not_to change(UserIp, :count) expect(response).to have_http_status(:ok) - expect(json["valid"]).to eq(true) + expect(json['valid']).to eq(true) + end + + it 'creates another UserIp for same user and different IP address' do + user = create(:user, inheritance_code: SecureRandom.uuid, role: 'guest') + + post '/users/verify', params: { code: user.inheritance_code } + expect(response).to have_http_status(:ok) + + allow_any_instance_of(ActionDispatch::Request) + .to receive(:remote_ip) + .and_return('203.0.113.11') + + expect { + post '/users/verify', params: { code: user.inheritance_code } + }.to change(UserIp, :count).by(1) + + expect(response).to have_http_status(:ok) + expect(json['valid']).to eq(true) end end - describe "GET /users/me" do - it "returns 404 when code not found" do - get "/users/me", params: { code: "nope" } + describe 'GET /users/me' do + it 'returns 404 when code not found' do + get '/users/me', params: { code: 'nope' } + expect(response).to have_http_status(:not_found) end - it "returns user slice when found" do - user = create(:user, inheritance_code: SecureRandom.uuid, name: "me", role: "guest") - get "/users/me", params: { code: user.inheritance_code } + it 'returns user slice when found' do + user = create(:user, inheritance_code: SecureRandom.uuid, name: 'me', role: 'guest') + + get '/users/me', params: { code: user.inheritance_code } expect(response).to have_http_status(:ok) - expect(json["id"]).to eq(user.id) - expect(json["name"]).to eq("me") - expect(json["inheritance_code"]).to eq(user.inheritance_code) - expect(json["role"]).to eq("guest") + expect(json['id']).to eq(user.id) + expect(json['name']).to eq('me') + expect(json['inheritance_code']).to eq(user.inheritance_code) + expect(json['role']).to eq('guest') + end + + it 'returns 403 when current IP address is banned' do + user = create(:user, inheritance_code: SecureRandom.uuid) + + IpAddress.create!( + ip_address: IPAddr.new(remote_ip).hton, + banned_at: Time.current + ) + + get '/users/me', params: { code: user.inheritance_code } + + expect(response).to have_http_status(:forbidden) end end end diff --git a/backend/spec/support/test_records.rb b/backend/spec/support/test_records.rb index 350b8da..b6a7cd5 100644 --- a/backend/spec/support/test_records.rb +++ b/backend/spec/support/test_records.rb @@ -2,14 +2,12 @@ module TestRecords def create_member_user! User.create!(name: 'spec user', inheritance_code: SecureRandom.hex(16), - role: 'member', - banned: false) + role: 'member') end def create_admin_user! User.create!(name: 'spec admin', inheritance_code: SecureRandom.hex(16), - role: 'admin', - banned: false) + role: 'admin') end end