ResetPasswordCommand::getMemberByEmailOrID()   A
last analyzed

Complexity

Conditions 2
Paths 2

Size

Total Lines 14
Code Lines 8

Duplication

Lines 0
Ratio 0 %

Importance

Changes 2
Bugs 1 Features 0
Metric Value
c 2
b 1
f 0
dl 0
loc 14
rs 9.4285
cc 2
eloc 8
nc 2
nop 0
1
<?php
2
3
use Symfony\Component\Console\Input\InputArgument;
4
5
class ResetPasswordCommand extends SilverstripeCommand
0 ignored issues
show
Coding Style Compatibility introduced by
PSR1 recommends that each class must be in a namespace of at least one level to avoid collisions.

You can fix this by adding a namespace to your class:

namespace YourVendor;

class YourClass { }

When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.

Loading history...
6
{
7
    /**
8
     * @var string
9
     */
10
    protected $name = 'security:resetpassword';
11
12
    /**
13
     * @var string
14
     */
15
    protected $description = 'Send a reset password link to an email address';
16
17
    public function fire()
18
    {
19
        $member = $this->getMemberByEmailOrID();
20
21
        if (!(bool) $member) {
22
            $this->error('Member not found');
23
        } else {
24
            $link = $this->sendResetPasswordEmail($member);
0 ignored issues
show
Documentation introduced by
$member is of type object<DataObject>|null, but the function expects a object<Member>.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
25
            $this->info('Email sent to '.$member->getName().'<'.$member->Email.'>');
26
            $this->line($link);
27
        }
28
    }
29
30
    /**
31
     * @return Member|null
0 ignored issues
show
Documentation introduced by
Should the return type not be DataObject|null?

This check compares the return type specified in the @return annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch.

Loading history...
32
     */
33
    protected function getMemberByEmailOrID()
34
    {
35
        $emailorid = $this->argument('emailorid');
36
37
        $member = null;
0 ignored issues
show
Unused Code introduced by
$member is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
38
39
        if (Str::contains($emailorid, '@')) {
0 ignored issues
show
Bug introduced by
It seems like $emailorid defined by $this->argument('emailorid') on line 35 can also be of type array; however, Str::contains() 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...
40
            $member = Member::get()->where("Email = '".Convert::raw2sql($emailorid)."'")->first();
41
        } else {
42
            $member = Member::get()->byID($emailorid);
0 ignored issues
show
Documentation introduced by
$emailorid is of type string|array, but the function expects a integer.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
43
        }
44
45
        return $member;
46
    }
47
48
    /**
49
     * Send the reset password email and return the generated link.
50
     *
51
     * @param Member $member
52
     *
53
     * @return string
54
     */
55
    protected function sendResetPasswordEmail(Member $member)
0 ignored issues
show
Coding Style introduced by
sendResetPasswordEmail uses the super-global variable $_SERVER which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
56
    {
57
        // hack ?
58
        global $_FILE_TO_URL_MAPPING;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

Instead of relying on global state, we recommend one of these alternatives:

1. Pass all data via parameters

function myFunction($a, $b) {
    // Do something
}

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

    public function __construct($a, $b) {
        $this->a = $a;
        $this->b = $b;
    }

    public function myFunction() {
        // Do something
    }
}
Loading history...
59
60
        if ($_FILE_TO_URL_MAPPING[BASE_PATH]) {
61
            $_SERVER['REQUEST_URI'] = $_FILE_TO_URL_MAPPING[BASE_PATH];
62
        }
63
64
        $token = $member->generateAutologinTokenAndStoreHash();
65
        $link = Security::getPasswordResetLink($member, $token);
66
67
        /* @var Member_ForgotPasswordEmail $email */
68
        $email = Member_ForgotPasswordEmail::create();
69
        $email->populateTemplate($member);
70
        $email->populateTemplate([
71
            'PasswordResetLink' => $link,
72
        ]);
73
        $email->setTo($member->Email);
74
        $email->send();
75
76
        return $link;
77
    }
78
79
    /**
80
     * @return array
81
     */
82
    protected function getArguments()
83
    {
84
        return [
85
            ['emailorid', InputArgument::REQUIRED, 'The emailaddress or ID of the Member'],
86
        ];
87
    }
88
}
89