feat: Improved password security policy (#2345)

Co-authored-by: Pranav Raj S <pranav@chatwoot.com>
This commit is contained in:
Sojan Jose
2021-06-07 17:26:08 +05:30
committed by GitHub
parent d1b3c7b0c2
commit 467b45b427
36 changed files with 284 additions and 151 deletions

View File

@@ -2,7 +2,7 @@
class AccountBuilder
include CustomExceptions::Account
pattr_initialize [:account_name!, :email!, :confirmed!, :user, :user_full_name, :user_password]
pattr_initialize [:account_name!, :email!, :confirmed, :user, :user_full_name, :user_password]
def perform
if @user.nil?
@@ -61,11 +61,9 @@ class AccountBuilder
end
def create_user
password = user_password || SecureRandom.alphanumeric(12)
@user = User.new(email: @email,
password: password,
password_confirmation: password,
password: user_password,
password_confirmation: user_password,
name: @user_full_name)
@user.confirm if @confirmed
@user.save!

View File

@@ -58,9 +58,10 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController
end
def new_agent_params
time = Time.now.to_i
# intial string ensures the password requirements are met
temp_password = "1!aA#{SecureRandom.alphanumeric(12)}"
params.require(:agent).permit(:email, :name, :role)
.merge!(password: time, password_confirmation: time, inviter: current_user)
.merge!(password: temp_password, password_confirmation: temp_password, inviter: current_user)
end
def agents

View File

@@ -18,7 +18,7 @@ class Api::V1::AccountsController < Api::BaseController
account_name: account_params[:account_name],
user_full_name: account_params[:user_full_name],
email: account_params[:email],
confirmed: confirmed?,
user_password: account_params[:password],
user: current_user
).perform
if @user
@@ -46,17 +46,13 @@ class Api::V1::AccountsController < Api::BaseController
private
def confirmed?
super_admin? && params[:confirmed]
end
def fetch_account
@account = current_user.accounts.find(params[:id])
@current_account_user = @account.account_users.find_by(user_id: current_user.id)
end
def account_params
params.permit(:account_name, :email, :name, :locale, :domain, :support_email, :auto_resolve_duration, :user_full_name)
params.permit(:account_name, :email, :name, :password, :locale, :domain, :support_email, :auto_resolve_duration, :user_full_name)
end
def check_signup_enabled

View File

@@ -20,7 +20,7 @@ class Api::V1::Widget::MessagesController < Api::V1::Widget::BaseController
@message.update!(message_update_params[:message])
end
rescue StandardError => e
render json: { error: @contact.errors, message: e.message }.to_json, status: 500
render json: { error: @contact.errors, message: e.message }.to_json, status: :internal_server_error
end
private

View File

@@ -17,13 +17,8 @@ module AccessTokenAuthHelper
Current.user = @resource if current_user.is_a?(User)
end
def super_admin?
@resource.present? && @resource.is_a?(SuperAdmin)
end
def validate_bot_access_token!
return if Current.user.is_a?(User)
return if super_admin?
return if agent_bot_accessible?
render_unauthorized('Access to this endpoint is not authorized for bots')

View File

@@ -1,34 +1,29 @@
class DeviseOverrides::ConfirmationsController < Devise::ConfirmationsController
include AuthHelper
skip_before_action :require_no_authentication, raise: false
skip_before_action :authenticate_user!, raise: false
def create
@confirmable = User.find_by(confirmation_token: params[:confirmation_token])
render_confirmation_success and return if @confirmable&.confirm
if confirm
render_confirmation_success
else
render_confirmation_error
end
render_confirmation_error
end
protected
def confirm
@confirmable&.confirm || (@confirmable&.confirmed_at && @confirmable&.reset_password_token)
end
private
def render_confirmation_success
render json: { "message": 'Success', "redirect_url": create_reset_token_link(@confirmable) }, status: :ok
send_auth_headers(@confirmable)
render partial: 'devise/auth.json', locals: { resource: @confirmable }
end
def render_confirmation_error
if @confirmable.blank?
render json: { "message": 'Invalid token', "redirect_url": '/' }, status: 422
render json: { message: 'Invalid token', redirect_url: '/' }, status: :unprocessable_entity
elsif @confirmable.confirmed_at
render json: { "message": 'Already confirmed', "redirect_url": '/' }, status: 422
render json: { message: 'Already confirmed', redirect_url: '/' }, status: :unprocessable_entity
else
render json: { "message": 'Failure', "redirect_url": '/' }, status: 422
render json: { message: 'Failure', redirect_url: '/' }, status: :unprocessable_entity
end
end

