Merge pull request #1172 from nextcloud/core_cleanup

Core controller cleanup

core/Application.php

@@ -73,14 +73,6 @@ class Application extends App {
 				$c->query('TimeFactory')
 			);
 		});
-		$container->registerService('UserController', function(SimpleContainer $c) {
-			return new UserController(
-				$c->query('AppName'),
-				$c->query('Request'),
-				$c->query('UserManager'),
-				$c->query('Defaults')
-			);
-		});
 		$container->registerService('LoginController', function(SimpleContainer $c) {
 			return new LoginController(
 				$c->query('AppName'),

core/Controller/AvatarController.php

@@ -29,8 +29,8 @@ namespace OC\Core\Controller;
 
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
-use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\DataDisplayResponse;
+use OCP\AppFramework\Http\JSONResponse;
 use OCP\Files\File;
 use OCP\Files\IRootFolder;
 use OCP\Files\NotFoundException;
@@ -111,7 +111,7 @@ class AvatarController extends Controller {
 	 *
 	 * @param string $userId
 	 * @param int $size
-	 * @return DataResponse|DataDisplayResponse
+	 * @return JSONResponse|DataDisplayResponse
 	 */
 	public function getAvatar($userId, $size) {
 		if ($size > 2048) {
@@ -128,13 +128,13 @@ class AvatarController extends Controller {
 			$resp->setETag($avatar->getEtag());
 		} catch (NotFoundException $e) {
 			$user = $this->userManager->get($userId);
-			$resp = new DataResponse([
+			$resp = new JSONResponse([
 				'data' => [
 					'displayname' => $user->getDisplayName(),
 				],
 			]);
 		} catch (\Exception $e) {
-			$resp = new DataResponse([
+			$resp = new JSONResponse([
 				'data' => [
 					'displayname' => '',
 				],
@@ -152,25 +152,22 @@ class AvatarController extends Controller {
 	 * @NoAdminRequired
 	 *
 	 * @param string $path
-	 * @return DataResponse
+	 * @return JSONResponse
 	 */
 	public function postAvatar($path) {
 		$files = $this->request->getUploadedFile('files');
 
-		$headers = [];
-
 		if (isset($path)) {
 			$path = stripslashes($path);
 			$userFolder = $this->rootFolder->getUserFolder($this->userId);
 			$node = $userFolder->get($path);
 			if (!($node instanceof File)) {
-				return new DataResponse(['data' => ['message' => $this->l->t('Please select a file.')]], Http::STATUS_OK, $headers);
+				return new JSONResponse(['data' => ['message' => $this->l->t('Please select a file.')]]);
 			}
 			if ($node->getSize() > 20*1024*1024) {
-				return new DataResponse(
+				return new JSONResponse(
 					['data' => ['message' => $this->l->t('File is too big')]],
-					Http::STATUS_BAD_REQUEST,
+					Http::STATUS_BAD_REQUEST
-					$headers
 				);
 			}
 			$content = $node->getContent();
@@ -181,28 +178,25 @@ class AvatarController extends Controller {
 				!\OC\Files\Filesystem::isFileBlacklisted($files['tmp_name'][0])
 			) {
 				if ($files['size'][0] > 20*1024*1024) {
-					return new DataResponse(
+					return new JSONResponse(
 						['data' => ['message' => $this->l->t('File is too big')]],
-						Http::STATUS_BAD_REQUEST,
+						Http::STATUS_BAD_REQUEST
-						$headers
 					);
 				}
 				$this->cache->set('avatar_upload', file_get_contents($files['tmp_name'][0]), 7200);
 				$content = $this->cache->get('avatar_upload');
 				unlink($files['tmp_name'][0]);
 			} else {
-				return new DataResponse(
+				return new JSONResponse(
 					['data' => ['message' => $this->l->t('Invalid file provided')]],
-					Http::STATUS_BAD_REQUEST,
+					Http::STATUS_BAD_REQUEST
-					$headers
 				);
 			}
 		} else {
 			//Add imgfile
-			return new DataResponse(
+			return new JSONResponse(
 				['data' => ['message' => $this->l->t('No image or file provided')]],
-				Http::STATUS_BAD_REQUEST,
+				Http::STATUS_BAD_REQUEST
-				$headers
 			);
 		}
 
@@ -214,57 +208,54 @@ class AvatarController extends Controller {
 			if ($image->valid()) {
 				$mimeType = $image->mimeType();
 				if ($mimeType !== 'image/jpeg' && $mimeType !== 'image/png') {
-					return new DataResponse(
+					return new JSONResponse(
 						['data' => ['message' => $this->l->t('Unknown filetype')]],
-						Http::STATUS_OK,
+						Http::STATUS_OK
-						$headers
 					);
 				}
 
 				$this->cache->set('tmpAvatar', $image->data(), 7200);
-				return new DataResponse(
+				return new JSONResponse(
 					['data' => 'notsquare'],
-					Http::STATUS_OK,
+					Http::STATUS_OK
-					$headers
 				);
 			} else {
-				return new DataResponse(
+				return new JSONResponse(
 					['data' => ['message' => $this->l->t('Invalid image')]],
-					Http::STATUS_OK,
+					Http::STATUS_OK
-					$headers
 				);
 			}
 		} catch (\Exception $e) {
 			$this->logger->logException($e, ['app' => 'core']);
-			return new DataResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_OK, $headers);
+			return new JSONResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_OK);
 		}
 	}
 
 	/**
 	 * @NoAdminRequired
      *
-	 * @return DataResponse
+	 * @return JSONResponse
 	 */
 	public function deleteAvatar() {
 		try {
 			$avatar = $this->avatarManager->getAvatar($this->userId);
 			$avatar->remove();
-			return new DataResponse();
+			return new JSONResponse();
 		} catch (\Exception $e) {
 			$this->logger->logException($e, ['app' => 'core']);
-			return new DataResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
+			return new JSONResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
 		}
 	}
 
 	/**
 	 * @NoAdminRequired
 	 *
-	 * @return DataResponse|DataDisplayResponse
+	 * @return JSONResponse|DataDisplayResponse
 	 */
 	public function getTmpAvatar() {
 		$tmpAvatar = $this->cache->get('tmpAvatar');
 		if (is_null($tmpAvatar)) {
-			return new DataResponse(['data' => [
+			return new JSONResponse(['data' => [
 										'message' => $this->l->t("No temporary profile picture available, try again")
 									]],
 									Http::STATUS_NOT_FOUND);
@@ -286,22 +277,22 @@ class AvatarController extends Controller {
 	 * @NoAdminRequired
 	 *
 	 * @param array $crop
-	 * @return DataResponse
+	 * @return JSONResponse
 	 */
 	public function postCroppedAvatar($crop) {
 		if (is_null($crop)) {
-			return new DataResponse(['data' => ['message' => $this->l->t("No crop data provided")]],
+			return new JSONResponse(['data' => ['message' => $this->l->t("No crop data provided")]],
 									Http::STATUS_BAD_REQUEST);
 		}
 
 		if (!isset($crop['x'], $crop['y'], $crop['w'], $crop['h'])) {
-			return new DataResponse(['data' => ['message' => $this->l->t("No valid crop data provided")]],
+			return new JSONResponse(['data' => ['message' => $this->l->t("No valid crop data provided")]],
 									Http::STATUS_BAD_REQUEST);
 		}
 
 		$tmpAvatar = $this->cache->get('tmpAvatar');
 		if (is_null($tmpAvatar)) {
-			return new DataResponse(['data' => [
+			return new JSONResponse(['data' => [
 										'message' => $this->l->t("No temporary profile picture available, try again")
 									]],
 									Http::STATUS_BAD_REQUEST);
@@ -314,13 +305,13 @@ class AvatarController extends Controller {
 			$avatar->set($image);
 			// Clean up
 			$this->cache->remove('tmpAvatar');
-			return new DataResponse(['status' => 'success']);
+			return new JSONResponse(['status' => 'success']);
 		} catch (\OC\NotSquareException $e) {
-			return new DataResponse(['data' => ['message' => $this->l->t('Crop is not square')]],
+			return new JSONResponse(['data' => ['message' => $this->l->t('Crop is not square')]],
 									Http::STATUS_BAD_REQUEST);
 		} catch (\Exception $e) {
 			$this->logger->logException($e, ['app' => 'core']);
-			return new DataResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
+			return new JSONResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
 		}
 	}
 }

core/Controller/LoginController.php

@@ -25,7 +25,6 @@
 
 namespace OC\Core\Controller;
 
-use OC\AppFramework\Utility\TimeFactory;
 use OC\Authentication\TwoFactorAuth\Manager;
 use OC\Security\Bruteforce\Throttler;
 use OC\User\Session;

core/Controller/LostController.php

@@ -40,7 +40,6 @@ use \OCP\IConfig;
 use OCP\IUserManager;
 use OCP\Mail\IMailer;
 use OCP\Security\ISecureRandom;
-use OCP\Security\StringUtils;
 
 /**
  * Class LostController
@@ -144,7 +143,7 @@ class LostController extends Controller {
 	}
 
 	/**
-	 * @param string $userId
+	 * @param string $token
 	 * @param string $userId
 	 * @throws \Exception
 	 */
@@ -161,7 +160,7 @@ class LostController extends Controller {
 			throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is expired'));
 		}
 
-		if (!StringUtils::equals($splittedToken[1], $token)) {
+		if (!hash_equals($splittedToken[1], $token)) {
 			throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid'));
 		}
 	}

core/Controller/TokenController.php

@@ -24,13 +24,10 @@
 namespace OC\Core\Controller;
 
 use OC\AppFramework\Http;
-use OC\AppFramework\Utility\TimeFactory;
-use OC\Authentication\Token\DefaultTokenProvider;
 use OC\Authentication\Token\IProvider;
 use OC\Authentication\Token\IToken;
 use OC\Authentication\TwoFactorAuth\Manager as TwoFactorAuthManager;
 use OC\User\Manager as UserManager;
-use OCA\User_LDAP\User\Manager;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IRequest;
@@ -100,9 +97,9 @@ class TokenController extends Controller {
 
 		$token = $this->secureRandom->generate(128);
 		$this->tokenProvider->generateToken($token, $user->getUID(), $loginName, $password, $name, IToken::PERMANENT_TOKEN);
-		return [
+		return new JSONResponse([
 			'token' => $token,
-		];
+		]);
 	}
 
 }

core/Controller/TwoFactorChallengeController.php

@@ -96,7 +96,7 @@ class TwoFactorChallengeController extends Controller {
 	 *
 	 * @param string $challengeProviderId
 	 * @param string $redirect_url
-	 * @return TemplateResponse
+	 * @return TemplateResponse|RedirectResponse
 	 */
 	public function showChallenge($challengeProviderId, $redirect_url) {
 		$user = $this->userSession->getUser();

core/Controller/UserController.php

@@ -26,26 +26,20 @@ namespace OC\Core\Controller;
 use \OCP\AppFramework\Controller;
 use \OCP\AppFramework\Http\JSONResponse;
 use \OCP\IRequest;
+use \OCP\IUserManager;
 
 class UserController extends Controller {
 	/**
-	 * @var \OCP\IUserManager
+	 * @var IUserManager
 	 */
 	protected $userManager;
 
-	/**
-	 * @var \OC_Defaults
-	 */
-	protected $defaults;
-
 	public function __construct($appName,
 								IRequest $request,
-								$userManager,
+								IUserManager $userManager
-								$defaults
 	) {
 		parent::__construct($appName, $request);
 		$this->userManager = $userManager;
-		$this->defaults = $defaults;
 	}
 
 	/**

tests/Core/Controller/AvatarControllerTest.php

@@ -228,7 +228,7 @@ class AvatarControllerTest extends \Test\TestCase {
 		$this->logger->expects($this->once())
 			->method('logException')
 			->with(new \Exception("foo"));
-		$expectedResponse = new Http\DataResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST);
+		$expectedResponse = new Http\JSONResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST);
 		$this->assertEquals($expectedResponse, $this->avatarController->deleteAvatar());
 	}
 
@@ -377,7 +377,7 @@ class AvatarControllerTest extends \Test\TestCase {
 		$this->logger->expects($this->once())
 			->method('logException')
 			->with(new \Exception("foo"));
-		$expectedResponse = new Http\DataResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_OK);
+		$expectedResponse = new Http\JSONResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_OK);
 		$this->assertEquals($expectedResponse, $this->avatarController->postAvatar('avatar.jpg'));
 	}
 
@@ -437,7 +437,7 @@ class AvatarControllerTest extends \Test\TestCase {
 		$this->logger->expects($this->once())
 			->method('logException')
 			->with(new \Exception('foo'));
-		$expectedResponse = new Http\DataResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST);
+		$expectedResponse = new Http\JSONResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST);
 		$this->assertEquals($expectedResponse, $this->avatarController->postCroppedAvatar(['x' => 0, 'y' => 0, 'w' => 10, 'h' => 11]));
 	}
 

tests/Core/Controller/TokenControllerTest.php

@@ -41,15 +41,17 @@ class TokenControllerTest extends TestCase {
 	protected function setUp() {
 		parent::setUp();
 
-		$this->request = $this->getMock('\OCP\IRequest');
+		$this->request = $this->getMockBuilder('\OCP\IRequest')->getMock();
 		$this->userManager = $this->getMockBuilder('\OC\User\Manager')
 			->disableOriginalConstructor()
 			->getMock();
-		$this->tokenProvider = $this->getMock('\OC\Authentication\Token\IProvider');
+		$this->tokenProvider = $this->getMockBuilder('\OC\Authentication\Token\IProvider')
+			->getMock();
 		$this->twoFactorAuthManager = $this->getMockBuilder('\OC\Authentication\TwoFactorAuth\Manager')
 			->disableOriginalConstructor()
 			->getMock();
-		$this->secureRandom = $this->getMock('\OCP\Security\ISecureRandom');
+		$this->secureRandom = $this->getMockBuilder('\OCP\Security\ISecureRandom')
+			->getMock();
 
 		$this->tokenController = new TokenController('core', $this->request, $this->userManager, $this->tokenProvider, $this->twoFactorAuthManager, $this->secureRandom);
 	}
@@ -77,7 +79,7 @@ class TokenControllerTest extends TestCase {
 	}
 
 	public function testWithValidCredentials() {
-		$user = $this->getMock('\OCP\IUser');
+		$user = $this->getMockBuilder('\OCP\IUser')->getMock();
 		$this->userManager->expects($this->once())
 			->method('checkPassword')
 			->with('john', '123456')
@@ -96,9 +98,9 @@ class TokenControllerTest extends TestCase {
 		$this->tokenProvider->expects($this->once())
 			->method('generateToken')
 			->with('verysecurerandomtoken', 'john', 'john', '123456', 'unknown client', IToken::PERMANENT_TOKEN);
-		$expected = [
+		$expected = new JSONResponse([
 			'token' => 'verysecurerandomtoken'
-		];
+		]);
 
 		$actual = $this->tokenController->generateToken('john', '123456');
 
@@ -106,7 +108,7 @@ class TokenControllerTest extends TestCase {
 	}
 
 	public function testWithValidCredentialsBut2faEnabled() {
-		$user = $this->getMock('\OCP\IUser');
+		$user = $this->getMockBuilder('\OCP\IUser')->getMock();
 		$this->userManager->expects($this->once())
 			->method('checkPassword')
 			->with('john', '123456')