Issues in LDAPGroupExtension.php - New Issues - Inspection of "Adding composer alias" - silverstripe/silverstripe-activedirectory - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( f879e1...c8a3df )

by Daniel

created 2016-12-20 15:52 UTC

code/extensions/LDAPGroupExtension.php (1 issue)

Labels

Bug 1

Severity

Minor 1

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php
/**
 * Class LDAPGroupExtension
 *
 * Adds a field to map an LDAP group to a SilverStripe {@link Group}
 */
class LDAPGroupExtension extends DataExtension
{
    /**
     * @var array
     */
    private static $db = [
        // Unique user identifier, same field is used by SAMLMemberExtension
        'GUID' => 'Varchar(50)',
        'DN' => 'Text',
        'LastSynced' => 'SS_Datetime'
    ];

    /**
     * A SilverStripe group can have several mappings to LDAP groups.
     * @var array
     */
    private static $has_many = [
        'LDAPGroupMappings' => 'LDAPGroupMapping'
    ];

    /**
     * Add a field to the Group_Members join table so we can keep track
     * of Members added to a mapped Group.
     *
     * See {@link LDAPService::updateMemberFromLDAP()} for more details
     * on how this gets used.
     *
     * @var array
     */
    private static $many_many_extraFields = [
        'Members' => [
            'IsImportedFromLDAP' => 'Boolean'
        ]
    ];

    public function updateCMSFields(FieldList $fields)
    {
        // Add read-only LDAP metadata fields.
        $fields->addFieldToTab('Root.LDAP', new ReadonlyField('GUID'));
        $fields->addFieldToTab('Root.LDAP', new ReadonlyField('DN'));
        $fields->addFieldToTab('Root.LDAP', new ReadonlyField(
            'LastSynced',
            _t('LDAPGroupExtension.LASTSYNCED', 'Last synced'))
        );

        if ($this->owner->GUID) {
            $fields->replaceField('Title', new ReadonlyField('Title'));
            $fields->replaceField('Description', new ReadonlyField('Description'));
            // Surface the code which is normally hidden from the CMS user.
            $fields->addFieldToTab('Root.Members', new ReadonlyField('Code'), 'Members');

            $message = _t(
                'LDAPGroupExtension.INFOIMPORTED',
                'This group is automatically imported from LDAP.'
            );
            $fields->addFieldToTab(
                'Root.Members',
                new LiteralField(
                    'Info',
                    sprintf('<p class="message warning">%s</p>', $message)
                ),
                'Title'
            );

            $fields->addFieldToTab('Root.LDAP', new ReadonlyField(
                'LDAPGroupMappingsRO',
                _t('LDAPGroupExtension.AUTOMAPPEDGROUPS', 'Automatically mapped LDAP Groups'),
                implode('; ', $this->owner->LDAPGroupMappings()->column('DN'))
            ));
        } else {
            $field = GridField::create(
                'LDAPGroupMappings',
                _t('LDAPGroupExtension.MAPPEDGROUPS', 'Mapped LDAP Groups'),
                $this->owner->LDAPGroupMappings()
            );
            $config = GridFieldConfig_RecordEditor::create();
            $config->getComponentByType('GridFieldAddNewButton')
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.
    }
}
                ->setButtonName(_t('LDAPGroupExtension.ADDMAPPEDGROUP', 'Add LDAP group mapping'));

            $field->setConfig($config);
            $fields->addFieldToTab('Root.LDAP', $field);
        }
    }

    /**
     * LDAPGroupMappings are inherently relying on groups and can be removed now.
     */
    public function onBeforeDelete()
    {
        foreach ($this->owner->LDAPGroupMappings() as $mapping) {
            $mapping->delete();
        }
    }
}


Push — master ( f879e1...c8a3df )

code/extensions/LDAPGroupExtension.php (1 issue)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Available Fixes

