diff --git a/.overcommit.yml b/.overcommit.yml index f5905b8c1..a50b90cd4 100644 --- a/.overcommit.yml +++ b/.overcommit.yml @@ -12,7 +12,7 @@ CommitMsg: MessageFormat: enabled: true - pattern: ^(\((doc|bug|feat|security|dev|i18n|api|test|quality|ui|merge)\) [\w ]++(\n\n.+)?)|(Version (\d+\.?)+)|(Merge branch .*) - expected_pattern_message: (doc|bug|feat|security|dev|i18n|api|test|quality|ui|merge) title\n\ndescription + pattern: ^(\((doc|bug|feat|security|dev|i18n|api|test|quality|ui|merge|wip)\) [\w ]++(\n\n.+)?)|(Version (\d+\.?)+)|(Merge branch .*) + expected_pattern_message: (doc|bug|feat|security|dev|i18n|api|test|quality|ui|merge|wip) title\n\ndescription sample_message: (bug) no validation on date\n\nThe birthdate was not validated... diff --git a/CHANGELOG.md b/CHANGELOG.md index cfa4203a3..5e90ee97e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ - Ability to define multiple accounting journal codes - OpenAPI endpoint to fetch accounting data - Add reservation deadline parameter (#414) +- Verify current password at server side when changing password +- Password strengh indicator - Updated OpenAPI documentation - Updated OpenID Connect documentation - OpenAPI users endpoint offer ability to filter by created_after diff --git a/app/controllers/api/members_controller.rb b/app/controllers/api/members_controller.rb index f54e7c66d..c0e83192c 100644 --- a/app/controllers/api/members_controller.rb +++ b/app/controllers/api/members_controller.rb @@ -47,7 +47,7 @@ class API::MembersController < API::ApiController authorize @member members_service = Members::MembersService.new(@member) - if members_service.update(user_params) + if members_service.update(user_params, current_user, params[:user][:current_password]) # Update password without logging out bypass_sign_in(@member) unless current_user.id != params[:id].to_i render :show, status: :ok, location: member_path(@member) @@ -235,7 +235,7 @@ class API::MembersController < API::ApiController ], statistic_profile_attributes: %i[id gender birthday]) - elsif current_user.admin? || current_user.manager? + elsif current_user.privileged? params.require(:user).permit(:username, :email, :password, :password_confirmation, :is_allow_contact, :is_allow_newsletter, :group_id, tag_ids: [], profile_attributes: [:id, :first_name, :last_name, :phone, :interest, :software_mastered, :website, :job, diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index 285e87e3c..838e31028 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -1,19 +1,17 @@ # frozen_string_literal: true -# Devise controller used for the "forgotten password" feature +# Devise controller used for the "forgotten password" feature and to check the current's user password class PasswordsController < Devise::PasswordsController # POST /users/password.json def create self.resource = resource_class.send_reset_password_instructions(resource_params) yield resource if block_given? - if successfully_sent?(resource) - respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name)) - end + respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name)) if successfully_sent?(resource) end # POST /password/verify def verify - current_user.valid_password?(params[:password]) ? head(200) : head(404) + current_user.valid_password?(params[:password]) ? head(:ok) : head(:not_found) end end diff --git a/app/frontend/src/javascript/components/user/change-password.tsx b/app/frontend/src/javascript/components/user/change-password.tsx index cb85d4607..6a28c082b 100644 --- a/app/frontend/src/javascript/components/user/change-password.tsx +++ b/app/frontend/src/javascript/components/user/change-password.tsx @@ -8,7 +8,7 @@ import { useTranslation } from 'react-i18next'; import Authentication from '../../api/authentication'; import { FieldValues } from 'react-hook-form/dist/types/fields'; import { PasswordInput } from './password-input'; -import { FormState } from 'react-hook-form/dist/types/form'; +import { FormState, UseFormSetValue } from 'react-hook-form/dist/types/form'; import MemberAPI from '../../api/member'; import { User } from '../../models/user'; @@ -18,13 +18,14 @@ interface ChangePasswordProp { currentFormPassword: string, formState: FormState, user: User, + setValue: UseFormSetValue, } /** * This component shows a button that trigger a modal dialog to verify the user's current password. * If the user's current password is correct, the modal dialog is closed and the button is replaced by a form to set the new password. */ -export const ChangePassword = ({ register, onError, currentFormPassword, formState, user }: ChangePasswordProp) => { +export const ChangePassword = ({ register, onError, currentFormPassword, formState, user, setValue }: ChangePasswordProp) => { const { t } = useTranslation('shared'); const [isModalOpen, setIsModalOpen] = React.useState(false); @@ -68,6 +69,7 @@ export const ChangePassword = ({ register, onE return handleSubmit((data: { password: string }) => { Authentication.verifyPassword(data.password).then(res => { if (res) { + setValue('current_password', data.password); setIsConfirmedPassword(true); toggleConfirmationModal(); } else { @@ -85,6 +87,7 @@ export const ChangePassword = ({ register, onE {t('app.shared.change_password.change_my_password')} } {isConfirmedPassword &&
+
} diff --git a/app/frontend/src/javascript/components/user/password-input.tsx b/app/frontend/src/javascript/components/user/password-input.tsx index 4d87ad96c..e3b5e41d7 100644 --- a/app/frontend/src/javascript/components/user/password-input.tsx +++ b/app/frontend/src/javascript/components/user/password-input.tsx @@ -3,6 +3,10 @@ import { FieldValues } from 'react-hook-form/dist/types/fields'; import { useTranslation } from 'react-i18next'; import { FormInput } from '../form/form-input'; import { FormState } from 'react-hook-form/dist/types/form'; +import { PasswordStrength } from './password-strength'; +import * as React from 'react'; +import { useState } from 'react'; +import { Eye, EyeSlash } from 'phosphor-react'; interface PasswordInputProps { register: UseFormRegister, @@ -16,9 +20,32 @@ interface PasswordInputProps { export const PasswordInput = ({ register, currentFormPassword, formState }: PasswordInputProps) => { const { t } = useTranslation('shared'); + const [password, setPassword] = useState(null); + const [inputType, setInputType] = useState<'password'|'text'>('password'); + + /** + * Callback triggered when the user types a password + */ + const handlePasswordChange = (event: React.ChangeEvent) => { + setPassword(event.target.value); + }; + + /** + * Switch the password characters between hidden and displayed + */ + const toggleShowPassword = () => { + if (inputType === 'text') { + setInputType('password'); + } else { + setInputType('text'); + } + }; + return (
: } + addOnAction={toggleShowPassword} rules={{ required: true, validate: (value: string) => { @@ -29,9 +56,11 @@ export const PasswordInput = ({ register, curr } }} formState={formState} + onChange={handlePasswordChange} label={t('app.shared.password_input.new_password')} tooltip={t('app.shared.password_input.help')} - type="password" /> + type={inputType} /> + ', '?', '@', '[', ']', '^', '_', '{', '|', '}', '~', "'", '`', '"']; + +/** + * Shows a visual indicator of the password strength + */ +export const PasswordStrength: React.FC = ({ password }) => { + const { t } = useTranslation('shared'); + + const [strength, setStrength] = useState(null); + const [hasRequirements, setHasRequirements] = useState(false); + + /* + * zxcvbn library options + * @see https://zxcvbn-ts.github.io/zxcvbn/guide/getting-started/ + */ + const options = { + translations: null, + graphs: zxcvbnCommonPackage.adjacencyGraphs, + dictionary: LocaliseLib.zxcvbnDictionnaries() + }; + zxcvbnOptions.setOptions(options); + + /** + * Compute the strength of the given password and update the result in memory + */ + const updateStrength = () => { + if (typeof password === 'string') { + if (checkRequirements()) { + setHasRequirements(true); + const result = zxcvbn(password); + setStrength(result); + } else { + setHasRequirements(false); + } + } + }; + + /** + * Check if the provided password meet the minimal requirements + */ + const checkRequirements = (): boolean => { + if (typeof password === 'string') { + const chars = password.split(''); + return (chars.some(c => SPECIAL_CHARS.includes(c)) && + !!password.match(/[A-Z]/) && + !!password.match(/[a-z]/) && + !!password.match(/[0-9]/) && + password.length >= 12); + } + }; + + useEffect(_debounce(updateStrength, 500), [password]); + + return ( +
+ {password && !hasRequirements && <> + {t('app.shared.password_strength.not_in_requirements')} + } + {hasRequirements && strength && <> +
+ {t(`app.shared.password_strength.${strength.score}`)} + } +
+ ); +}; diff --git a/app/frontend/src/javascript/components/user/user-profile-form.tsx b/app/frontend/src/javascript/components/user/user-profile-form.tsx index 66b7cba8c..0ca1aca5e 100644 --- a/app/frontend/src/javascript/components/user/user-profile-form.tsx +++ b/app/frontend/src/javascript/components/user/user-profile-form.tsx @@ -255,7 +255,8 @@ export const UserProfileForm: React.FC = ({ action, size, onError={onError} currentFormPassword={output.password} user={user} - formState={formState} />} + formState={formState} + setValue={setValue} />} {action === 'create' && } diff --git a/app/frontend/src/javascript/lib/localise.ts b/app/frontend/src/javascript/lib/localise.ts new file mode 100644 index 000000000..68dfd1ad6 --- /dev/null +++ b/app/frontend/src/javascript/lib/localise.ts @@ -0,0 +1,46 @@ +import { IFablab } from '../models/fablab'; +import zxcvbnCommonPackage from '@zxcvbn-ts/language-common'; +import zxcvbnEnPackage from '@zxcvbn-ts/language-en'; +import zxcvbnDePackage from '@zxcvbn-ts/language-de'; +import zxcvbnEsPackage from '@zxcvbn-ts/language-es-es'; +import zxcvbnFrPackage from '@zxcvbn-ts/language-fr'; +import zxcvbnPtPackage from '@zxcvbn-ts/language-pt-br'; + +declare let Fablab: IFablab; +/** + * Localization specific handlers + */ +export default class LocaliseLib { + /** + * Bind the dictionnaries for the zxcvbn lib, to the current locale configuration of the app (APP_LOCALE). + */ + static zxcvbnDictionnaries = () => { + switch (Fablab.locale) { + case 'de': + return { + ...zxcvbnCommonPackage.dictionary, + ...zxcvbnDePackage.dictionary + }; + case 'es': + return { + ...zxcvbnCommonPackage.dictionary, + ...zxcvbnEsPackage.dictionary + }; + case 'fr': + return { + ...zxcvbnCommonPackage.dictionary, + ...zxcvbnFrPackage.dictionary + }; + case 'pt': + return { + ...zxcvbnCommonPackage.dictionary, + ...zxcvbnPtPackage.dictionary + }; + default: + return { + ...zxcvbnCommonPackage.dictionary, + ...zxcvbnEnPackage.dictionary + }; + } + }; +} diff --git a/app/frontend/src/javascript/models/user.ts b/app/frontend/src/javascript/models/user.ts index e7d5ccb9a..0aeb48372 100644 --- a/app/frontend/src/javascript/models/user.ts +++ b/app/frontend/src/javascript/models/user.ts @@ -19,6 +19,7 @@ export interface User { need_completion?: boolean, ip_address?: string, mapped_from_sso?: string[], + current_password?: string, password?: string, password_confirmation?: string, cgu?: boolean, // Accepted terms and conditions? diff --git a/app/frontend/src/stylesheets/application.scss b/app/frontend/src/stylesheets/application.scss index 8ca6bd24f..c2c7f50ad 100644 --- a/app/frontend/src/stylesheets/application.scss +++ b/app/frontend/src/stylesheets/application.scss @@ -132,8 +132,10 @@ @import "modules/trainings/training-form"; @import "modules/user/avatar"; @import "modules/user/avatar-input"; +@import "modules/user/change-password"; @import "modules/user/gender-input"; @import "modules/user/member-select"; +@import "modules/user/password-strength"; @import "modules/user/user-profile-form"; @import "modules/user/user-validation"; diff --git a/app/frontend/src/stylesheets/modules/user/change-password.scss b/app/frontend/src/stylesheets/modules/user/change-password.scss new file mode 100644 index 000000000..a17e83f56 --- /dev/null +++ b/app/frontend/src/stylesheets/modules/user/change-password.scss @@ -0,0 +1,15 @@ +.change-password { + .password-input { + animation: show 600ms 100ms cubic-bezier(0.38, 0.97, 0.56, 0.76) forwards; + opacity: 0; + transform: rotateX(-90deg); + transform-origin: top center; + } +} + +@keyframes show { + 100% { + opacity: 1; + transform: none; + } +} diff --git a/app/frontend/src/stylesheets/modules/user/password-strength.scss b/app/frontend/src/stylesheets/modules/user/password-strength.scss new file mode 100644 index 000000000..b75fcca94 --- /dev/null +++ b/app/frontend/src/stylesheets/modules/user/password-strength.scss @@ -0,0 +1,37 @@ +.password-strength { + margin-top: -1rem; + margin-bottom: 1.6rem; + display: flex; + align-items: center; + + .requirements-error { + color: var(--alert); + } + + .strength-bar { + width: 100%; + height: 1rem; + border: 1px solid var(--gray-soft); + border-radius: 0.5rem; + &.strength-0 { + background: linear-gradient(to right, var(--alert), white 20%); + } + &.strength-1 { + background: linear-gradient(to right, var(--alert), orange 20%, white 40%); + } + &.strength-2 { + background: linear-gradient(to right, var(--alert), orange 20%, yellow 40%, white 60%); + } + &.strength-3 { + background: linear-gradient(to right, var(--alert), orange 20%, yellow 40%, #5e790f 60%, white 80%); + } + &.strength-4 { + background: linear-gradient(to right, var(--alert), orange 20%, yellow 40%, #5e790f 60%, var(--success) 80%); + } + } + span { + margin-left: 2rem; + min-width: fit-content; + white-space: nowrap; + } +} diff --git a/app/models/user.rb b/app/models/user.rb index 74ddc19c6..570ff2b87 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -255,7 +255,7 @@ class User < ApplicationRecord end def password_complexity - return if password.blank? || SecurePassword.is_secured?(password) + return if password.blank? || SecurePassword.secured?(password) errors.add I18n.t('app.public.common.password_is_too_weak'), I18n.t('app.public.common.password_is_too_weak_explanations') end diff --git a/app/services/members/import_service.rb b/app/services/members/import_service.rb index b91137f86..9e844be11 100644 --- a/app/services/members/import_service.rb +++ b/app/services/members/import_service.rb @@ -16,7 +16,7 @@ class Members::ImportService params = row_to_params(row, user, password) if user service = Members::MembersService.new(user) - res = service.update(params) + res = service.update(params, import.user) log << { user: user.id, status: 'update', result: res } else user = User.new(params) @@ -26,14 +26,10 @@ class Members::ImportService end log << user.errors.to_hash unless user.errors.to_hash.empty? rescue StandardError => e - log << e.to_s - Rails.logger.error e - Rails.logger.debug e.backtrace + handle_error(log, e) end rescue ArgumentError => e - log << e.to_s - Rails.logger.error e - Rails.logger.debug e.backtrace + handle_error(log, e) end log end @@ -184,4 +180,10 @@ class Members::ImportService password end end + + def handle_error(log, error) + log << error.to_s + Rails.logger.error error + Rails.logger.debug error.backtrace + end end diff --git a/app/services/members/members_service.rb b/app/services/members/members_service.rb index ba21f312e..9d1ac8867 100644 --- a/app/services/members/members_service.rb +++ b/app/services/members/members_service.rb @@ -8,7 +8,7 @@ class Members::MembersService @member = member end - def update(params) + def update(params, operator, current_password = nil) if subscriber_group_change?(params) # here a group change is requested but unprocessable, handle the exception @member.errors.add(:group_id, I18n.t('members.unable_to_change_the_group_while_a_subscription_is_running')) @@ -30,6 +30,8 @@ class Members::MembersService end end + handle_password(params, operator, current_password) + Members::MembersService.handle_organization(params) not_complete = member.need_completion? @@ -171,4 +173,14 @@ class Members::MembersService def user_group_change?(params) @member.group_id && params[:group_id] && @member.group_id != params[:group_id].to_i end + + def handle_password(params, operator, current_password = nil) + return unless params[:password] && params[:password_confirmation] + + return if operator.privileged? + + raise SecurityError, 'current password not provided' if current_password.blank? + + raise SecurityError, 'current password does not match' unless @member.valid_password?(current_password) + end end diff --git a/app/services/secure_password.rb b/app/services/secure_password.rb index cbbfc13e5..0e574e531 100644 --- a/app/services/secure_password.rb +++ b/app/services/secure_password.rb @@ -1,18 +1,22 @@ +# frozen_string_literal: true + +# Ensure the passwords are secure enough class SecurePassword LOWER_LETTERS = ('a'..'z').to_a UPPER_LETTERS = ('A'..'Z').to_a DIGITS = ('0'..'9').to_a - SPECIAL_CHARS = ["!", "#", "$", "%", "&", "(", ")", "*", "+", ",", "-", ".", "/", ":", ";", "<", "=", ">", "?", "@", "[", "]", "^", "_", "{", "|", "}", "~", "'", "`", '"'] + SPECIAL_CHARS = ['!', '#', '$', '%', '&', '(', ')', '*', '+', ',', '-', '.', '/', ':', ';', '<', '=', '>', '?', '@', '[', ']', '^', '_', '{', + '|', '}', '~', "'", '`', '"'].freeze def self.generate - (LOWER_LETTERS.shuffle.first(4) + UPPER_LETTERS.shuffle.first(4) + DIGITS.shuffle.first(4) + SPECIAL_CHARS.shuffle.first(4)).shuffle.join + (LOWER_LETTERS.sample(4) + UPPER_LETTERS.sample(4) + DIGITS.sample(4) + SPECIAL_CHARS.sample(4)).shuffle.join end - def self.is_secured?(password) - password_as_array = password.split("") - password_as_array.any? {|c| c.in? LOWER_LETTERS } && - password_as_array.any? {|c| c.in? UPPER_LETTERS } && - password_as_array.any? {|c| c.in? DIGITS } && - password_as_array.any? {|c| c.in? SPECIAL_CHARS } + def self.secured?(password) + password_as_array = password.chars + password_as_array.any? { |c| c.in? LOWER_LETTERS } && + password_as_array.any? { |c| c.in? UPPER_LETTERS } && + password_as_array.any? { |c| c.in? DIGITS } && + password_as_array.any? { |c| c.in? SPECIAL_CHARS } end -end \ No newline at end of file +end diff --git a/config/locales/app.shared.en.yml b/config/locales/app.shared.en.yml index c928b62bc..7f0a79955 100644 --- a/config/locales/app.shared.en.yml +++ b/config/locales/app.shared.en.yml @@ -116,6 +116,13 @@ en: help: "Your password must be minimum 12 characters long, have at least one uppercase letter, one lowercase letter, one number and one special character." password_too_short: "Password is too short (must be at least 12 characters)" confirmation_mismatch: "Confirmation mismatch with password." + password_strength: + not_in_requirements: "Your password doesn't meet the minimal requirements" + 0: "Very weak password" + 1: "Weak password" + 2: "Almost ok" + 3: "Good password" + 4: "Excellent password" #project edition form project: name: "Name" diff --git a/package.json b/package.json index 24bbfea2e..2c5705331 100644 --- a/package.json +++ b/package.json @@ -81,6 +81,13 @@ "@types/react-dom": "^17.0.3", "@types/sortablejs": "1", "@uirouter/angularjs": "1.0.30", + "@zxcvbn-ts/core": "^2.1.0", + "@zxcvbn-ts/language-common": "^2.0.1", + "@zxcvbn-ts/language-de": "^2.1.0", + "@zxcvbn-ts/language-en": "^2.1.0", + "@zxcvbn-ts/language-es-es": "^2.1.1", + "@zxcvbn-ts/language-fr": "^2.2.0", + "@zxcvbn-ts/language-pt-br": "^2.1.0", "AngularDevise": "https://github.com/cloudspace/angular_devise.git#1.0.2", "angular": "1.8", "angular-animate": "1.7", diff --git a/yarn.lock b/yarn.lock index 6c1439166..3d8fd0bbf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3812,6 +3812,41 @@ resolved "https://registry.yarnpkg.com/@xtuc/long/-/long-4.2.2.tgz#d291c6a4e97989b5c61d9acf396ae4fe133a718d" integrity sha512-NuHqBY1PB/D8xU6s/thBgOAiAP7HOYDQ32+BFZILJ8ivkUkAHQnWfn6WhL79Owj1qmUnoN/YPhktdIoucipkAQ== +"@zxcvbn-ts/core@^2.1.0": + version "2.1.0" + resolved "https://registry.yarnpkg.com/@zxcvbn-ts/core/-/core-2.1.0.tgz#026ffaba5b09cb05ee80f28b0183f75217d267d1" + integrity sha512-doxol9xrO7LgyVJhguXe7vO0xthnIYmsOKoDwrLg0Ho2kkpQaVtM+AOQw+BkEiKIqNg1V48eUf4/cTzMElXdiA== + +"@zxcvbn-ts/language-common@^2.0.1": + version "2.0.1" + resolved "https://registry.yarnpkg.com/@zxcvbn-ts/language-common/-/language-common-2.0.1.tgz#01c371a64e86de417c317b4443aaa0a0f07f917b" + integrity sha512-P+v5MA/UNc9nb3FEOEoDgTyIGQc2vLc6m04pdf5YyuNOzrL0iNANhECk2TUp62JbrjouJVodqhMH0j1a8/24Bg== + +"@zxcvbn-ts/language-de@^2.1.0": + version "2.1.0" + resolved "https://registry.yarnpkg.com/@zxcvbn-ts/language-de/-/language-de-2.1.0.tgz#c312638d6f520df40ac953cd0c1c3bcc69701bb3" + integrity sha512-VAk6D8+1eaeyatFU6Uz9Odiqu58e4VtyWzqdy2EmajAuGzZ+jpZLWtAlRG/qfElAFKR1B7SUp7tHRApzEJywvQ== + +"@zxcvbn-ts/language-en@^2.1.0": + version "2.1.0" + resolved "https://registry.yarnpkg.com/@zxcvbn-ts/language-en/-/language-en-2.1.0.tgz#52c797914380546b191e5b915e0e9843116eae18" + integrity sha512-I3n4AAbArjPAZtwCrk9MQnSrcj5+9rq8sic2rUU44fP5QaR17Vk8zDt61+R9dnP9ZRsj09aAUYML4Ash05qZjQ== + +"@zxcvbn-ts/language-es-es@^2.1.1": + version "2.1.1" + resolved "https://registry.yarnpkg.com/@zxcvbn-ts/language-es-es/-/language-es-es-2.1.1.tgz#960b28bf58e537547293d555a36b1c42ef1ce66b" + integrity sha512-uDXU/z1df6YGmacFVcFhsvQ2Uu/EbMFCjLeNoM/95vH3GCTb/10eI5IlzjgSP4EG305vd9oNpBy6MODu+9SvNg== + +"@zxcvbn-ts/language-fr@^2.2.0": + version "2.2.0" + resolved "https://registry.yarnpkg.com/@zxcvbn-ts/language-fr/-/language-fr-2.2.0.tgz#0b7dd93ebba0044fbe733836bc7091b76d42afe1" + integrity sha512-KK+vIXm17mZyo7jLmV4T0fT6hh5NOBABdmkCBVpLyXq+rlZpdaz6HgoYLjqq2JbEU3KSZ+gv6qW+2N4dMk3Tlw== + +"@zxcvbn-ts/language-pt-br@^2.1.0": + version "2.1.0" + resolved "https://registry.yarnpkg.com/@zxcvbn-ts/language-pt-br/-/language-pt-br-2.1.0.tgz#27676c0ee8df538ffc98bc7bca2817d492c70428" + integrity sha512-g4HxJLBf546BSfM4zDq29CNpAmFl2UsgHrEjy6gUA4KBEVqEaYNnMNfvayEtM7PpnzfZjSyLLVVG6S02lR8w+g== + "@zxing/text-encoding@0.9.0": version "0.9.0" resolved "https://registry.yarnpkg.com/@zxing/text-encoding/-/text-encoding-0.9.0.tgz#fb50ffabc6c7c66a0c96b4c03e3d9be74864b70b"