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

pkp/pkp-lib#10571 limit email template access by user groups #10581

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
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
19 changes: 17 additions & 2 deletions api/v1/emailTemplates/PKPEmailTemplateController.php
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public function getMany(Request $illuminateRequest): JsonResponse

Hook::call('API::emailTemplates::params', [$collector, $illuminateRequest]);

$emailTemplates = $collector->getMany();
$emailTemplates = collect(Repo::emailTemplate()->filterTemplatesByUserAccess($collector->getMany(), $request->getUser(), $request->getContext()->getId()));

return response()->json([
'itemsMax' => $collector->getCount(),
Expand All @@ -176,6 +176,12 @@ public function get(Request $illuminateRequest): JsonResponse
], Response::HTTP_NOT_FOUND);
}

if (!Repo::emailTemplate()->isTemplateAccessibleToUser($request->getUser(), $emailTemplate, $request->getContext()->getId())) {
return response()->json([
'error' => __('api.emailTemplates.404.templateNotFound')
], Response::HTTP_NOT_FOUND);
}

return response()->json(Repo::emailTemplate()->getSchemaMap()->map($emailTemplate), Response::HTTP_OK);
}

Expand All @@ -198,6 +204,9 @@ public function add(Request $illuminateRequest): JsonResponse

$emailTemplate = Repo::emailTemplate()->newDataObject($params);
Repo::emailTemplate()->add($emailTemplate);

Repo::emailTemplate()->setEmailTemplateAccess($emailTemplate, $requestContext->getId(), $params['userGroupIds'], $params['isUnrestricted']);

$emailTemplate = Repo::emailTemplate()->getByKey($emailTemplate->getData('contextId'), $emailTemplate->getData('key'));

return response()->json(Repo::emailTemplate()->getSchemaMap()->map($emailTemplate), Response::HTTP_OK);
Expand Down Expand Up @@ -235,6 +244,11 @@ public function edit(Request $illuminateRequest): JsonResponse
$params['contextId'] = $requestContext->getId();
}


// If the user submitted an empty list (meaning all user groups were unchecked), the empty array is converted to null in the request's data.
// Convert back to an empty array.
$params['userGroupIds'] = $params['userGroupIds'] ?: [];

$errors = Repo::emailTemplate()->validate(
$emailTemplate,
$params,
Expand All @@ -246,6 +260,7 @@ public function edit(Request $illuminateRequest): JsonResponse
}

Repo::emailTemplate()->edit($emailTemplate, $params);
Repo::emailTemplate()->setEmailTemplateAccess($emailTemplate, $requestContext->getId(), $params['userGroupIds'], $params['isUnrestricted']);

$emailTemplate = Repo::emailTemplate()->getByKey(
// context ID is null if edited for the first time
Expand Down Expand Up @@ -275,7 +290,7 @@ public function delete(Request $illuminateRequest): JsonResponse

$props = Repo::emailTemplate()->getSchemaMap()->map($emailTemplate);
Repo::emailTemplate()->delete($emailTemplate);

Repo::emailTemplate()->deleteTemplateGroupAccess($requestContext->getId(), [$illuminateRequest->route('key')]);
return response()->json($props, Response::HTTP_OK);
}

Expand Down
33 changes: 33 additions & 0 deletions classes/components/forms/FieldEmailTemplateUnrestricted.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

/**
* @file classes/components/form/FieldEmailTemplateUnrestricted.php
*
* Copyright (c) 2014-2024 Simon Fraser University
* Copyright (c) 2000-2024 John Willinsky
* Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
*
* @class FieldEmailTemplateUnrestricted
*
* @ingroup classes_controllers_form
*
* @brief A component to indicate if an email template is unrestricted, i.e accessible to all user groups.
*/

namespace PKP\components\forms;

class FieldEmailTemplateUnrestricted extends Field
{
/** @copydoc Field::$component */
public $component = 'field-email-template-unrestricted';
public string $subNote = '';
/**
* @copydoc Field::getConfig()
*/
public function getConfig()
{
$config = parent::getConfig();
$config['subNote'] = $this->subNote;
return $config;
}
}
33 changes: 33 additions & 0 deletions classes/components/forms/FieldEmailTemplateUserGroupSettings.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

