Completed
Pull Request — master (#39)
by
unknown
02:02
created

OpauthController   B

Complexity

Total Complexity 51

Size/Duplication

Total Lines 447
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 13

Importance

Changes 30
Bugs 3 Features 1
Metric Value
wmc 51
c 30
b 3
f 1
lcom 1
cbo 13
dl 0
loc 447
rs 8.3206

20 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 6 2
A getResponseController() 0 12 2
A index() 0 18 3
A oauthCallback() 0 9 1
B finished() 0 65 7
B loginAndRedirect() 0 42 5
A profilecompletion() 0 15 2
A RegisterForm() 0 16 2
B doCompleteRegister() 0 30 5
A getOpauthResponse() 0 13 4
B validateOpauthResponse() 0 31 3
A requireResponseComponents() 0 7 3
A getResponseFromSession() 0 3 1
B handleOpauthException() 0 30 4
A getResponseFromRequest() 0 3 1
A Link() 0 6 1
A get_path() 0 6 1
A get_callback_path() 0 6 1
A Title() 0 6 2
A Form() 0 3 1

How to fix   Complexity   

Complex Class

Complex classes like OpauthController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use OpauthController, and based on these observations, apply Extract Interface, too.

1
<?php
2
3
/**
4
 * OpauthController
5
 * Wraps around Opauth for handling callbacks.
6
 * The SS equivalent of "index.php" and "callback.php" in the Opauth package.
7
 * @author Will Morgan <@willmorgan>
8
 * @author Dan Hensby <@dhensby>
9
 * @copyright Copyright (c) 2013, Better Brief LLP
10
 */
11
class OpauthController extends ContentController {
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...
12
13
	private static
14
		$allowed_actions = array(
0 ignored issues
show
Coding Style introduced by
The visibility should be declared for property $allowed_actions.

The PSR-2 coding standard requires that all properties in a class have their visibility explicitly declared. If you declare a property using

class A {
    var $property;
}

the property is implicitly global.

To learn more about the PSR-2, please see the PHP-FIG site on the PSR-2.

Loading history...
Unused Code introduced by
The property $allowed_actions is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
Coding Style introduced by
It is generally advisable to only define one property per statement.

Only declaring a single property per statement allows you to later on add doc comments more easily.

It is also recommended by PSR2, so it is a common style that many people expect.

Loading history...
15
			'index',
16
			'finished',
17
			'profilecompletion',
18
			'RegisterForm',
19
		),
20
		$url_handlers = array(
0 ignored issues
show
Unused Code introduced by
The property $url_handlers is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
21
			'finished' => 'finished',
22
		);
23
24
	/**
25
	 * Bitwise indicators to extensions what sort of action is happening
26
	 */
27
	const
28
		/**
29
		 * LOGIN = already a user with an OAuth ID
30
		 */
31
		AUTH_FLAG_LOGIN = 2,
32
		/**
33
		 * LINK = already a user, linking a new OAuth ID
34
		 */
35
		AUTH_FLAG_LINK = 4,
36
		/**
37
		 * REGISTER = new user, linking OAuth ID
38
		 */
39
		AUTH_FLAG_REGISTER = 8;
40
41
	protected
42
		$registerForm;
0 ignored issues
show
Coding Style introduced by
The visibility should be declared for property $registerForm.

The PSR-2 coding standard requires that all properties in a class have their visibility explicitly declared. If you declare a property using

class A {
    var $property;
}

the property is implicitly global.

To learn more about the PSR-2, please see the PHP-FIG site on the PSR-2.

Loading history...
43
44
	/**
45
	 * Fake a Page_Controller by using that class as a failover
46
	 */
47
	public function __construct($dataRecord = null) {
48
		if(class_exists('Page_Controller')) {
49
			$dataRecord = new Page_Controller($dataRecord);
50
		}
51
		parent::__construct($dataRecord);
52
	}
53
54
	/**
55
	 * Prepare the controller for handling the response to this request
56
	 *
57
	 * @param string $title Title to use
58
	 * @return Controller
59
	 */
60
	protected function getResponseController($title) {
61
		if(!class_exists('SiteTree')) return $this;
62
63
		// Use sitetree pages to render the opauth pages
64
		$tmpPage = new Page();
65
		$tmpPage->ID = -1;
66
		$tmpPage->Title = $title;
67
		
68
		$controller = ModelAsController::controller_for($tmpPage);
69
		$controller->init();
70
		return $controller;
71
	}
72
73
	/**
74
	 * This function only catches the request to pass it straight on.
75
	 * Opauth uses the last segment of the URL to identify the auth method.
76
	 * In _routes.yml we enforce a $Strategy request parameter to enforce this.
77
	 * Equivalent to "index.php" in the Opauth package.
78
	 * @todo: Validate the strategy works before delegating to Opauth.
79
	 */
80
	public function index(SS_HTTPRequest $request) {
81
82
		$strategy = $request->param('Strategy');
83
		$method = $request->param('StrategyMethod');
84
85
		if(!isset($strategy)) {
86
			return Security::permissionFailure($this);
87
		}
88
89
		// If there is no method then we redirect (not a callback)
90
		if(!isset($method)) {
91
			// Redirects:
92
			OpauthAuthenticator::opauth(true);
93
		}
94
		else {
95
			return $this->oauthCallback($request);
96
		}
97
	}
98
99
	/**
100
	 * This is executed when the Oauth provider redirects back to us
101
	 * Opauth handles everything sent back in this request.
102
	 */
103
	protected function oauthCallback(SS_HTTPRequest $request) {
104
105
		// Set up and run opauth with the correct params from the strategy:
106
		OpauthAuthenticator::opauth(true, array(
107
			'strategy'	=> $request->param('Strategy'),
108
			'action'	=> $request->param('StrategyMethod'),
109
		));
110
111
	}
112
113
	/**
114
	 * Equivalent to "callback.php" in the Opauth package.
115
	 * If there is a problem with the response, we throw an HTTP error.
116
	 * When done validating, we return back to the Authenticator continue auth.
117
	 * @throws SS_HTTPResponse_Exception if any validation errors
118
	 */
119
	public function finished(SS_HTTPRequest $request) {
120
121
		$opauth = OpauthAuthenticator::opauth(false);
122
123
		$response = $this->getOpauthResponse();
124
125
		if (!$response) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $response of type array is implicitly converted to a boolean; are you sure this is intended? If so, consider using empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
126
			$response = array();
127
		}
128
		// Clear the response as it is only to be read once (if Session)
129
		Session::clear('opauth');
130
131
		// Handle all Opauth validation in this handy function
132
		try {
133
			$this->validateOpauthResponse($opauth, $response);
134
		}
135
		catch(OpauthValidationException $e) {
136
			return $this->handleOpauthException($e);
137
		}
138
139
		$identity = OpauthIdentity::factory($response);
140
141
		$member = $identity->findOrCreateMember();
142
143
		// If the member exists, associate it with the identity and log in
144
		if($member->isInDB() && $member->validate()->valid()) {
145
			if(!$identity->exists()) {
146
				$identity->write();
147
				$flag = self::AUTH_FLAG_LINK;
148
			}
149
			else {
150
				$flag = self::AUTH_FLAG_LOGIN;
151
			}
152
153
			Session::set('OpauthIdentityID', $identity->ID);
154
		}
155
		else {
156
157
			$flag = self::AUTH_FLAG_REGISTER;
158
159
			// Write the identity
160
			$identity->write();
161
162
			// Keep a note of the identity ID
163
			Session::set('OpauthIdentityID', $identity->ID);
164
165
			// Even if written, check validation - we might not have full fields
166
			$validationResult = $member->validate();
167
			if(!$validationResult->valid()) {
168
				// Set up the register form before it's output
169
				$regForm = $this->RegisterForm();
170
				$regForm->loadDataFrom($member);
171
				$regForm->setSessionData($member);
172
				$regForm->validate();
173
				return $this->redirect($this->Link('profilecompletion'));
174
			}
175
			else {
176
				$member->extend('onBeforeOpauthRegister');
177
				$member->write();
178
				$identity->MemberID = $member->ID;
0 ignored issues
show
Documentation introduced by
The property MemberID does not exist on object<OpauthIdentity>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
179
				$identity->write();
180
			}
181
		}
182
		return $this->loginAndRedirect($member, $identity, $flag);
183
	}
184
185
	/**
186
	 * @param Member
187
	 * @param OpauthIdentity
188
	 * @param int $mode One or more AUTH_FLAGs.
189
	 */
190
	protected function loginAndRedirect(Member $member, OpauthIdentity $identity, $mode) {
191
		// Back up the BackURL as Member::logIn regenerates the session
192
		$backURL = Session::get('BackURL');
193
194
		// Check if we can log in:
195
		$canLogIn = $member->canLogIn();
196
197
		if(!$canLogIn->valid()) {
198
			$extendedURLs = $this->extend('getCantLoginBackURL', $member, $identity, $canLogIn, $mode);
199
			if(count($extendedURLs)) {
200
				$redirectURL = array_pop($extendedURLs);
201
				$this->redirect($redirectURL, 302);
202
				return;
203
			}
204
			Security::permissionFailure($this, $canLogIn->message());
205
			return;
206
		}
207
208
		// Decide where to go afterwards...
209
		if(!empty($backURL)) {
210
			$redirectURL = $backURL;
211
		}
212
		else {
213
			$redirectURL = Security::config()->default_login_dest;
214
		}
215
216
		$extendedURLs = $this->extend('getSuccessBackURL', $member, $identity, $redirectURL, $mode);
217
218
		if(count($extendedURLs)) {
219
			$redirectURL = array_pop($extendedURLs);
220
		}
221
222
		$member->logIn();
223
224
		// Clear any identity ID
225
		Session::clear('OpauthIdentityID');
226
		
227
		// Clear the BackURL
228
		Session::clear('BackURL');
229
230
		return $this->redirect($redirectURL);
231
	}
232
233
	public function profilecompletion(SS_HTTPRequest $request = null) {
234
		// Get response handler
235
		$controller = $this->getResponseController(_t('Opauth.PROFILECOMPLETIONTITLE', 'Complete your profile'));
0 ignored issues
show
Unused Code introduced by
$controller 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...
236
		
237
		if(!Session::get('OpauthIdentityID')) {
238
			Security::permissionFailure($this);
239
		}
240
		// Redirect to complete register step by adding in extra info
241
		return $this->renderWith(array(
242
				'OpauthController_profilecompletion',
243
				'Security_profilecompletion',
244
				'Page',
245
			)
246
		);
247
	}
248
249
	public function RegisterForm(SS_HTTPRequest $request = null, Member $member = null, $result = null) {
250
		if(!isset($this->registerForm)) {
251
			$form = Injector::inst()->create('OpauthRegisterForm', $this, 'RegisterForm', $result);
252
			$form->populateFromSources($request, $member, $result);
253
			// Set manually the form action due to how routing works
254
			$form->setFormAction(Controller::join_links(
255
				self::config()->opauth_path,
256
				'RegisterForm'
257
			));
258
			$this->registerForm = $form;
259
		}
260
		else {
261
			$this->registerForm->populateFromSources($request, $member, $result);
262
		}
263
		return $this->registerForm;
264
	}
265
266
	public function doCompleteRegister($data, $form, $request) {
267
		$member = new Member();
268
		$form->saveInto($member);
269
		$identityID = Session::get('OpauthIdentityID');
270
		$identity = DataObject::get_by_id('OpauthIdentity', $identityID);
271
		$validationResult = $member->validate();
272
		$existing = Member::get()->filter('Email', $member->Email)->first();
273
		$emailCollision = $existing && $existing->exists();
274
		// If not valid then we have to manually transpose errors to the form
275
		if(!$validationResult->valid() || $emailCollision) {
276
			$errors = $validationResult->messageList();
277
			$form->setRequiredFields($errors);
278
			// Mandatory check on the email address
279
			if($emailCollision) {
280
				$form->addErrorMessage('Email', _t(
281
					'OpauthRegisterForm.ERROREMAILTAKEN',
282
					'It looks like this email has already been used'
283
				), 'required');
284
			}
285
			return $this->redirect('profilecompletion');
286
		}
287
		// If valid then write and redirect
288
		else {
289
			$member->extend('onBeforeOpauthRegister');
290
			$member->write();
291
			$identity->MemberID = $member->ID;
292
			$identity->write();
293
			return $this->loginAndRedirect($member, $identity, self::AUTH_FLAG_REGISTER);
294
		}
295
	}
296
297
	/**
298
	 * Returns the response from the Oauth callback.
299
	 * @throws InvalidArugmentException
300
	 * @return array The response
301
	 */
302
	protected function getOpauthResponse() {
303
		$config = OpauthAuthenticator::get_opauth_config();
304
		$transportMethod = $config['callback_transport'];
305
		switch($transportMethod) {
306
			case 'session':
307
				return $this->getResponseFromSession();
308
			case 'get':
309
			case 'post':
310
				return $this->getResponseFromRequest($transportMethod);
311
			default:
312
				throw new InvalidArgumentException('Invalid transport method: ' . $transportMethod);
313
		}
314
	}
315
316
	/**
317
	 * Validates the Oauth response for Opauth.
318
	 * @throws InvalidArgumentException
319
	 */
320
	protected function validateOpauthResponse($opauth, $response) {
321
		if(!empty($response['error'])) {
322
			throw new OpauthValidationException('Oauth provider error', 1, $response['error']);
323
		}
324
325
		// Required components within the response
326
		$this->requireResponseComponents(
327
			array('auth', 'timestamp', 'signature'),
328
			$response
329
		);
330
331
		// More required components within the auth section...
332
		$this->requireResponseComponents(
333
			array('provider', 'uid'),
334
			$response['auth']
335
		);
336
337
		$invalidReason = '';
338
339
		/**
340
		 * @todo: improve this signature check. it's a bit weak.
341
		 */
342
		if(!$opauth->validate(
343
			sha1(print_r($response['auth'], true)),
344
			$response['timestamp'],
345
			$response['signature'],
346
			$invalidReason
347
		)) {
348
			throw new OpauthValidationException('Invalid auth response', 3, $invalidReason);
349
		}
350
	}
351
352
	/**
353
	 * Shorthand for quickly finding missing components and complaining about it
354
	 * @throws InvalidArgumentException
355
	 */
356
	protected function requireResponseComponents(array $components, $response) {
357
		foreach($components as $component) {
358
			if(empty($response[$component])) {
359
				throw new OpauthValidationException('Required component missing', 2, $component);
360
			}
361
		}
362
	}
363
364
	/**
365
	 * @return array Opauth response from session
366
	 */
367
	protected function getResponseFromSession() {
368
		return Session::get('opauth');
369
	}
370
371
	/**
372
	 * @param OpauthValidationException $e
373
	 */
374
	protected function handleOpauthException(OpauthValidationException $e) {
375
		$data = $e->getData();
376
		$loginFormName = 'OpauthLoginForm_LoginForm';
377
		$message = '';
378
		switch($e->getCode()) {
379
			case 1: // provider error
380
				$message = _t(
381
					'OpauthLoginForm.OAUTHFAILURE',
382
					'There was a problem logging in with {provider}.',
383
					array(
0 ignored issues
show
Documentation introduced by
array('provider' => $data['provider']) is of type array<string,?,{"provider":"?"}>, but the function expects a string.

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...
384
						'provider' => $data['provider'],
385
					)
386
				);
387
				break;
388
			case 2: // validation error
389
			case 3: // invalid auth response
390
				$message = _t(
391
					'OpauthLoginForm.RESPONSEVALIDATIONFAILURE',
392
					'There was a problem logging in - {message}',
393
					array(
0 ignored issues
show
Documentation introduced by
array('message' => $e->getMessage()) is of type array<string,string,{"message":"string"}>, but the function expects a string.

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...
394
						'message' => $e->getMessage(),
395
					)
396
				);
397
				break;
398
		}
399
		// Set form message, redirect to login with permission failure
400
		Form::messageForForm($loginFormName, $message, 'bad');
401
		// always redirect to login
402
		Security::permissionFailure($this, $message);
403
	}
404
405
	/**
406
	 * Looks at $method (GET, POST, PUT etc) for the response.
407
	 * @return array Opauth response
408
	 */
409
	protected function getResponseFromRequest($method) {
410
		return unserialize(base64_decode($this->request->{$method.'Var'}('opauth')));
411
	}
412
413
	public function Link($action = null) {
414
		return Controller::join_links(
415
			self::config()->opauth_path,
416
			$action
417
		);
418
	}
419
420
	/**
421
	 * 'path' param for use in Opauth's config
422
	 * MUST have trailling slash for Opauth needs
423
	 * @return string
424
	 */
425
	public static function get_path() {
426
		return Controller::join_links(
427
			self::config()->opauth_path,
428
			'strategy/'
429
		);
430
	}
431
432
	/**
433
	 * 'callback_url' param for use in Opauth's config
434
	 * MUST have trailling slash for Opauth needs
435
	 * @return string
436
	 */
437
	public static function get_callback_path() {
438
		return Controller::join_links(
439
			self::config()->opauth_path,
440
			'finished/'
441
		);
442
	}
443
444
////**** Template variables ****////
445
	function Title() {
0 ignored issues
show
Best Practice introduced by
It is generally recommended to explicitly declare the visibility for methods.

Adding explicit visibility (private, protected, or public) is generally recommend to communicate to other developers how, and from where this method is intended to be used.

Loading history...
446
		if($this->action == 'profilecompletion') {
447
			return _t('OpauthController.PROFILECOMPLETIONTITLE', 'Complete your profile');
448
		}
449
		return _t('OpauthController.TITLE', 'Social Login');
450
	}
451
452
	public function Form() {
453
		return $this->RegisterForm();
454
	}
455
////**** END Template variables ****////
456
457
}
458