fix: remove long-unused desktop option

appinfo/app.php

@@ -110,31 +110,6 @@
 	$redirectSituation = true;
 }
 
-// If a request to OCS or remote.php is sent by the official desktop clients it can
-// be intercepted as it supports SAML. All other clients don't yet and thus we
-// require the usage of application specific passwords there.
-//
-// However, it is an opt-in setting to use SAML for the desktop clients. For better
-// UX (users don't have to reauthenticate) we default to disallow the access via
-// SAML at the moment.
-$useSamlForDesktopClients = $config->getAppValue('user_saml', 'general-use_saml_auth_for_desktop', '0');
-if ($useSamlForDesktopClients === '1') {
-	$currentUrl = substr(explode('?', $request->getRequestUri(), 2)[0], strlen(\OC::$WEBROOT));
-	if (substr($currentUrl, 0, 12) === '/remote.php/' || substr($currentUrl, 0, 5) === '/ocs/') {
-		if (!$userSession->isLoggedIn() && $request->isUserAgent([\OCP\IRequest::USER_AGENT_CLIENT_DESKTOP])) {
-			$redirectSituation = true;
-
-			if (preg_match('/^.*\/(\d+\.\d+\.\d+).*$/', $request->getHeader('USER_AGENT'), $matches) === 1) {
-				$versionstring = $matches[1];
-
-				if (version_compare($versionstring, '2.5.0', '>=') === true) {
-					$redirectSituation = false;
-				}
-			}
-		}
-	}
-}
-
 $multipleUserBackEnds = $samlSettings->allowMultipleUserBackEnds();
 $configuredIdps = $samlSettings->getListOfIdps();
 $showLoginOptions = ($multipleUserBackEnds || count($configuredIdps) > 1) && $type === 'saml';

appinfo/info.xml

@@ -43,6 +43,9 @@ While theoretically any other authentication provider implementing either one of
 		<post-migration>
 			<step>OCA\User_SAML\Migration\RememberLocalGroupsForPotentialMigrations</step>
 		</post-migration>
+		<live-migration>
+			<step>OCA\User_SAML\Migration\CleanupRemovedConfig</step>
+		</live-migration>
 	</repair-steps>
 	<commands>
 		<command>OCA\User_SAML\Command\ConfigCreate</command>

lib/Migration/CleanupRemovedConfig.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OCA\User_SAML\Migration;
+
+use OCP\IConfig;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+
+class CleanupRemovedConfig implements IRepairStep {
+
+	public function __construct(protected IConfig $config) {
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	public function getName() {
+		return 'Cleans up config keys that are not used anymore';
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	public function run(IOutput $output) {
+		$this->config->deleteAppValue('user_saml', 'general-use_saml_auth_for_desktop');
+	}
+}

lib/SAMLSettings.php

@@ -20,7 +20,7 @@ class SAMLSettings {
 	private const LOADED_ALL = 2;
 
 	// list of global settings which are valid for every idp:
-	// 'general-require_provisioned_account', 'general-allow_multiple_user_back_ends', 'general-use_saml_auth_for_desktop'
+	// 'general-require_provisioned_account', 'general-allow_multiple_user_back_ends'
 
 	// IdP-specific keys
 	public const IDP_CONFIG_KEYS = [

lib/Settings/Admin.php

@@ -192,13 +192,7 @@ public function getForm() {
 				'text' => $this->l10n->t('Only allow authentication if an account exists on some other backend (e.g. LDAP).', [$this->defaults->getName()]),
 				'type' => 'checkbox',
 				'global' => true,
-				'value' => $this->config->getAppValue('user_saml', 'general-require_provisioned_account', 0)
-			];
-			$generalSettings['use_saml_auth_for_desktop'] = [
-				'text' => $this->l10n->t('Use SAML auth for the %s desktop clients (requires user re-authentication)', [$this->defaults->getName()]),
-				'type' => 'checkbox',
-				'global' => true,
-				'value' => $this->config->getAppValue('user_saml', 'general-use_saml_auth_for_desktop', 0)
+				'value' => $this->config->getAppValue('user_saml', 'general-require_provisioned_account', '0')
 			];
 			$generalSettings['idp0_display_name'] = [
 				'text' => $this->l10n->t('Optional display name of the identity provider (default: "SSO & SAML log in")'),
@@ -210,7 +204,7 @@ public function getForm() {
 				'type' => 'checkbox',
 				'hideForEnv' => true,
 				'global' => true,
-				'value' => $this->config->getAppValue('user_saml', 'general-allow_multiple_user_back_ends')
+				'value' => $this->config->getAppValue('user_saml', 'general-allow_multiple_user_back_ends', '0')
 			];
 		}
 

tests/integration/features/bootstrap/FeatureContext.php

@@ -111,7 +111,6 @@ public function theSettingIsSetTo($settingName,
 			'type',
 			'general-require_provisioned_account',
 			'general-allow_multiple_user_back_ends',
-			'general-use_saml_auth_for_desktop',
 			'localGroupsCheckForMigration',
 		])) {
 			$this->changedSettings[] = $settingName;

tests/unit/Settings/AdminTest.php

@@ -47,9 +47,9 @@ public function formDataProvider() {
 		$this->l10n
 			->expects($this->any())
 			->method('t')
-			->will($this->returnCallback(function ($text, $parameters = []) {
+			->willReturnCallback(function ($text, $parameters = []) {
 				return vsprintf($text, $parameters);
-			}));
+			});
 
 		$serviceProviderFields = [
 			'x509cert' => 'X.509 certificate of the Service Provider',
@@ -94,11 +94,6 @@ public function formDataProvider() {
 				'type' => 'checkbox',
 				'global' => true,
 			],
-			'use_saml_auth_for_desktop' => [
-				'text' => 'Use SAML auth for the Nextcloud desktop clients (requires user re-authentication)',
-				'type' => 'checkbox',
-				'global' => true,
-			],
 			'allow_multiple_user_back_ends' => [
 				'text' => $this->l10n->t('Allow the use of multiple user back-ends (e.g. LDAP)'),
 				'type' => 'checkbox',
@@ -231,7 +226,6 @@ public function testGetFormWithoutType() {
 			->willReturn('');
 
 		$params = $this->formDataProvider();
-		unset($params['general']['use_saml_auth_for_desktop']);
 		unset($params['general']['idp0_display_name']);
 		unset($params['general']['allow_multiple_user_back_ends']);
 		$params['type'] = '';
@@ -248,25 +242,23 @@ public function testGetFormWithSaml() {
 				2 => 'Provider 2',
 			]);
 		$this->config
-			->expects($this->exactly(4)) # mode + three global values
+			->expects($this->exactly(3)) # mode + three global values
 			->method('getAppValue')
 			->withConsecutive(
 				['user_saml', 'type'],
 				['user_saml', 'general-require_provisioned_account'],
-				['user_saml', 'general-use_saml_auth_for_desktop'],
 				['user_saml', 'general-allow_multiple_user_back_ends'],
 			)
-			->willReturnOnConsecutiveCalls('saml', 0, 0, '');
+			->willReturnOnConsecutiveCalls('saml', '0', '0');
 		$this->defaults
 			->expects($this->any())
 			->method('getName')
 			->willReturn('Nextcloud');
 
 		$params = $this->formDataProvider();
 		$params['type'] = 'saml';
-		$params['general']['require_provisioned_account']['value'] = 0;
-		$params['general']['use_saml_auth_for_desktop']['value'] = 0;
-		$params['general']['allow_multiple_user_back_ends']['value'] = '';
+		$params['general']['require_provisioned_account']['value'] = '0';
+		$params['general']['allow_multiple_user_back_ends']['value'] = '0';
 
 		$expected = new TemplateResponse('user_saml', 'admin', $params);
 		$this->assertEquals($expected, $this->admin->getForm());