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

Add option to allow groups to be determined by registering email address #677

Closed
wants to merge 2 commits into from

Conversation

Bottswana
Copy link

Hello,

To implement changes for a specific requirement, we have implemented the following features as part of the registration module:

  • Allow the mapping of email domains -> groups, instead of relying entirely on the default group mapping
  • Allow the option of only sending activation emails to the group admins (subadmins) instead of the subadmins + global admins

Additionally, we changed the response text for non-permitted domains, and if the user already exists, to a generic response instead of a detailed error, to reduce the opportunity for unauthenticated account enumeration (even though this is heavily restricted by the rate limiter) as this was a security requirement.

I'm not sure to what degree you are interested in these features upstream, so if you would like these options please let me know any changes required to make this eligible for merging.

The changes allow for a workflow where subdomains of a corporate domain can self-register and have the relevant department lead authorise their account creation. For example:

email@hr.corp.com -> HR Group -> Emails HR group admins for approval
email@it.corp.com -> IT Group -> Emails IT group admins for approval
email@corp.com -> Default group -> Emails default group admins for approval

Note: I'm not entirely familiar with how the l18n implementation with vue is working here, so I added some translation strings directly for english. I'm not sure if this is the correct approach.

Additionally, currently the mapping is stored in a new database table. I have not pushed a migration as part of this PR as I would like to verify this method is acceptable. If you have an alternative way of storing the mappings you would prefer, please let me know.

The simple migration I have used for my custom version of this module is as follows:

	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
		/** @var ISchemaWrapper $schema */
		$schema = $schemaClosure();

		if (!$schema->hasTable('registration_group')) {
			$table = $schema->createTable('registration_group');
			$table->addColumn('id', Types::INTEGER, [
				'autoincrement' => true,
				'notnull' => true,
				'unsigned' => true,
			]);
			$table->addColumn('email_domains', Types::STRING, [
				'notnull' => true,
			]);
			$table->addColumn('group_id', Types::STRING, [
				'notnull' => false,
			]);
			$table->setPrimaryKey(['id']);
		}
		return $schema;
	}

Signed-off-by: James Botting <james@bottswanamedia.info>
Merge of the following commits from branch:

Rebuild webpack

Prevent account enumeration

The granular errors shown here can allow an attacker to enumerate valid accounts in the system and increase the attack surface

Relocate user disable code

If any of the code should error after the creation of the user, the user may then have a valid account to login with when this is not expected. Relocate the code to disable the user immediately after user creation

Update backend code for changes
@Bottswana Bottswana changed the title Implement Group Mapping Add option to allow groups to be determined by registering email address Jul 26, 2024
Signed-off-by: James Botting <james@bottswanamedia.info>
Copy link

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant