Page MenuHomePhorge
Files F117884645

D5078.1775352882.diff
No OneTemporary
Actions

Authored By
Unknown
Size
10 KB
Referenced Files
None
Subscribers
None

D5078.1775352882.diff
View Options

diff --git a/src/app/Http/Controllers/API/V4/NGINXController.php b/src/app/Http/Controllers/API/V4/NGINXController.php
--- a/src/app/Http/Controllers/API/V4/NGINXController.php
+++ b/src/app/Http/Controllers/API/V4/NGINXController.php
@@ -2,11 +2,9 @@
namespace App\Http\Controllers\API\V4;
-use App\Auth\Utils as AuthUtils;
use App\Http\Controllers\Controller;
use App\User;
use Illuminate\Http\Request;
-use Illuminate\Support\Facades\Hash;
class NGINXController extends Controller
{
@@ -30,25 +28,13 @@
throw new \Exception("Empty password");
}
- if ($userid = AuthUtils::tokenValidate($password)) {
- $user = User::find($userid);
- if ($user && $user->email == $login) {
- return $user;
- }
-
- throw new \Exception("Password mismatch");
- }
-
- $user = User::where('email', $login)->first();
- if (!$user) {
- throw new \Exception("User not found");
- }
+ $result = User::findAndAuthenticate($login, $password, null, false);
- if (!Hash::check($password, $user->password)) {
- throw new \Exception("Password mismatch");
+ if (empty($result['user'])) {
+ throw new \Exception($result['errorMessage'] ?? "Unknown error");
}
- return $user;
+ return $result['user'];
}
/**
diff --git a/src/app/User.php b/src/app/User.php
--- a/src/app/User.php
+++ b/src/app/User.php
@@ -416,6 +416,18 @@
return false;
}
+ /**
+ * Check if multi factor authentication is enabled
+ *
+ * @return bool
+ */
+ public function isMFAEnabled(): bool
+ {
+ return \App\CompanionApp::where('user_id', $this->id)
+ ->where('mfa_enabled', true)
+ ->exists();
+ }
+
/**
* Returns whether this user is restricted.
*
@@ -712,51 +724,32 @@
/**
* Validate the user credentials
*
- * @param string $username The username.
- * @param string $password The password in plain text.
- * @param bool $updatePassword Store the password if currently empty
- *
- * @return bool true on success
+ * @param string $password The password in plain text.
*/
- public function validateCredentials(string $username, string $password, bool $updatePassword = true): bool
+ public function validatePassword(string $password): bool
{
$authenticated = false;
- if ($this->email === \strtolower($username)) {
- if (!empty($this->password)) {
- if (Hash::check($password, $this->password)) {
- $authenticated = true;
- }
- } elseif (!empty($this->password_ldap)) {
- if (substr($this->password_ldap, 0, 6) == "{SSHA}") {
- $salt = substr(base64_decode(substr($this->password_ldap, 6)), 20);
-
- $hash = '{SSHA}' . base64_encode(
- sha1($password . $salt, true) . $salt
- );
-
- if ($hash == $this->password_ldap) {
- $authenticated = true;
- }
- } elseif (substr($this->password_ldap, 0, 9) == "{SSHA512}") {
- $salt = substr(base64_decode(substr($this->password_ldap, 9)), 64);
+ if (!empty($this->password)) {
+ $authenticated = Hash::check($password, $this->password);
+ } elseif (!empty($this->password_ldap)) {
+ if (substr($this->password_ldap, 0, 6) == "{SSHA}") {
+ $salt = substr(base64_decode(substr($this->password_ldap, 6)), 20);
+ $hash = '{SSHA}' . base64_encode(sha1($password . $salt, true) . $salt);
- $hash = '{SSHA512}' . base64_encode(
- pack('H*', hash('sha512', $password . $salt)) . $salt
- );
+ $authenticated = $hash === $this->password_ldap;
+ } elseif (substr($this->password_ldap, 0, 9) == "{SSHA512}") {
+ $salt = substr(base64_decode(substr($this->password_ldap, 9)), 64);
+ $hash = '{SSHA512}' . base64_encode(pack('H*', hash('sha512', $password . $salt)) . $salt);
- if ($hash == $this->password_ldap) {
- $authenticated = true;
- }
- }
- } else {
- \Log::error("Incomplete credentials for {$this->email}");
+ $authenticated = $hash === $this->password_ldap;
}
+ } else {
+ \Log::error("Missing password for {$this->email}");
}
if ($authenticated) {
- // TODO: update last login time
- if ($updatePassword && (empty($this->password) || empty($this->password_ldap))) {
+ if (empty($this->password) || empty($this->password_ldap)) {
$this->password = $password;
$this->save();
}
@@ -783,18 +776,6 @@
return in_array(\App\Utils::countryForIP($ip), $countryCodes);
}
- /**
- * Check if multi factor verification is enabled
- *
- * @return bool
- */
- public function mfaEnabled(): bool
- {
- return \App\CompanionApp::where('user_id', $this->id)
- ->where('mfa_enabled', true)
- ->exists();
- }
-
/**
* Retrieve and authenticate a user
*
@@ -824,8 +805,7 @@
$error = AuthAttempt::REASON_PASSWORD;
}
} else {
- // Check user password
- if (!$user->validateCredentials($username, $password)) {
+ if (!$user->validatePassword($password)) {
$error = AuthAttempt::REASON_PASSWORD;
}
}
@@ -848,7 +828,7 @@
}
// Check 2FA - Companion App
- if (!$error && $user->mfaEnabled()) {
+ if (!$error && $user->isMFAEnabled()) {
$attempt = AuthAttempt::recordAuthAttempt($user, $clientIP);
if (!$attempt->waitFor2FA()) {
$error = AuthAttempt::REASON_2FA;
diff --git a/src/tests/Feature/UserTest.php b/src/tests/Feature/UserTest.php
--- a/src/tests/Feature/UserTest.php
+++ b/src/tests/Feature/UserTest.php
@@ -1577,6 +1577,52 @@
$this->assertInstanceOf(\App\Wallet::class, $ned->wallets->first());
}
+ /**
+ * Test User password validation
+ */
+ public function testValidatePassword(): void
+ {
+ Queue::fake();
+
+ $user = $this->getTestUser('user-test@' . \config('app.domain'), ['password' => 'test']);
+ $hash = $user->password;
+ $ldap = "{SSHA512}7iaw3Ur350mqGo7jwQrpkj9hiYB3Lkc/iBml1JQODbJ6wYX4oOHV+E+IvIh/1nsUNzLDBMxfqa2Ob1f1ACio/w==";
+ $attrs = $user->getAttributes();
+
+ // Note: A User has two password properties, on a successful validation missing password should get updated
+
+ // Wrong password
+ $user->setRawAttributes(array_merge($attrs, ['password_ldap' => null]));
+ $this->assertFalse($user->validatePassword('wrong'));
+ $this->assertTrue($user->password_ldap === null);
+
+ // Valid password (in 'password_ldap' only)
+ $user->setRawAttributes(array_merge($attrs, ['password_ldap' => $ldap, 'password' => null]));
+ $user->save();
+ $user->refresh();
+ $this->assertTrue($user->password === null);
+ $this->assertTrue($user->validatePassword('test'));
+ $this->assertTrue($user->password_ldap == $ldap);
+ $this->assertTrue(strlen($user->password) == strlen($hash)); // @phpstan-ignore-line
+ $user->refresh();
+ $this->assertTrue($user->password_ldap == $ldap);
+ $this->assertTrue(strlen($user->password) == strlen($hash)); // @phpstan-ignore-line
+
+ // Valid password (in 'password' only)
+ $user->setRawAttributes(array_merge($attrs, ['password_ldap' => null, 'password' => $hash]));
+ $user->save();
+ $user->refresh();
+ $this->assertTrue($user->password_ldap === null);
+ $this->assertTrue($user->validatePassword('test'));
+ $this->assertTrue(strlen($user->password_ldap) == strlen($ldap)); // @phpstan-ignore-line
+ $this->assertTrue(strlen($user->password) == strlen($hash));
+ $user->refresh();
+ $this->assertTrue($user->password_ldap == $ldap);
+ $this->assertTrue(strlen($user->password) == strlen($hash));
+
+ // TODO: sha1 passwords in password_ldap (or remove this code)
+ }
+
/**
* Tests for User::findAndAuthenticate()
*/
@@ -1592,5 +1638,4 @@
$token = AuthUtils::tokenCreate($this->getTestUser('ned@kolab.org')->id);
$this->assertFalse(isset(User::findAndAuthenticate($user->email, $token)['user']));
}
-
}
diff --git a/src/tests/Unit/UserTest.php b/src/tests/Unit/UserTest.php
--- a/src/tests/Unit/UserTest.php
+++ b/src/tests/Unit/UserTest.php
@@ -40,31 +40,6 @@
$this->assertSame($ssh512, $user->password_ldap);
}
- /**
- * Test User password validation
- */
- public function testPasswordValidation(): void
- {
- $user = new User(['email' => 'user@email.com']);
- $user->password = 'test';
-
- $this->assertSame(true, $user->validateCredentials('user@email.com', 'test'));
- $this->assertSame(false, $user->validateCredentials('user@email.com', 'wrong'));
- $this->assertSame(true, $user->validateCredentials('User@Email.Com', 'test'));
- $this->assertSame(false, $user->validateCredentials('wrong', 'test'));
-
- // Ensure the fallback to the ldap_password works if the current password is empty
- $ssh512 = "{SSHA512}7iaw3Ur350mqGo7jwQrpkj9hiYB3Lkc/iBml1JQODbJ"
- . "6wYX4oOHV+E+IvIh/1nsUNzLDBMxfqa2Ob1f1ACio/w==";
- $ldapUser = new User(['email' => 'user2@email.com']);
- $ldapUser->setRawAttributes(['password' => '', 'password_ldap' => $ssh512, 'email' => 'user2@email.com']);
- $this->assertSame($ldapUser->password, '');
- $this->assertSame($ldapUser->password_ldap, $ssh512);
-
- $this->assertSame(true, $ldapUser->validateCredentials('user2@email.com', 'test', false));
- $ldapUser->delete();
- }
-
/**
* Test User role mutator
*/

File Metadata

Mime Type
text/plain
Expires
Sun, Apr 5, 1:34 AM (18 h, 12 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18828611
Default Alt Text
D5078.1775352882.diff (10 KB)

Event Timeline