mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #8743 from nextcloud/strict_settings 

Strict settings
This commit is contained in:
Roeland Jago Douma 2018-03-12 12:37:03 +01:00 committed by GitHub
parent 430c478ba3 7fc85bbf78
commit af89db3407
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 56 additions and 114 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
settings/Controller/ChangePasswordController.php
View File

@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
/**
*
*
@ -26,12 +27,12 @@
*/
namespace OC\Settings\Controller;
use OC\Group\Manager as GroupManager;
use OC\HintException;
use OC\User\Session;
use OCP\App\IAppManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\JSONResponse;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\IRequest;
use OCP\IUser;
@ -49,7 +50,7 @@ class ChangePasswordController extends Controller {
/** @var IL10N */
private $l;
/** @var IGroupManager */
/** @var GroupManager */
private $groupManager;
/** @var Session */
@ -58,24 +59,12 @@ class ChangePasswordController extends Controller {
/** @var IAppManager */
private $appManager;
/**
* ChangePasswordController constructor.
*
* @param string $appName
* @param IRequest $request
* @param $userId
* @param IUserManager $userManager
* @param IUserSession $userSession
* @param IGroupManager $groupManager
* @param IAppManager $appManager
* @param IL10N $l
*/
public function __construct($appName,
public function __construct(string $appName,
IRequest $request,
$userId,
string $userId,
IUserManager $userManager,
IUserSession $userSession,
IGroupManager $groupManager,
GroupManager $groupManager,
IAppManager $appManager,
IL10N $l) {
parent::__construct($appName, $request);
@ -98,7 +87,7 @@ class ChangePasswordController extends Controller {
*
* @return JSONResponse
*/
public function changePersonalPassword($oldpassword = '', $newpassword = null) {
public function changePersonalPassword(string $oldpassword = '', string $newpassword = null): JSONResponse {
/** @var IUser $user */
$user = $this->userManager->checkPassword($this->userId, $oldpassword);
if ($user === false) {
@ -148,7 +137,7 @@ class ChangePasswordController extends Controller {
*
* @return JSONResponse
*/
public function changeUserPassword($username = null, $password = null, $recoveryPassword = null) {
public function changeUserPassword(string $username = null, string $password = null, string $recoveryPassword = null): JSONResponse {
if ($username === null) {
return new JSONResponse([
'status' => 'error',

120
settings/Controller/UsersController.php
View File

@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
@ -39,6 +40,7 @@ namespace OC\Settings\Controller;
use OC\Accounts\AccountManager;
use OC\AppFramework\Http;
use OC\ForbiddenException;
use OC\Group\Manager as GroupManager;
use OC\HintException;
use OC\Settings\Mailer\NewUserMailHelper;
use OC\Security\IdentityProof\Manager;
@ -50,7 +52,6 @@ use OCP\Files\Config\IUserMountCache;
use OCP\Encryption\IEncryptionModule;
use OCP\Encryption\IManager;
use OCP\IConfig;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\ILogger;
use OCP\IRequest;
@ -76,7 +77,7 @@ class UsersController extends Controller {
private $isAdmin;
/** @var IUserManager */
private $userManager;
/** @var IGroupManager */
/** @var GroupManager */
private $groupManager;
/** @var IConfig */
private $config;
@ -109,36 +110,13 @@ class UsersController extends Controller {
/** @var IManager */
private $encryptionManager;
/**
* @param string $appName
* @param IRequest $request
* @param IUserManager $userManager
* @param IGroupManager $groupManager
* @param IUserSession $userSession
* @param IConfig $config
* @param bool $isAdmin
* @param IL10N $l10n
* @param ILogger $log
* @param IMailer $mailer
* @param IURLGenerator $urlGenerator
* @param IAppManager $appManager
* @param IAvatarManager $avatarManager
* @param AccountManager $accountManager
* @param ISecureRandom $secureRandom
* @param NewUserMailHelper $newUserMailHelper
* @param Manager $keyManager
* @param IJobList $jobList
* @param IUserMountCache $userMountCache
* @param IManager $encryptionManager
*/
public function __construct($appName,
public function __construct(string $appName,
IRequest $request,
IUserManager $userManager,
IGroupManager $groupManager,
GroupManager $groupManager,
IUserSession $userSession,
IConfig $config,
$isAdmin,
bool $isAdmin,
IL10N $l10n,
ILogger $log,
IMailer $mailer,
@ -185,7 +163,7 @@ class UsersController extends Controller {
* @param array|null $userGroups
* @return array
*/
private function formatUserForIndex(IUser $user, array $userGroups = null) {
private function formatUserForIndex(IUser $user, array $userGroups = null): array {
// TODO: eliminate this encryption specific code below and somehow
// hook in additional user info from other apps
@ -261,7 +239,7 @@ class UsersController extends Controller {
* @param array $userIDs Array with schema [$uid => $displayName]
* @return IUser[]
*/
private function getUsersForUID(array $userIDs) {
private function getUsersForUID(array $userIDs): array {
$users = [];
foreach ($userIDs as $uid => $displayName) {
$users[$uid] = $this->userManager->get($uid);
@ -281,7 +259,7 @@ class UsersController extends Controller {
*
* TODO: Tidy up and write unit tests - code is mainly static method calls
*/
public function index($offset = 0, $limit = 10, $gid = '', $pattern = '', $backend = '') {
public function index(int $offset = 0, int $limit = 10, string $gid = '', string $pattern = '', string $backend = ''): DataResponse {
// Remove backends
if (!empty($backend)) {
$activeBackends = $this->userManager->getBackends();
@ -375,11 +353,11 @@ class UsersController extends Controller {
* @param string $email
* @return DataResponse
*/
public function create($username, $password, array $groups = [], $email = '') {
public function create(string $username, string $password, array $groups = [], $email = ''): DataResponse {
if ($email !== '' && !$this->mailer->validateMailAddress($email)) {
return new DataResponse(
[
'message' => (string)$this->l10n->t('Invalid mail address')
'message' => $this->l10n->t('Invalid mail address')
],
Http::STATUS_UNPROCESSABLE_ENTITY
);
@ -396,7 +374,7 @@ class UsersController extends Controller {
continue;
}
if (!$this->groupManager->getSubAdmin()->isSubAdminofGroup($currentUser, $groupObject)) {
if (!$this->groupManager->getSubAdmin()->isSubAdminOfGroup($currentUser, $groupObject)) {
unset($groups[$key]);
}
}
@ -415,7 +393,7 @@ class UsersController extends Controller {
if ($this->userManager->userExists($username)) {
return new DataResponse(
[
'message' => (string)$this->l10n->t('A user with that name already exists.')
'message' => $this->l10n->t('A user with that name already exists.')
],
Http::STATUS_CONFLICT
);
@ -426,7 +404,7 @@ class UsersController extends Controller {
if ($email === '') {
return new DataResponse(
[
'message' => (string)$this->l10n->t('To send a password link to the user an email address is required.')
'message' => $this->l10n->t('To send a password link to the user an email address is required.')
],
Http::STATUS_UNPROCESSABLE_ENTITY
);
@ -494,7 +472,7 @@ class UsersController extends Controller {
return new DataResponse(
[
'message' => (string)$this->l10n->t('Unable to create user.')
'message' => $this->l10n->t('Unable to create user.')
],
Http::STATUS_FORBIDDEN
);
@ -508,7 +486,7 @@ class UsersController extends Controller {
* @param string $id
* @return DataResponse
*/
public function destroy($id) {
public function destroy(string $id): DataResponse {
$userId = $this->userSession->getUser()->getUID();
$user = $this->userManager->get($id);
@ -517,7 +495,7 @@ class UsersController extends Controller {
[
'status' => 'error',
'data' => [
'message' => (string)$this->l10n->t('Unable to delete user.')
'message' => $this->l10n->t('Unable to delete user.')
]
],
Http::STATUS_FORBIDDEN
@ -529,32 +507,30 @@ class UsersController extends Controller {
[
'status' => 'error',
'data' => [
'message' => (string)$this->l10n->t('Authentication error')
'message' => $this->l10n->t('Authentication error')
]
],
Http::STATUS_FORBIDDEN
);
}
if ($user) {
if ($user->delete()) {
return new DataResponse(
[
'status' => 'success',
'data' => [
'username' => $id
]
],
Http::STATUS_NO_CONTENT
);
}
if ($user && $user->delete()) {
return new DataResponse(
[
'status' => 'success',
'data' => [
'username' => $id
]
],
Http::STATUS_NO_CONTENT
);
}
return new DataResponse(
[
'status' => 'error',
'data' => [
'message' => (string)$this->l10n->t('Unable to delete user.')
'message' => $this->l10n->t('Unable to delete user.')
]
],
Http::STATUS_FORBIDDEN
@ -568,12 +544,12 @@ class UsersController extends Controller {
* @param int $enabled
* @return DataResponse
*/
public function setEnabled($id, $enabled) {
public function setEnabled(string $id, int $enabled): DataResponse {
$enabled = (bool)$enabled;
if ($enabled) {
$errorMsgGeneral = (string)$this->l10n->t('Error while enabling user.');
$errorMsgGeneral = $this->l10n->t('Error while enabling user.');
} else {
$errorMsgGeneral = (string)$this->l10n->t('Error while disabling user.');
$errorMsgGeneral = $this->l10n->t('Error while disabling user.');
}
$userId = $this->userSession->getUser()->getUID();
@ -596,7 +572,7 @@ class UsersController extends Controller {
[
'status' => 'error',
'data' => [
'message' => (string)$this->l10n->t('Authentication error')
'message' => $this->l10n->t('Authentication error')
]
],
Http::STATUS_FORBIDDEN
@ -638,7 +614,7 @@ class UsersController extends Controller {
* @param bool $onlyVerificationCode only return verification code without updating the data
* @return DataResponse
*/
public function getVerificationCode($account, $onlyVerificationCode) {
public function getVerificationCode(string $account, bool $onlyVerificationCode): DataResponse {
$user = $this->userSession->getUser();
@ -648,7 +624,7 @@ class UsersController extends Controller {
$accountData = $this->accountManager->getUser($user);
$cloudId = $user->getCloudId();
$message = "Use my Federated Cloud ID to share with me: " . $cloudId;
$message = 'Use my Federated Cloud ID to share with me: ' . $cloudId;
$signature = $this->signMessage($user, $message);
$code = $message . ' ' . $signature;
@ -697,7 +673,7 @@ class UsersController extends Controller {
*
* @return int
*/
protected function getCurrentTime() {
protected function getCurrentTime(): int {
return time();
}
@ -709,7 +685,7 @@ class UsersController extends Controller {
*
* @return string base64 encoded signature
*/
protected function signMessage(IUser $user, $message) {
protected function signMessage(IUser $user, string $message): string {
$privateKey = $this->keyManager->getKey($user)->getPrivate();
openssl_sign(json_encode($message), $signature, $privateKey, OPENSSL_ALGO_SHA512);
return base64_encode($signature);
@ -755,7 +731,7 @@ class UsersController extends Controller {
[
'status' => 'error',
'data' => [
'message' => (string)$this->l10n->t('Invalid mail address')
'message' => $this->l10n->t('Invalid mail address')
]
],
Http::STATUS_UNPROCESSABLE_ENTITY
@ -799,7 +775,7 @@ class UsersController extends Controller {
'websiteScope' => $data[AccountManager::PROPERTY_WEBSITE]['scope'],
'address' => $data[AccountManager::PROPERTY_ADDRESS]['value'],
'addressScope' => $data[AccountManager::PROPERTY_ADDRESS]['scope'],
'message' => (string)$this->l10n->t('Settings saved')
'message' => $this->l10n->t('Settings saved')
]
],
Http::STATUS_OK
@ -823,7 +799,7 @@ class UsersController extends Controller {
* @param array $data
* @throws ForbiddenException
*/
protected function saveUserSettings(IUser $user, $data) {
protected function saveUserSettings(IUser $user, array $data) {
// keep the user back-end up-to-date with the latest display name and email
// address
@ -861,7 +837,7 @@ class UsersController extends Controller {
*
* @return DataResponse
*/
public function stats() {
public function stats(): DataResponse {
$userCount = 0;
if ($this->isAdmin) {
$countByBackend = $this->userManager->countUsers();
@ -904,7 +880,7 @@ class UsersController extends Controller {
* @param string $displayName
* @return DataResponse
*/
public function setDisplayName($username, $displayName) {
public function setDisplayName(string $username, string $displayName) {
$currentUser = $this->userSession->getUser();
$user = $this->userManager->get($username);
@ -961,7 +937,7 @@ class UsersController extends Controller {
* @param string $mailAddress
* @return DataResponse
*/
public function setEMailAddress($id, $mailAddress) {
public function setEMailAddress(string $id, string $mailAddress) {
$user = $this->userManager->get($id);
if (!$this->isAdmin
&& !$this->groupManager->getSubAdmin()->isUserAccessible($this->userSession->getUser(), $user)
@ -970,7 +946,7 @@ class UsersController extends Controller {
[
'status' => 'error',
'data' => [
'message' => (string)$this->l10n->t('Forbidden')
'message' => $this->l10n->t('Forbidden')
]
],
Http::STATUS_FORBIDDEN
@ -982,7 +958,7 @@ class UsersController extends Controller {
[
'status' => 'error',
'data' => [
'message' => (string)$this->l10n->t('Invalid mail address')
'message' => $this->l10n->t('Invalid mail address')
]
],
Http::STATUS_UNPROCESSABLE_ENTITY
@ -994,7 +970,7 @@ class UsersController extends Controller {
[
'status' => 'error',
'data' => [
'message' => (string)$this->l10n->t('Invalid user')
'message' => $this->l10n->t('Invalid user')
]
],
Http::STATUS_UNPROCESSABLE_ENTITY
@ -1007,7 +983,7 @@ class UsersController extends Controller {
[
'status' => 'error',
'data' => [
'message' => (string)$this->l10n->t('Unable to change mail address')
'message' => $this->l10n->t('Unable to change mail address')
]
],
Http::STATUS_FORBIDDEN
@ -1025,7 +1001,7 @@ class UsersController extends Controller {
'data' => [
'username' => $id,
'mailAddress' => $mailAddress,
'message' => (string)$this->l10n->t('Email saved')
'message' => $this->l10n->t('Email saved')
]
],
Http::STATUS_OK

23
tests/Settings/Controller/UsersControllerTest.php
View File

@ -1878,29 +1878,6 @@ class UsersControllerTest extends \Test\TestCase {
$this->assertEquals($expectedResponse, $response);
}
public function testSetDisplayNameNull() {
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('userName');
$this->userSession
->expects($this->once())
->method('getUser')
->willReturn($user);
$expectedResponse = new DataResponse(
[
'status' => 'error',
'data' => [
'message' => 'Authentication error',
],
]
);
$controller = $this->getController(true);
$response = $controller->setDisplayName(null, 'displayName');
$this->assertEquals($expectedResponse, $response);
}
public function dataSetDisplayName() {
$data = [];