DirectoryTree / LdapRecord-Laravel

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

[Bug] User group exists returns true on null #197

Closed nerdalertdk closed 4 years ago

nerdalertdk commented 4 years ago

Environment (please complete the following information):

Describe the bug:

I'm trying to make an login Rule, base code is from your example

Why do this return true

Input:

    public function isValid()
    {
        $group = Group::find('cn=donotexists,cn=groups,cn=accounts,dc=domain,dc=tld');

        dump( $group );

        dd( $this->user->groups()->recursive()->exists( $group ));
    }

Output:

null
true

it also returns true on false the only way I got it to return false was supplying an string such as 'false'

How I fixed it but would call this a hack $group = Group::find('cn=donotexists,cn=groups,cn=accounts,dc=domain,dc=tld') ?: 'false';

User Class

<?php

namespace App\Ldap\Models\Freeipa;

class User extends Entry
{
    /**
     * The object classes of the LDAP model.
     *
     * @var array
     */
    public static $objectClasses = [
        'top',
        'person',
        'organizationalperson',
        'inetorgperson',
        'inetuser',
        'posixaccount'
    ];

    /**
     * Retrieve groups that the current user is apart of.
     */
    public function groups()
    {
        return $this->hasManyIn(Group::class, 'memberof');
    }
}

Group class

<?php

namespace App\Ldap\Models\Freeipa;

use LdapRecord\Models\Model;

class Group extends Entry
{
    /**
     * The object classes of the LDAP model.
     *
     * @var array
     */
    public static $objectClasses = [
        'top',
        'groupofnames',
        'nestedgroup',
        'ipausergroup',
        'ipaobject',
        'posixgroup',
    ];

    /**
     * The groups relationship.
     *
     * Retrieves groups that the current group is apart of.
     *
     * @return \LdapRecord\Models\Relations\HasMany
     */
    public function groups()
    {
        return $this->hasMany(static::class, 'member');
    }

    /**
     * Retrieve the members of the group.
     */
    public function members()
    {
        return $this->hasMany([
            Group::class, User::class
        ], 'memberof')->using($this, 'member');
    }

}
stevebauman commented 4 years ago

Hi @nerdalertdk,

If null is passed into the exists() method, then it simply checks if the relation contains any results.

https://github.com/DirectoryTree/LdapRecord/blob/9bc847b50a70503428154ad760c0b1a40b47027a/src/Models/Relations/Relation.php#L169

I will patch this to ensure if a null argument is passed into the method, then false will be returned. 👍

stevebauman commented 4 years ago

Okay patched -- I'll have a release out shortly with this fix.

In the interim, I'd recommend using Group::findOrFail() to ensure the group you are requiring to be checked can be found:

$group = Group::findOrFail('cn=donotexists,cn=groups,cn=accounts,dc=domain,dc=tld');

Or, use the group itself to determine if the authenticating user is a member of the group:

return Group::findOrFail('cn=donotexists,cn=groups,cn=accounts,dc=domain,dc=tld')
    ->members()
    ->recursive()
    ->exists($this->user);
stevebauman commented 4 years ago

v1.11.0 is now out with this patch. Thanks again for the report! 😄

nerdalertdk commented 4 years ago

I actually just copied you documentation code :)

It’s use for a rule