Changeset View
Standalone View
src/app/Http/Controllers/API/V4/NGINXController.php
<?php | <?php | ||||
namespace App\Http\Controllers\API\V4; | namespace App\Http\Controllers\API\V4; | ||||
use App\Http\Controllers\Controller; | use App\Http\Controllers\Controller; | ||||
use Illuminate\Http\Request; | use Illuminate\Http\Request; | ||||
use Illuminate\Support\Facades\Hash; | use Illuminate\Support\Facades\Hash; | ||||
use Illuminate\Support\Str; | use Illuminate\Support\Str; | ||||
class NGINXController extends Controller | class NGINXController extends Controller | ||||
{ | { | ||||
/** | /** | ||||
* Authentication request. | * Authorize with the provided credentials. | ||||
* | * | ||||
* @todo: Separate IMAP(+STARTTLS) from IMAPS, same for SMTP/submission. => | * @throws \Exception $e If the authorization fails. | ||||
machniak: `$e` is redundant here. And I'd put @throws after @return. Also, please add @params. | |||||
* I suppose that's not necessary given that we have the information avialable in the headers? | |||||
* | * | ||||
* @param \Illuminate\Http\Request $request The API request. | * @return \App\User The user | ||||
* | |||||
* @return \Illuminate\Http\Response The response | |||||
*/ | */ | ||||
public function authenticate(Request $request) | private function authorizeRequest($login, $password, $clientIP) | ||||
{ | { | ||||
/** | |||||
* Auth-Login-Attempt: 1 | |||||
* Auth-Method: plain | |||||
* Auth-Pass: simple123 | |||||
* Auth-Protocol: imap | |||||
* Auth-Ssl: on | |||||
* Auth-User: john@kolab.org | |||||
* Client-Ip: 127.0.0.1 | |||||
* Host: 127.0.0.1 | |||||
* | |||||
* Auth-SSL: on | |||||
* Auth-SSL-Verify: SUCCESS | |||||
* Auth-SSL-Subject: /CN=example.com | |||||
* Auth-SSL-Issuer: /CN=example.com | |||||
* Auth-SSL-Serial: C07AD56B846B5BFF | |||||
* Auth-SSL-Fingerprint: 29d6a80a123d13355ed16b4b04605e29cb55a5ad | |||||
*/ | |||||
\Log::debug("Authentication attempt"); | |||||
\Log::debug($request->headers); | |||||
$login = $request->headers->get('Auth-User', null); | |||||
if (empty($login)) { | if (empty($login)) { | ||||
return $this->byebye($request, "Empty login"); | throw new \Exception("Empty login"); | ||||
} | } | ||||
// validate password, otherwise bye bye | |||||
$password = $request->headers->get('Auth-Pass', null); | |||||
if (empty($password)) { | if (empty($password)) { | ||||
return $this->byebye($request, "Empty password"); | throw new \Exception("Empty password"); | ||||
} | } | ||||
$clientIP = $request->headers->get('Client-Ip', null); | |||||
if (empty($clientIP)) { | if (empty($clientIP)) { | ||||
return $this->byebye($request, "No client ip"); | throw new \Exception("No client ip"); | ||||
} | } | ||||
// validate user exists, otherwise bye bye | |||||
$user = \App\User::where('email', $login)->first(); | $user = \App\User::where('email', $login)->first(); | ||||
if (!$user) { | if (!$user) { | ||||
Done Inline ActionsFor activesync we have to support user\domain syntax. https://git.kolab.org/diffusion/S/browse/master/lib/kolab_sync.php$123 machniak: For activesync we have to support user\domain syntax. https://git.kolab. | |||||
Done Inline ActionsI'll look into that. mollekopf: I'll look into that. | |||||
return $this->byebye($request, "User not found"); | throw new \Exception("User not found"); | ||||
} | } | ||||
// TODO: validate the user's domain is A-OK (active, confirmed, not suspended, ldapready) | // TODO: validate the user's domain is A-OK (active, confirmed, not suspended, ldapready) | ||||
// TODO: validate the user is A-OK (active, not suspended, ldapready, imapready) | // TODO: validate the user is A-OK (active, not suspended, ldapready, imapready) | ||||
if (!Hash::check($password, $user->password)) { | if (!Hash::check($password, $user->password)) { | ||||
$attempt = \App\AuthAttempt::recordAuthAttempt($user, $clientIP); | $attempt = \App\AuthAttempt::recordAuthAttempt($user, $clientIP); | ||||
// Avoid setting a password failure reason if we previously accepted the location. | // Avoid setting a password failure reason if we previously accepted the location. | ||||
if (!$attempt->isAccepted()) { | if (!$attempt->isAccepted()) { | ||||
$attempt->reason = \App\AuthAttempt::REASON_PASSWORD; | $attempt->reason = \App\AuthAttempt::REASON_PASSWORD; | ||||
$attempt->save(); | $attempt->save(); | ||||
$attempt->notify(); | $attempt->notify(); | ||||
} | } | ||||
\Log::info("Failed authentication attempt due to password mismatch for user: {$login}"); | throw new \Exception("Password mismatch"); | ||||
return $this->byebye($request, "Password mismatch"); | |||||
} | } | ||||
// validate country of origin against restrictions, otherwise bye bye | // validate country of origin against restrictions, otherwise bye bye | ||||
$countryCodes = json_decode($user->getSetting('limit_geo', "[]")); | $countryCodes = json_decode($user->getSetting('limit_geo', "[]")); | ||||
\Log::debug("Countries for {$user->email}: " . var_export($countryCodes, true)); | \Log::debug("Countries for {$user->email}: " . var_export($countryCodes, true)); | ||||
if (!empty($countryCodes)) { | if (!empty($countryCodes)) { | ||||
$country = \App\Utils::countryForIP($clientIP); | $country = \App\Utils::countryForIP($clientIP); | ||||
if (!in_array($country, $countryCodes)) { | if (!in_array($country, $countryCodes)) { | ||||
\Log::info( | \Log::info( | ||||
"Failed authentication attempt due to country code mismatch ({$country}) for user: {$login}" | "Failed authentication attempt due to country code mismatch ({$country}) for user: {$login}" | ||||
); | ); | ||||
$attempt = \App\AuthAttempt::recordAuthAttempt($user, $clientIP); | $attempt = \App\AuthAttempt::recordAuthAttempt($user, $clientIP); | ||||
$attempt->deny(\App\AuthAttempt::REASON_GEOLOCATION); | $attempt->deny(\App\AuthAttempt::REASON_GEOLOCATION); | ||||
$attempt->notify(); | $attempt->notify(); | ||||
return $this->byebye($request, "Country code mismatch"); | throw new \Exception("Country code mismatch"); | ||||
} | } | ||||
} | } | ||||
// TODO: Apply some sort of limit for Auth-Login-Attempt -- docs say it is the number of | // TODO: Apply some sort of limit for Auth-Login-Attempt -- docs say it is the number of | ||||
// attempts over the same authAttempt. | // attempts over the same authAttempt. | ||||
// Check 2fa | // Check 2fa | ||||
if ($user->getSetting('2fa_enabled', false)) { | if ($user->getSetting('2fa_enabled', false)) { | ||||
$authAttempt = \App\AuthAttempt::recordAuthAttempt($user, $clientIP); | $authAttempt = \App\AuthAttempt::recordAuthAttempt($user, $clientIP); | ||||
if (!$authAttempt->waitFor2FA()) { | if (!$authAttempt->waitFor2FA()) { | ||||
return $this->byebye($request, "2fa failed"); | throw new \Exception("2fa failed"); | ||||
} | |||||
} | |||||
return $user; | |||||
} | |||||
/** | |||||
* Convert domain.tld\username into username@domain for activesync | |||||
* | |||||
* @param string $username The original username. | |||||
* | |||||
* @return string The username in canonical form | |||||
*/ | |||||
private function normalizeUsername($username) { | |||||
$usernameParts = explode("\\", $username); | |||||
if (count($usernameParts) == 2) { | |||||
$username = $usernameParts[1]; | |||||
if (!strpos($username, '@') && !empty($usernameParts[0])) { | |||||
$username .= '@' . $usernameParts[0]; | |||||
} | |||||
} | |||||
return $username; | |||||
} | } | ||||
/** | |||||
* Authentication request from the ngx_http_auth_request_module | |||||
* | |||||
* @param \Illuminate\Http\Request $request The API request. | |||||
* | |||||
* @return \Illuminate\Http\Response The response | |||||
*/ | |||||
public function httpauth(Request $request) | |||||
{ | |||||
/** | |||||
Php-Auth-Pw: simple123 | |||||
Php-Auth-User: john@kolab.org | |||||
Sec-Fetch-Dest: document | |||||
Sec-Fetch-Mode: navigate | |||||
Sec-Fetch-Site: cross-site | |||||
Sec-Gpc: 1 | |||||
Upgrade-Insecure-Requests: 1 | |||||
User-Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:93.0) Gecko/20100101 Firefox/93.0 | |||||
X-Forwarded-For: 31.10.153.58 | |||||
X-Forwarded-Proto: https | |||||
X-Original-Uri: /iRony/ | |||||
X-Real-Ip: 31.10.153.58 | |||||
*/ | |||||
\Log::debug("Authentication attempt"); | |||||
\Log::debug($request->headers); | |||||
machniakUnsubmitted Done Inline ActionsI'd make it a single Log::debug() call, like in byebye(). machniak: I'd make it a single Log::debug() call, like in byebye(). | |||||
$username = $this->normalizeUsername($request->headers->get('Php-Auth-User', "")); | |||||
$password = $request->headers->get('Php-Auth-Pw', null); | |||||
if (empty($password)) { | |||||
\Log::debug("Empty password provided."); | |||||
machniakUnsubmitted Done Inline ActionsWell, we already log the request headers above. So, this debug line looks redundant. machniak: Well, we already log the request headers above. So, this debug line looks redundant. | |||||
mollekopfAuthorUnsubmitted Done Inline ActionsI think it makes sense to have an explicit message given we stop processing at this point, but the message could be better indeed. mollekopf: I think it makes sense to have an explicit message given we stop processing at this point, but… | |||||
return response("", 401); | |||||
} | |||||
try { | |||||
$this->authorizeRequest( | |||||
$username, | |||||
$password, | |||||
$request->headers->get('X-Real-Ip', null), | |||||
); | |||||
} catch (\Exception $e) { | |||||
\Log::debug("Authentication attempt failed: {$e->getMessage()}"); | |||||
return response("", 403); | |||||
machniakUnsubmitted Done Inline ActionsI'm not sure, but I'm wondering whether for cases like this (and 401 above) we should use byebye() method, of course extended to handle all cases, but it would be some unification on the code level (debug logging). machniak: I'm not sure, but I'm wondering whether for cases like this (and 401 above) we should use… | |||||
mollekopfAuthorUnsubmitted Done Inline ActionsI don't think that would help at the moment. The byebye method only works for the imap/smtp responses which uses headers, and the httpauth module uses the http status code instead. While we could introduce a similar method to combine status code + debug message, it doesn't seem to really improve the situation to me (two lines of code vs 1 line + a function). mollekopf: I don't think that would help at the moment. The byebye method only works for the imap/smtp… | |||||
} | |||||
\Log::debug("Authentication attempt succeeded"); | |||||
return response(""); | |||||
} | |||||
/** | |||||
* Authentication request. | |||||
* | |||||
* @todo: Separate IMAP(+STARTTLS) from IMAPS, same for SMTP/submission. => | |||||
* I suppose that's not necessary given that we have the information avialable in the headers? | |||||
* | |||||
* @param \Illuminate\Http\Request $request The API request. | |||||
* | |||||
* @return \Illuminate\Http\Response The response | |||||
*/ | |||||
public function authenticate(Request $request) | |||||
{ | |||||
/** | |||||
* Auth-Login-Attempt: 1 | |||||
* Auth-Method: plain | |||||
* Auth-Pass: simple123 | |||||
* Auth-Protocol: imap | |||||
* Auth-Ssl: on | |||||
* Auth-User: john@kolab.org | |||||
* Client-Ip: 127.0.0.1 | |||||
* Host: 127.0.0.1 | |||||
* | |||||
* Auth-SSL: on | |||||
* Auth-SSL-Verify: SUCCESS | |||||
* Auth-SSL-Subject: /CN=example.com | |||||
* Auth-SSL-Issuer: /CN=example.com | |||||
* Auth-SSL-Serial: C07AD56B846B5BFF | |||||
* Auth-SSL-Fingerprint: 29d6a80a123d13355ed16b4b04605e29cb55a5ad | |||||
*/ | |||||
\Log::debug("Authentication attempt"); | |||||
\Log::debug($request->headers); | |||||
machniakUnsubmitted Done Inline ActionsI'd make it a single Log::debug() call, like in byebye(). machniak: I'd make it a single Log::debug() call, like in byebye(). | |||||
$password = $request->headers->get('Auth-Pass', null); | |||||
try { | |||||
$user = $this->authorizeRequest( | |||||
$request->headers->get('Auth-User', null), | |||||
$password, | |||||
$request->headers->get('Client-Ip', null), | |||||
); | |||||
} catch (\Exception $e) { | |||||
return $this->byebye($request, $e->getMessage()); | |||||
} | } | ||||
// All checks passed | // All checks passed | ||||
switch ($request->headers->get('Auth-Protocol')) { | switch ($request->headers->get('Auth-Protocol')) { | ||||
case "imap": | case "imap": | ||||
return $this->authenticateIMAP($request, $user->getSetting('guam_enabled', false), $password); | return $this->authenticateIMAP($request, $user->getSetting('guam_enabled', false), $password); | ||||
case "smtp": | case "smtp": | ||||
return $this->authenticateSMTP($request, $password); | return $this->authenticateSMTP($request, $password); | ||||
▲ Show 20 Lines • Show All 83 Lines • Show Last 20 Lines |
$e is redundant here. And I'd put @throws after @return. Also, please add @params.