Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Added query_string LDAP config option#7420

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
xabbuh merged 4 commits intosymfony:masterfromlsmith77:ldap_query_string
Feb 28, 2017

Conversation

@lsmith77
Copy link
Contributor

@lsmith77lsmith77 commentedJan 26, 2017
edited
Loading

@nietonfir
Copy link
Contributor

Maybe merge with#7414 ?

@lsmith77
Copy link
ContributorAuthor

ah doh .. didn't see this PR :-/

lets wait for feedback.

for example I am not sure if it made sense adding another example or not like I did.

@nietonfir
Copy link
Contributor

No problem. ;-)
Your example looks fine to me.

@lsmith77
Copy link
ContributorAuthor

integrated#7414

nietonfir reacted with heart emoji

@lsmith77lsmith77force-pushed theldap_query_string branch 2 times, most recently froma9bd37a to1735f08CompareJanuary 27, 2017 09:51
fabpot added a commit to symfony/symfony that referenced this pull requestJan 28, 2017
… of searching for the DN (lsmith77, nietonfir)This PR was merged into the 3.3-dev branch.Discussion----------[Security] make LdapBindAuthenticationProvider capable of searching for the DN| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#16823,#20905| License       | MIT| Doc PR        |symfony/symfony-docs#7420I guess due to the separation between the user and auth provider something like the following isn't ok  (note: the following works just fine for me but if course in the end the username is the DN and not the user provided shorter username):```diffdiff --git a/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php b/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.phpindex5ebb09a..18d7825 100644--- a/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php+++ b/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php@@ -70,7 +70,7 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider      */     protected function checkAuthentication(UserInterface $user, UsernamePasswordToken $token)     {-        $username = $token->getUsername();+        $username = $user->getUsername();         $password = $token->getCredentials();         if ('' === $password) {@@ -78,10 +78,7 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider         }         try {-            $username = $this->ldap->escape($username, '', LdapInterface::ESCAPE_DN);-            $dn = str_replace('{username}', $username, $this->dnString);--            $this->ldap->bind($dn, $password);+            $this->ldap->bind($username, $password);         } catch (ConnectionException $e) {             throw new BadCredentialsException('The presented password is invalid.');         }diff --git a/src/Symfony/Component/Security/Core/User/LdapUserProvider.php b/src/Symfony/Component/Security/Core/User/LdapUserProvider.phpindexfc42419..8194c4c 100644--- a/src/Symfony/Component/Security/Core/User/LdapUserProvider.php+++ b/src/Symfony/Component/Security/Core/User/LdapUserProvider.php@@ -115,7 +115,7 @@ class LdapUserProvider implements UserProviderInterface     {         $password = $this->getPassword($entry);-        return new User($username, $password, $this->defaultRoles);+        return new User($entry->getDn(), $password, $this->defaultRoles);     }     /**```Therefore I created an entire new auth provider.Commits-------8ddd533 Merge pull request#1 from nietonfir/http_basic_ldapa783e5c Update HttpBasicLdapFactorya30191f make LdapBindAuthenticationProvider capable of searching for the DN
@lsmith77
Copy link
ContributorAuthor

the relevant code branch was merged

chalasr pushed a commit to chalasr/symfony that referenced this pull requestJan 29, 2017
…capable of searching for the DN (lsmith77, nietonfir)This PR was merged into the 3.3-dev branch.Discussion----------[Security] make LdapBindAuthenticationProvider capable of searching for the DN| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |symfony#16823,symfony#20905| License       | MIT| Doc PR        |symfony/symfony-docs#7420I guess due to the separation between the user and auth provider something like the following isn't ok  (note: the following works just fine for me but if course in the end the username is the DN and not the user provided shorter username):```diffdiff --git a/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php b/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.phpindex5ebb09a..18d7825 100644--- a/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php+++ b/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php@@ -70,7 +70,7 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider      */     protected function checkAuthentication(UserInterface $user, UsernamePasswordToken $token)     {-        $username = $token->getUsername();+        $username = $user->getUsername();         $password = $token->getCredentials();         if ('' === $password) {@@ -78,10 +78,7 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider         }         try {-            $username = $this->ldap->escape($username, '', LdapInterface::ESCAPE_DN);-            $dn = str_replace('{username}', $username, $this->dnString);--            $this->ldap->bind($dn, $password);+            $this->ldap->bind($username, $password);         } catch (ConnectionException $e) {             throw new BadCredentialsException('The presented password is invalid.');         }diff --git a/src/Symfony/Component/Security/Core/User/LdapUserProvider.php b/src/Symfony/Component/Security/Core/User/LdapUserProvider.phpindexfc42419..8194c4c 100644--- a/src/Symfony/Component/Security/Core/User/LdapUserProvider.php+++ b/src/Symfony/Component/Security/Core/User/LdapUserProvider.php@@ -115,7 +115,7 @@ class LdapUserProvider implements UserProviderInterface     {         $password = $this->getPassword($entry);-        return new User($username, $password, $this->defaultRoles);+        return new User($entry->getDn(), $password, $this->defaultRoles);     }     /**```Therefore I created an entire new auth provider.Commits-------8ddd533 Merge pull request#1 from nietonfir/http_basic_ldapa783e5c Update HttpBasicLdapFactorya30191f make LdapBindAuthenticationProvider capable of searching for the DN
@javiereguiluzjaviereguiluz changed the titleadded query_string LDAP config optionAdded query_string LDAP config optionFeb 1, 2017
@javiereguiluzjaviereguiluz added this to the3.3 milestoneFeb 1, 2017
**type**: ``string`` **default**: ``null``