1			<?php
2			/**
3			* Class LDAPGroupExtension
4			*
5			* Adds a field to map an LDAP group to a SilverStripe {@link Group}
6			*/
7			class LDAPGroupExtension extends DataExtension
8			{
9			/**
10			* @var array
11			*/
12			private static $db = [
13			// Unique user identifier, same field is used by SAMLMemberExtension
14			'GUID' => 'Varchar(50)',
15			'DN' => 'Text',
16			'LastSynced' => 'SS_Datetime'
17			];
18
19			/**
20			* A SilverStripe group can have several mappings to LDAP groups.
21			* @var array
22			*/
23			private static $has_many = [
24			'LDAPGroupMappings' => 'LDAPGroupMapping'
25			];
26
27			/**
28			* Add a field to the Group_Members join table so we can keep track
29			* of Members added to a mapped Group.
30			*
31			* See {@link LDAPService::updateMemberFromLDAP()} for more details
32			* on how this gets used.
33			*
34			* @var array
35			*/
36			private static $many_many_extraFields = [
37			'Members' => [
38			'IsImportedFromLDAP' => 'Boolean'
39			]
40			];
41
42			public function updateCMSFields(FieldList $fields)
43			{
44			// Add read-only LDAP metadata fields.
45			$fields->addFieldToTab('Root.LDAP', new ReadonlyField('GUID'));
46			$fields->addFieldToTab('Root.LDAP', new ReadonlyField('DN'));
47			$fields->addFieldToTab('Root.LDAP', new ReadonlyField(
48			'LastSynced',
49			_t('LDAPGroupExtension.LASTSYNCED', 'Last synced'))
50			);
51
52			if ($this->owner->GUID) {
53			$fields->replaceField('Title', new ReadonlyField('Title'));
54			$fields->replaceField('Description', new ReadonlyField('Description'));
55			// Surface the code which is normally hidden from the CMS user.
56			$fields->addFieldToTab('Root.Members', new ReadonlyField('Code'), 'Members');
57
58			$message = _t(
59			'LDAPGroupExtension.INFOIMPORTED',
60			'This group is automatically imported from LDAP.'
61			);
62			$fields->addFieldToTab(
63			'Root.Members',
64			new LiteralField(
65			'Info',
66			sprintf('<p class="message warning">%s</p>', $message)
67			),
68			'Title'
69			);
70
71			$fields->addFieldToTab('Root.LDAP', new ReadonlyField(
72			'LDAPGroupMappingsRO',
73			_t('LDAPGroupExtension.AUTOMAPPEDGROUPS', 'Automatically mapped LDAP Groups'),
74			implode('; ', $this->owner->LDAPGroupMappings()->column('DN'))
75			));
76			} else {
77			$field = GridField::create(
78			'LDAPGroupMappings',
79			_t('LDAPGroupExtension.MAPPEDGROUPS', 'Mapped LDAP Groups'),
80			$this->owner->LDAPGroupMappings()
81			);
82			$config = GridFieldConfig_RecordEditor::create();
83			$config->getComponentByType('GridFieldAddNewButton')
			0 ignored issues – show Bug introduced 2016-03-22 20:04 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a concrete implementation and not the interface `GridFieldComponent` as the method `setButtonName()` does only exist in the following implementations of said interface: `GridFieldAddNewButton`. 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...
84			->setButtonName(_t('LDAPGroupExtension.ADDMAPPEDGROUP', 'Add LDAP group mapping'));
85
86			$field->setConfig($config);
87			$fields->addFieldToTab('Root.LDAP', $field);
88			}
89			}
90
91			/**
92			* LDAPGroupMappings are inherently relying on groups and can be removed now.
93			*/
94			public function onBeforeDelete()
95			{
96			foreach ($this->owner->LDAPGroupMappings() as $mapping) {
97			$mapping->delete();
98			}
99			}
100			}
101

silverstripe / silverstripe-activedirectory

Push — master ( f879e1...c8a3df )

code/extensions/LDAPGroupExtension.php (1 issue)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Available Fixes

Duplication Side-by-Side

Filter issues like