Page MenuHomePhorge
Differential D2984 Diff 9400 src/app/Http/Controllers/API/V4/NGINXController.php

Changeset View

Standalone View

View Options

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. => * @param string $login The login name
* I suppose that's not necessary given that we have the information avialable in the headers? * @param string $password The password
* @param string $clientIP The client ip
machniakUnsubmitted
Done
Inline Actions

$e is redundant here. And I'd put @throws after @return. Also, please add @params.

machniak: `$e` is redundant here. And I'd put @throws after @return. Also, please add @params.
* *
* @param \Illuminate\Http\Request $request The API request. * @return \App\User The user
* *
* @return \Illuminate\Http\Response The response * @throws \Exception If the authorization fails.
*/ */
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) {
machniakUnsubmitted
Done
Inline Actions

For 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.
mollekopfAuthorUnsubmitted
Done
Inline Actions

I'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\n{$request->headers}");
machniakUnsubmitted
Done
Inline Actions

I'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("Authentication attempt failed: Empty password provided.");
return response("", 401);
machniakUnsubmitted
Done
Inline Actions

Well, 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 Actions

I 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…
}
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 Actions

I'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 Actions

I 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\n{$request->headers}");
$password = $request->headers->get('Auth-Pass', null);
machniakUnsubmitted
Done
Inline Actions

I'd make it a single Log::debug() call, like in byebye().

machniak: I'd make it a single Log::debug() call, like in byebye().
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