This is the string which will be used to query for the DN. The ``{username}``
placeholder will be replaced with the user-provided value (his login).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

We should use "their" instead of "his".

This is the string which will be used to query for the DN. The ``{username}``
placeholder will be replaced with the user-provided value (his login).
Depending on your LDAP server's configuration, you will need to override
this value. This setting is only necessary if the users DN cannot be derived
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

user's

placeholder will be replaced with the user-provided value (his login).
Depending on your LDAP server's configuration, you will need to override
this value. This setting is only necessary if the users DN cannot be derived
statically using the `dn_string` config option.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

we need double backticks here

..code-block::php
$container->loadFromExtension('security', array(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

we should add the// app/config/security.php comment above

# ...
form_login_ldap:
login_path:login
check_path:login_check
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Recently we updated the docs to use the same route forlogin_path andcheck_path. I think we should do that here too.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

if I look at form_login.rst it seems like those config options are simply omitted there .. that being said there are also some previously existing places in this file where we have the check_path in examples .. so should i remove login_path/check_path for all examples in this file?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

diff --git a/security/ldap.rst b/security/ldap.rstindex bd812fd..d91e977 100644--- a/security/ldap.rst+++ b/security/ldap.rst@@ -310,8 +310,6 @@ Configuration example for form login                 main:                     # ...                     form_login_ldap:-                        login_path: login-                        check_path: login_check                         # ...                         service: ldap                         dn_string: 'uid={username},dc=example,dc=com'@@ -329,8 +327,6 @@ Configuration example for form login             <config>                 <firewall name="main">                     <form-login-ldap-                            login-path="login"-                            check-path="login_check"                             service="ldap"                             dn-string="uid={username},dc=example,dc=com" />                 </firewall>@@ -343,8 +339,6 @@ Configuration example for form login             'firewalls' => array(                 'main' => array(                     'form_login_ldap' => array(-                        'login_path' => 'login',-                        'check_path' => 'login_check',                         'service' => 'ldap',                         'dn_string' => 'uid={username},dc=example,dc=com',                         // ...@@ -419,8 +413,6 @@ Configuration example for form login and query_string                 main:                     # ...                     form_login_ldap:-                        login_path: login-                        check_path: login_check                         # ...                         service: ldap                         dn_string: 'dc=example,dc=com'@@ -439,8 +431,6 @@ Configuration example for form login and query_string             <config>                 <firewall name="main">                     <form-login-ldap-                            login-path="login"-                            check-path="login_check"                             service="ldap"                             dn-string="dc=example,dc=com"                             query-string="(&amp;(uid={username})(memberOf=cn=users,ou=Services,dc=example,dc=com))" />@@ -455,8 +445,6 @@ Configuration example for form login and query_string             'firewalls' => array(                 'main' => array(                     'form_login_ldap' => array(-                        'login_path' => 'login',-                        'check_path' => 'login_check',                         'service' => 'ldap',                         'dn_string' => 'dc=example,dc=com',                         'query_string' => '(&(uid={username})(memberOf=cn=users,ou=Services,dc=example,dc=com))',

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

looks good to me too

@lsmith77
Copy link
ContributorAuthor

@xabbuh ok .. all comments should be addressed

nietonfir reacted with thumbs up emoji

@xabbuh
Copy link
Member

👍

Status: Reviewed

@xabbuh
Copy link
Member

Thank you@lsmith77.

@xabbuhxabbuh merged commitb82cafd intosymfony:masterFeb 28, 2017
xabbuh added a commit that referenced this pull requestFeb 28, 2017
…reguiluz, lsmith77)This PR was merged into the master branch.Discussion----------Added query_string LDAP config optiondocs forsymfony/symfony#21402Commits-------b82cafd clean up446ba38 added query_string LDAP config optioned58da8 Minor rewordf133269 Explain the query_string ldap authentication provider configuration key
@xabbuh
Copy link
Member

And thank you very much@nietonfir!

@lsmith77lsmith77 deleted the ldap_query_string branchFebruary 28, 2017 21:53
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@xabbuhxabbuhxabbuh left review comments

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

5 participants

@lsmith77@nietonfir@xabbuh@javiereguiluz@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp