Ads - Code Metrics - Inspection of "Merge pull request #557 from harupu/dev" - XOOPS/XoopsCore - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( fc2c42...d0941c )

by Richard

created 2017-10-04 20:58 UTC

Ads A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	86
Duplicated Lines	3.49 %

Coupling/Cohesion

Components	1
Dependencies	1

Test Coverage

Coverage

9.09%

Importance

Changes

Metric	Value
dl	3
loc	86
ccs	3
cts	33
cp	0.0909
rs	10
c	0
b	0
f	0
wmc	11
lcom	1
cbo	1

3 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	0	4	1
D	authenticate()	3	44	9
A	getUPN()	0	5	1

How to fix Duplicated Code

<?php
/*
 You may not change or alter any portion of this comment or credits
 of supporting developers from this source code or any supporting source code
 which is considered copyrighted (c) material of the original comment or credit authors.

 This program is distributed in the hope that it will be useful,
 but WITHOUT ANY WARRANTY; without even the implied warranty of
 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
*/

namespace Xoops\Auth;

use Xoops\Core\Database\Connection;

/**
 * Authentication class for Active Directory
 *
 * @category  Xoops\Auth
 * @package   Ldap
 * @author    Pierre-Eric MENUET <[email protected]>
 * @copyright 2000-2014 XOOPS Project (http://xoops.org)
 * @license   GNU GPL 2 or later (http://www.gnu.org/licenses/gpl-2.0.html)
 * @link      http://xoops.org
 * @since     2.0
 */
class Ads extends Ldap
{
    /**
     * Authentication Service constructor
     *
     * @param Connection|null $dao database
     *
     * @return void

     */
    public function __construct(Connection $dao = null)
    {
        parent::__construct($dao);
    }

    /**
     * Authenticate  user again LDAP directory (Bind)
     *         2 options :
     *         Authenticate directly with uname in the DN
     *         Authenticate with manager, search the dn
     *
     * @param string $uname Username
     * @param string $pwd   Password
     *
     * @return bool
     */
    public function authenticate($uname, $pwd = null)
    {
        $authenticated = false;
        if (!extension_loaded('ldap')) {
            $this->setErrors(0, \XoopsLocale::E_EXTENSION_PHP_LDAP_NOT_LOADED);
            return $authenticated;
        }
        $this->ds = ldap_connect($this->ldap_server, $this->ldap_port);
        if ($this->ds) {
            ldap_set_option($this->ds, LDAP_OPT_PROTOCOL_VERSION, $this->ldap_version);
            ldap_set_option($this->ds, LDAP_OPT_REFERRALS, 0);
            if ($this->ldap_use_TLS) { // We use TLS secure connection
                if (!ldap_start_tls($this->ds)) {
                    $this->setErrors(0, \XoopsLocale::E_TLS_CONNECTION_NOT_OPENED);
                }
            }
            // remove the domain name prefix from the username
            $uname = explode("\\", $uname);
            $uname = (sizeof($uname) > 0) ? $uname[sizeof($uname) - 1] : $uname = $uname[0];
            // If the uid is not in the DN we proceed to a search
            // The uid is not always in the dn
            $userUPN = $this->getUPN($uname);
            if (!$userUPN) {
                return false;
            }
            // We bind as user to test the credentials
            $authenticated = ldap_bind($this->ds, $userUPN, stripslashes($pwd));
            if ($authenticated) {
                // We load the Xoops User database
                $dn = $this->getUserDN($uname);
                if ($dn) {
                    return $this->loadXoopsUser($dn, $uname, $pwd);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}
                } else {
                    return false;
                }
            } else {
                $this->setErrors(ldap_errno($this->ds), ldap_err2str(ldap_errno($this->ds)) . '(' . $userUPN . ')');
            }
        } else {
            $this->setErrors(0, \XoopsLocale::E_CANNOT_CONNECT_TO_SERVER);
        }
        @ldap_close($this->ds);
// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
        return $authenticated;
    }

    /**
     * Return the UPN = userPrincipalName (Active Directory)
     *         userPrincipalName = [email protected]    Often abbreviated to UPN, and
     *         looks like an email address.  Very useful for logging on especially in
     *         a large Forest.   Note UPN must be unique in the forest.
     *
     * @param string $uname username
     *
     * @return string userDN
     */
    public function getUPN($uname)
    {
        $userDN = $uname . '@' . $this->ldap_domain_name;
        return $userDN;
    }
}


1			<?php
2			/*
3			You may not change or alter any portion of this comment or credits
4			of supporting developers from this source code or any supporting source code
5			which is considered copyrighted (c) material of the original comment or credit authors.
6
7			This program is distributed in the hope that it will be useful,
8			but WITHOUT ANY WARRANTY; without even the implied warranty of
9			MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
10			*/
11
12			namespace Xoops\Auth;
13
14			use Xoops\Core\Database\Connection;
15
16			/**
17			* Authentication class for Active Directory
18			*
19			* @category Xoops\Auth
20			* @package Ldap
21			* @author Pierre-Eric MENUET <[email protected]>
22			* @copyright 2000-2014 XOOPS Project (http://xoops.org)
23			* @license GNU GPL 2 or later (http://www.gnu.org/licenses/gpl-2.0.html)
24			* @link http://xoops.org
25			* @since 2.0
26			*/
27			class Ads extends Ldap
28			{
29			/**
30			* Authentication Service constructor
31			*
32			* @param Connection\|null $dao database
33			*
34			* @return void
			0 ignored issues – show Comprehensibility Best Practice introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report Adding a `@return` annotation to constructors is generally not recommended as a constructor does not have a meaningful return value. Adding a `@return` annotation to a constructor is not recommended, since a constructor does not have a meaningful return value. Please refer to the PHP core documentation on constructors. Loading history...
35			*/
36	2		public function __construct(Connection $dao = null)
37			{
38	2		parent::__construct($dao);
39	2		}
40
41			/**
42			* Authenticate user again LDAP directory (Bind)
43			* 2 options :
44			* Authenticate directly with uname in the DN
45			* Authenticate with manager, search the dn
46			*
47			* @param string $uname Username
48			* @param string $pwd Password
49			*
50			* @return bool
51			*/
52			public function authenticate($uname, $pwd = null)
53			{
54			$authenticated = false;
55			if (!extension_loaded('ldap')) {
56			$this->setErrors(0, \XoopsLocale::E_EXTENSION_PHP_LDAP_NOT_LOADED);
57			return $authenticated;
58			}
59			$this->ds = ldap_connect($this->ldap_server, $this->ldap_port);
60			if ($this->ds) {
61			ldap_set_option($this->ds, LDAP_OPT_PROTOCOL_VERSION, $this->ldap_version);
62			ldap_set_option($this->ds, LDAP_OPT_REFERRALS, 0);
63			if ($this->ldap_use_TLS) { // We use TLS secure connection
64			if (!ldap_start_tls($this->ds)) {
65			$this->setErrors(0, \XoopsLocale::E_TLS_CONNECTION_NOT_OPENED);
66			}
67			}
68			// remove the domain name prefix from the username
69			$uname = explode("\\", $uname);
70			$uname = (sizeof($uname) > 0) ? $uname[sizeof($uname) - 1] : $uname = $uname[0];
71			// If the uid is not in the DN we proceed to a search
72			// The uid is not always in the dn
73			$userUPN = $this->getUPN($uname);
74			if (!$userUPN) {
75			return false;
76			}
77			// We bind as user to test the credentials
78			$authenticated = ldap_bind($this->ds, $userUPN, stripslashes($pwd));
79			if ($authenticated) {
80			// We load the Xoops User database
81			$dn = $this->getUserDN($uname);
82			if ($dn) {
83			return $this->loadXoopsUser($dn, $uname, $pwd);
			0 ignored issues – show Bug introduced 2016-02-04 22:14 UTC by Report Bug Copy Issue Report It seems like `$dn` defined by `$this->getUserDN($uname)` on line `81` can also be of type `boolean`; however, `Xoops\Auth\Ldap::loadXoopsUser()` does only seem to accept `string`, maybe add an additional type check? If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /** * @return array\|string */ function returnsDifferentValues($x) { if ($x) { return 'foo'; } return array(); } $x = returnsDifferentValues($y); if (is_array($x)) { // $x is an array. } If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
84			} else {
85			return false;
86			}
87		View Code Duplication	} else {
88			$this->setErrors(ldap_errno($this->ds), ldap_err2str(ldap_errno($this->ds)) . '(' . $userUPN . ')');
89			}
90			} else {
91			$this->setErrors(0, \XoopsLocale::E_CANNOT_CONNECT_TO_SERVER);
92			}
93			@ldap_close($this->ds);
			0 ignored issues – show Security Best Practice introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended. If you suppress an error, we recommend checking for the error condition explicitly: // For example instead of @mkdir($dir); // Better use if (@mkdir($dir) === false) { throw new \RuntimeException('The directory '.$dir.' could not be created.'); } Loading history...
94			return $authenticated;
95			}
96
97			/**
98			* Return the UPN = userPrincipalName (Active Directory)
99			* userPrincipalName = [email protected] Often abbreviated to UPN, and
100			* looks like an email address. Very useful for logging on especially in
101			* a large Forest. Note UPN must be unique in the forest.
102			*
103			* @param string $uname username
104			*
105			* @return string userDN
106			*/
107			public function getUPN($uname)
108			{
109			$userDN = $uname . '@' . $this->ldap_domain_name;
110			return $userDN;
111			}
112			}
113

XOOPS / XoopsCore

Push — master ( fc2c42...d0941c )

Ads A

Complexity

Size/Duplication

Coupling/Cohesion

Test Coverage

Importance

3 Methods

How to fix Duplicated Code

Duplicated Code

Duplication Side-by-Side

Filter issues like