View File

@@ -13,7 +13,7 @@ class DeviseOverrides::PasswordsController < Devise::PasswordsController
send_auth_headers(@recoverable)
render partial: 'devise/auth.json', locals: { resource: @recoverable }
else
render json: { "message": 'Invalid token', "redirect_url": '/' }, status: 422
render json: { message: 'Invalid token', redirect_url: '/' }, status: :unprocessable_entity
end
end
@@ -27,7 +27,7 @@ class DeviseOverrides::PasswordsController < Devise::PasswordsController
end
end
protected
private
def reset_password_and_confirmation(recoverable)
recoverable.confirm unless recoverable.confirmed? # confirm if user resets password without confirming anytime before
@@ -40,7 +40,7 @@ class DeviseOverrides::PasswordsController < Devise::PasswordsController
def build_response(message, status)
render json: {
"message": message
message: message
}, status: status
end
end

View File

@@ -11,7 +11,6 @@ class SuperAdminDashboard < Administrate::BaseDashboard
id: Field::Number,
email: Field::String,
password: Field::Password,
access_token: Field::HasOne,
remember_created_at: Field::DateTime,
sign_in_count: Field::Number,
current_sign_in_at: Field::DateTime,
@@ -30,7 +29,6 @@ class SuperAdminDashboard < Administrate::BaseDashboard
COLLECTION_ATTRIBUTES = %i[
id
email
access_token
].freeze
# SHOW_PAGE_ATTRIBUTES

View File

