Skip to content

LDAP minor fixes#4784

Merged
AdSchellevis merged 10 commits intoopnsense:masterfrom
kulikov-a:ldap_fixes
Mar 6, 2021
Merged

LDAP minor fixes#4784
AdSchellevis merged 10 commits intoopnsense:masterfrom
kulikov-a:ldap_fixes

Conversation

@kulikov-a
Copy link
Member

@kulikov-a kulikov-a commented Mar 6, 2021

Hi!
based on https://forum.opnsense.org/index.php?topic=21605.0

  • baseDN igonred in search function if ldapauthcontainers is set. help text for "Authentication containers" input fixed

    $searchpaths = array($this->baseSearchDN);
    if (!empty($this->ldapAuthcontainers)) {
    $searchpaths = explode(';', $this->ldapAuthcontainers);
    }

  • In select "Authentication containers" modal Close button is actually a Save button (writes changes to the "Authentication containers" input)

  • Some debug info added to System: Access: Tester page for LDAP servers
    tester1
    tester2

thanks!

@AdSchellevis
Copy link
Member

@kulikov-a maybe we better add a method getLastErrors() for the error feedback and move this into a separate array, it makes it more explicit that this is error feedback and not something returned by the authenticator. Other than that it's definitely an improvement to make sure errors are returned to the user here.

@kulikov-a
Copy link
Member Author

@AdSchellevis
thanks! more correct of course
I will try to do so. for now I'm converting it to a draft

@kulikov-a kulikov-a marked this pull request as draft March 6, 2021 07:55
@kulikov-a
Copy link
Member Author

kulikov-a commented Mar 6, 2021

@AdSchellevis it's kind of done.

  • moved errors to a separate array.
  • slightly tweaked the logLdapError function: it turned out that a LDAP_OPT_ERROR_STRING can contain not only commas but also new lines and even tabs special characters.
  • added Cancel button to "Authentication containers" modal.
  • added some debug (and the message for this case in modal window) for "Authentication containers" empty search results
    (https://forum.opnsense.org/index.php?topic=21605.msg103543#msg103543)

please take a look when you have time

@kulikov-a kulikov-a marked this pull request as ready for review March 6, 2021 13:07
@AdSchellevis
Copy link
Member

@kulikov-a looks good, thanks, I'll pull it in to master, haven't tried it myself yet, but don't see any reason why it shouldn't.

@AdSchellevis AdSchellevis merged commit 035319f into opnsense:master Mar 6, 2021
@kulikov-a
Copy link
Member Author

Wow! thanks!!!

@AdSchellevis
Copy link
Member

@kulikov-a made a small adjustment after testing (3277c0b), when you can't connect, there's an ldap connect error but no LDAP_OPT_ERROR_STRING, so it's better to omit it in that case.

@kulikov-a
Copy link
Member Author

@AdSchellevis thanks! it is always better to add existing ones )
I should have noticed it. thank!

AdSchellevis pushed a commit that referenced this pull request Mar 14, 2021
Improve error handling in ldap authentication
fichtner pushed a commit that referenced this pull request Mar 28, 2021
Improve error handling in ldap authentication

(cherry picked from commit 035319f)
(cherry picked from commit 3277c0b)
(cherry picked from commit d9f2799)
AdSchellevis added a commit that referenced this pull request Mar 30, 2021
 PHP Fatal error:  Uncaught Error: Call to undefined method OPNsense\Auth\Radius::getLastAuthErrors() in /usr/local/www/diag_authentication.php:76
fichtner pushed a commit that referenced this pull request Mar 31, 2021
 PHP Fatal error:  Uncaught Error: Call to undefined method OPNsense\Auth\Radius::getLastAuthErrors() in /usr/local/www/diag_authentication.php:76

(cherry picked from commit a7ae8c4)
fichtner pushed a commit that referenced this pull request Mar 31, 2021
 PHP Fatal error:  Uncaught Error: Call to undefined method OPNsense\Auth\Radius::getLastAuthErrors() in /usr/local/www/diag_authentication.php:76

(cherry picked from commit a7ae8c4)
@kulikov-a kulikov-a deleted the ldap_fixes branch November 12, 2021 09:40
oshogbo pushed a commit to DynFi/opnsense-core that referenced this pull request Mar 3, 2022
oshogbo pushed a commit to DynFi/opnsense-core that referenced this pull request Mar 3, 2022
oshogbo pushed a commit to DynFi/opnsense-core that referenced this pull request Mar 3, 2022
…/core#4784

 PHP Fatal error:  Uncaught Error: Call to undefined method OPNsense\Auth\Radius::getLastAuthErrors() in /usr/local/www/diag_authentication.php:76
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants