Merge pull request #14170 from uberbrady/saml_nonce_storage

Enforce SAML assertion ID uniqueness and notValidOnOrAfter attribute [FD-37019]
This commit is contained in:
snipe 2024-01-25 20:14:40 +00:00 committed by GitHub
commit 85a158eaef
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 111 additions and 3 deletions

View file

@ -0,0 +1,44 @@
<?php
namespace App\Console\Commands;
use Illuminate\Console\Command;
use App\Models\SamlNonce;
class SamlClearExpiredNonces extends Command
{
/**
* The name and signature of the console command.
*
* @var string
*/
protected $signature = 'saml:clear_expired_nonces';
/**
* The console command description.
*
* @var string
*/
protected $description = 'Clears out expired SAML assertions from the saml_nonces table';
/**
* Create a new command instance.
*
* @return void
*/
public function __construct()
{
parent::__construct();
}
/**
* Execute the console command.
*
* @return int
*/
public function handle()
{
SamlNonce::where('not_valid_after','<=',now())->delete();
return 0;
}
}

View file

@ -25,6 +25,7 @@ class Kernel extends ConsoleKernel
$schedule->command('backup:clean')->daily();
$schedule->command('snipeit:upcoming-audits')->daily();
$schedule->command('auth:clear-resets')->everyFifteenMinutes();
$schedule->command('saml:clear_expired_nonces')->weekly();
}
/**

View file

@ -3,6 +3,7 @@
namespace App\Http\Controllers\Auth;
use App\Http\Controllers\Controller;
use App\Models\SamlNonce;
use App\Models\Setting;
use App\Models\User;
use App\Models\Ldap;
@ -109,7 +110,14 @@ class LoginController extends Controller
try {
$user = $saml->samlLogin($samlData);
$notValidAfter = new \Carbon\Carbon(@$samlData['assertionNotOnOrAfter']);
if(\Carbon::now()->greaterThanOrEqualTo($notValidAfter)) {
abort(400,"Expired SAML Assertion");
}
if(SamlNonce::where('nonce', @$samlData['nonce'])->count() > 0) {
abort(400,"Assertion has already been used");
}
Log::debug("okay, fine, this is a new nonce then. Good for you.");
if (!is_null($user)) {
Auth::login($user);
} else {
@ -123,10 +131,14 @@ class LoginController extends Controller
$user->last_login = \Carbon::now();
$user->saveQuietly();
}
$s = new SamlNonce();
$s->nonce = @$samlData['nonce'];
$s->not_valid_after = $notValidAfter;
$s->save();
} catch (\Exception $e) {
\Log::debug('There was an error authenticating the SAML user: '.$e->getMessage());
throw new \Exception($e->getMessage());
throw $e;
}
// Fallthrough with better logging

15
app/Models/SamlNonce.php Normal file
View file

@ -0,0 +1,15 @@
<?php
namespace App\Models;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;
class SamlNonce extends Model
{
use HasFactory;
protected $fillable = ['nonce','not_on_or_after'];
public $timestamps = false;
}

View file

@ -394,6 +394,8 @@ class Saml
'nameIdSPNameQualifier' => $auth->getNameIdSPNameQualifier(),
'sessionIndex' => $auth->getSessionIndex(),
'sessionExpiration' => $auth->getSessionExpiration(),
'nonce' => $auth->getLastAssertionId(),
'assertionNotOnOrAfter' => $auth->getLastAssertionNotOnOrAfter(),
];
}

View file

@ -0,0 +1,34 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
class CreateSamlNonceTable extends Migration
{
/**
* Run the migrations.
*
* @return void
*/
public function up()
{
if (! Schema::hasTable('saml_nonces') ) {
Schema::create('saml_nonces', function (Blueprint $table) {
$table->id();
$table->string('nonce')->index();
$table->datetime('not_valid_after')->index();
});
}
}
/**
* Reverse the migrations.
*
* @return void
*/
public function down()
{
Schema::dropIfExists('saml_nonces');
}
}