Skip to content
Merged
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
12 changes: 12 additions & 0 deletions lib/Controller/LoginController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion lib/Service/ProviderService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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,
];
}

Expand Down
39 changes: 37 additions & 2 deletions lib/Service/ProvisioningService.php
Original file line number Diff line number Diff line change
Expand Up @@ -385,18 +385,19 @@ 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');

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) {
Expand All @@ -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);
}
}
Expand All @@ -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;
}
}
15 changes: 15 additions & 0 deletions src/components/SettingsForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,21 @@
<p class="settings-hint">
{{ 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", ...]') }}
</p>
<p>
<label for="group-whitelist-regex">{{ t('user_oidc', 'Group whitelist regex') }}</label>
<input id="group-whitelist-regex"
v-model="localProvider.settings.groupWhitelistRegex"
type="text">
</p>
<p class="settings-hint">
{{ 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' }) }}
</p>
<NcCheckboxRadioSwitch :checked.sync="localProvider.settings.restrictLoginToGroups" wrapper-element="div">
{{ t('user_oidc', 'Restrict login for users that are not in any whitelisted group') }}
</NcCheckboxRadioSwitch>
<p class="settings-hint">
{{ t('user_oidc', 'Users that are not part of any whitelisted group are not created and can not login') }}
</p>
<NcCheckboxRadioSwitch :checked.sync="localProvider.settings.checkBearer" wrapper-element="div">
{{ t('user_oidc', 'Check Bearer token on API and WebDav requests') }}
</NcCheckboxRadioSwitch>
Expand Down
3 changes: 0 additions & 3 deletions tests/psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,5 @@
<MissingDependency>
<code><![CDATA[Image]]></code>
</MissingDependency>
<RedundantCondition>
<code><![CDATA[isset($group->displayName)]]></code>
</RedundantCondition>
</file>
</files>
3 changes: 3 additions & 0 deletions tests/unit/Db/UserMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ class UserMapperTest extends TestCase {
/** @var LocalIdService|MockObject */
private $idService;

/** @var IDBConnection|MockObject */
private $db;

/** @var UserMapper|MockObject */
private $userMapper;

Expand Down
8 changes: 8 additions & 0 deletions tests/unit/Service/ProviderServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ public function testGetProvidersWithSettings() {
'extraClaims' => '1',
'providerBasedId' => true,
'groupProvisioning' => true,
'groupWhitelistRegex' => '1',
'restrictLoginToGroups' => true,
],
],
[
Expand Down Expand Up @@ -126,6 +128,8 @@ public function testGetProvidersWithSettings() {
'extraClaims' => '1',
'providerBasedId' => true,
'groupProvisioning' => true,
'groupWhitelistRegex' => '1',
'restrictLoginToGroups' => true,
],
],
], $this->providerService->getProvidersWithSettings());
Expand Down Expand Up @@ -161,6 +165,8 @@ public function testSetSettings() {
'mappingBiography' => 'biography',
'mappingPhonenumber' => 'phone',
'mappingGender' => 'gender',
'groupWhitelistRegex' => '',
'restrictLoginToGroups' => false,
];
$this->config->expects(self::any())
->method('getAppValue')
Expand Down Expand Up @@ -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(
Expand Down
62 changes: 56 additions & 6 deletions tests/unit/Service/ProvisioningServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ class ProvisioningServiceTest extends TestCase {
/** @var ProvisioningService | MockObject */
private $providerService;

/** @var LdapService | MockObject */
private $ldapService;

/** @var UserMapper | MockObject */
private $userMapper;

Expand Down Expand Up @@ -170,7 +173,9 @@ public function dataProvisionUserGroups() {
'displayName' => 'groupName1'
]
],
]
],
'',
true,
],
[
'1',
Expand All @@ -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);
Expand Down