[stable-7] fix: Use assertGroupMemberships also in the DAV plugin

lib/AppInfo/Application.php

@@ -25,6 +25,7 @@
 use OCA\User_SAML\SAMLSettings;
 use OCA\User_SAML\Service\SessionService;
 use OCA\User_SAML\UserBackend;
+use OCA\User_SAML\UserData;
 use OCP\AppFramework\App;
 use OCP\AppFramework\Bootstrap\IBootContext;
 use OCP\AppFramework\Bootstrap\IBootstrap;
@@ -71,6 +72,8 @@ public function register(IRegistrationContext $context): void {
 			$_SERVER,
 			$c->get(SAMLSettings::class),
 			$c->get(SessionService::class),
+			$c->get(UserData::class),
+			$c->get(UserBackend::class),
 		));
 	}
 

lib/Controller/SAMLController.php

@@ -77,7 +77,7 @@ private function autoprovisionIfPossible(): void {
 			throw new NoUserFoundException('IDP parameter for the UID not found. Possible parameters are: ' . json_encode(array_keys($auth)));
 		}
 
-		$this->assertGroupMemberships();
+		$this->userBackend->assertGroupMemberships();
 
 		if ($this->userData->getOriginalUid() === '') {
 			$this->logger->error('Given UID is not valid, please check your attribute mapping', ['app' => $this->appName]);
@@ -114,34 +114,6 @@ private function autoprovisionIfPossible(): void {
 		}
 	}
 
-	/**
-	 * @throws UserFilterViolationException
-	 */
-	protected function assertGroupMemberships(): void {
-		if (!$this->userBackend->autoprovisionAllowed()) {
-			// return early, when users are provided by a different backend
-			// - mappings are not available/configurable in that case
-			// - control is solely based on presence and enabled-state of the user
-			return;
-		}
-
-		$groups = $this->userData->getGroups();
-		$settings = $this->samlSettings->get($this->session->get('user_saml.Idp') ?? 1);
-
-		$rejectGroupsString = $settings['saml-user-filter-reject_groups'] ?? '';
-		$rejectGroups = array_map('trim', explode(',', $rejectGroupsString));
-
-		if (!empty(array_intersect($groups, $rejectGroups))) {
-			throw new UserFilterViolationException('User is member of a rejection group.');
-		}
-
-		$requireGroupsString = trim($settings['saml-user-filter-require_groups'] ?? '');
-		$requireGroups = array_map('trim', explode(',', $requireGroupsString));
-		if (!empty($requireGroupsString) && empty(array_intersect($groups, $requireGroups))) {
-			throw new UserFilterViolationException('User is not member of a required group.');
-		}
-	}
-
 	/**
 	 * @PublicPage
 	 * @UseSession

lib/DavPlugin.php

@@ -8,6 +8,7 @@
 namespace OCA\User_SAML;
 
 use OCA\DAV\Connector\Sabre\Auth;
+use OCA\User_SAML\Exceptions\UserFilterViolationException;
 use OCA\User_SAML\Service\SessionService;
 use OCP\IAppConfig;
 use OCP\ISession;
@@ -26,6 +27,8 @@ public function __construct(
 		private readonly array $auth,
 		private readonly SAMLSettings $samlSettings,
 		private readonly SessionService $sessionService,
+		private readonly UserData $userData,
+		private readonly UserBackend $userBackend,
 	) {
 	}
 
@@ -42,6 +45,12 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
 		) {
 			$uidMapping = $this->samlSettings->get(1)['general-uid_mapping'];
 			if (isset($this->auth[$uidMapping])) {
+				$this->userData->setAttributes($this->auth);
+				try {
+					$this->userBackend->assertGroupMemberships();
+				} catch (UserFilterViolationException) {
+					return;
+				}
 				$this->session->set(Auth::DAV_AUTHENTICATED, $this->auth[$uidMapping]);
 				$this->sessionService->prepareEnvironmentBasedSession($this->auth);
 			}

lib/UserBackend.php

@@ -8,6 +8,7 @@
 namespace OCA\User_SAML;
 
 use OC\Security\CSRF\CsrfTokenManager;
+use OCA\User_SAML\Exceptions\UserFilterViolationException;
 use OCA\User_SAML\Model\SessionData;
 use OCP\Authentication\IApacheBackend;
 use OCP\EventDispatcher\IEventDispatcher;
@@ -412,6 +413,36 @@ public function getBackendName() {
 		return 'user_saml';
 	}
 
+	/**
+	 * Checks the group filter against the attributes set on UserData.
+	 *
+	 * @throws UserFilterViolationException
+	 */
+	public function assertGroupMemberships(): void {
+		if (!$this->autoprovisionAllowed()) {
+			// return early, when users are provided by a different backend
+			// - mappings are not available/configurable in that case
+			// - control is solely based on presence and enabled-state of the user
+			return;
+		}
+
+		$groups = $this->userData->getGroups();
+		$settings = $this->settings->get($this->session->get('user_saml.Idp') ?? 1);
+
+		$rejectGroupsString = $settings['saml-user-filter-reject_groups'] ?? '';
+		$rejectGroups = array_map('trim', explode(',', $rejectGroupsString));
+
+		if (!empty(array_intersect($groups, $rejectGroups))) {
+			throw new UserFilterViolationException('User is member of a rejection group.');
+		}
+
+		$requireGroupsString = trim($settings['saml-user-filter-require_groups'] ?? '');
+		$requireGroups = array_map('trim', explode(',', $requireGroupsString));
+		if (!empty($requireGroupsString) && empty(array_intersect($groups, $requireGroups))) {
+			throw new UserFilterViolationException('User is not member of a required group.');
+		}
+	}
+
 	/**
 	 * Whether autoprovisioning is enabled or not
 	 */

tests/unit/Controller/SAMLControllerTest.php

@@ -10,7 +10,6 @@
 use Exception;
 use OCA\User_SAML\Controller\SAMLController;
 use OCA\User_SAML\Exceptions\NoUserFoundException;
-use OCA\User_SAML\Exceptions\UserFilterViolationException;
 use OCA\User_SAML\SAMLSettings;
 use OCA\User_SAML\Service\SessionService;
 use OCA\User_SAML\UserBackend;
@@ -368,87 +367,6 @@ public function dataTestGetSSODisplayName() {
 		];
 	}
 
-	public function userFilterDataProvider(): array {
-		return [
-			[ // 0 - test rejection by membership
-				'Group C',
-				'Group A, Group B',
-				true,
-				'User is member of a rejection group.',
-			],
-			[ // 1 - test rejection by required membership
-				'Group D',
-				'Group B',
-				true,
-				'User is not member of a required group.',
-			],
-			[ // 2 - test satisfy all requirements
-				'Group D',
-				'Group A',
-				false,
-			],
-			[ // 3 - test no filtering
-				null,
-				null,
-				false,
-			],
-		];
-	}
-
-	/**
-	 * @dataProvider userFilterDataProvider
-	 */
-	public function testUserFilter(?string $rejectGroups, ?string $requireGroups, bool $isException, string $message = ''): void {
-		$this->userData->expects($this->any())
-			->method('getGroups')
-			->willReturn(['Group A', 'Group C']);
-
-		$this->session->expects($this->any())
-			->method('get')
-			->with('user_saml.Idp')
-			->willReturn(1);
-
-		$settings = [];
-		if ($rejectGroups !== null && $requireGroups !== null) {
-			$settings = [
-				'saml-user-filter-reject_groups' => $rejectGroups,
-				'saml-user-filter-require_groups' => $requireGroups,
-			];
-		}
-		$this->samlSettings->expects($this->any())
-			->method('get')
-			->with(1)
-			->willReturn($settings);
-
-		$this->userBackend->expects($this->any())
-			->method('autoprovisionAllowed')
-			->willReturn(true);
-
-		if ($isException) {
-			$this->expectException(UserFilterViolationException::class);
-			$this->expectExceptionMessage($message);
-		} else {
-			// Nothing to assert other than no exception being thrown
-			$this->assertTrue(true);
-		}
-
-		$this->invokePrivate($this->samlController, 'assertGroupMemberships');
-	}
-
-	public function testUserFilterNotApplicable(): void {
-		$this->userData->expects($this->never())
-			->method('getGroups');
-
-		$this->session->expects($this->never())
-			->method('get');
-
-		$this->userBackend->expects($this->any())
-			->method('autoprovisionAllowed')
-			->willReturn(false);
-
-		$this->invokePrivate($this->samlController, 'assertGroupMemberships');
-	}
-
 	public function testLoginWithUnconfiguredIdpRedirectsToGenericError(): void {
 		$this->config->expects($this->once())
 			->method('getAppValue')

tests/unit/DavPluginTest.php

@@ -0,0 +1,86 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\User_SAML\Tests;
+
+use OCA\User_SAML\DavPlugin;
+use OCA\User_SAML\Exceptions\UserFilterViolationException;
+use OCA\User_SAML\SAMLSettings;
+use OCA\User_SAML\Service\SessionService;
+use OCA\User_SAML\UserBackend;
+use OCA\User_SAML\UserData;
+use OCP\IAppConfig;
+use OCP\ISession;
+use Override;
+use PHPUnit\Framework\MockObject\MockObject;
+use Sabre\HTTP\RequestInterface;
+use Sabre\HTTP\ResponseInterface;
+use Test\TestCase;
+
+class DavPluginTest extends TestCase {
+	private ISession&MockObject $session;
+	private SessionService&MockObject $sessionService;
+	private UserData&MockObject $userData;
+	private UserBackend&MockObject $userBackend;
+	private DavPlugin $davPlugin;
+
+	#[Override]
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->session = $this->createMock(ISession::class);
+		$this->sessionService = $this->createMock(SessionService::class);
+		$this->userData = $this->createMock(UserData::class);
+		$this->userBackend = $this->createMock(UserBackend::class);
+
+		$appConfig = $this->createMock(IAppConfig::class);
+		$appConfig->method('getValueString')
+			->with('user_saml', 'type', 'unset')
+			->willReturn('environment-variable');
+		$samlSettings = $this->createMock(SAMLSettings::class);
+		$samlSettings->method('get')
+			->with(1)
+			->willReturn(['general-uid_mapping' => 'REMOTE_USER']);
+
+		$this->davPlugin = new DavPlugin(
+			$this->session,
+			$appConfig,
+			['REMOTE_USER' => 'alice'],
+			$samlSettings,
+			$this->sessionService,
+			$this->userData,
+			$this->userBackend,
+		);
+	}
+
+	public function testBeforeMethodPreparesSession(): void {
+		$this->userData->expects($this->once())
+			->method('setAttributes')
+			->with(['REMOTE_USER' => 'alice']);
+		$this->userBackend->expects($this->once())
+			->method('assertGroupMemberships');
+		$this->session->expects($this->once())
+			->method('set');
+		$this->sessionService->expects($this->once())
+			->method('prepareEnvironmentBasedSession')
+			->with(['REMOTE_USER' => 'alice']);
+
+		$this->davPlugin->beforeMethod($this->createMock(RequestInterface::class), $this->createMock(ResponseInterface::class));
+	}
+
+	public function testBeforeMethodRespectsUserFilter(): void {
+		$this->userBackend->expects($this->once())
+			->method('assertGroupMemberships')
+			->willThrowException(new UserFilterViolationException('User is member of a rejection group.'));
+		$this->session->expects($this->never())
+			->method('set');
+		$this->sessionService->expects($this->never())
+			->method('prepareEnvironmentBasedSession');
+
+		$this->davPlugin->beforeMethod($this->createMock(RequestInterface::class), $this->createMock(ResponseInterface::class));
+	}
+}