diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 970ef16e..75c56fd3 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -454,6 +454,18 @@ public function code(string $state = '', string $code = '', string $scope = '', return $this->build403TemplateResponse($message, Http::STATUS_BAD_REQUEST, ['reason' => 'failed to provision user']); } + // prevent login of users that are not in a whitelisted group (if activated) + $restrictLoginToGroups = $this->providerService->getSetting($providerId, ProviderService::SETTING_RESTRICT_LOGIN_TO_GROUPS, '0'); + if ($restrictLoginToGroups === '1') { + $syncGroups = $this->provisioningService->getSyncGroupsOfToken($providerId, $idTokenPayload); + + if ($syncGroups === null || count($syncGroups) === 0) { + $this->logger->debug('Prevented user from login as user is not part of a whitelisted group'); + $message = $this->l10n->t('You do not have permission to log in to this instance. If you think this is an error, please contact an administrator.'); + return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, ['reason' => 'user not in any whitelisted group']); + } + } + $autoProvisionAllowed = (!isset($oidcSystemConfig['auto_provision']) || $oidcSystemConfig['auto_provision']); // in case user is provisioned by user_ldap, userManager->search() triggers an ldap search which syncs the results diff --git a/lib/Service/ProviderService.php b/lib/Service/ProviderService.php index c460ed88..ff55bdb6 100644 --- a/lib/Service/ProviderService.php +++ b/lib/Service/ProviderService.php @@ -47,6 +47,8 @@ class ProviderService { public const SETTING_JWKS_CACHE_TIMESTAMP = 'jwksCacheTimestamp'; public const SETTING_PROVIDER_BASED_ID = 'providerBasedId'; public const SETTING_GROUP_PROVISIONING = 'groupProvisioning'; + public const SETTING_GROUP_WHITELIST_REGEX = 'groupWhitelistRegex'; + public const SETTING_RESTRICT_LOGIN_TO_GROUPS = 'restrictLoginToGroups'; public const BOOLEAN_SETTINGS_DEFAULT_VALUES = [ self::SETTING_GROUP_PROVISIONING => false, @@ -55,6 +57,7 @@ class ProviderService { self::SETTING_UNIQUE_UID => true, self::SETTING_CHECK_BEARER => false, self::SETTING_SEND_ID_TOKEN_HINT => false, + self::SETTING_RESTRICT_LOGIN_TO_GROUPS => false, ]; public function __construct( @@ -159,7 +162,9 @@ private function getSupportedSettings(): array { self::SETTING_BEARER_PROVISIONING, self::SETTING_EXTRA_CLAIMS, self::SETTING_PROVIDER_BASED_ID, - self::SETTING_GROUP_PROVISIONING + self::SETTING_GROUP_PROVISIONING, + self::SETTING_GROUP_WHITELIST_REGEX, + self::SETTING_RESTRICT_LOGIN_TO_GROUPS, ]; } diff --git a/lib/Service/ProvisioningService.php b/lib/Service/ProvisioningService.php index 02122a1d..342ef17c 100644 --- a/lib/Service/ProvisioningService.php +++ b/lib/Service/ProvisioningService.php @@ -385,10 +385,12 @@ private function setUserAvatar(string $userId, string $avatarAttribute): void { } } - public function provisionUserGroups(IUser $user, int $providerId, object $idTokenPayload): void { + public function getSyncGroupsOfToken(int $providerId, object $idTokenPayload) { $groupsAttribute = $this->providerService->getSetting($providerId, ProviderService::SETTING_MAPPING_GROUPS, 'groups'); $groupsData = $idTokenPayload->{$groupsAttribute} ?? null; + $groupsWhitelistRegex = $this->getGroupWhitelistRegex($providerId); + $event = new AttributeMappedEvent(ProviderService::SETTING_MAPPING_GROUPS, $idTokenPayload, json_encode($groupsData)); $this->eventDispatcher->dispatchTyped($event); $this->logger->debug('Group mapping event dispatched'); @@ -396,7 +398,6 @@ public function provisionUserGroups(IUser $user, int $providerId, object $idToke if ($event->hasValue() && $event->getValue() !== null) { // casted to null if empty value $groups = json_decode($event->getValue() ?? ''); - $userGroups = $this->groupManager->getUserGroups($user); $syncGroups = []; foreach ($groups as $k => $v) { @@ -413,13 +414,35 @@ public function provisionUserGroups(IUser $user, int $providerId, object $idToke continue; } + if ($groupsWhitelistRegex && !preg_match($groupsWhitelistRegex, $group->gid)) { + $this->logger->debug('Skipped group `' . $group->gid . '` for importing as not part of whitelist'); + continue; + } + $group->gid = $this->idService->getId($providerId, $group->gid); $syncGroups[] = $group; } + return $syncGroups; + } + + return null; + } + + public function provisionUserGroups(IUser $user, int $providerId, object $idTokenPayload): void { + $groupsWhitelistRegex = $this->getGroupWhitelistRegex($providerId); + + $syncGroups = $this->getSyncGroupsOfToken($providerId, $idTokenPayload); + + if ($syncGroups !== null) { + + $userGroups = $this->groupManager->getUserGroups($user); foreach ($userGroups as $group) { if (!in_array($group->getGID(), array_column($syncGroups, 'gid'))) { + if ($groupsWhitelistRegex && !preg_match($groupsWhitelistRegex, $group->getGID())) { + continue; + } $group->removeUser($user); } } @@ -437,4 +460,16 @@ public function provisionUserGroups(IUser $user, int $providerId, object $idToke } } } + + public function getGroupWhitelistRegex(int $providerId): string { + $regex = $this->providerService->getSetting($providerId, ProviderService::SETTING_GROUP_WHITELIST_REGEX, ''); + + // If regex does not start with '/', add '/' to the beginning and end + // Only check first character to allow for flags at the end of the regex + if ($regex && substr($regex, 0, 1) !== '/') { + $regex = '/' . $regex . '/'; + } + + return $regex; + } } diff --git a/src/components/SettingsForm.vue b/src/components/SettingsForm.vue index 81eee52b..3e369c5f 100644 --- a/src/components/SettingsForm.vue +++ b/src/components/SettingsForm.vue @@ -240,6 +240,21 @@

{{ t('user_oidc', 'This will create and update the users groups depending on the groups claim in the id token. The Format of the groups claim value should be [{gid: "1", displayName: "group1"}, ...] or ["group1", "group2", ...]') }}

+

+ + +

+

+ {{ t('user_oidc', 'Only groups matching the whitelist regex will be created, updated and deleted by the group claim. For example: {regex} allows all groups which ID starts with {substr}', { regex: '/^blue/', substr: 'blue' }) }} +

+ + {{ t('user_oidc', 'Restrict login for users that are not in any whitelisted group') }} + +

+ {{ t('user_oidc', 'Users that are not part of any whitelisted group are not created and can not login') }} +

{{ t('user_oidc', 'Check Bearer token on API and WebDav requests') }} diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index bee65b4a..6d291951 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -4,8 +4,5 @@ - - displayName)]]> - diff --git a/tests/unit/Db/UserMapperTest.php b/tests/unit/Db/UserMapperTest.php index b36af9bc..2812f312 100644 --- a/tests/unit/Db/UserMapperTest.php +++ b/tests/unit/Db/UserMapperTest.php @@ -20,6 +20,9 @@ class UserMapperTest extends TestCase { /** @var LocalIdService|MockObject */ private $idService; + /** @var IDBConnection|MockObject */ + private $db; + /** @var UserMapper|MockObject */ private $userMapper; diff --git a/tests/unit/Service/ProviderServiceTest.php b/tests/unit/Service/ProviderServiceTest.php index 2a7db068..7806028d 100644 --- a/tests/unit/Service/ProviderServiceTest.php +++ b/tests/unit/Service/ProviderServiceTest.php @@ -88,6 +88,8 @@ public function testGetProvidersWithSettings() { 'extraClaims' => '1', 'providerBasedId' => true, 'groupProvisioning' => true, + 'groupWhitelistRegex' => '1', + 'restrictLoginToGroups' => true, ], ], [ @@ -126,6 +128,8 @@ public function testGetProvidersWithSettings() { 'extraClaims' => '1', 'providerBasedId' => true, 'groupProvisioning' => true, + 'groupWhitelistRegex' => '1', + 'restrictLoginToGroups' => true, ], ], ], $this->providerService->getProvidersWithSettings()); @@ -161,6 +165,8 @@ public function testSetSettings() { 'mappingBiography' => 'biography', 'mappingPhonenumber' => 'phone', 'mappingGender' => 'gender', + 'groupWhitelistRegex' => '', + 'restrictLoginToGroups' => false, ]; $this->config->expects(self::any()) ->method('getAppValue') @@ -193,6 +199,8 @@ public function testSetSettings() { [Application::APP_ID, 'provider-1-' . ProviderService::SETTING_EXTRA_CLAIMS, '', 'claim1 claim2'], [Application::APP_ID, 'provider-1-' . ProviderService::SETTING_PROVIDER_BASED_ID, '', '0'], [Application::APP_ID, 'provider-1-' . ProviderService::SETTING_GROUP_PROVISIONING, '', '1'], + [Application::APP_ID, 'provider-1-' . ProviderService::SETTING_GROUP_WHITELIST_REGEX, '', ''], + [Application::APP_ID, 'provider-1-' . ProviderService::SETTING_RESTRICT_LOGIN_TO_GROUPS, '', '0'], ]); Assert::assertEquals( diff --git a/tests/unit/Service/ProvisioningServiceTest.php b/tests/unit/Service/ProvisioningServiceTest.php index c972a85e..1c830434 100644 --- a/tests/unit/Service/ProvisioningServiceTest.php +++ b/tests/unit/Service/ProvisioningServiceTest.php @@ -34,6 +34,9 @@ class ProvisioningServiceTest extends TestCase { /** @var ProvisioningService | MockObject */ private $providerService; + /** @var LdapService | MockObject */ + private $ldapService; + /** @var UserMapper | MockObject */ private $userMapper; @@ -170,7 +173,9 @@ public function dataProvisionUserGroups() { 'displayName' => 'groupName1' ] ], - ] + ], + '', + true, ], [ '1', @@ -179,26 +184,71 @@ public function dataProvisionUserGroups() { 'groups' => [ 'group2' ], - ] + ], + '', + true, + ], + [ + '1_Group_Import', + 'Imported from OIDC', + (object)[ + 'groups' => [ + (object)[ + 'gid' => '1_Group_Import', + 'displayName' => 'Imported from OIDC', + ], + (object)[ + 'gid' => '10_Group_NoImport', + 'displayName' => 'Not Imported', + ] + ], + ], + '/^1_/', + false + ], + [ + '1', + 'users_nextcloud', + (object)[ + 'groups' => [ + 'users_nextcloud', + 'users', + ], + ], + 'nextcloud', + false, ], ]; } /** @dataProvider dataProvisionUserGroups */ - public function testProvisionUserGroups(string $gid, string $displayName, object $payload): void { + public function testProvisionUserGroups(string $gid, string $displayName, object $payload, string $group_whitelist, bool $expect_delete_local_group): void { $user = $this->createMock(IUser::class); $group = $this->createMock(IGroup::class); + $local_group = $this->createMock(IGroup::class); $providerId = 421; $this->providerService ->method('getSetting') - ->with($providerId, ProviderService::SETTING_MAPPING_GROUPS, 'groups') - ->willReturn('groups'); + ->will($this->returnValueMap( + [ + [$providerId, ProviderService::SETTING_GROUP_WHITELIST_REGEX, '', $group_whitelist], + [$providerId, ProviderService::SETTING_MAPPING_GROUPS, 'groups', 'groups'], + ] + )); $this->groupManager ->method('getUserGroups') ->with($user) - ->willReturn([]); + ->willReturn([$local_group]); + + $local_group + ->method('getGID') + ->willReturn('local_group'); + + $local_group->expects($expect_delete_local_group ? self::once() : self::never()) + ->method('removeUser') + ->with($user); $this->idService->method('getId') ->willReturn($gid);