LDAPServiceTest::testUpdateMemberFromLDAP() - Code Metrics - Inspection of "SS4 security changes and unit test fixes" - silverstripe/silverstripe-activedirectory - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#116)

by Franco

created 2017-09-03 11:20 UTC

LDAPServiceTest::testUpdateMemberFromLDAP() A

↳ Parent: LDAPServiceTest

Complexity

Conditions	1
Paths	1

Size

Total Lines	23
Code Lines	15

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	23
rs	9.0856
c	0
b	0
f	0
cc	1
eloc	15
nc	1
nop	0

<?php

namespace SilverStripe\ActiveDirectory\Tests\Services;

use SilverStripe\ActiveDirectory\Extensions\LDAPGroupExtension;
use SilverStripe\ActiveDirectory\Extensions\LDAPMemberExtension;
use SilverStripe\ActiveDirectory\Model\LDAPGateway;
use SilverStripe\ActiveDirectory\Services\LDAPService;
use SilverStripe\ActiveDirectory\Tests\FakeGatewayTest;
use SilverStripe\Core\Config\Config;
use SilverStripe\Security\Group;
use SilverStripe\Security\Member;

/**
 * @coversDefaultClass \SilverStripe\ActiveDirectory\Services\LDAPService
 * @package activedirectory
 */
class LDAPServiceTest extends FakeGatewayTest
{
    /**
     * {@inheritDoc}
     * @var bool
     */
    protected $usesDatabase = true;

    /**
     * {@inheritDoc}
     */
    public function setUp()
    {
        parent::setUp();

        Config::modify()->set(LDAPGateway::class, 'options', ['host' => '1.2.3.4']);
        Config::modify()->set(LDAPService::class, 'groups_search_locations', [
            'CN=Users,DC=playpen,DC=local',
            'CN=Others,DC=playpen,DC=local'
        ]);
        // Prevent other module extension hooks from executing during write() etc.
        Config::modify()->remove(Member::class, 'extensions');
        Config::modify()->remove(Group::class, 'extensions');
        Config::modify()->set(Member::class, 'update_ldap_from_local', false);
        Config::modify()->set(Member::class, 'create_users_in_ldap', false);
        Config::modify()->set(
            Group::class,
            'extensions',
            [LDAPGroupExtension::class]
        );
        Config::modify()->set(
            Member::class,
            'extensions',
            [LDAPMemberExtension::class]
        );

        // Disable Monolog logging to stderr by default if you don't give it a handler
        $this->service->getLogger()->pushHandler(new \Monolog\Handler\NullHandler);
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
    }

    public function testGroups()
    {
        $expected = [
            'CN=Group1,CN=Users,DC=playpen,DC=local' => ['dn' => 'CN=Group1,CN=Users,DC=playpen,DC=local'],
            'CN=Group2,CN=Users,DC=playpen,DC=local' => ['dn' => 'CN=Group2,CN=Users,DC=playpen,DC=local'],
            'CN=Group3,CN=Users,DC=playpen,DC=local' => ['dn' => 'CN=Group3,CN=Users,DC=playpen,DC=local'],
            'CN=Group4,CN=Users,DC=playpen,DC=local' => ['dn' => 'CN=Group4,CN=Users,DC=playpen,DC=local'],
            'CN=Group5,CN=Users,DC=playpen,DC=local' => ['dn' => 'CN=Group5,CN=Users,DC=playpen,DC=local'],
            'CN=Group6,CN=Others,DC=playpen,DC=local' => ['dn' => 'CN=Group6,CN=Others,DC=playpen,DC=local'],
            'CN=Group7,CN=Others,DC=playpen,DC=local' => ['dn' => 'CN=Group7,CN=Others,DC=playpen,DC=local'],
            'CN=Group8,CN=Others,DC=playpen,DC=local' => ['dn' => 'CN=Group8,CN=Others,DC=playpen,DC=local']
        ];

        $results = $this->service->getGroups();

        $this->assertEquals($expected, $results);
    }

    public function testUpdateMemberFromLDAP()
    {
        Config::modify()->set(
            Member::class,
            'ldap_field_mappings',
            [
                'givenname' => 'FirstName',
                'sn' => 'Surname',
                'mail' => 'Email',
            ]
        );

        $member = new Member();
        $member->GUID = '123';

        $this->service->updateMemberFromLDAP($member);

        $this->assertTrue($member->ID > 0, 'updateMemberFromLDAP writes the member');
        $this->assertEquals('123', $member->GUID, 'GUID remains the same');
        $this->assertEquals('Joe', $member->FirstName, 'FirstName updated from LDAP');
        $this->assertEquals('Bloggs', $member->Surname, 'Surname updated from LDAP');
        $this->assertEquals('[email protected]', $member->Email, 'Email updated from LDAP');
    }
}


