DirectoryTree / LdapRecord-Laravel

Multi-domain LDAP Authentication & Management for Laravel.
https://ldaprecord.com/docs/laravel/v3
MIT License
483 stars 51 forks source link

[Question] Unable to display LDAP error messages when use plain authentication with Breeze #621

Closed jorgelloret closed 5 months ago

jorgelloret commented 6 months ago

Environment:

Describe the bug: First of all, thank you for this project that saves us time in our developments.

I am setting up a plain authentication: https://ldaprecord.com/docs/laravel/v3/auth/plain/laravel-breeze. Access by username is configured, instead of email. The login works perfectly.

I have seen that in the authentication by database, there is a section for error messages: https://ldaprecord.com/docs/laravel/v3/auth/database/laravel-breeze/#displaying-ldap-error-messages

I have tried to apply it, but I get the generic error: “These credentials do not match our records”.

I tried this test: https://github.com/DirectoryTree/LdapRecord-Laravel/issues/285#issuecomment-852448519 and I got the error you mentioned (I see that error isn't translatable). This generates an error that, if you are logged in, you get an error 500 indicating that the LDAP server cannot be contacted. How can I handle it so that, in production, it is not a simple error screen?

Here is the code for my file app/Http/Controllers/Auth/AuthenticatedSessionController.php:

<?php

namespace App\Http\Controllers\Auth;

use App\Http\Controllers\Controller;
use App\Http\Requests\Auth\LoginRequest;
use App\Providers\RouteServiceProvider;
use Illuminate\Http\RedirectResponse;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Auth;
use Illuminate\View\View;
use LdapRecord\Laravel\Auth\ListensForLdapBindFailure;

class AuthenticatedSessionController extends Controller
{
    use ListensForLdapBindFailure;

    protected $username = 'username';

    public function __construct()
    {
        $this->listenForLdapBindFailure();
    }

    /**
     * Display the login view.
     */
    public function create(): View
    {
        return view('auth.login');
    }

    /**
     * Handle an incoming authentication request.
     */
    public function store(LoginRequest $request): RedirectResponse
    {
        $request->authenticate();

        $request->session()->regenerate();

        return redirect()->intended(RouteServiceProvider::HOME);
    }

    /**
     * Destroy an authenticated session.
     */
    public function destroy(Request $request): RedirectResponse
    {
        Auth::guard('web')->logout();

        $request->session()->invalidate();

        $request->session()->regenerateToken();

        return redirect('/');
    }
}
stevebauman commented 5 months ago

Hi @jorgelloret,

If you are using plain authentication, then a persistent connection to your LDAP server is required for all active user sessions (persistent in the sense that, for every request, an LDAP connection will be made). If users are logged into your web application using plain authentication and the connection to the server drops or is inaccessible, then an LdapRecord\Auth\BindException will be thrown on subsequent requests to your application. Your application is responsible for handling this scenario, as LdapRecord-Laravel can only be responsible for handling authentication errors (as it is the authentication driver).

To handle the exception in your application, you could update your App\Exceptions\Handler and return a response to the end user, and also perform additional logic when it occurs:

use LdapRecord\Auth\BindException;

$this->renderable(function (BindException $e, Request $request) {
    return response()->view('errors.ldap-connection-error');
});

Hope this helps. Let me know if you need further clarification.

jorgelloret commented 5 months ago

@stevebauman thanks for the response.

Related to the previous message, can I make it so that when the server denies access via rules, a message other than the generic one is displayed?

stevebauman commented 5 months ago

@jorgelloret Happy to help.

Related to the previous message, can I make it so that when the server denies access via rules, a message other than the generic one is displayed?

At the moment no, unless you manually throw a ValidationException inside of the rule itself:

use Illuminate\Validation\ValidationException;

public function passes(LdapRecord $user, Eloquent $model = null): bool
{
    $administrators = Group::find('cn=Administrators,dc=local,dc=com');

    if ($user->groups()->recursive()->exists($administrators)) {
        return true;
    }

    throw ValidationException::withMessages([
        'email' => 'You are not a member of the "Administrators" group.',
    ]);
}

That's a good point though -- I'll make an update to LdapRecord-Laravel that fires an event with your authentication rule that failed so you can instead listen for that event, and then throw the exception in the listener.

Something similar to:

class HandleAuthRuleFailure
{
    public function handle(RuleFailed $event)
    {
        $message = match (true) {
            $event->rule instanceof OnlyAdministrators => 'You are not a member of the "Administrators" group.',
            default => 'You are not authorized to login.',
        };

        throw ValidationException::withMessages([
            'email' => $message,
        ]);
    }
}
jorgelloret commented 5 months ago

@stevebauman Thanks, it's working. I'm using the rule to validate if the user is a personal identifier. Since we have generic users, they should not be able to log in:

<?php

namespace App\Ldap\Rules;

use Illuminate\Database\Eloquent\Model as Eloquent;
use LdapRecord\Laravel\Auth\Rule;
use LdapRecord\Models\Model as LdapRecord;
use Orumad\SpanishValidator\Validator;
use Illuminate\Validation\ValidationException;

class OnlySpanishPersonalID implements Rule
{
    /**
     * Check if the rule passes validation.
     */
    public function passes(LdapRecord $user, Eloquent $model = null): bool
    {
        $validator = new Validator();

        if ($validator->isValidPersonalId($user->samaccountname[0])) {
            return true;
        }

        throw ValidationException::withMessages([
            'username' => 'Estas usando un usuario genérico.',
        ]);
    }
}
stevebauman commented 5 months ago

Hey @jorgelloret, I've just released v3.2.0 which includes both the RulePassed and RuleFailed authentication events, where you can now throw this exception (if you prefer):

namespace App\Listeners;

use LdapRecord\Events\Auth\RuleFailed;
use App\Ldap\Rules\OnlySpanishPersonalID;

class HandleAuthRuleFailure
{
    public function handle(RuleFailed $event)
    {
        $message = match (true) {
            $event->rule instanceof OnlySpanishPersonalID => 'Estas usando un usuario genérico.',
        };

        throw ValidationException::withMessages([
            'username' => $message,
        ]);
    }
}
Event::listen(
    \LdapRecord\Events\Auth\RuleFailed::class, 
    \App\Listeners\HandleAuthRuleFailure::class
);
jorgelloret commented 5 months ago

@stevebauman thanks for all, it is working. 👍

FYI, there is a bug in your example, in case someone needs to use it in the future. In both codes, LdapRecord\Events\Auth\RuleFailed is LdapRecord\Laravel\Events\Auth\RuleFailed. And, in the listener, we need to include: use Illuminate\Validation\ValidationException;

By the way, I have a scope to a OU (https://ldaprecord.com/docs/laravel/v3/auth/plain/configuration/#scopes). Can I show a message when the user is not in scope, or should I create a rule for it?

stevebauman commented 5 months ago

@jorgelloret thanks for those corrections 👍

By the way, I have a scope to a OU (https://ldaprecord.com/docs/laravel/v3/auth/plain/configuration/#scopes). Can I show a message when the user is not in scope, or should I create a rule for it?

No unfortunately, as scoping to an OU is just restricting the result of the query to locate your user. You can use a rule for this purpose if you need it 👍