Use placeholder values for password fields in external storage webui

This prevents the password from being sent to the webui.

While an admin will always be able to retrieve the passwords (as they
can do arbitrairy code execution by design) this prevents casual
password snooping

Signed-off-by: Robin Appelman <robin@icewind.nl>
This commit is contained in:
Robin Appelman 2020-03-12 19:45:23 +01:00
parent 24d0fb9fcd
commit 0d112d7901
No known key found for this signature in database
GPG Key ID: 42B69D8A64526EFB
8 changed files with 65 additions and 37 deletions

View File

@ -113,7 +113,7 @@ class GlobalStoragesController extends StoragesController {
$this->updateStorageStatus($newStorage); $this->updateStorageStatus($newStorage);
return new DataResponse( return new DataResponse(
$newStorage, $this->formatStorageForUI($newStorage),
Http::STATUS_CREATED Http::STATUS_CREATED
); );
} }
@ -180,7 +180,7 @@ class GlobalStoragesController extends StoragesController {
$this->updateStorageStatus($storage, $testOnly); $this->updateStorageStatus($storage, $testOnly);
return new DataResponse( return new DataResponse(
$storage, $this->formatStorageForUI($storage),
Http::STATUS_OK Http::STATUS_OK
); );

View File

@ -31,6 +31,7 @@ namespace OCA\Files_External\Controller;
use OCA\Files_External\Lib\Auth\AuthMechanism; use OCA\Files_External\Lib\Auth\AuthMechanism;
use OCA\Files_External\Lib\Backend\Backend; use OCA\Files_External\Lib\Backend\Backend;
use OCA\Files_External\Lib\DefinitionParameter;
use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException;
use OCA\Files_External\Lib\StorageConfig; use OCA\Files_External\Lib\StorageConfig;
use OCA\Files_External\NotFoundException; use OCA\Files_External\NotFoundException;
@ -146,9 +147,9 @@ abstract class StoragesController extends Controller {
$mountPoint = $storage->getMountPoint(); $mountPoint = $storage->getMountPoint();
if ($mountPoint === '') { if ($mountPoint === '') {
return new DataResponse( return new DataResponse(
array( [
'message' => (string)$this->l10n->t('Invalid mount point') 'message' => (string)$this->l10n->t('Invalid mount point'),
), ],
Http::STATUS_UNPROCESSABLE_ENTITY Http::STATUS_UNPROCESSABLE_ENTITY
); );
} }
@ -156,9 +157,9 @@ abstract class StoragesController extends Controller {
if ($storage->getBackendOption('objectstore')) { if ($storage->getBackendOption('objectstore')) {
// objectstore must not be sent from client side // objectstore must not be sent from client side
return new DataResponse( return new DataResponse(
array( [
'message' => (string)$this->l10n->t('Objectstore forbidden') 'message' => (string)$this->l10n->t('Objectstore forbidden'),
), ],
Http::STATUS_UNPROCESSABLE_ENTITY Http::STATUS_UNPROCESSABLE_ENTITY
); );
} }
@ -170,11 +171,11 @@ abstract class StoragesController extends Controller {
if ($backend->checkDependencies()) { if ($backend->checkDependencies()) {
// invalid backend // invalid backend
return new DataResponse( return new DataResponse(
array( [
'message' => (string)$this->l10n->t('Invalid storage backend "%s"', [ 'message' => (string)$this->l10n->t('Invalid storage backend "%s"', [
$backend->getIdentifier() $backend->getIdentifier(),
]) ]),
), ],
Http::STATUS_UNPROCESSABLE_ENTITY Http::STATUS_UNPROCESSABLE_ENTITY
); );
} }
@ -182,22 +183,22 @@ abstract class StoragesController extends Controller {
if (!$backend->isVisibleFor($this->service->getVisibilityType())) { if (!$backend->isVisibleFor($this->service->getVisibilityType())) {
// not permitted to use backend // not permitted to use backend
return new DataResponse( return new DataResponse(
array( [
'message' => (string)$this->l10n->t('Not permitted to use backend "%s"', [ 'message' => (string)$this->l10n->t('Not permitted to use backend "%s"', [
$backend->getIdentifier() $backend->getIdentifier(),
]) ]),
), ],
Http::STATUS_UNPROCESSABLE_ENTITY Http::STATUS_UNPROCESSABLE_ENTITY
); );
} }
if (!$authMechanism->isVisibleFor($this->service->getVisibilityType())) { if (!$authMechanism->isVisibleFor($this->service->getVisibilityType())) {
// not permitted to use auth mechanism // not permitted to use auth mechanism
return new DataResponse( return new DataResponse(
array( [
'message' => (string)$this->l10n->t('Not permitted to use authentication mechanism "%s"', [ 'message' => (string)$this->l10n->t('Not permitted to use authentication mechanism "%s"', [
$authMechanism->getIdentifier() $authMechanism->getIdentifier(),
]) ]),
), ],
Http::STATUS_UNPROCESSABLE_ENTITY Http::STATUS_UNPROCESSABLE_ENTITY
); );
} }
@ -205,9 +206,9 @@ abstract class StoragesController extends Controller {
if (!$backend->validateStorage($storage)) { if (!$backend->validateStorage($storage)) {
// unsatisfied parameters // unsatisfied parameters
return new DataResponse( return new DataResponse(
array( [
'message' => (string)$this->l10n->t('Unsatisfied backend parameters') 'message' => (string)$this->l10n->t('Unsatisfied backend parameters'),
), ],
Http::STATUS_UNPROCESSABLE_ENTITY Http::STATUS_UNPROCESSABLE_ENTITY
); );
} }
@ -215,7 +216,7 @@ abstract class StoragesController extends Controller {
// unsatisfied parameters // unsatisfied parameters
return new DataResponse( return new DataResponse(
[ [
'message' => (string)$this->l10n->t('Unsatisfied authentication mechanism parameters') 'message' => (string)$this->l10n->t('Unsatisfied authentication mechanism parameters'),
], ],
Http::STATUS_UNPROCESSABLE_ENTITY Http::STATUS_UNPROCESSABLE_ENTITY
); );
@ -272,7 +273,7 @@ abstract class StoragesController extends Controller {
// FIXME: convert storage exceptions to StorageNotAvailableException // FIXME: convert storage exceptions to StorageNotAvailableException
$storage->setStatus( $storage->setStatus(
StorageNotAvailableException::STATUS_ERROR, StorageNotAvailableException::STATUS_ERROR,
get_class($e).': '.$e->getMessage() get_class($e) . ': ' . $e->getMessage()
); );
} }
} }
@ -283,7 +284,7 @@ abstract class StoragesController extends Controller {
* @return DataResponse * @return DataResponse
*/ */
public function index() { public function index() {
$storages = $this->service->getStorages(); $storages = $this->formatStoragesForUI($this->service->getStorages());
return new DataResponse( return new DataResponse(
$storages, $storages,
@ -291,6 +292,29 @@ abstract class StoragesController extends Controller {
); );
} }
protected function formatStoragesForUI(array $storages): array {
return array_map(function ($storage) {
return $this->formatStorageForUI($storage);
}, $storages);
}
protected function formatStorageForUI(StorageConfig $storage): StorageConfig {
/** @var DefinitionParameter[] $parameters */
$parameters = array_merge($storage->getBackend()->getParameters(), $storage->getAuthMechanism()->getParameters());
$options = $storage->getBackendOptions();
foreach ($options as $key => $value) {
foreach ($parameters as $parameter) {
if ($parameter->getName() === $key && $parameter->getType() === DefinitionParameter::VALUE_PASSWORD) {
$storage->setBackendOption($key, DefinitionParameter::UNMODIFIED_PLACEHOLDER);
break;
}
}
}
return $storage;
}
/** /**
* Get an external storage entry. * Get an external storage entry.
* *
@ -307,14 +331,14 @@ abstract class StoragesController extends Controller {
} catch (NotFoundException $e) { } catch (NotFoundException $e) {
return new DataResponse( return new DataResponse(
[ [
'message' => (string)$this->l10n->t('Storage with ID "%d" not found', array($id)) 'message' => (string)$this->l10n->t('Storage with ID "%d" not found', [$id]),
], ],
Http::STATUS_NOT_FOUND Http::STATUS_NOT_FOUND
); );
} }
return new DataResponse( return new DataResponse(
$storage, $this->formatStorageForUI($storage),
Http::STATUS_OK Http::STATUS_OK
); );
} }
@ -332,7 +356,7 @@ abstract class StoragesController extends Controller {
} catch (NotFoundException $e) { } catch (NotFoundException $e) {
return new DataResponse( return new DataResponse(
[ [
'message' => (string)$this->l10n->t('Storage with ID "%d" not found', array($id)) 'message' => (string)$this->l10n->t('Storage with ID "%d" not found', [$id]),
], ],
Http::STATUS_NOT_FOUND Http::STATUS_NOT_FOUND
); );

View File

@ -85,7 +85,7 @@ class UserGlobalStoragesController extends StoragesController {
* @NoAdminRequired * @NoAdminRequired
*/ */
public function index() { public function index() {
$storages = $this->service->getUniqueStorages(); $storages = $this->formatStoragesForUI($this->service->getUniqueStorages());
// remove configuration data, this must be kept private // remove configuration data, this must be kept private
foreach ($storages as $storage) { foreach ($storages as $storage) {
@ -133,7 +133,7 @@ class UserGlobalStoragesController extends StoragesController {
$this->sanitizeStorage($storage); $this->sanitizeStorage($storage);
return new DataResponse( return new DataResponse(
$storage, $this->formatStorageForUI($storage),
Http::STATUS_OK Http::STATUS_OK
); );
} }
@ -182,7 +182,7 @@ class UserGlobalStoragesController extends StoragesController {
$this->sanitizeStorage($storage); $this->sanitizeStorage($storage);
return new DataResponse( return new DataResponse(
$storage, $this->formatStorageForUI($storage),
Http::STATUS_OK Http::STATUS_OK
); );

View File

@ -148,7 +148,7 @@ class UserStoragesController extends StoragesController {
$this->updateStorageStatus($newStorage); $this->updateStorageStatus($newStorage);
return new DataResponse( return new DataResponse(
$newStorage, $this->formatStorageForUI($newStorage),
Http::STATUS_CREATED Http::STATUS_CREATED
); );
} }
@ -208,7 +208,7 @@ class UserStoragesController extends StoragesController {
$this->updateStorageStatus($storage, $testOnly); $this->updateStorageStatus($storage, $testOnly);
return new DataResponse( return new DataResponse(
$storage, $this->formatStorageForUI($storage),
Http::STATUS_OK Http::STATUS_OK
); );

View File

@ -51,7 +51,6 @@ use OCA\Files_External\Lib\VisibilityTrait;
* Object can affect storage mounting * Object can affect storage mounting
*/ */
class AuthMechanism implements \JsonSerializable { class AuthMechanism implements \JsonSerializable {
/** Standard authentication schemes */ /** Standard authentication schemes */
const SCHEME_NULL = 'null'; const SCHEME_NULL = 'null';
const SCHEME_BUILTIN = 'builtin'; const SCHEME_BUILTIN = 'builtin';

View File

@ -27,6 +27,9 @@ namespace OCA\Files_External\Lib;
* Parameter for an external storage definition * Parameter for an external storage definition
*/ */
class DefinitionParameter implements \JsonSerializable { class DefinitionParameter implements \JsonSerializable {
// placeholder value for password fields, when the client updates a storage configuration
// placeholder values are ignored and the field is left unmodified
const UNMODIFIED_PLACEHOLDER = '__unmodified__';
/** Value constants */ /** Value constants */
const VALUE_TEXT = 0; const VALUE_TEXT = 0;

View File

@ -154,5 +154,4 @@ trait FrontendDefinitionTrait {
} }
return true; return true;
} }
} }

View File

@ -36,6 +36,7 @@ use OCA\Files_External\Lib\Auth\AuthMechanism;
use OCA\Files_External\Lib\Auth\InvalidAuth; use OCA\Files_External\Lib\Auth\InvalidAuth;
use OCA\Files_External\Lib\Backend\Backend; use OCA\Files_External\Lib\Backend\Backend;
use OCA\Files_External\Lib\Backend\InvalidBackend; use OCA\Files_External\Lib\Backend\InvalidBackend;
use OCA\Files_External\Lib\DefinitionParameter;
use OCA\Files_External\Lib\StorageConfig; use OCA\Files_External\Lib\StorageConfig;
use OCA\Files_External\NotFoundException; use OCA\Files_External\NotFoundException;
use OCP\Files\Config\IUserMountCache; use OCP\Files\Config\IUserMountCache;
@ -427,7 +428,9 @@ abstract class StoragesService {
$changedOptions = array_diff_assoc($updatedStorage->getMountOptions(), $oldStorage->getMountOptions()); $changedOptions = array_diff_assoc($updatedStorage->getMountOptions(), $oldStorage->getMountOptions());
foreach ($changedConfig as $key => $value) { foreach ($changedConfig as $key => $value) {
$this->dbConfig->setConfig($id, $key, $value); if ($value !== DefinitionParameter::UNMODIFIED_PLACEHOLDER) {
$this->dbConfig->setConfig($id, $key, $value);
}
} }
foreach ($changedOptions as $key => $value) { foreach ($changedOptions as $key => $value) {
$this->dbConfig->setOption($id, $key, $value); $this->dbConfig->setOption($id, $key, $value);