Completed
Push — authenticator-refactor ( 3617c4...16f104 )
by Sam
05:36
created

CookieAuthenticationHandler::authenticateRequest()   C

Complexity

Conditions 12
Paths 22

Size

Total Lines 76
Code Lines 42

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
cc 12
eloc 42
nc 22
nop 1
dl 0
loc 76
rs 5.2846
c 0
b 0
f 0

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
namespace SilverStripe\Security\MemberAuthenticator;
4
5
use SilverStripe\ORM\DataObject;
6
use SilverStripe\Security\Member;
7
use SilverStripe\Control\HTTPRequest;
8
use SilverStripe\Control\HTTPResponse;
9
use SilverStripe\Security\AuthenticationHandler as AuthenticationHandlerInterface;
10
use SilverStripe\Security\IdentityStore;
11
use SilverStripe\Security\RememberLoginHash;
12
use SilverStripe\Security\Security;
13
use SilverStripe\ORM\FieldType\DBDatetime;
14
use SilverStripe\Control\Cookie;
15
16
/**
17
 * Authenticate a member pased on a session cookie
18
 */
19
class CookieAuthenticationHandler implements AuthenticationHandlerInterface, IdentityStore
20
{
21
22
    private $deviceCookieName;
23
    private $tokenCookieName;
24
    private $cascadeLogInTo;
25
26
    /**
27
     * Get the name of the cookie used to track this device
28
     *
29
     * @return string
30
     */
31
    public function getDeviceCookieName()
32
    {
33
        return $this->deviceCookieName;
34
    }
35
36
    /**
37
     * Set the name of the cookie used to track this device
38
     *
39
     * @param string $cookieName
0 ignored issues
show
Documentation introduced by
There is no parameter named $cookieName. Did you maybe mean $deviceCookieName?

This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function. It has, however, found a similar but not annotated parameter which might be a good fit.

Consider the following example. The parameter $ireland is not defined by the method finale(...).

/**
 * @param array $germany
 * @param array $ireland
 */
function finale($germany, $island) {
    return "2:1";
}

The most likely cause is that the parameter was changed, but the annotation was not.

Loading history...
40
     * @return null
41
     */
42
    public function setDeviceCookieName($deviceCookieName)
43
    {
44
        $this->deviceCookieName = $deviceCookieName;
45
    }
46
47
    /**
48
     * Get the name of the cookie used to store an login token
49
     *
50
     * @return string
51
     */
52
    public function getTokenCookieName()
53
    {
54
        return $this->tokenCookieName;
55
    }
56
57
    /**
58
     * Set the name of the cookie used to store an login token
59
     *
60
     * @param string $cookieName
0 ignored issues
show
Documentation introduced by
There is no parameter named $cookieName. Did you maybe mean $tokenCookieName?

This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function. It has, however, found a similar but not annotated parameter which might be a good fit.

Consider the following example. The parameter $ireland is not defined by the method finale(...).

/**
 * @param array $germany
 * @param array $ireland
 */
function finale($germany, $island) {
    return "2:1";
}

The most likely cause is that the parameter was changed, but the annotation was not.

Loading history...
61
     * @return null
62
     */
63
    public function setTokenCookieName($tokenCookieName)
64
    {
65
        $this->tokenCookieName = $tokenCookieName;
66
    }
67
68
    /**
69
     * Once a member is found by authenticateRequest() pass it to this identity store
70
     *
71
     * @return IdentityStore
72
     */
73
    public function getCascadeLogInTo()
74
    {
75
        return $this->cascadeLogInTo;
76
    }
77
78
    /**
79
     * Set the name of the cookie used to store an login token
80
     *
81
     * @param $cascadeLogInTo
82
     * @return null
83
     */
84
    public function setCascadeLogInTo(IdentityStore $cascadeLogInTo)
85
    {
86
        $this->cascadeLogInTo = $cascadeLogInTo;
87
    }
88
89
    /**
90
     * @inherit
91
     */
92
    public function authenticateRequest(HTTPRequest $request)
93
    {
94
        $uidAndToken = Cookie::get($this->getTokenCookieName());
95
        $deviceID = Cookie::get($this->getDeviceCookieName());
96
97
        // @todo Consider better placement of database_is_ready test
98
        if (!$deviceID || strpos($uidAndToken, ':') === false || !Security::database_is_ready()) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $deviceID of type string|null is loosely compared to false; this is ambiguous if the string can be empty. You might want to explicitly use === null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For string values, the empty string '' is a special case, in particular the following results might be unexpected:

''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
Loading history...
99
            return;
100
        }
101
102
        list($uid, $token) = explode(':', $uidAndToken, 2);
103
104
        if (!$uid || !$token) {
105
            return;
106
        }
107
108
        /** @var Member $member */
109
        $member = Member::get()->byID($uid);
110
111
        /** @var RememberLoginHash $rememberLoginHash */
112
        $rememberLoginHash = null;
113
114
        // check if autologin token matches
115
        if ($member) {
116
            $hash = $member->encryptWithUserSettings($token);
117
            $rememberLoginHash = RememberLoginHash::get()
118
                ->filter(array(
119
                    'MemberID' => $member->ID,
120
                    'DeviceID' => $deviceID,
121
                    'Hash' => $hash
122
                ))->first();
123
124
            if (!$rememberLoginHash) {
125
                $member = null;
126
            } else {
127
                // Check for expired token
128
                $expiryDate = new \DateTime($rememberLoginHash->ExpiryDate);
129
                $now = DBDatetime::now();
130
                $now = new \DateTime($now->Rfc2822());
131
                if ($now > $expiryDate) {
132
                    $member = null;
133
                }
134
            }
135
        }
136
137
        if ($member) {
138
            if ($this->cascadeLogInTo) {
139
                // @todo look at how to block "regular login" triggers from happening here
140
                // @todo deal with the fact that the Session::current_session() isn't correct here :-/
141
                $this->cascadeLogInTo->logIn($member, false, $request);
142
                //\SilverStripe\Dev\Debug::message('here');
143
            }
144
145
            // @todo Consider whether response should be part of logIn() as well
146
147
            // Renew the token
148
            if ($rememberLoginHash) {
149
                $rememberLoginHash->renew();
150
                $tokenExpiryDays = RememberLoginHash::config()->uninherited('token_expiry_days');
151
                Cookie::set(
152
                    $this->getTokenCookieName(),
153
                    $member->ID . ':' . $rememberLoginHash->getToken(),
154
                    $tokenExpiryDays,
155
                    null,
156
                    null,
157
                    false,
158
                    true
159
                );
160
            }
161
162
            return $member;
163
164
            // Audit logging hook
165
            $member->extend('memberAutoLoggedIn');
0 ignored issues
show
Unused Code introduced by
// Audit logging hook $m...('memberAutoLoggedIn'); does not seem to be reachable.

This check looks for unreachable code. It uses sophisticated control flow analysis techniques to find statements which will never be executed.

Unreachable code is most often the result of return, die or exit statements that have been added for debug purposes.

function fx() {
    try {
        doSomething();
        return true;
    }
    catch (\Exception $e) {
        return false;
    }

    return false;
}

In the above example, the last return false will never be executed, because a return statement has already been met in every possible execution path.

Loading history...
166
        }
167
    }
168
169
    /**
170
     * @inherit
171
     */
172
    public function logIn(Member $member, $persistent, HTTPRequest $request)
173
    {
174
        // @todo couple the cookies to the response object
175
176
        // Cleans up any potential previous hash for this member on this device
177
        if ($alcDevice = Cookie::get($this->getDeviceCookieName())) {
178
            RememberLoginHash::get()->filter('DeviceID', $alcDevice)->removeAll();
179
        }
180
181
        // Set a cookie for persistent log-ins
182
        if ($persistent) {
183
            $rememberLoginHash = RememberLoginHash::generate($member);
184
            $tokenExpiryDays = RememberLoginHash::config()->uninherited('token_expiry_days');
185
            $deviceExpiryDays = RememberLoginHash::config()->uninherited('device_expiry_days');
186
            Cookie::set(
187
                $this->getTokenCookieName(),
188
                $member->ID . ':' . $rememberLoginHash->getToken(),
189
                $tokenExpiryDays,
190
                null,
191
                null,
192
                null,
0 ignored issues
show
Documentation introduced by
null is of type null, but the function expects a boolean.

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...
193
                true
194
            );
195
            Cookie::set(
196
                $this->getDeviceCookieName(),
197
                $rememberLoginHash->DeviceID,
198
                $deviceExpiryDays,
199
                null,
200
                null,
201
                null,
0 ignored issues
show
Documentation introduced by
null is of type null, but the function expects a boolean.

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...
202
                true
203
            );
204
205
        // Clear a cookie for non-persistent log-ins
206
        } else {
207
            $this->logOut($request);
208
        }
209
    }
210
211
    /**
212
     * @inherit
213
     */
214
    public function logOut(HTTPRequest $request)
215
    {
216
        // @todo couple the cookies to the response object
217
218
        Cookie::set($this->getTokenCookieName(), null);
219
        Cookie::set($this->getDeviceCookieName(), null);
220
        Cookie::force_expiry($this->getTokenCookieName());
221
        Cookie::force_expiry($this->getDeviceCookieName());
222
    }
223
}
224