Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Login Enabled" Reverts After Login Attempt #8670

Closed
2 tasks done
bahhhhh opened this issue Nov 3, 2020 · 15 comments
Closed
2 tasks done

"Login Enabled" Reverts After Login Attempt #8670

bahhhhh opened this issue Nov 3, 2020 · 15 comments
Assignees
Labels

Comments

@bahhhhh
Copy link

bahhhhh commented Nov 3, 2020

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):

  • OS: [e.g. iOS]
  • Browser [e.g. chrome, safari]
  • Version [e.g. 22]

Smartphone (please complete the following information):

  • Device: MacBook Pro
  • OS: 10.15.7 (19H2)
  • Browser: Safari 14.0 (15610.1.28.1.9, 15610)
  • Version [e.g. 22]

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
Copy link
Owner

snipe commented Nov 3, 2020

@uberbrady want to take a look?

@snipe
Copy link
Owner

snipe commented Nov 3, 2020

@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
Copy link
Author

bahhhhh commented Nov 3, 2020

@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
Copy link
Owner

snipe commented Nov 10, 2020

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
A free open source IT asset/license management system - snipe/snipe-it

@bahhhhh
Copy link
Author

bahhhhh commented Nov 10, 2020

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
Copy link
Owner

snipe commented Nov 10, 2020

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

@snipe
Copy link
Owner

snipe commented Nov 13, 2020

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

@snipe
Copy link
Owner

snipe commented Nov 13, 2020

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
Copy link
Owner

snipe commented Nov 13, 2020

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 <[email protected]>
     *
     * @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 <[email protected]>
     *
     * @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
Copy link
Owner

snipe commented Nov 13, 2020

Is this AD or LDAP?

@snipe
Copy link
Owner

snipe commented Nov 13, 2020

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

@snipe
Copy link
Owner

snipe commented Nov 13, 2020

(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
Copy link
Author

bahhhhh commented Nov 15, 2020

Is this AD or LDAP?

It's LDAP (from FreeIPA)

@bahhhhh
Copy link
Author

bahhhhh commented Nov 17, 2020

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

Thank you thank you!

@uberbrady
Copy link
Collaborator

Glad to hear it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants