diff --git a/approot/messages.xml b/approot/messages.xml index b599b76c..36d4bedd 100644 --- a/approot/messages.xml +++ b/approot/messages.xml @@ -17,6 +17,7 @@ Bad login / password combination Login throttled, please try again later Confirm password +Password must match the login name! New password Old password Password too weak @@ -94,7 +95,6 @@ Demo account Demo account already exists! -Demo password must be the login name! Last login User already exists! New user diff --git a/approot/messages_fr.xml b/approot/messages_fr.xml index 5b688e0e..bf3ba440 100644 --- a/approot/messages_fr.xml +++ b/approot/messages_fr.xml @@ -17,6 +17,7 @@ Mauvaise combinaison login / mot de passe Trop de tentatives de connexion, veuillez réessayer plus tard Confirmation du mot de passe +Le password doit être égal au login ! Nouveau mot de passe Ancien mot de passe Mot de passe trop faible @@ -94,7 +95,6 @@ Compte de démonstration Le compte de démonstration existe déjà ! -Le password doit être égal au login ! Date du dernier login L'utilisateur existe déjà ! Nouvel utilisateur diff --git a/approot/messages_it.xml b/approot/messages_it.xml index ad690776..f285e9dc 100644 --- a/approot/messages_it.xml +++ b/approot/messages_it.xml @@ -17,6 +17,7 @@ Errata combinazione di Login / Password Superati i tentativi di accesso, riprova più tardi Conferma la password +La password deve essere il nome utente! Nuova password Vecchia password La password è troppo debole @@ -93,7 +94,6 @@ Account demo L'account demo è già esistente! -La password dell'account demo deve essere il nome utente! Ultimo accesso Utente già esistente! Crea utente diff --git a/approot/messages_zh.xml b/approot/messages_zh.xml index 5e914553..e7f5dab7 100644 --- a/approot/messages_zh.xml +++ b/approot/messages_zh.xml @@ -17,6 +17,7 @@ 无效的登陆 / 密码组合 登录已被限制,请稍后再试 确认密码 +演示密码必须是登录名! 新密码 旧密码 密码太弱 @@ -94,7 +95,6 @@ 演示账号 演示账号已存在! -演示密码必须是登录名! 最后登录 用户已存在! 新建用户 diff --git a/src/libs/auth/impl/internal/InternalPasswordService.cpp b/src/libs/auth/impl/internal/InternalPasswordService.cpp index 2d17c044..82acd56b 100644 --- a/src/libs/auth/impl/internal/InternalPasswordService.cpp +++ b/src/libs/auth/impl/internal/InternalPasswordService.cpp @@ -80,16 +80,16 @@ namespace Auth return true; } - bool - InternalPasswordService::isPasswordSecureEnough(std::string_view password, const PasswordValidationContext& context) const + IPasswordService::PasswordAcceptabilityResult + InternalPasswordService::checkPasswordAcceptability(std::string_view password, const PasswordValidationContext& context) const { switch (context.userType) { case Database::UserType::ADMIN: case Database::UserType::REGULAR: - return _validator.evaluateStrength(std::string {password}, context.loginName, "").isValid(); + return _validator.evaluateStrength(std::string {password}, context.loginName, "").isValid() ? PasswordAcceptabilityResult::OK : PasswordAcceptabilityResult::TooWeak; case Database::UserType::DEMO: - return true; // no constraint + return password == context.loginName ? PasswordAcceptabilityResult::OK : PasswordAcceptabilityResult::MustMatchLoginName; } throw NotImplementedException {}; @@ -106,8 +106,15 @@ namespace Auth if (!user) throw Exception {"User not found!"}; - if (!isPasswordSecureEnough(newPassword, PasswordValidationContext {user->getLoginName(), user->getType()} )) - throw PasswordTooWeakException {}; + switch (checkPasswordAcceptability(newPassword, PasswordValidationContext {user->getLoginName(), user->getType()})) + { + case PasswordAcceptabilityResult::OK: + break; + case PasswordAcceptabilityResult::TooWeak: + throw PasswordTooWeakException {}; + case PasswordAcceptabilityResult::MustMatchLoginName: + throw PasswordMustMatchLoginNameException {}; + } user.modify()->setPasswordHash(passwordHash); getAuthTokenService().clearAuthTokens(session, userId); diff --git a/src/libs/auth/impl/internal/InternalPasswordService.hpp b/src/libs/auth/impl/internal/InternalPasswordService.hpp index 67a5da08..6e599204 100644 --- a/src/libs/auth/impl/internal/InternalPasswordService.hpp +++ b/src/libs/auth/impl/internal/InternalPasswordService.hpp @@ -41,7 +41,7 @@ namespace Auth std::string_view password) override; bool canSetPasswords() const override; - bool isPasswordSecureEnough(std::string_view loginName, const PasswordValidationContext& context) const override; + PasswordAcceptabilityResult checkPasswordAcceptability(std::string_view loginName, const PasswordValidationContext& context) const override; void setPassword(Database::Session& session, Database::IdType userId, std::string_view newPassword) override; Database::User::PasswordHash hashPassword(std::string_view password) const; diff --git a/src/libs/auth/impl/pam/PAMPasswordService.cpp b/src/libs/auth/impl/pam/PAMPasswordService.cpp index 7d8e1af2..47b75932 100644 --- a/src/libs/auth/impl/pam/PAMPasswordService.cpp +++ b/src/libs/auth/impl/pam/PAMPasswordService.cpp @@ -186,8 +186,8 @@ namespace Auth return false; } - bool - PAMPasswordService::isPasswordSecureEnough(std::string_view, const PasswordValidationContext&) const + IPasswordService::PasswordAcceptabilityResult + PAMPasswordService::checkPasswordAcceptability(std::string_view, const PasswordValidationContext&) const { throw NotImplementedException {}; } diff --git a/src/libs/auth/impl/pam/PAMPasswordService.hpp b/src/libs/auth/impl/pam/PAMPasswordService.hpp index 1b1bcc1c..b6213e17 100644 --- a/src/libs/auth/impl/pam/PAMPasswordService.hpp +++ b/src/libs/auth/impl/pam/PAMPasswordService.hpp @@ -36,7 +36,7 @@ namespace Auth std::string_view password) override; bool canSetPasswords() const override; - bool isPasswordSecureEnough(std::string_view loginName, const PasswordValidationContext& context) const override; + PasswordAcceptabilityResult checkPasswordAcceptability(std::string_view loginName, const PasswordValidationContext& context) const override; void setPassword(Database::Session& session, Database::IdType userId, std::string_view newPassword) override; diff --git a/src/libs/auth/include/auth/IPasswordService.hpp b/src/libs/auth/include/auth/IPasswordService.hpp index 0c4b1cc5..a5b9a0bc 100644 --- a/src/libs/auth/include/auth/IPasswordService.hpp +++ b/src/libs/auth/include/auth/IPasswordService.hpp @@ -56,15 +56,21 @@ namespace Auth std::optional userId {}; std::optional expiry {}; }; - virtual CheckResult checkUserPassword(Database::Session& session, - const boost::asio::ip::address& clientAddress, - std::string_view loginName, - std::string_view password) = 0; + virtual CheckResult checkUserPassword(Database::Session& session, + const boost::asio::ip::address& clientAddress, + std::string_view loginName, + std::string_view password) = 0; - virtual bool canSetPasswords() const = 0; + virtual bool canSetPasswords() const = 0; - virtual bool isPasswordSecureEnough(std::string_view password, const PasswordValidationContext& context) const = 0; - virtual void setPassword(Database::Session& session, Database::IdType userId, std::string_view newPassword) = 0; + enum class PasswordAcceptabilityResult + { + OK, + TooWeak, + MustMatchLoginName, + }; + virtual PasswordAcceptabilityResult checkPasswordAcceptability(std::string_view password, const PasswordValidationContext& context) const = 0; + virtual void setPassword(Database::Session& session, Database::IdType userId, std::string_view newPassword) = 0; }; std::unique_ptr createPasswordService(std::string_view authPasswordBackend, std::size_t maxThrottlerEntryCount, IAuthTokenService& authTokenService); diff --git a/src/libs/auth/include/auth/Types.hpp b/src/libs/auth/include/auth/Types.hpp index dec88e0a..8b97df6e 100644 --- a/src/libs/auth/include/auth/Types.hpp +++ b/src/libs/auth/include/auth/Types.hpp @@ -36,16 +36,34 @@ namespace Auth NotImplementedException() : Auth::Exception {"Not implemented"} {} }; + class UserNotFoundException : public Exception + { + public: + UserNotFoundException() : Auth::Exception {"User not found"} {} + }; + struct PasswordValidationContext { std::string loginName; Database::UserType userType; }; - class PasswordTooWeakException : public Exception + class PasswordException : public Exception { public: - PasswordTooWeakException() : Auth::Exception {"Password too weak"} {} + using Exception::Exception; + }; + + class PasswordTooWeakException : public PasswordException + { + public: + PasswordTooWeakException() : PasswordException {"Password too weak"} {} + }; + + class PasswordMustMatchLoginNameException : public PasswordException + { + public: + PasswordMustMatchLoginNameException() : PasswordException {"Password must match login name"} {} }; } diff --git a/src/libs/subsonic/impl/SubsonicResource.cpp b/src/libs/subsonic/impl/SubsonicResource.cpp index fa22a14b..2c0a7e1a 100644 --- a/src/libs/subsonic/impl/SubsonicResource.cpp +++ b/src/libs/subsonic/impl/SubsonicResource.cpp @@ -559,11 +559,15 @@ handleChangePassword(RequestContext& context) Service::get()->setPassword(context.dbSession, userId, password); } - catch (Auth::PasswordTooWeakException&) + catch (const Auth::PasswordMustMatchLoginNameException&) + { + throw PasswordMustMatchLoginNameGenericError {}; + } + catch (const Auth::PasswordTooWeakException&) { throw PasswordTooWeakGenericError {}; } - catch (Auth::Exception& authException) + catch (const Auth::Exception& authException) { throw UserNotAuthorizedError {}; } @@ -658,6 +662,11 @@ handleCreateUserRequest(RequestContext& context) { Service::get()->setPassword(context.dbSession, userId, password); } + catch (const Auth::PasswordMustMatchLoginNameException&) + { + removeCreatedUser(); + throw PasswordMustMatchLoginNameGenericError {}; + } catch (const Auth::PasswordTooWeakException&) { removeCreatedUser(); @@ -1711,6 +1720,10 @@ handleUpdateUserRequest(RequestContext& context) { Service<::Auth::IPasswordService>()->setPassword(context.dbSession, userId, decodePasswordIfNeeded(*password)); } + catch (const Auth::PasswordMustMatchLoginNameException&) + { + throw PasswordMustMatchLoginNameGenericError {}; + } catch (const Auth::PasswordTooWeakException&) { throw PasswordTooWeakGenericError {}; diff --git a/src/libs/subsonic/impl/SubsonicResponse.hpp b/src/libs/subsonic/impl/SubsonicResponse.hpp index bd7339c4..a19c195d 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.hpp +++ b/src/libs/subsonic/impl/SubsonicResponse.hpp @@ -152,6 +152,11 @@ class PasswordTooWeakGenericError : public GenericError std::string getMessage() const override { return "Password too weak"; } }; +class PasswordMustMatchLoginNameGenericError : public GenericError +{ + std::string getMessage() const override { return "Password must match login name"; } +}; + class DemoUserCannotChangePasswordGenericError : public GenericError { std::string getMessage() const override { return "Demo user cannot change its password"; } diff --git a/src/lms/ui/SettingsView.cpp b/src/lms/ui/SettingsView.cpp index 59e0cbff..fc446feb 100644 --- a/src/lms/ui/SettingsView.cpp +++ b/src/lms/ui/SettingsView.cpp @@ -276,6 +276,14 @@ class SettingsModel : public Wt::WFormModel validator(SettingsModel::ListenBrainzTokenField)->setMandatory(usesListenBrainz); } } + if (_authPasswordService) + { + if (_withOldPassword) + setValue(PasswordOldField, ""); + + setValue(PasswordField, ""); + setValue(PasswordConfirmField, ""); + } } private: diff --git a/src/lms/ui/admin/UserView.cpp b/src/lms/ui/admin/UserView.cpp index 20ac8b43..e0467939 100644 --- a/src/lms/ui/admin/UserView.cpp +++ b/src/lms/ui/admin/UserView.cpp @@ -66,7 +66,7 @@ class UserModel : public Wt::WFormModel if (authPasswordService) { addField(PasswordField); - setValidator(PasswordField, createPasswordStrengthValidator([this] { return ::Auth::PasswordValidationContext {getLoginName(), Wt::asNumber(value(DemoField)) ? UserType::DEMO : UserType::REGULAR}; })); + setValidator(PasswordField, createPasswordStrengthValidator([this] { return ::Auth::PasswordValidationContext {getLoginName(), getUserType()}; })); if (!userId) validator(PasswordField)->setMandatory(true); } @@ -122,6 +122,19 @@ class UserModel : public Wt::WFormModel throw UserNotAllowedException {}; } + Database::UserType getUserType() const + { + if (_userId) + { + auto transaction {LmsApp->getDbSession().createSharedTransaction()}; + + const Database::User::pointer user {Database::User::getById(LmsApp->getDbSession(), *_userId)}; + return user->getType(); + } + + return Wt::asNumber(value(DemoField)) ? UserType::DEMO : UserType::REGULAR; + } + std::string getLoginName() const { if (_userId) @@ -131,18 +144,8 @@ class UserModel : public Wt::WFormModel const Database::User::pointer user {Database::User::getById(LmsApp->getDbSession(), *_userId)}; return user->getLoginName(); } - else - return valueText(LoginField).toUTF8(); - } - void validatePassword(Wt::WString& error) const - { - if (!valueText(PasswordField).empty() && Wt::asNumber(value(DemoField))) - { - // Demo account: password must be the same as the login name - if (valueText(PasswordField) != getLoginName()) - error = Wt::WString::tr("Lms.Admin.User.demo-password-invalid"); - } + return valueText(LoginField).toUTF8(); } bool validateField(Field field) @@ -157,13 +160,6 @@ class UserModel : public Wt::WFormModel if (user) error = Wt::WString::tr("Lms.Admin.User.user-already-exists"); } - else if (field == PasswordField) - { - if (Wt::asNumber(value(DemoField))) - setValidator(PasswordField, {}); - - validatePassword(error); - } else if (field == DemoField) { auto transaction {LmsApp->getDbSession().createSharedTransaction()}; diff --git a/src/lms/ui/common/PasswordValidator.cpp b/src/lms/ui/common/PasswordValidator.cpp index b923fe80..443d556f 100644 --- a/src/lms/ui/common/PasswordValidator.cpp +++ b/src/lms/ui/common/PasswordValidator.cpp @@ -48,10 +48,17 @@ namespace UserInterface const ::Auth::PasswordValidationContext context {_passwordValidationContextGetFunc()}; - if (Service<::Auth::IPasswordService>::get()->isPasswordSecureEnough(input.toUTF8(), context)) - return Wt::WValidator::Result {Wt::ValidationState::Valid}; + switch (Service<::Auth::IPasswordService>::get()->checkPasswordAcceptability(input.toUTF8(), context)) + { + case ::Auth::IPasswordService::PasswordAcceptabilityResult::OK: + return Wt::WValidator::Result {Wt::ValidationState::Valid}; + case ::Auth::IPasswordService::PasswordAcceptabilityResult::TooWeak: + return Wt::WValidator::Result {Wt::ValidationState::Invalid, Wt::WString::tr("Lms.password-too-weak")}; + case ::Auth::IPasswordService::PasswordAcceptabilityResult::MustMatchLoginName: + return Wt::WValidator::Result {Wt::ValidationState::Invalid, Wt::WString::tr("Lms.password-must-match-login")}; + } - return Wt::WValidator::Result {Wt::ValidationState::Invalid, Wt::WString::tr("Lms.password-too-weak")}; + throw LmsException {"internal error"}; } std::shared_ptr