Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core controller cleanup #1172

Merged
merged 3 commits into from
Aug 30, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions core/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,6 @@ public function __construct(array $urlParams=array()){
$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'),
Expand Down
75 changes: 33 additions & 42 deletions core/Controller/AvatarController.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@

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;
Expand Down Expand Up @@ -111,7 +111,7 @@ public function __construct($appName,
*
* @param string $userId
* @param int $size
* @return DataResponse|DataDisplayResponse
* @return JSONResponse|DataDisplayResponse
*/
public function getAvatar($userId, $size) {
if ($size > 2048) {
Expand All @@ -128,13 +128,13 @@ public function getAvatar($userId, $size) {
$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' => '',
],
Expand All @@ -152,25 +152,22 @@ public function getAvatar($userId, $size) {
* @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,
$headers
Http::STATUS_BAD_REQUEST
);
}
$content = $node->getContent();
Expand All @@ -181,28 +178,25 @@ public function postAvatar($path) {
!\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,
$headers
Http::STATUS_BAD_REQUEST
);
}
$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,
$headers
Http::STATUS_BAD_REQUEST
);
}
} else {
//Add imgfile
return new DataResponse(
return new JSONResponse(
['data' => ['message' => $this->l->t('No image or file provided')]],
Http::STATUS_BAD_REQUEST,
$headers
Http::STATUS_BAD_REQUEST
);
}

Expand All @@ -214,57 +208,54 @@ public function postAvatar($path) {
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,
$headers
Http::STATUS_OK
);
}

$this->cache->set('tmpAvatar', $image->data(), 7200);
return new DataResponse(
return new JSONResponse(
['data' => 'notsquare'],
Http::STATUS_OK,
$headers
Http::STATUS_OK
);
} else {
return new DataResponse(
return new JSONResponse(
['data' => ['message' => $this->l->t('Invalid image')]],
Http::STATUS_OK,
$headers
Http::STATUS_OK
);
}
} 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);
Expand All @@ -286,22 +277,22 @@ public function getTmpAvatar() {
* @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);
Expand All @@ -314,13 +305,13 @@ public function postCroppedAvatar($crop) {
$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);
}
}
}
1 change: 0 additions & 1 deletion core/Controller/LoginController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions core/Controller/LostController.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
use OCP\IUserManager;
use OCP\Mail\IMailer;
use OCP\Security\ISecureRandom;
use OCP\Security\StringUtils;

/**
* Class LostController
Expand Down Expand Up @@ -144,7 +143,7 @@ public function resetform($token, $userId) {
}

/**
* @param string $userId
* @param string $token
* @param string $userId
* @throws \Exception
*/
Expand All @@ -161,7 +160,7 @@ private function checkPasswordResetToken($token, $userId) {
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'));
}
}
Expand Down
7 changes: 2 additions & 5 deletions core/Controller/TokenController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -100,9 +97,9 @@ public function generateToken($user, $password, $name = 'unknown client') {

$token = $this->secureRandom->generate(128);
$this->tokenProvider->generateToken($token, $user->getUID(), $loginName, $password, $name, IToken::PERMANENT_TOKEN);
return [
return new JSONResponse([
'token' => $token,
];
]);
}

}
2 changes: 1 addition & 1 deletion core/Controller/TwoFactorChallengeController.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public function selectChallenge($redirect_url) {
*
* @param string $challengeProviderId
* @param string $redirect_url
* @return TemplateResponse
* @return TemplateResponse|RedirectResponse
*/
public function showChallenge($challengeProviderId, $redirect_url) {
$user = $this->userSession->getUser();
Expand Down
12 changes: 3 additions & 9 deletions core/Controller/UserController.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,20 @@
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,
$defaults
IUserManager $userManager
) {
parent::__construct($appName, $request);
$this->userManager = $userManager;
$this->defaults = $defaults;
}

/**
Expand Down
6 changes: 3 additions & 3 deletions tests/Core/Controller/AvatarControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public function testDeleteAvatarException() {
$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());
}

Expand Down Expand Up @@ -377,7 +377,7 @@ public function testPostAvatarException() {
$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'));
}

Expand Down Expand Up @@ -437,7 +437,7 @@ public function testPostCroppedAvatarException() {
$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]));
}

Expand Down
Loading