1			<?php
2
3			namespace SilverStripe\ActiveDirectory\Tests\Services;
4
5			use SilverStripe\ActiveDirectory\Extensions\LDAPGroupExtension;
6			use SilverStripe\ActiveDirectory\Extensions\LDAPMemberExtension;
7			use SilverStripe\ActiveDirectory\Model\LDAPGateway;
8			use SilverStripe\ActiveDirectory\Services\LDAPService;
9			use SilverStripe\ActiveDirectory\Tests\FakeGatewayTest;
10			use SilverStripe\Core\Config\Config;
11			use SilverStripe\Security\Group;
12			use SilverStripe\Security\Member;
13
14			/**
15			* @coversDefaultClass \SilverStripe\ActiveDirectory\Services\LDAPService
16			* @package activedirectory
17			*/
18			class LDAPServiceTest extends FakeGatewayTest
19			{
20			/**
21			* {@inheritDoc}
22			* @var bool
23			*/
24			protected $usesDatabase = true;
25
26			/**
27			* {@inheritDoc}
28			*/
29			public function setUp()
30			{
31			parent::setUp();
32
33			Config::modify()->set(LDAPGateway::class, 'options', ['host' => '1.2.3.4']);
34			Config::modify()->set(LDAPService::class, 'groups_search_locations', [
35			'CN=Users,DC=playpen,DC=local',
36			'CN=Others,DC=playpen,DC=local'
37			]);
38			// Prevent other module extension hooks from executing during write() etc.
39			Config::modify()->remove(Member::class, 'extensions');
40			Config::modify()->remove(Group::class, 'extensions');
41			Config::modify()->set(Member::class, 'update_ldap_from_local', false);
42			Config::modify()->set(Member::class, 'create_users_in_ldap', false);
43			Config::modify()->set(
44			Group::class,
45			'extensions',
46			[LDAPGroupExtension::class]
47			);
48			Config::modify()->set(
49			Member::class,
50			'extensions',
51			[LDAPMemberExtension::class]
52			);
53
54			// Disable Monolog logging to stderr by default if you don't give it a handler
55			$this->service->getLogger()->pushHandler(new \Monolog\Handler\NullHandler);
			0 ignored issues – show Bug introduced 2017-08-30 05:56 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Psr\Log\LoggerInterface` as the method `pushHandler()` does only exist in the following implementations of said interface: `Monolog\Logger`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
56			}
57
58			public function testGroups()
59			{
60			$expected = [
61			'CN=Group1,CN=Users,DC=playpen,DC=local' => ['dn' => 'CN=Group1,CN=Users,DC=playpen,DC=local'],
62			'CN=Group2,CN=Users,DC=playpen,DC=local' => ['dn' => 'CN=Group2,CN=Users,DC=playpen,DC=local'],
63			'CN=Group3,CN=Users,DC=playpen,DC=local' => ['dn' => 'CN=Group3,CN=Users,DC=playpen,DC=local'],
64			'CN=Group4,CN=Users,DC=playpen,DC=local' => ['dn' => 'CN=Group4,CN=Users,DC=playpen,DC=local'],
65			'CN=Group5,CN=Users,DC=playpen,DC=local' => ['dn' => 'CN=Group5,CN=Users,DC=playpen,DC=local'],
66			'CN=Group6,CN=Others,DC=playpen,DC=local' => ['dn' => 'CN=Group6,CN=Others,DC=playpen,DC=local'],
67			'CN=Group7,CN=Others,DC=playpen,DC=local' => ['dn' => 'CN=Group7,CN=Others,DC=playpen,DC=local'],
68			'CN=Group8,CN=Others,DC=playpen,DC=local' => ['dn' => 'CN=Group8,CN=Others,DC=playpen,DC=local']
69			];
70
71			$results = $this->service->getGroups();
72
73			$this->assertEquals($expected, $results);
74			}
75
76			public function testUpdateMemberFromLDAP()
77			{
78			Config::modify()->set(
79			Member::class,
80			'ldap_field_mappings',
81			[
82			'givenname' => 'FirstName',
83			'sn' => 'Surname',
84			'mail' => 'Email',
85			]
86			);
87
88			$member = new Member();
89			$member->GUID = '123';
90
91			$this->service->updateMemberFromLDAP($member);
92
93			$this->assertTrue($member->ID > 0, 'updateMemberFromLDAP writes the member');
94			$this->assertEquals('123', $member->GUID, 'GUID remains the same');
95			$this->assertEquals('Joe', $member->FirstName, 'FirstName updated from LDAP');
96			$this->assertEquals('Bloggs', $member->Surname, 'Surname updated from LDAP');
97			$this->assertEquals('[email protected]', $member->Email, 'Email updated from LDAP');
98			}
99			}
100

silverstripe / silverstripe-activedirectory

Pull Request — master (#116)

LDAPServiceTest::testUpdateMemberFromLDAP() A

Complexity

Size

Duplication

Importance

Available Fixes

Duplication Side-by-Side

Filter issues like