Skip to content

Test #1

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Test #1

wants to merge 4 commits into from

Conversation

FrancescoBorzi
Copy link
Owner

test

FrancescoBorzi pushed a commit that referenced this pull request Feb 17, 2017
…er in autowired classes (brainexe)

This PR was squashed before being merged into the 3.1 branch (closes symfony#21372).

Discussion
----------

[DependencyInjection] Fixed variadic method parameter in autowired classes

| Q             | A
| ------------- | ---
| Branch?       | 3.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | --
| License       | MIT

Autowiring classes containing methods with variadic method parameter throws a ReflectionException in compile process:

```
PHP Fatal error:  Uncaught ReflectionException: Internal error: Failed to retrieve the default value in /.../vendor/symfony/dependency-injection/Compiler/AutowirePass.php:437
Stack trace:
#0 /.../vendor/symfony/dependency-injection/Compiler/AutowirePass.php(437): ReflectionParameter->getDefaultValue()
#1 /.../vendor/symfony/dependency-injection/Compiler/AutowirePass.php(80): Symfony\Component\DependencyInjection\Compiler\AutowirePass::getResourceMetadataForMethod(Object(ReflectionMethod))
symfony#2 /.../vendor/symfony/dependency-injection/Compiler/AutowirePass.php(105): Symfony\Component\DependencyInjection\Compiler\AutowirePass::createResourceForClass(Object(ReflectionClass))
symfony#3 /.../vendor/symfony/dependency-injection/Compiler/AutowirePass.php(48): Symfony\Component\DependencyInjection\Compiler\AutowirePass->completeDefinition('__controller.Sw...', Object(Symfony\Component\DependencyInjection\Definition), Array)
symfony#4 /.../vendor/symfony/dependency-injection/Compiler/Compiler in /.../vendor/symfony/dependency-injection/Compiler/AutowirePass.php on line 437
```

**Example:**
```
<?php
class FooVariadic
{
    public function bar(...$arguments)
    {
    }
}

$method = new ReflectionMethod(FooVariadic::class, 'bar');
$parameter = $method->getParameters()[0];
$parameter->getDefaultValue(); // -> ReflectionException: Internal error: Failed to retrieve the default value in ...
```

Commits
-------

a7f63de [DependencyInjection] Fixed variadic method parameter in autowired classes
FrancescoBorzi pushed a commit that referenced this pull request Feb 17, 2017
FrancescoBorzi pushed a commit that referenced this pull request Feb 17, 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#7420

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

```diff
diff --git a/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php b/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php
index 5ebb09a..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.php
index fc42419..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_ldap
a783e5c Update HttpBasicLdapFactory
a30191f make LdapBindAuthenticationProvider capable of searching for the DN
FrancescoBorzi pushed a commit that referenced this pull request Feb 17, 2017
…utput trimming (ogizanagi)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[FrameworkBundle] Revert AbstractDescriptorTest output trimming

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no, but fixes an annoying/unhelping test output and fixes false positives
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | https://github.com/symfony/symfony/pull/21129/files#diff-0e52187cbf1067a310538287da74ddb5R178
| License       | MIT
| Doc PR        | N/A

#### Before output when having a failing test, for instance, in a `TextDescriptor` fixture:

```diff
There was 1 failure:

1) Symfony\Bundle\FrameworkBundle\Tests\Console\Descriptor\TextDescriptorTest::testDescribeContainerDefinitionWhichIsAnAlias with data set #1 (Symfony\Component\DependencyInjection\Alias Object (...), ' // This serv...------', Symfony\Component\DependencyInjection\ContainerBuilder Object (...), array('alias_2'))
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-' // This service is an alias for the service service_2 Information for Service "service_2" =================================== ------------------ ---------------------------------  Option   Value  ------------------ --------------------------------- Class Full\Qualified\Class2 Tags tag1 (attr1: val1, attr2: val2) tag1 (attr3: val3) tag2 Calls setMailer Public no Synthetic yes Lazy no Shared yes Abstract no Autowired no Autowiring Types - Required File /path/to/file Factory Service factory.service Factory Method get ------------------ ---------------------------------'
+' // This service is an alias for the service service_2 Information for Service "service_2" =================================== ------------------ ---------------------------------  Option   Value  ------------------ --------------------------------- Service ID service_2 Class Full\Qualified\Class2 Tags tag1 (attr1: val1, attr2: val2) tag1 (attr3: val3) tag2 Calls setMailer Public no Synthetic yes Lazy no Shared yes Abstract no Autowired no Autowiring Types - Required File /path/to/file Factory Service factory.service Factory Method get ------------------ ---------------------------------'
```

#### After:

```diff
There was 1 failure:

1) Symfony\Bundle\FrameworkBundle\Tests\Console\Descriptor\TextDescriptorTest::testDescribeContainerDefinitionWhichIsAnAlias with data set #1 (Symfony\Component\DependencyInjection\Alias Object (...), ' // This serv...------', Symfony\Component\DependencyInjection\ContainerBuilder Object (...), array('alias_2'))
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
  ------------------ ---------------------------------
+  Service ID         service_2
   Class              Full\Qualified\Class2
   Tags               tag1 (attr1: val1, attr2: val2)
                      tag1 (attr3: val3)
                      tag2
   Calls              setMailer
   Public             no
   Synthetic          yes
   Lazy               no
   Shared             yes
   Abstract           no
   Autowired          no
   Autowiring Types   -
   Required File      /path/to/file
   Factory Service    factory.service
   Factory Method     get
  ------------------ ---------------------------------'
```

Commits
-------

efd00ba [FrameworkBundle] Revert AbstractDescriptorTest output trimming
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant