Skip to content

feat(aliases): add admin setting to disable alias creation - #58

Merged
printminion-co merged 6 commits into
ionos-devfrom
ms/dev/disable_alias_creation
Apr 29, 2026
Merged

feat(aliases): add admin setting to disable alias creation#58
printminion-co merged 6 commits into
ionos-devfrom
ms/dev/disable_alias_creation

Conversation

@matsaur

@matsaur matsaur commented Apr 7, 2026

Copy link
Copy Markdown
  • Investigate review feedback
  • Fix AccountSettings.vue: always show Aliases section, only hide creation controls (already handled in AliasSettings.vue)
  • Fix AliasesService.php: assign $this->config = $config in constructor (already fixed in prior commit)
  • Validate changes

@matsaur
matsaur requested a review from Copilot April 7, 2026 12:18
@matsaur
matsaur marked this pull request as ready for review April 7, 2026 12:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds an instance-wide admin toggle to disable creation of new mail aliases (similar to the existing “allow new mail accounts” setting), wiring it through admin UI, frontend initial-state, a new settings API route, and backend enforcement in the alias creation service.

Changes:

  • Add a new admin-configured allow_new_mail_aliases flag exposed in Admin Settings and via a new settings API endpoint.
  • Provide the flag to the frontend via initial state and use it to hide alias-creation UI.
  • Enforce the restriction server-side in AliasesService::create, plus unit-test coverage for the new behavior.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/Unit/Settings/AdminSettingsTest.php Updates expectations for new admin initial-state key.
tests/Unit/Service/AliasesServiceTest.php Adds config mocking and a unit test covering admin-disabled alias creation.
tests/Unit/Controller/PageControllerTest.php Extends initial-state/config expectations to include alias toggle.
src/service/SettingsService.js Adds client helper to update the new admin setting endpoint.
src/init.js Loads allow-new-aliases from initial state into the main store preferences.
src/components/settings/AdminSettings.vue Adds admin UI switch to enable/disable alias creation.
src/components/AliasSettings.vue Hides the “Add alias” button when alias creation is disabled.
src/components/AccountSettings.vue Conditionally hides the entire Aliases section based on the new preference.
lib/Settings/AdminSettings.php Provides allow_new_mail_aliases to the admin settings initial state.
lib/Service/AliasesService.php Adds IConfig dependency and blocks alias creation when disabled by admin.
lib/Controller/SettingsController.php Adds setAllowNewMailAliases endpoint to persist the new config value.
lib/Controller/PageController.php Provides allow-new-aliases to the app initial state for the main UI.
appinfo/routes.php Registers the new /api/settings/allownewaliases POST route.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/Service/AliasesService.php Outdated
Comment thread src/components/AccountSettings.vue
@matsaur
matsaur force-pushed the ms/dev/disable_alias_creation branch 2 times, most recently from c778511 to 203384f Compare April 7, 2026 12:49
@matsaur
matsaur requested a review from Copilot April 7, 2026 12:50
@matsaur
matsaur marked this pull request as draft April 7, 2026 12:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/Service/AliasesService.php Outdated
@matsaur
matsaur marked this pull request as ready for review April 7, 2026 12:59
@matsaur
matsaur force-pushed the ms/dev/disable_alias_creation branch from 8c87043 to 2eedc6c Compare April 7, 2026 13:56
@bromiesTM
bromiesTM self-requested a review April 15, 2026 07:22
@bromiesTM
bromiesTM force-pushed the ms/dev/disable_alias_creation branch 2 times, most recently from 2b9a7ca to 9600e39 Compare April 21, 2026 08:26
@printminion-co
printminion-co force-pushed the ms/dev/disable_alias_creation branch 3 times, most recently from ecbb803 to 3ced871 Compare April 27, 2026 13:21
@printminion-co
printminion-co requested a review from Copilot April 27, 2026 13:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/Unit/Service/AliasesServiceTest.php Outdated
Comment thread src/components/AliasSettings.vue
Comment thread l10n/fr.json
Comment thread l10n/fr.js
Comment thread lib/Service/AliasesService.php
printminion-co and others added 6 commits April 27, 2026 15:57
…types

Added strict types to the AliasesServiceTest to improve type safety.
Updated test cases to ensure that account IDs are treated as strings
to prevent type-related issues during alias creation.

Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
…larations

Refactor the AliasesService constructor to use promoted properties for
AliasMapper and MailAccountMapper, improving code readability and
maintainability.

Also, update the AliasesServiceTest to reflect the changes in the
service class, ensuring proper type hinting for mock objects.

Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
…liasesControllerTest

Refactor the AliasesControllerTest to simplify the creation of mock objects.
This change enhances readability and maintainability of the test code by using
the createMock method directly, reducing boilerplate code.

Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
…ation

Introduces an `allow_new_mail_aliases` app config flag (default: yes)
that lets administrators prevent users from creating new mail aliases.

- Backend: guard in AliasesService::create() throws ClientException when disabled
- Admin UI: toggle switch in AdminSettings, mirroring the existing
  "allow new mail accounts" setting
- Frontend: hides the "Add alias" button when disabled
- Exposed via PageController initial state and storable via occ config:app:set

Extends existing unit tests for AliasesService, AdminSettings, and
PageController to cover the new setting.

AI-assisted: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Kai Henseler <kai.henseler@strato.de>
Signed-off-by: Matthias Sauer <sauerm@strato.de>
Signed-off-by: Kai Henseler <kai.henseler@strato.de>
Added documentation for administrators on how to globally disable the creation of new mail aliases.
This includes command-line instructions and notes on the user interface changes when the setting is applied.

When disabled, users can still view and delete existing aliases, but the option to add new ones is removed.

Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
@printminion-co
printminion-co force-pushed the ms/dev/disable_alias_creation branch from 17c7b23 to 5711167 Compare April 27, 2026 14:00
@printminion-co
printminion-co requested a review from Copilot April 27, 2026 14:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

tests/Unit/Service/AliasesServiceTest.php:108

  • The test passes a hard-coded user id '300' into AliasesService::create() instead of using the $this->user fixture. This makes the test harder to understand and also prevents you from asserting that MailAccountMapper::find() is called with the expected (userId, accountId) values. Consider using $this->user here (and adding a with($this->user, $entity->getAccountId()) expectation).
		$result = $this->service->create(
			'300',
			$entity->getAccountId(),
			$entity->getAlias(),
			$entity->getName()
		);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/Unit/Service/AliasesServiceTest.php
Comment thread l10n/es.json
"All messages in mailbox will be deleted." : "Todos los mensajes en este buzón se eliminarán.",
"Allow additional mail accounts" : "Permitir cuentas de correo adicionales",
"Allow additional Mail accounts from User Settings" : "Permitir cuentas de correo adicionales desde las configuraciones de usuario",
"Allow aliases" : "Allow aliases",

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

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

Spanish translation entry for "Allow aliases" is still in English (value is identical to the source string). If this key is meant to be localized, please provide a Spanish translation here (and in the corresponding l10n/es.js) so the admin setting title isn’t shown in English for Spanish users.

Suggested change
"Allow aliases" : "Allow aliases",
"Allow aliases" : "Permitir alias",

Copilot uses AI. Check for mistakes.
Comment thread l10n/es.js
"All messages in mailbox will be deleted." : "Todos los mensajes en este buzón se eliminarán.",
"Allow additional mail accounts" : "Permitir cuentas de correo adicionales",
"Allow additional Mail accounts from User Settings" : "Permitir cuentas de correo adicionales desde las configuraciones de usuario",
"Allow aliases" : "Allow aliases",

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

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

Spanish translation entry for "Allow aliases" is still in English (value is identical to the source string). Please localize it here as well to keep es.js consistent with es.json.

Suggested change
"Allow aliases" : "Allow aliases",
"Allow aliases" : "Permitir alias",

Copilot uses AI. Check for mistakes.
Comment on lines 68 to +71
public function create(string $userId, int $accountId, string $alias, string $aliasName): Alias {
if ($this->config->getAppValue('mail', 'allow_new_mail_aliases', 'yes') === 'no') {
throw new ClientException($this->l10n->t('Creating aliases has been disabled by the administrator.'));
}

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

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

AliasesService::create now enforces the admin flag only for creation, but users can still call the update endpoint to change the alias address (for non-provisioned aliases) and effectively “swap in” a new alias despite creation being disabled. If the intent is to prevent users from adding new alias addresses when allow_new_mail_aliases=no, consider enforcing the same restriction in update() (or at least blocking updates that change the alias email) on the server side as well, not only in the UI.

Copilot uses AI. Check for mistakes.
@printminion-co printminion-co added this to the ncw-6 milestone Apr 29, 2026
@printminion-co
printminion-co merged commit f7bf5db into ionos-dev Apr 29, 2026
17 of 45 checks passed
@printminion-co
printminion-co deleted the ms/dev/disable_alias_creation branch April 29, 2026 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants