Refresh only last login time when necessary (avoid hammering the db, ref #420)

src/libs/services/auth/impl/AuthServiceBase.cpp

@@ -19,6 +19,7 @@
 
 #include "AuthServiceBase.hpp"
 
+#include <cstdlib>
 #include "database/Db.hpp"
 #include "database/Session.hpp"
 #include "database/User.hpp"
@@ -29,19 +30,18 @@ namespace Auth
     using namespace Database;
 
     AuthServiceBase::AuthServiceBase(Db& db)
-	: _db {db}
+        : _db{ db }
     {}
 
-	UserId
+    UserId AuthServiceBase::getOrCreateUser(std::string_view loginName)
-	AuthServiceBase::getOrCreateUser(std::string_view loginName)
     {
-		Session& session {getDbSession()};
+        Session& session{ getDbSession() };
-		auto transaction {session.createWriteTransaction()};
+        auto transaction{ session.createWriteTransaction() };
 
-		User::pointer user {User::find(session, loginName)};
+        User::pointer user{ User::find(session, loginName) };
         if (!user)
         {
-			const UserType type {User::getCount(session) == 0 ? UserType::ADMIN : UserType::REGULAR};
+            const UserType type{ User::getCount(session) == 0 ? UserType::ADMIN : UserType::REGULAR };
 
             LMS_LOG(AUTH, DEBUG, "Creating user '" << loginName << "', admin = " << (type == UserType::ADMIN));
 
@@ -52,19 +52,31 @@ namespace Auth
         return user->getId();
     }
 
-	void
+    void AuthServiceBase::onUserAuthenticated(UserId userId)
-	AuthServiceBase::onUserAuthenticated(UserId userId)
     {
-		Session& session {getDbSession()};
+        Session& session{ getDbSession() };
-		auto transaction {session.createWriteTransaction()};
 
-		User::pointer user {User::find(session, userId)};
+        // Update last login only if relevant (avoid hammering write accesses to the database)
-		if (user)
+        {
-			user.modify()->setLastLogin(Wt::WDateTime::currentDateTime());
+            auto transaction{ session.createReadTransaction() };
+
+            const User::pointer user{ User::find(session, userId) };
+            if (!user)
+                return;
+
+            if (std::abs(Wt::WDateTime::currentDateTime().secsTo(user->getLastLogin())) < 60)
+                return;
         }
 
-	Session&
+        {
-	AuthServiceBase::getDbSession()
+            auto transaction{ session.createWriteTransaction() };
+
+            if (User::pointer user{ User::find(session, userId) })
+                user.modify()->setLastLogin(Wt::WDateTime::currentDateTime());
+        }
+    }
+
+    Session& AuthServiceBase::getDbSession()
     {
         return _db.getTLSSession();
     }

src/libs/services/auth/impl/internal/InternalPasswordService.cpp

@@ -30,7 +30,7 @@
 namespace Auth
 {
     InternalPasswordService::InternalPasswordService(Database::Db& db, std::size_t maxThrottlerEntries, IAuthTokenService& authTokenService)
-		: PasswordServiceBase {db, maxThrottlerEntries, authTokenService}
+        : PasswordServiceBase{ db, maxThrottlerEntries, authTokenService }
     {
         _validator.setMinimumLength(Wt::Auth::PasswordStrengthType::OneCharClass, 4);
         _validator.setMinimumLength(Wt::Auth::PasswordStrengthType::TwoCharClass, 4);
@@ -41,17 +41,16 @@ namespace Auth
         _validator.setMinimumMatchLength(3);
     }
 
-	bool
+    bool InternalPasswordService::checkUserPassword(std::string_view loginName, std::string_view password)
-	InternalPasswordService::checkUserPassword(std::string_view loginName, std::string_view password)
     {
         LMS_LOG(AUTH, DEBUG, "Checking internal password for user '" << loginName << "'");
 
         Database::User::PasswordHash passwordHash;
         {
-			Database::Session& session {getDbSession()};
+            Database::Session& session{ getDbSession() };
-			auto transaction {session.createReadTransaction()};
+            auto transaction{ session.createReadTransaction() };
 
-			const Database::User::pointer user {Database::User::find(session, loginName)};
+            const Database::User::pointer user{ Database::User::find(session, loginName) };
             if (!user)
             {
                 LMS_LOG(AUTH, DEBUG, "hashing random stuff");
@@ -70,62 +69,58 @@ namespace Auth
             }
         }
 
-		return _hashFunc.verify(std::string {password}, std::string {passwordHash.salt}, std::string {passwordHash.hash});
+        return _hashFunc.verify(std::string{ password }, std::string{ passwordHash.salt }, std::string{ passwordHash.hash });
     }
 
-	bool
+    bool InternalPasswordService::canSetPasswords() const
-	InternalPasswordService::canSetPasswords() const
     {
         return true;
     }
 
-	IPasswordService::PasswordAcceptabilityResult
+    IPasswordService::PasswordAcceptabilityResult InternalPasswordService::checkPasswordAcceptability(std::string_view password, const PasswordValidationContext& context) const
-	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() ? PasswordAcceptabilityResult::OK : PasswordAcceptabilityResult::TooWeak;
+            return _validator.evaluateStrength(std::string{ password }, context.loginName, "").isValid() ? PasswordAcceptabilityResult::OK : PasswordAcceptabilityResult::TooWeak;
         case Database::UserType::DEMO:
             return password == context.loginName ? PasswordAcceptabilityResult::OK : PasswordAcceptabilityResult::MustMatchLoginName;
         }
 
-		throw NotImplementedException {};
+        throw NotImplementedException{};
     }
 
-	void
+    void InternalPasswordService::setPassword(Database::UserId userId, std::string_view newPassword)
-	InternalPasswordService::setPassword(Database::UserId userId, std::string_view newPassword)
     {
-		const Database::User::PasswordHash passwordHash {hashPassword(newPassword)};
+        const Database::User::PasswordHash passwordHash{ hashPassword(newPassword) };
 
-		Database::Session& session {getDbSession()};
+        Database::Session& session{ getDbSession() };
-		auto transaction {session.createWriteTransaction()};
+        auto transaction{ session.createWriteTransaction() };
 
-		Database::User::pointer user {Database::User::find(session, userId)};
+        Database::User::pointer user{ Database::User::find(session, userId) };
         if (!user)
-			throw Exception {"User not found!"};
+            throw Exception{ "User not found!" };
 
-		switch (checkPasswordAcceptability(newPassword, PasswordValidationContext {user->getLoginName(), user->getType()}))
+        switch (checkPasswordAcceptability(newPassword, PasswordValidationContext{ user->getLoginName(), user->getType() }))
         {
         case PasswordAcceptabilityResult::OK:
             break;
         case PasswordAcceptabilityResult::TooWeak:
-				throw PasswordTooWeakException {};
+            throw PasswordTooWeakException{};
         case PasswordAcceptabilityResult::MustMatchLoginName:
-				throw PasswordMustMatchLoginNameException {};
+            throw PasswordMustMatchLoginNameException{};
         }
 
         user.modify()->setPasswordHash(passwordHash);
         getAuthTokenService().clearAuthTokens(userId);
     }
 
-	Database::User::PasswordHash
+    Database::User::PasswordHash InternalPasswordService::hashPassword(std::string_view password) const
-	InternalPasswordService::hashPassword(std::string_view password) const
     {
-		const std::string salt {Wt::WRandom::generateId(32)};
+        const std::string salt{ Wt::WRandom::generateId(32) };
 
-		return {salt, _hashFunc.compute(std::string {password}, salt)};
+        return { salt, _hashFunc.compute(std::string {password}, salt) };
     }
 
     void

src/libs/services/auth/impl/internal/InternalPasswordService.hpp

@@ -45,7 +45,7 @@ namespace Auth
         Database::User::PasswordHash	hashPassword(std::string_view password) const;
         void							hashRandomPassword() const;
 
-			const Wt::Auth::BCryptHashFunction	_hashFunc {7}; // TODO parametrize this
+        const Wt::Auth::BCryptHashFunction	_hashFunc{ 7 }; // TODO parametrize this
         Wt::Auth::PasswordStrengthValidator	_validator;
     };