diff --git a/_build/data/transport.core.menus.php b/_build/data/transport.core.menus.php
index a7ee89b2078..f2b10446666 100644
--- a/_build/data/transport.core.menus.php
+++ b/_build/data/transport.core.menus.php
@@ -94,7 +94,7 @@
[
'text' => 'media',
'description' => '',
- 'permissions' => 'file_manager',
+ 'permissions' => '',
'action' => '',
'icon' => '',
'children' => [
diff --git a/_build/test/Tests/Controllers/MediaAccessPolicyTest.php b/_build/test/Tests/Controllers/MediaAccessPolicyTest.php
new file mode 100644
index 00000000000..9cece80d105
--- /dev/null
+++ b/_build/test/Tests/Controllers/MediaAccessPolicyTest.php
@@ -0,0 +1,59 @@
+assertFileExists($menusFile);
+
+ $contents = file_get_contents($menusFile);
+ $this->assertMatchesRegularExpression(
+ "/'text' => 'media',\s*\n\s*'description' => '',\s*\n\s*'permissions' => '',/",
+ $contents,
+ 'Media parent menu must not require file_manager; child items enforce their own permissions.'
+ );
+ $this->assertMatchesRegularExpression(
+ "/'text' => 'file_browser',[\s\S]*?'permissions' => 'file_manager',/",
+ $contents,
+ 'Media Browser child must still require file_manager.'
+ );
+ }
+
+ public function testBrowserFileGetUsesViewSourcePolicy()
+ {
+ $file = MODX_CORE_PATH . 'src/Revolution/Processors/Browser/File/Get.php';
+ $contents = file_get_contents($file);
+ $this->assertStringContainsString("public \$policy = 'view';", $contents);
+ $this->assertStringNotContainsString("checkPolicy('delete')", $contents);
+ }
+
+ public function testFileTreePageLinkRequiresFileViewPermission()
+ {
+ $sourceFile = MODX_CORE_PATH . 'src/Revolution/Sources/modMediaSource.php';
+ $contents = file_get_contents($sourceFile);
+ $this->assertStringContainsString(
+ "&& \$this->hasPermission('file_view')\n && \$canView",
+ $contents
+ );
+ }
+
+ public function testDirectoryChmodLexiconDescribesVisibility()
+ {
+ $lexiconFile = MODX_CORE_PATH . 'lexicon/en/permissions.inc.php';
+ $contents = file_get_contents($lexiconFile);
+ $this->assertStringContainsString('set directory visibility', $contents);
+ $this->assertStringContainsString('Does not include uploading files', $contents);
+ $this->assertStringContainsString('Does not include creating empty files', $contents);
+ }
+}
diff --git a/_build/test/Tests/Controllers/System/SystemFileEditControllerTest.php b/_build/test/Tests/Controllers/System/SystemFileEditControllerTest.php
new file mode 100644
index 00000000000..895793a073b
--- /dev/null
+++ b/_build/test/Tests/Controllers/System/SystemFileEditControllerTest.php
@@ -0,0 +1,45 @@
+assertTrue($this->controller->checkPermissions());
+ }
+
+ public function testCanSaveGatesOnFileUpdateAndSourceSavePolicy()
+ {
+ $controllerFile = MODX_MANAGER_PATH . 'controllers/default/system/file/edit.class.php';
+ $contents = file_get_contents($controllerFile);
+ $this->assertStringContainsString(
+ "\$this->canSave = \$this->modx->hasPermission('file_update') && \$source->checkPolicy('save');",
+ $contents
+ );
+ $this->assertStringContainsString("\$source->checkPolicy('view')", $contents);
+ $this->assertStringNotContainsString("\$this->canSave = true;", $contents);
+ }
+
+ public function testMediaBrowserViewDefinesUnpackFileHandler()
+ {
+ $browserFile = MODX_MANAGER_PATH . 'assets/modext/widgets/media/modx.browser.js';
+ $contents = file_get_contents($browserFile);
+ $this->assertStringContainsString('unpackFile: function', $contents);
+ $this->assertStringContainsString('file: data.pathRelative', $contents);
+ }
+}
diff --git a/core/lexicon/en/permissions.inc.php b/core/lexicon/en/permissions.inc.php
index 43d2ae252d5..2bb103bf696 100644
--- a/core/lexicon/en/permissions.inc.php
+++ b/core/lexicon/en/permissions.inc.php
@@ -40,7 +40,7 @@
$_lang['perm.delete_template_desc'] = 'To delete or remove any Templates.';
$_lang['perm.delete_tv_desc'] = 'To delete or remove any TVs.';
$_lang['perm.delete_user_desc'] = 'To delete or remove any Users.';
-$_lang['perm.directory_chmod_desc'] = 'To chmod a physical directory.';
+$_lang['perm.directory_chmod_desc'] = 'To set directory visibility (public/private) in the manager. Also requires the Media Source save policy.';
$_lang['perm.directory_create_desc'] = 'To create a physical directory.';
$_lang['perm.directory_list_desc'] = 'To list subdirectories for a physical directory.';
$_lang['perm.directory_remove_desc'] = 'To delete a physical directory.';
@@ -65,15 +65,15 @@
$_lang['perm.error_log_erase_desc'] = 'To erase the error log.';
$_lang['perm.error_log_view_desc'] = 'To view the error log.';
$_lang['perm.export_static_desc'] = 'To export the site to static HTML.';
-$_lang['perm.file_create_desc'] = 'To create a file.';
+$_lang['perm.file_create_desc'] = 'To create a new empty file. Does not include uploading files.';
$_lang['perm.file_list_desc'] = 'To list files within a given physical directory.';
$_lang['perm.file_manager_desc'] = 'To use the file manager utility.';
$_lang['perm.file_remove_desc'] = 'To delete physical files.';
$_lang['perm.file_tree_desc'] = 'To view the Files Tree on the left nav.';
-$_lang['perm.file_update_desc'] = 'To edit the content of physical files. WARNING: grants ability to execute arbitrary server-side code.';
-$_lang['perm.file_upload_desc'] = 'To upload files to a directory.';
+$_lang['perm.file_update_desc'] = 'To edit the content of physical files. Also requires file_view and the Media Source save policy. WARNING: grants ability to execute arbitrary server-side code.';
+$_lang['perm.file_upload_desc'] = 'To upload files to a directory. Does not include creating empty files.';
$_lang['perm.file_unpack_desc'] = 'To extract zip archives.';
-$_lang['perm.file_view_desc'] = 'To view the contents of a file.';
+$_lang['perm.file_view_desc'] = 'To view the contents of a file. Media Source view policy must also pass.';
$_lang['perm.flush_sessions_desc'] = 'Can flush Sessions across the site.';
$_lang['perm.frames_desc'] = 'To use the MODX Manager UI at all.';
$_lang['perm.help_desc'] = 'To view the Help page.';
diff --git a/core/src/Revolution/Processors/Browser/File/Get.php b/core/src/Revolution/Processors/Browser/File/Get.php
index 653ead9a36b..bd185c95dbf 100644
--- a/core/src/Revolution/Processors/Browser/File/Get.php
+++ b/core/src/Revolution/Processors/Browser/File/Get.php
@@ -23,22 +23,10 @@
class Get extends Browser
{
public $permission = 'file_view';
+ public $policy = 'view';
public $languageTopics = ['file'];
- /**
- * @return array|bool|string
- */
- public function initialize()
- {
- if (!$this->getSource() || !$this->source->checkPolicy('delete')) {
- return $this->failure($this->modx->lexicon('permission_denied'));
- }
-
- return true;
- }
-
-
/**
* @return array|bool|mixed|string
*/
diff --git a/core/src/Revolution/Sources/modMediaSource.php b/core/src/Revolution/Sources/modMediaSource.php
index 2d43cf04911..34b7630e331 100644
--- a/core/src/Revolution/Sources/modMediaSource.php
+++ b/core/src/Revolution/Sources/modMediaSource.php
@@ -1869,6 +1869,7 @@ protected function buildFileList($path, $ext, $image_extensions, $bases, $proper
$editAction = $this->getEditActionId();
$canSave = $this->checkPolicy('save');
$canRemove = $this->checkPolicy('remove');
+ $canView = $this->checkPolicy('view');
$id = rawurlencode(htmlspecialchars_decode($path, ENT_COMPAT));
$cls = [];
@@ -1888,7 +1889,11 @@ protected function buildFileList($path, $ext, $image_extensions, $bases, $proper
$cls[] = 'pupdate';
}
$page = null;
- if (!$this->isFileBinary($path)) {
+ if (
+ !$this->isFileBinary($path)
+ && $this->hasPermission('file_view')
+ && $canView
+ ) {
$page = !empty($editAction)
? '?a=' . $editAction . '&file=' . $id . '&wctx=' . $this->ctx->get('key') . '&source=' . $this->get('id')
: null;
@@ -1932,10 +1937,10 @@ protected function buildFileList($path, $ext, $image_extensions, $bases, $proper
$preview_image = $this->buildManagerImagePreview($path, $ext, $imageWidth, $imageHeight, $bases, $properties);
// Once minimum php requirement is brought up to 7.4+, heredoc closing can be indented
$file_list['qtip'] = <<
QTIP;
@@ -1960,7 +1965,11 @@ protected function buildFileBrowserViewList($path, $ext, $image_extensions, $bas
$editAction = $this->getEditActionId();
$page = null;
- if (!$this->isFileBinary($path)) {
+ if (
+ !$this->isFileBinary($path)
+ && $this->hasPermission('file_view')
+ && $this->checkPolicy('view')
+ ) {
$page = !empty($editAction)
? '?a=' . $editAction . '&file=' . $path . '&wctx=' . $this->ctx->get('key') . '&source=' . $this->get('id')
: null;
diff --git a/core/src/Revolution/Sources/modS3MediaSource.php b/core/src/Revolution/Sources/modS3MediaSource.php
index c6db2ac5d21..22b4a07470e 100644
--- a/core/src/Revolution/Sources/modS3MediaSource.php
+++ b/core/src/Revolution/Sources/modS3MediaSource.php
@@ -732,7 +732,11 @@ protected function buildFileBrowserViewList($path, $ext, $image_extensions, $bas
$editAction = $this->getEditActionId();
$page = null;
- if (!$this->isFileBinary($path)) {
+ if (
+ !$this->isFileBinary($path)
+ && $this->hasPermission('file_view')
+ && $this->checkPolicy('view')
+ ) {
$page = !empty($editAction)
? '?a=' . $editAction .
'&file=' . $path .
diff --git a/manager/assets/modext/widgets/media/modx.browser.js b/manager/assets/modext/widgets/media/modx.browser.js
index 6723d3257f5..6b4241d3572 100644
--- a/manager/assets/modext/widgets/media/modx.browser.js
+++ b/manager/assets/modext/widgets/media/modx.browser.js
@@ -192,6 +192,37 @@ Ext.extend(MODx.browser.View,MODx.DataView,{
w.show(e.target);
}
+ // eslint-disable-next-line comma-style, comma-spacing
+ ,unpackFile: function(item, e) {
+ const node = this.cm.activeNode,
+ data = this.lookup[node.id];
+ MODx.msg.confirm({
+ text: `${_('file_download_unzip')} ${data.name}`,
+ url: MODx.config.connector_url,
+ params: {
+ action: 'Browser/File/Unpack',
+ file: data.pathRelative,
+ wctx: MODx.ctx || '',
+ source: this.config.source
+ },
+ listeners: {
+ success: {
+ fn: function() {
+ if (this.config.tree) {
+ if (this.config.tree.cm.activeNode && this.config.tree.cm.activeNode.id.match(/.*?\/$/)) {
+ this.config.tree.refreshParentNode();
+ } else {
+ this.config.tree.refresh();
+ }
+ }
+ this.run();
+ },
+ scope: this
+ }
+ }
+ });
+ }
+
,downloadFile: function(item,e) {
var node = this.cm.activeNode;
var data = this.lookup[node.id];
diff --git a/manager/controllers/default/system/file/edit.class.php b/manager/controllers/default/system/file/edit.class.php
index 60de7d64891..95f45e2f63d 100644
--- a/manager/controllers/default/system/file/edit.class.php
+++ b/manager/controllers/default/system/file/edit.class.php
@@ -93,6 +93,12 @@ public function process(array $scriptProperties = [])
return false;
}
+ if (!$source->checkPolicy('view')) {
+ $this->failure($this->modx->lexicon('permission_denied'));
+
+ return false;
+ }
+
if ($this->fileRecord = $source->getObjectContents($this->filename)) {
$this->fileRecord['source'] = $source->get('id');
}
@@ -114,7 +120,7 @@ public function process(array $scriptProperties = [])
if (!empty($this->fileRecord['last_modified'])) {
$this->fileRecord['last_modified'] = $formatter->formatDateTime($this->fileRecord['last_modified']);
}
- $this->canSave = true;
+ $this->canSave = $this->modx->hasPermission('file_update') && $source->checkPolicy('save');
$placeholders['fa'] = $this->fileRecord;
$placeholders['OnFileEditFormPrerender'] = $this->fireEvents();
diff --git a/setup/includes/upgrades/common/3.3.0-clear-media-parent-permission.php b/setup/includes/upgrades/common/3.3.0-clear-media-parent-permission.php
new file mode 100644
index 00000000000..c10e1317155
--- /dev/null
+++ b/setup/includes/upgrades/common/3.3.0-clear-media-parent-permission.php
@@ -0,0 +1,25 @@
+getObject(modMenu::class, [
+ 'text' => 'media',
+ 'permissions' => 'file_manager',
+]);
+if ($mediaMenu instanceof modMenu) {
+ $mediaMenu->set('permissions', '');
+ if ($mediaMenu->save()) {
+ $mediaMenu->rebuildCache('');
+ }
+}
diff --git a/setup/includes/upgrades/mysql/3.3.0-pl.php b/setup/includes/upgrades/mysql/3.3.0-pl.php
new file mode 100644
index 00000000000..efff0ed755b
--- /dev/null
+++ b/setup/includes/upgrades/mysql/3.3.0-pl.php
@@ -0,0 +1,12 @@
+