snipe / snipe-it

A free open source IT asset/license management system
https://snipeitapp.com
GNU Affero General Public License v3.0
11.12k stars 3.19k forks source link

"Login Enabled" Reverts After Login Attempt #8670

Closed bahhhhh closed 3 years ago

bahhhhh commented 4 years ago

Please confirm you have done the following before posting your bug report:

Describe the bug I've set "Login Enabled" to "Yes" for a few of our users, yet when they try logging in, the setting reverts back to "No".

FWIW, our users are imported using LDAP.

To Reproduce Steps to reproduce the behavior:

  1. Enable the "This user can login" option on a user's page
  2. Have the user try logging in
  3. Refresh the user's page and see the "Login Enabled" setting set to "No"

Expected behavior The user should be able to logi n using their LDAP credentials

Screenshots N/A

Server (please complete the following information):

Desktop (please complete the following information):

Smartphone (please complete the following information):

Error Messages

(1/1) TypeError
Return value of App\Services\LdapAd::ldapLogin() must be an instance of App\Models\User, null returned

See this image: https://drive.google.com/file/d/13f8Ah89LCE6du-gja2vSkNGODfkq_edF/view?usp=sharing

Additional context N/A

Add any other context about the problem here. N/A

snipe commented 4 years ago

@uberbrady want to take a look?

snipe commented 4 years ago

@bahhhhh Can you create a test user (NOT imported via LDAP) and see if you can reproduce it with that user? I'm trying to see if it's LDAP related or not.

bahhhhh commented 4 years ago

@bahhhhh Can you create a test user (NOT imported via LDAP) and see if you can reproduce it with that user? I'm trying to see if it's LDAP related or not.

Good idea! I just tried and the non-LDAP user is able to log in.

snipe commented 4 years ago

Do you have LDAP-sync set up as well, or only login? We did fix an "activated" bug in the past week or so that was related to LDAP. If you pull latest from master, are you still seeing this?

https://github.com/snipe/snipe-it/commits/master/app/Services/LdapAd.php

GitHub
snipe/snipe-it
A free open source IT asset/license management system - snipe/snipe-it
bahhhhh commented 4 years ago

I just updated Snipe-IT to v5.0.6 - build 5526 (master) and I'm afraid the issue remains.

Here's a screenshot of the LDAP page:

snipe commented 4 years ago

Balls. @uberbrady, you want to take a peek?

snipe commented 4 years ago

@bahhhhh Do you have LDAP-sync set up as well, or only login?

snipe commented 4 years ago

Also do you have anything in your app logs that might shed any light? Even just debugging stuff. I don't see anything in the LoginController that would change that value.

snipe commented 4 years ago

I'm thinking it might be related to this bit from the LdapAD model?

 /**
     * Set the user information based on the LDAP settings.
     *
     * @author Wes Hulette <jwhulette@gmail.com>
     *
     * @since 5.0.0
     *
     * @param \Adldap\Models\User $user
     * @param null|Collection     $defaultLocation
     * @param null|Collection     $mappedLocations
     *
     * @return null|\App\Models\User
     */
    public function processUser(AdldapUser $user, ?Collection $defaultLocation=null, ?Collection $mappedLocations=null): ?User
    {
        // Only sync active users
        if(!$user) {
            return null;
        }
        $snipeUser = [];
        $snipeUser['username']        = $user->{$this->ldapSettings['ldap_username_field']}[0] ?? '';
        $snipeUser['employee_number'] = $user->{$this->ldapSettings['ldap_emp_num']}[0] ?? '';
        $snipeUser['lastname']        = $user->{$this->ldapSettings['ldap_lname_field']}[0] ?? '';
        $snipeUser['firstname']       = $user->{$this->ldapSettings['ldap_fname_field']}[0] ?? '';
        $snipeUser['email']           = $user->{$this->ldapSettings['ldap_email']}[0] ?? '';
        $snipeUser['title']           = $user->getTitle() ?? '';
        $snipeUser['telephonenumber'] = $user->getTelephoneNumber() ?? '';
        $snipeUser['location_id']     = $this->getLocationId($user, $defaultLocation, $mappedLocations);
        $snipeUser['activated']       = $this->getActiveStatus($user);

        return $this->setUserModel($snipeUser);
    }

Specifically this part:

 $snipeUser['activated']       = $this->getActiveStatus($user);

That points to this method:

/**
     * Set the active status of the user.
     *
     * @author Wes Hulette <jwhulette@gmail.com>
     *
     * @since 5.0.0
     *
     * @param \Adldap\Models\User $user
     *
     * @return int
     */
    private function getActiveStatus(AdldapUser $user): int
    {
        $activeStatus = 0;
        /*
         * Check to see if we are connected to an AD server
         * if so, check the Active Directory User Account Control Flags
         */
        if ($user->hasAttribute($user->getSchema()->userAccountControl())) {
            $activeStatus = (in_array($user->getUserAccountControl(), self::AD_USER_ACCOUNT_CONTROL_FLAGS)) ? 1 : 0;
        } else {
            // If there is no activated flag, assume this is handled via the OU and activate the users
            if (false == $this->ldapSettings['ldap_active_flag']) {
                $activeStatus = 1;
            }
        }

        return $activeStatus;
    }
snipe commented 4 years ago

Is this AD or LDAP?

snipe commented 4 years ago

I've just added some additional debugging info on develop - hopefully that will shed some light

snipe commented 4 years ago

(I also attempted a hack around for the login deactivation. I believe this does not solve the crux of the problem, which I think is a logic bug in the LdapAd model, but it might work for you for right now until we can more formally fix this.)

bahhhhh commented 4 years ago

Is this AD or LDAP?

It's LDAP (from FreeIPA)

bahhhhh commented 3 years ago

@snipe I just updated to v5.0.7 - build 5615 (master) and this seems to be fixed.

Thank you thank you!

uberbrady commented 3 years ago

Glad to hear it!