diff --git a/.gitignore b/.gitignore index 2aacd31..915e9e9 100644 --- a/.gitignore +++ b/.gitignore @@ -2,7 +2,7 @@ /testdb /*.dll /*.exe -userman +/userman userman.exe userman-test-* libuserman.a diff --git a/dub.sdl b/dub.sdl index f5f27cb..d1f46b1 100644 --- a/dub.sdl +++ b/dub.sdl @@ -5,6 +5,9 @@ copyright "Copyright (c) 2012-2020 rejectedsoftware e.K." license "GPL-3.0" dependency "vibe-d" version=">=0.9.7 <0.11.0" +dependency "botan" version="*" # needed for bcrypt, available since initial version + +subConfiguration "botan" "full" configuration "application" { targetType "executable" diff --git a/dub.selections.json b/dub.selections.json index b857050..c0512b5 100644 --- a/dub.selections.json +++ b/dub.selections.json @@ -1,13 +1,14 @@ { "fileVersion": 1, "versions": { - "botan": "1.12.18", - "botan-math": "1.0.3", - "diet-ng": "1.8.2", - "eventcore": "0.9.35", + "botan": "1.13.8", + "botan-math": "1.0.4", + "diet-ng": "1.8.4", + "during": "0.3.0", + "eventcore": "0.9.38", "libasync": "0.8.6", "libevent": "2.0.2+2.0.16", - "memutils": "1.0.10", + "memutils": "1.0.11", "mir-linux-kernel": "1.2.1", "openssl": "3.3.4", "openssl-static": "1.0.5+3.0.8", diff --git a/source/userman/api.d b/source/userman/api.d index b91353e..dd98c49 100644 --- a/source/userman/api.d +++ b/source/userman/api.d @@ -7,12 +7,12 @@ */ module userman.api; -import userman.db.controller : UserManController, UserManCommonSettings; +import userman.db.controller : UserManCommonSettings, UserManController; static import userman.db.controller; import vibe.data.json : Json; -import vibe.http.router : URLRouter; import vibe.http.common : enforceHTTP; +import vibe.http.router : URLRouter; import vibe.http.status : HTTPStatus; import vibe.inet.url : URL; import vibe.web.rest; @@ -72,6 +72,9 @@ interface UserManUserAPI { @property Collection!UserManUserPropertyAPI properties(User.ID _user); /// Tests a username/e-mail and password combination for validity. + UserLoginInfo testLoginInfo(string email_or_name, string password); + + /// Deprecated: you should use testLoginInfo instead! User.ID testLogin(string name, string password); /// Registers a new user. @@ -165,6 +168,15 @@ struct User { } } +/// Information about a user coming from a login attempt. +struct UserLoginInfo { + /// User ID of the login credentials + User.ID userId; + /// Set to true if the current password is deemed too insecure now. + /// In that case, a password reset should be mandatory. + bool needsPasswordChange; +} + /// Interface suitable for manipulating group information interface UserManGroupAPI { @safe: @@ -277,11 +289,18 @@ private class UserManUserAPIImpl : UserManUserAPI { return Collection!UserManUserPropertyAPI(m_properties, _id); } + UserLoginInfo testLoginInfo(string email_or_name, string password) + { + auto ret = m_ctrl.validateLogin(email_or_name, password); + enforceHTTP(!ret.userId.isNull, HTTPStatus.unauthorized, "Wrong user name or password."); + return UserLoginInfo(ret.userId.get(), ret.needsPasswordChange); + } + User.ID testLogin(string name, string password) { - auto ret = m_ctrl.testLogin(name, password); - enforceHTTP(!ret.isNull, HTTPStatus.unauthorized, "Wrong user name or password."); - return ret.get(); + auto ret = m_ctrl.validateLogin(name, password); + enforceHTTP(!ret.userId.isNull, HTTPStatus.unauthorized, "Wrong user name or password."); + return ret.userId.get(); } User.ID register(string email, string name, string full_name, string password) diff --git a/source/userman/db/controller.d b/source/userman/db/controller.d index fbe897f..bdddd11 100644 --- a/source/userman/db/controller.d +++ b/source/userman/db/controller.d @@ -10,13 +10,13 @@ module userman.db.controller; public import userman.userman; import userman.id; +import diet.html; import vibe.data.serialization; import vibe.db.mongo.mongo; import vibe.http.router; import vibe.mail.smtp; import vibe.stream.memory; import vibe.utils.validation; -import diet.html; import std.algorithm; import std.array; @@ -77,8 +77,8 @@ class UserManController { user.name = name; user.fullName = full_name; user.auth.method = "password"; - user.auth.passwordHash = generatePasswordHash(password); - assert(validatePasswordHash(user.auth.passwordHash, password)); + user.auth.passwordHash = generateBcryptHash(password); + assert(validateBcryptHash(user.auth.passwordHash, password)); user.email = email; if( need_activation ) user.activationCode = generateActivationCode(); @@ -127,17 +127,43 @@ class UserManController { } } - Nullable!(User.ID) testLogin(string name, string password) + static struct LoginResult + { + /// If set, the username and password combination was found in the DB. + Nullable!(User.ID) userId; + /// Set to true if the current password is deemed too insecure now. + /// In that case, a password reset should be mandatory. + bool needsPasswordChange; + } + + /// Checks if the login credentials are correct, returns one of: + /// - invalid username or password (result is LoginResult.init) + /// - valid user, but password reset is required (userId is set, needsPasswordChange is true) + /// - valid: valid user (userId is set, needsPasswordChange is false) + LoginResult validateLogin(string emailOrName, string password) { - string password_ = password; - auto user = getUserByEmailOrName(name); - assert(password == password_); // this used to be false to to a Nullable related codegen issue - Nullable!(User.ID) ret; - if (validatePasswordHash(user.auth.passwordHash, password_)) - ret = user.id; + auto user = getUserByEmailOrName(emailOrName); + LoginResult ret; + bool validPassword; + if (user.auth.passwordHash.length && user.auth.passwordHash[0] == '$') { + validPassword = validateBcryptHash(user.auth.passwordHash, password); + } else { + validPassword = validatePasswordHashMD5(user.auth.passwordHash, password); + ret.needsPasswordChange = true; + } + + if (!validPassword) + return LoginResult.init; + + ret.userId = user.id; return ret; } + deprecated("Use validateLogin instead") Nullable!(User.ID) testLogin(string name, string password) + { + return validateLogin(name, password).userId; + } + void activateUser(string email, string activation_code) { email = email.toLower(); @@ -208,7 +234,7 @@ class UserManController { usr.resetCode = ""; updateUser(usr); enforce(reset_code == code, "Invalid request code, please request a new one."); - usr.auth.passwordHash = generatePasswordHash(new_password); + usr.auth.passwordHash = generateBcryptHash(new_password); updateUser(usr); } @@ -263,8 +289,8 @@ class UserManController { */ static bool isValidGroupID(string name) { - import std.ascii : isAlpha, isDigit; import std.algorithm : splitter; + import std.ascii : isAlpha, isDigit; if (name.length < 1) return false; foreach (p; name.splitter('.')) { @@ -322,7 +348,29 @@ string generateActivationCode() return ret.data(); } -string generatePasswordHash(string password) +string generateBcryptHash(in string password, ushort work_factor = 12) +@trusted { + import botan.passhash.bcrypt : generateBcrypt; + import botan.rng.auto_rng; + + static AutoSeededRNG rng; + if (rng is null) + rng = new AutoSeededRNG; + + return generateBcrypt(password, rng, work_factor); +} + +bool validateBcryptHash(in string password_hash, in string password) +@trusted { + import botan.passhash.bcrypt : checkBcrypt; + + return checkBcrypt(password, password_hash); +} + +deprecated("Use generatePasswordHashMD5 for old (insecure) behavior or generateBcryptHash") alias generatePasswordHash = generatePasswordHashMD5; +deprecated("Use validatePasswordHashMD5 for old (insecure) behavior or validateBcryptHash") alias validatePasswordHash = validatePasswordHashMD5; + +deprecated("This method is insecure") string generatePasswordHashMD5(string password) @safe { import std.base64 : Base64; @@ -333,7 +381,7 @@ string generatePasswordHash(string password) return Base64.encode(salt ~ hash).idup; } -bool validatePasswordHash(string password_hash, string password) +bool validatePasswordHashMD5(string password_hash, string password) @safe { import std.base64 : Base64; @@ -347,10 +395,16 @@ bool validatePasswordHash(string password_hash, string password) return hash == hashcmp; } +deprecated unittest { + auto h = generatePasswordHashMD5("foobar"); + assert(!validatePasswordHashMD5(h, "foo")); + assert(validatePasswordHashMD5(h, "foobar")); +} + unittest { - auto h = generatePasswordHash("foobar"); - assert(!validatePasswordHash(h, "foo")); - assert(validatePasswordHash(h, "foobar")); + auto h = generateBcryptHash("foobar"); + assert(!validateBcryptHash(h, "foo")); + assert(validateBcryptHash(h, "foobar")); } private ubyte[16] md5hash(scope const(ubyte)[] salt, const string[] strs...) diff --git a/source/userman/db/file.d b/source/userman/db/file.d index d6b39c2..d6defef 100644 --- a/source/userman/db/file.d +++ b/source/userman/db/file.d @@ -14,10 +14,10 @@ import vibe.data.json; import vibe.textfilter.urlencode; import vibe.utils.validation; +import std.conv; import std.datetime; import std.exception; import std.string; -import std.conv; import std.uuid; @@ -181,7 +181,7 @@ class FileUserManController : UserManController { { auto usr = getUser(user); usr.auth.method = "password"; - usr.auth.passwordHash = generatePasswordHash(password); + usr.auth.passwordHash = generateBcryptHash(password); updateUser(usr); } diff --git a/source/userman/db/mongo.d b/source/userman/db/mongo.d index e41bf52..a6561ca 100644 --- a/source/userman/db/mongo.d +++ b/source/userman/db/mongo.d @@ -159,7 +159,7 @@ class MongoUserManController : UserManController { override void setPassword(User.ID user, string password) { m_users.updateOne(["_id": user.bsonObjectIDValue], ["$set": - ["auth.method": "password", "auth.passwordHash": generatePasswordHash(password)]]); + ["auth.method": "password", "auth.passwordHash": generateBcryptHash(password)]]); } override void setProperty(User.ID user, string name, Json value) diff --git a/source/userman/db/redis.d b/source/userman/db/redis.d index 2c0ad62..f1a6309 100644 --- a/source/userman/db/redis.d +++ b/source/userman/db/redis.d @@ -9,18 +9,18 @@ module userman.db.redis; import userman.db.controller; -import vibe.db.redis.redis; -import vibe.db.redis.idioms; -import vibe.db.redis.types; import vibe.data.bson; import vibe.data.json; +import vibe.db.redis.idioms; +import vibe.db.redis.redis; +import vibe.db.redis.types; import vibe.utils.validation; +import std.conv; import std.datetime; import std.exception; -import std.string; -import std.conv; import std.range : front; +import std.string; class RedisUserManController : UserManController { @@ -287,7 +287,7 @@ class RedisUserManController : UserManController { AuthInfo auth = m_userAuthInfo[user.longValue]; auth.method = "password"; - auth.passwordHash = generatePasswordHash(password); + auth.passwordHash = generateBcryptHash(password); m_userAuthInfo[user.longValue] = auth; } diff --git a/source/userman/web.d b/source/userman/web.d index 419004f..8af4f45 100644 --- a/source/userman/web.d +++ b/source/userman/web.d @@ -298,10 +298,15 @@ class UserManWebInterface { @noAuth @errorDisplay!getLogin void postLogin(string name, string password, string redirect = "") { + import std.string : chomp; + User user; + string redirectUrl = redirect.length ? redirect : m_prefix; try { - auto uid = m_api.users.testLogin(name, password); - user = m_api.users[uid].get(); + auto result = m_api.users.testLoginInfo(name, password); + if (result.needsPasswordChange) + redirectUrl = m_prefix.chomp("/") ~ "/profile?changepw=true"; + user = m_api.users[result.userId].get(); } catch (Exception e) { import std.encoding : sanitize; logDebug("Error logging in: %s", e.toString().sanitize); @@ -314,7 +319,7 @@ class UserManWebInterface { m_sessUserName = user.name; m_sessUserFullName = user.fullName; m_sessUserID = user.id.toString(); - .redirect(redirect.length ? redirect : m_prefix); + .redirect(redirectUrl); } @noAuth @@ -430,14 +435,14 @@ class UserManWebInterface { } @anyAuth - void getProfile(HTTPServerRequest req, User _user, string _error = "") + void getProfile(HTTPServerRequest req, User _user, string _error = "", bool changepw = false) { req.form["full_name"] = _user.fullName; req.form["email"] = _user.email; bool useUserNames = m_settings.useUserNames; auto user = _user; string error = _error; - render!("userman.profile.dt", user, useUserNames, error); + render!("userman.profile.dt", user, useUserNames, error, changepw); } @anyAuth @errorDisplay!getProfile diff --git a/source/userman/webadmin.d b/source/userman/webadmin.d index e4d7c3b..082c1f2 100644 --- a/source/userman/webadmin.d +++ b/source/userman/webadmin.d @@ -17,7 +17,7 @@ import vibe.utils.validation; import vibe.web.auth; import vibe.web.web; -import std.algorithm : min, max; +import std.algorithm : max, min; import std.array : appender; import std.conv : to; import std.exception; @@ -63,21 +63,21 @@ class UserManWebAdminInterface { { import std.algorithm.searching : canFind; - User.ID uid; - try uid = m_api.users.testLogin(name, password); + UserLoginInfo info; + try info = m_api.users.testLoginInfo(name, password); catch (Exception e) { import std.encoding : sanitize; logDebug("Error logging in: %s", e.toString().sanitize); throw new Exception("Invalid user/email or password."); } - auto user = m_api.users[uid].get(); + auto user = m_api.users[info.userId].get(); enforce(user.active, "The account is not yet activated."); - enforce(m_api.users[uid].getGroups().canFind(adminGroupName), "User is not an administrator."); + enforce(m_api.users[info.userId].getGroups().canFind(adminGroupName), "User is not an administrator."); m_authUser = user.id; m_authUserDisplayName = user.fullName; - .redirect(redirect); + .redirect(info.needsPasswordChange ? "/profile?changepw=true" : redirect); } @noAuth @errorDisplay!getLogin @@ -393,9 +393,9 @@ struct ValidGroupName { static Nullable!ValidGroupName fromStringValidate(string str, string* err) { - import vibe.utils.validation : validateIdent; import std.algorithm : splitter; import std.array : appender; + import vibe.utils.validation : validateIdent; // work around disabled default construction auto ret = Nullable!ValidGroupName(ValidGroupName(null)); diff --git a/views/userman.profile.dt b/views/userman.profile.dt index d526174..1c3a9ec 100644 --- a/views/userman.profile.dt +++ b/views/userman.profile.dt @@ -4,30 +4,34 @@ block title - title = "Edit profile"; block userman.content - h2 Personal information + - if (!changepw) + h2 Personal information - form(action="profile", method="POST") - table - col.caption - - if (useUserNames) + form(action="profile", method="POST") + table + col.caption + - if (useUserNames) + tr + td User name + td= user.name tr - td User name - td= user.name - tr - td Display name - td - input(type="text", name="full_name", value=req.form["full_name"]) - tr - td E-mail address - td - input(type="email", name="email", value=req.form["email"]) - tr - td - td - button(type="submit") Apply + td Display name + td + input(type="text", name="full_name", value=req.form["full_name"]) + tr + td E-mail address + td + input(type="email", name="email", value=req.form["email"]) + tr + td + td + button(type="submit") Apply h2 Change password + - if (changepw) + p.error Your password was insecurely saved on our servers - please set a new password to ensure your account's safety. + form(action="profile", method="POST") table col.caption