/**
* @file classes/components/form/FieldEmailTemplateUserGroupSettings.php
*
* Copyright (c) 2014-2024 Simon Fraser University
* Copyright (c) 2000-2024 John Willinsky
* Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
*
* @class FieldEmailTemplateUserGroupSettings
*
* @ingroup classes_controllers_form
*
* @brief A component managing user groups assigned to an email template
*/

namespace PKP\components\forms;

class FieldEmailTemplateUserGroupSettings extends Field
{
/** @copydoc Field::$component */
public $component = 'field-email-template-user-group-settings';

/**
* @copydoc Field::getConfig()
*/
public function getConfig()
{
$config = parent::getConfig();

return $config;
}
}
10 changes: 10 additions & 0 deletions classes/components/forms/emailTemplate/EmailTemplateForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

namespace PKP\components\forms\emailTemplate;

use PKP\components\forms\FieldEmailTemplateUnrestricted;
use PKP\components\forms\FieldEmailTemplateUserGroupSettings;
use PKP\components\forms\FieldPreparedContent;
use PKP\components\forms\FieldText;
use PKP\components\forms\FormComponent;
Expand Down Expand Up @@ -46,6 +48,14 @@ public function __construct(string $action, array $locales)
'isMultilingual' => true,
'toolbar' => 'bold italic superscript subscript | link | blockquote bullist numlist',
'plugins' => 'paste,link,lists',
]))->addField(new FieldEmailTemplateUnrestricted('isUnrestricted', [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a lot more of the setup logic for the Vue components can be done in PHP so that these can both be done with existing FieldOptions components. I mentioned this in the ui-library PR as well and we can chat about it more in our next one-on-one (probably easier to share screen and code together than writing it all out here).

'type' => 'checkbox',
'label' => __('admin.workflow.email.userGroup.assign.unrestricted'),
'subNote' => __('admin.workflow.email.userGroup.unrestricted.template.note')
]))
->addField(new FieldEmailTemplateUserGroupSettings('userGroupIds', [
'type' => 'checkbox',
'label' => __('admin.workflow.email.userGroup.allowed'),
]));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,7 @@ public function fetchTemplateBody(array $args, PKPRequest $request): ?JSONMessag
};
$template = Repo::emailTemplate()->getByKey($context->getId(), $request->getUserVar('template'));

if (!$template) {
if (!$template || ! Repo::emailTemplate()->isTemplateAccessibleToUser($request->getUser(), $template, $context->getId())) {
return null;
}

Expand Down
8 changes: 6 additions & 2 deletions classes/decision/steps/Email.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,18 @@ protected function getEmailTemplates(): array
$emailTemplates = collect();
if ($this->mailable::getEmailTemplateKey()) {
$emailTemplate = Repo::emailTemplate()->getByKey($context->getId(), $this->mailable::getEmailTemplateKey());
if ($emailTemplate) {
if ($emailTemplate && Repo::emailTemplate()->isTemplateAccessibleToUser($request->getUser(), $emailTemplate, $context->getId())) {
$emailTemplates->add($emailTemplate);
}
Repo::emailTemplate()
->getCollector($context->getId())
->alternateTo([$this->mailable::getEmailTemplateKey()])
->getMany()
->each(fn (EmailTemplate $e) => $emailTemplates->add($e));
->each(function (EmailTemplate $e) use ($context, $request, $emailTemplates) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion, but since this isn't an arrow function anymore, I would be more verbose about the variable name, just to make it easier to read at a glance.

if (Repo::emailTemplate()->isTemplateAccessibleToUser($request->getUser(), $e, $context->getId())) {
$emailTemplates->add($e);
}
});
}

return Repo::emailTemplate()->getSchemaMap()->mapMany($emailTemplates)->toArray();
Expand Down
55 changes: 53 additions & 2 deletions classes/emailTemplate/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,17 @@ public function getMainEmailTemplatesFilename()
* skipping others
* @param bool $skipExisting If true, do not install email templates
* that already exist in the database
* @param bool $recordTemplateGroupAccess - If true, records the templates as unrestricted.
* By default, it is set to false to ensure compatibility with older processes (e.g., migrations)
* where the `email_template_user_group_access` table may not exist at the time of execution.
*
*/
public function installEmailTemplates(
string $templatesFile,
array $locales = [],
?string $emailKey = null,
bool $skipExisting = false
bool $skipExisting = false,
$recordTemplateGroupAccess = false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mention this directly below, but I don't think this optional parameter will be necessary. I think the assumption of "unrestricted if no additional data provided" below is enough.

): bool {
$xmlDao = new XMLDAO();
$data = $xmlDao->parseStruct($templatesFile, ['email']);
Expand Down Expand Up @@ -281,6 +285,20 @@ public function installEmailTemplates(
$this->installAlternateEmailTemplates($contextId, $attrs['key']);
}
}

if ($recordTemplateGroupAccess) {
// Default to true if `isUnrestricted` is not set.
$isUnrestricted = $attrs['isUnrestricted'] ?? '1';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this check is enough to cover any plugin-related issues or the email template XML not having this flag. I don't think you need the $recordTempalteGroupAccess parameter at all.


if ($isUnrestricted !== '1' && $isUnrestricted !== '0') {
throw new Exception('Invalid value given for the `isUnrestricted` attribute on the ' . $attrs['key'] . ' template.');
}

$contextIds = app()->get('context')->getIds();
foreach ($contextIds as $contextId) {
Repo::emailTemplate()->markTemplateAsUnrestricted($attrs['key'], (bool)$isUnrestricted, $contextId);
}
}
}
return true;
}
Expand Down Expand Up @@ -381,7 +399,7 @@ public function installAlternateEmailTemplates(int $contextId, ?string $emailKey
'Tried to install email template as an alternate to `' . $alternateTo . '`, but no default template exists with this key. Installing ' . $alternateTo . ' email template first',
E_USER_WARNING
);
$this->installEmailTemplates(Repo::emailTemplate()->dao->getMainEmailTemplatesFilename(), [], $alternateTo);
$this->installEmailTemplates(Repo::emailTemplate()->dao->getMainEmailTemplatesFilename(), [], $alternateTo, false, true);
}

DB::table($this->table)->insert([
Expand Down Expand Up @@ -448,4 +466,37 @@ protected function getUniqueKey(EmailTemplate $emailTemplate): string

return $key;
}


/**
* Sets email template's unrestricted status to their defaults
*/
public function setTemplateDefaultUnrestirctedSetting(int $contextId, ?array $emailKeys = null)
{
$xmlDao = new XMLDAO();
$data = $xmlDao->parseStruct($this->getMainEmailTemplatesFilename(), ['email']);

if (!isset($data['email'])) {
return false;
}

foreach ($data['email'] as $entry) {
$attrs = $entry['attributes'];

if ($emailKeys !== null && !in_array($attrs['key'], $emailKeys)) {
continue;
}

// Default to true if `isUnrestricted` is not set.
$isUnrestricted = $attrs['isUnrestricted'] ?? '1';

if ($isUnrestricted !== '1' && $isUnrestricted !== '0') {
throw new Exception('Invalid value given for the `isUnrestricted` attribute on the ' . $attrs['key'] . ' template.');
}

Repo::emailTemplate()->markTemplateAsUnrestricted($attrs['key'], (bool)$isUnrestricted, $contextId);
}

return true;
}
}
34 changes: 34 additions & 0 deletions classes/emailTemplate/EmailTemplateAccessGroup.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs file docblock.


namespace PKP\emailTemplate;

use Eloquence\Behaviours\HasCamelCasing;
use Eloquence\Database\Model;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless you've gotten advice to the contrary, it's probably best to extend the Laravel model directly through use Illuminate\Database\Eloquent\Model; rather than the one under the Eloquence namespace. At a quick glance, this seems to be what the other PKP classes that extend Eloquent models are doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an error on my part. It should be extending the Illuminate\Database\Eloquent\Model.

use Illuminate\Contracts\Database\Eloquent\Builder;

class EmailTemplateAccessGroup extends Model
{
use HasCamelCasing;
public $timestamps = false;
protected $primaryKey = 'email_template_user_group_access_id';
protected $table = 'email_template_user_group_access';
protected $fillable = ['userGroupId', 'contextId','emailKey'];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after 'contextId',



public function scopeWithEmailKey(Builder $query, ?array $keys): Builder
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would include a comment on what these scopes do for reference (e.g. optionally filter by email key). Same for the two below as well.

{
return $query->when(!empty($keys), function ($query) use ($keys) {
return $query->whereIn('email_key', $keys);
});
}

public function scopeWithContextId(Builder $query, int $contextId): Builder
{
return $query->where('context_id', $contextId);
}

public function scopeWithGroupIds(Builder $query, array $ids): Builder
{
return $query->whereIn('user_group_id', $ids);
}
}
Loading