@@ -29,6 +29,7 @@ export default {
account_name: creds.accountName.trim(),
user_full_name: creds.fullName.trim(),
email: creds.email,
password: creds.password,
})
.then(response => {
setAuthCredentials(response);
@@ -95,8 +96,18 @@ export default {
},
verifyPasswordToken({ confirmationToken }) {
return axios.post('auth/confirmation', {
confirmation_token: confirmationToken,
return new Promise((resolve, reject) => {
axios
.post('auth/confirmation', {
confirmation_token: confirmationToken,
})
.then(response => {
setAuthCredentials(response);
resolve(response);
})
.catch(error => {
reject(error.response);
});
});
},

View File

@@ -21,12 +21,10 @@ export default {
methods: {
async confirmToken() {
try {
const {
data: { redirect_url: redirectURL },
} = await Auth.verifyPasswordToken({
await Auth.verifyPasswordToken({
confirmationToken: this.confirmationToken,
});
window.location = redirectURL;
window.location = DEFAULT_REDIRECT_URL;
} catch (error) {
window.location = DEFAULT_REDIRECT_URL;
}

View File

@@ -120,8 +120,12 @@ export default {
window.location = DEFAULT_REDIRECT_URL;
}
})
.catch(() => {
this.showAlert(this.$t('SET_NEW_PASSWORD.API.ERROR_MESSAGE'));
.catch(error => {
let errorMessage = this.$t('SET_NEW_PASSWORD.API.ERROR_MESSAGE');
if (error?.data?.message) {
errorMessage = error.data.message;
}
this.showAlert(errorMessage);
});
},
},

View File

@@ -25,6 +25,17 @@
"
@blur="$v.credentials.fullName.$touch"
/>
<woot-input
v-model.trim="credentials.email"
type="email"
:class="{ error: $v.credentials.email.$error }"
:label="$t('REGISTER.EMAIL.LABEL')"
:placeholder="$t('REGISTER.EMAIL.PLACEHOLDER')"
:error="
$v.credentials.email.$error ? $t('REGISTER.EMAIL.ERROR') : ''
"
@blur="$v.credentials.email.$touch"
/>
<woot-input
v-model="credentials.accountName"
:class="{ error: $v.credentials.accountName.$error }"
@@ -38,15 +49,31 @@
@blur="$v.credentials.accountName.$touch"
/>
<woot-input
v-model.trim="credentials.email"
type="email"
:class="{ error: $v.credentials.email.$error }"
:label="$t('REGISTER.EMAIL.LABEL')"
:placeholder="$t('REGISTER.EMAIL.PLACEHOLDER')"
v-model.trim="credentials.password"
type="password"
:class="{ error: $v.credentials.password.$error }"
:label="$t('LOGIN.PASSWORD.LABEL')"
:placeholder="$t('SET_NEW_PASSWORD.PASSWORD.PLACEHOLDER')"
:error="
$v.credentials.email.$error ? $t('REGISTER.EMAIL.ERROR') : ''
$v.credentials.password.$error
? $t('SET_NEW_PASSWORD.PASSWORD.ERROR')
: ''
"
@blur="$v.credentials.email.$touch"
@blur="$v.credentials.password.$touch"
/>
<woot-input
v-model.trim="credentials.confirmPassword"
type="password"
:class="{ error: $v.credentials.confirmPassword.$error }"
:label="$t('SET_NEW_PASSWORD.CONFIRM_PASSWORD.LABEL')"
:placeholder="$t('SET_NEW_PASSWORD.CONFIRM_PASSWORD.PLACEHOLDER')"
:error="
$v.credentials.confirmPassword.$error
? $t('SET_NEW_PASSWORD.CONFIRM_PASSWORD.ERROR')
: ''
"
@blur="$v.credentials.confirmPassword.$touch"
/>
<woot-submit-button
:disabled="isSignupInProgress"
@@ -89,6 +116,8 @@ export default {
accountName: '',
fullName: '',
email: '',
password: '',
confirmPassword: '',
},
isSignupInProgress: false,
error: '',
@@ -108,6 +137,19 @@ export default {
required,
email,
},
password: {
required,
minLength: minLength(6),
},
confirmPassword: {
required,
isEqPassword(value) {
if (value !== this.credentials.password) {
return false;
}
return true;
},
},
},
},
computed: {

View File

@@ -124,8 +124,8 @@ export default {
this.errorMessage = this.$t('PROFILE_SETTINGS.PASSWORD_UPDATE_SUCCESS');
} catch (error) {
this.errorMessage = this.$t('RESET_PASSWORD.API.ERROR_MESSAGE');
if (error?.response?.data?.error) {
this.errorMessage = error.response.data.error;
if (error?.response?.data?.message) {
this.errorMessage = error.response.data.message;
}
} finally {
this.isPasswordChanging = false;

View File

@@ -21,7 +21,5 @@
class SuperAdmin < ApplicationRecord
# Include default devise modules. Others available are:
# :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
devise :database_authenticatable, :trackable, :rememberable, :validatable
include AccessTokenable
devise :database_authenticatable, :trackable, :rememberable, :validatable, :password_has_required_content
end

View File

@@ -53,7 +53,8 @@ class User < ApplicationRecord
:rememberable,
:trackable,
:validatable,
:confirmable
:confirmable,
:password_has_required_content
enum availability: { online: 0, offline: 1, busy: 2 }

View File

@@ -4,7 +4,7 @@
# layout will be rendered with erb and other content in html format
# Further processing in liquid is implemented in mailers
# Note: rails resolver looks for templates in cache first
# NOTE: rails resolver looks for templates in cache first
# which we don't want to happen here
# so we are overriding find_all method in action view resolver
# If anything breaks - look into rails : actionview/lib/action_view/template/resolver.rb

View File

@@ -79,12 +79,12 @@ class Notification::PushNotificationService
def fcm_options
{
"notification": {
"title": notification.notification_type.titleize,
"body": notification.push_message_title
notification: {
title: notification.notification_type.titleize,
body: notification.push_message_title
},
"data": { notification: notification.push_event_data.to_json },
"collapse_key": "chatwoot_#{notification.primary_actor_type.downcase}_#{notification.primary_actor_id}"
data: { notification: notification.push_event_data.to_json },
collapse_key: "chatwoot_#{notification.primary_actor_type.downcase}_#{notification.primary_actor_id}"
}
end
end