Add a username slug to the user URL
This way it won't be possible to browse all user URLs by just going to /users/1, /users/2, /users/3, ... and collect usernames, which might not be desirable in some cases. Note we could use the username as a URL parameter and just find the user with `@user = User.find_by!(id: id, username: username)`, but since usernames might contain strange characters, this might lead to strange/ugly URLs. Finally, note we're using `username.to_s` in order to cover the case where the username is `nil` (as is the case with erased users).
This commit is contained in:
@@ -1,6 +1,7 @@
|
|||||||
class DirectMessagesController < ApplicationController
|
class DirectMessagesController < ApplicationController
|
||||||
before_action :authenticate_user!
|
before_action :authenticate_user!
|
||||||
load_and_authorize_resource :user, instance_name: :receiver
|
load_and_authorize_resource :user, instance_name: :receiver
|
||||||
|
before_action :check_slug
|
||||||
load_resource through: :receiver, through_association: :direct_messages_received
|
load_resource through: :receiver, through_association: :direct_messages_received
|
||||||
authorize_resource except: :new
|
authorize_resource except: :new
|
||||||
|
|
||||||
@@ -28,6 +29,12 @@ class DirectMessagesController < ApplicationController
|
|||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
|
def check_slug
|
||||||
|
slug = params[:user_id].split("-", 2)[1]
|
||||||
|
|
||||||
|
raise ActiveRecord::RecordNotFound unless @receiver.slug == slug.to_s
|
||||||
|
end
|
||||||
|
|
||||||
def direct_message_params
|
def direct_message_params
|
||||||
params.require(:direct_message).permit(allowed_params)
|
params.require(:direct_message).permit(allowed_params)
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
class UsersController < ApplicationController
|
class UsersController < ApplicationController
|
||||||
load_and_authorize_resource
|
load_and_authorize_resource
|
||||||
|
before_action :check_slug
|
||||||
helper_method :valid_interests_access?
|
helper_method :valid_interests_access?
|
||||||
|
|
||||||
def show
|
def show
|
||||||
@@ -8,6 +9,12 @@ class UsersController < ApplicationController
|
|||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
|
def check_slug
|
||||||
|
slug = params[:id].split("-", 2)[1]
|
||||||
|
|
||||||
|
raise ActiveRecord::RecordNotFound unless @user.slug == slug.to_s
|
||||||
|
end
|
||||||
|
|
||||||
def valid_interests_access?(user)
|
def valid_interests_access?(user)
|
||||||
user.public_interests || user == current_user
|
user.public_interests || user == current_user
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -433,6 +433,14 @@ class User < ApplicationRecord
|
|||||||
(Tenant.current_secrets.dig(:security, :lockable, :unlock_in) || 1).to_f.hours
|
(Tenant.current_secrets.dig(:security, :lockable, :unlock_in) || 1).to_f.hours
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def to_param
|
||||||
|
"#{id}-#{slug}"
|
||||||
|
end
|
||||||
|
|
||||||
|
def slug
|
||||||
|
username.to_s.parameterize
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def clean_document_number
|
def clean_document_number
|
||||||
|
|||||||
62
spec/controllers/direct_messages_controller_spec.rb
Normal file
62
spec/controllers/direct_messages_controller_spec.rb
Normal file
@@ -0,0 +1,62 @@
|
|||||||
|
require "rails_helper"
|
||||||
|
|
||||||
|
describe DirectMessagesController do
|
||||||
|
before { sign_in(create :user, :level_two) }
|
||||||
|
|
||||||
|
describe "GET new" do
|
||||||
|
let!(:user) { create(:user, username: "James Jameson") }
|
||||||
|
|
||||||
|
it "finds a user by ID and slug" do
|
||||||
|
get :new, params: { user_id: "#{user.id}-james-jameson" }
|
||||||
|
|
||||||
|
expect(response).to be_successful
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not find a user by just an ID" do
|
||||||
|
expect do
|
||||||
|
get :new, params: { user_id: user.id }
|
||||||
|
end.to raise_error ActiveRecord::RecordNotFound
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not find a user by just a slug" do
|
||||||
|
expect do
|
||||||
|
get :new, params: { user_id: "james-jameson" }
|
||||||
|
end.to raise_error ActiveRecord::RecordNotFound
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not find a user with the wrong slug" do
|
||||||
|
expect do
|
||||||
|
get :new, params: { user_id: "#{user.id}-James Jameson" }
|
||||||
|
end.to raise_error ActiveRecord::RecordNotFound
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "POST create" do
|
||||||
|
let!(:user) { create(:user, username: "James Jameson") }
|
||||||
|
let(:message_params) { { direct_message: { title: "Hello!", message: "How are you doing?" }} }
|
||||||
|
|
||||||
|
it "finds a user by ID and slug" do
|
||||||
|
post :create, params: message_params.merge(user_id: "#{user.id}-james-jameson")
|
||||||
|
|
||||||
|
expect(response).to be_successful
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not find a user by just an ID" do
|
||||||
|
expect do
|
||||||
|
post :create, params: message_params.merge(user_id: user.id)
|
||||||
|
end.to raise_error ActiveRecord::RecordNotFound
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not find a user by just a slug" do
|
||||||
|
expect do
|
||||||
|
post :create, params: message_params.merge(user_id: "james-jameson")
|
||||||
|
end.to raise_error ActiveRecord::RecordNotFound
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not find a user with the wrong slug" do
|
||||||
|
expect do
|
||||||
|
post :create, params: message_params.merge(user_id: "#{user.id}-James Jameson")
|
||||||
|
end.to raise_error ActiveRecord::RecordNotFound
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
39
spec/controllers/users_controller_spec.rb
Normal file
39
spec/controllers/users_controller_spec.rb
Normal file
@@ -0,0 +1,39 @@
|
|||||||
|
require "rails_helper"
|
||||||
|
|
||||||
|
describe UsersController do
|
||||||
|
describe "GET show" do
|
||||||
|
let!(:user) { create(:user, username: "James Jameson") }
|
||||||
|
|
||||||
|
it "finds a user by ID and slug" do
|
||||||
|
get :show, params: { id: "#{user.id}-james-jameson" }
|
||||||
|
|
||||||
|
expect(response).to be_successful
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not find a user by just an ID" do
|
||||||
|
expect do
|
||||||
|
get :show, params: { id: user.id }
|
||||||
|
end.to raise_error ActiveRecord::RecordNotFound
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not find a user by just a slug" do
|
||||||
|
expect do
|
||||||
|
get :show, params: { id: "james-jameson" }
|
||||||
|
end.to raise_error ActiveRecord::RecordNotFound
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not find a user with the wrong slug" do
|
||||||
|
expect do
|
||||||
|
get :show, params: { id: "#{user.id}-James Jameson" }
|
||||||
|
end.to raise_error ActiveRecord::RecordNotFound
|
||||||
|
end
|
||||||
|
|
||||||
|
it "finds users without username by ID" do
|
||||||
|
user.erase
|
||||||
|
|
||||||
|
get :show, params: { id: user.id }
|
||||||
|
|
||||||
|
expect(response).to be_successful
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
Reference in New Issue
Block a user