Add client_id and id_token_hint to IdP logout

This adds two parameters to the end_session_endpoint IdP URL which the
user gets redirected to when singleLogout is triggered.

These paramters are:
- client_id: the client ID of the current session's provider
- id_token_hint: the raw id_token that was obtained during the code
  callback of this session's login flow (set in session variable `oidc.id_token`).

Some providers (e.g. node-oidc-provider and Keycloak) require this when using the code OAuth flow.

Because passing id_token_hint reveals the id_token to the user agent, a
app setting was also added to optionally turn this behaviour off (default is
turned on).

Builds upon PR nextcloud#373 / issue nextcloud#336
Fixes issue nextcloud#449

lib/Command/UpsertProvider.php

@@ -59,6 +59,7 @@ protected function configure() {
 			->addOption('scope', 'o', InputOption::VALUE_OPTIONAL, 'OpenID requested value scopes, if not set defaults to "openid email profile"')
 			->addOption('unique-uid', null, InputOption::VALUE_OPTIONAL, 'Flag if unique user ids shall be used or not. 1 to enable (default), 0 to disable')
 			->addOption('check-bearer', null, InputOption::VALUE_OPTIONAL, 'Flag if Nextcloud API/WebDav calls should check the Bearer token against this provider or not. 1 to enable (default), 0 to disable')
+			->addOption('sendIdTokenHint', null, InputOption::VALUE_OPTIONAL, 'Flag if ID token should be included as a parameter to the end_session_endpoint URL when using unified logout. 1 to enable (default), 0 to disable')
 			->addOption('mapping-display-name', null, InputOption::VALUE_OPTIONAL, 'Attribute mapping of the display name')
 			->addOption('mapping-email', null, InputOption::VALUE_OPTIONAL, 'Attribute mapping of the email address')
 			->addOption('mapping-quota', null, InputOption::VALUE_OPTIONAL, 'Attribute mapping of the quota')
@@ -148,6 +149,9 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 		if (($checkBearer = $input->getOption('check-bearer')) !== null) {
 			$this->providerService->setSetting($provider->getId(), ProviderService::SETTING_CHECK_BEARER, (string)$checkBearer === '0' ? '0' : '1');
 		}
+		if (($sendIdTokenHint = $input->getOption('send-id-token-hint')) !== null) {
+			$this->providerService->setSetting($provider->getId(), ProviderService::SETTING_SEND_ID_TOKEN_HINT, (string)$sendIdTokenHint === '0' ? '0' : '1');
+		}
 		if (($uniqueUid = $input->getOption('unique-uid')) !== null) {
 			$this->providerService->setSetting($provider->getId(), ProviderService::SETTING_UNIQUE_UID, (string)$uniqueUid === '0' ? '0' : '1');
 		}

lib/Controller/LoginController.php

@@ -63,6 +63,7 @@ class LoginController extends Controller {
 	private const NONCE = 'oidc.nonce';
 	public const PROVIDERID = 'oidc.providerid';
 	private const REDIRECT_AFTER_LOGIN = 'oidc.redirect';
+	private const ID_TOKEN = 'oidc.id_token';
 
 	/** @var ISecureRandom */
 	private $random;
@@ -317,9 +318,10 @@ public function code($state = '', $code = '', $scope = '', $error = '', $error_d
 		$this->eventDispatcher->dispatchTyped(new TokenObtainedEvent($data, $provider, $discovery));
 
 		// TODO: proper error handling
+		$idTokenRaw = $data['id_token'];
 		$jwks = $this->discoveryService->obtainJWK($provider);
 		JWT::$leeway = 60;
-		$idTokenPayload = JWT::decode($data['id_token'], $jwks, array_keys(JWT::$supported_algs));
+		$idTokenPayload = JWT::decode($idTokenRaw, $jwks, array_keys(JWT::$supported_algs));
 
 		$this->logger->debug('Parsed the JWT payload: ' . json_encode($idTokenPayload, JSON_THROW_ON_ERROR));
 
@@ -354,6 +356,8 @@ public function code($state = '', $code = '', $scope = '', $error = '', $error_d
 			return new JSONResponse(['Failed to provision user']);
 		}
 
+		$this->session->set(self::ID_TOKEN, $idTokenRaw);
+
 		$this->logger->debug('Logging user in');
 
 		$this->userSession->setUser($user);
@@ -475,13 +479,26 @@ public function singleLogoutService() {
 		$oidcSystemConfig = $this->config->getSystemValue('user_oidc', []);
 		$targetUrl = $this->urlGenerator->getAbsoluteURL('/');
 		if (!isset($oidcSystemConfig['single_logout']) || $oidcSystemConfig['single_logout']) {
-			$providerId = (int)$this->session->get(self::PROVIDERID);
-			$provider = $this->providerMapper->getProvider($providerId);
-			$targetUrl = $this->discoveryService->obtainDiscovery($provider)['end_session_endpoint'] ?? $this->urlGenerator->getAbsoluteURL('/');
-			if ($targetUrl) {
-				$targetUrl .= '?post_logout_redirect_uri=' . $this->urlGenerator->getAbsoluteURL('/');
+			$providerId = $this->session->get(self::PROVIDERID);
+			if ($providerId) {
+				$provider = $this->providerMapper->getProvider((int)$providerId);
+				$endSessionEndpoint = $this->discoveryService->obtainDiscovery($provider)['end_session_endpoint'];
+				if ($endSessionEndpoint) {
+					$endSessionEndpoint .= '?post_logout_redirect_uri=' . $targetUrl;
+					$endSessionEndpoint .= '&client_id=' . $provider->getClientId();
+					$shouldSendIdToken = $this->providerService->getSetting(
+						$provider->getId(),
+						ProviderService::SETTING_SEND_ID_TOKEN_HINT, '0'
+					) === '1';
+					$idToken = $this->session->get(self::ID_TOKEN);
+					if ($shouldSendIdToken && $idToken) {
+						$endSessionEndpoint .= '&id_token_hint=' . $idToken;
+					}
+					$targetUrl = $endSessionEndpoint;
+				}
 			}
 		}
+
 		// cleanup related oidc session
 		$this->sessionMapper->deleteFromNcSessionId($this->session->getId());
 

lib/Service/ProviderService.php

@@ -34,6 +34,7 @@
 
 class ProviderService {
 	public const SETTING_CHECK_BEARER = 'checkBearer';
+	public const SETTING_SEND_ID_TOKEN_HINT = 'sendIdTokenHint';
 	public const SETTING_UNIQUE_UID = 'uniqueUid';
 	public const SETTING_MAPPING_UID = 'mappingUid';
 	public const SETTING_MAPPING_UID_DEFAULT = 'sub';
@@ -43,6 +44,10 @@ class ProviderService {
 	public const SETTING_EXTRA_CLAIMS = 'extraClaims';
 	public const SETTING_JWKS_CACHE = 'jwksCache';
 	public const SETTING_JWKS_CACHE_TIMESTAMP = 'jwksCacheTimestamp';
+	private const BOOLEAN_SETTINGS = array(
+		self::SETTING_UNIQUE_UID, self::SETTING_CHECK_BEARER,
+		self::SETTING_SEND_ID_TOKEN_HINT
+	);
 
 	/** @var IConfig */
 	private $config;
@@ -127,20 +132,21 @@ private function getSupportedSettings(): array {
 			self::SETTING_MAPPING_UID,
 			self::SETTING_UNIQUE_UID,
 			self::SETTING_CHECK_BEARER,
+			self::SETTING_SEND_ID_TOKEN_HINT,
 			self::SETTING_EXTRA_CLAIMS,
 		];
 	}
 
 	private function convertFromJSON(string $key, $value): string {
-		if ($key === self::SETTING_UNIQUE_UID || $key === self::SETTING_CHECK_BEARER) {
+		if (in_array($key, self::BOOLEAN_SETTINGS)) {
 			$value = $value ? '1' : '0';
 		}
 		return (string)$value;
 	}
 
 	private function convertToJSON(string $key, $value) {
 		// default is disabled (if not set)
-		if ($key === self::SETTING_UNIQUE_UID || $key === self::SETTING_CHECK_BEARER) {
+		if (in_array($key, self::BOOLEAN_SETTINGS)) {
 			return $value === '1';
 		}
 		return (string)$value;

src/components/AdminSettings.vue

@@ -143,6 +143,7 @@ export default {
 				settings: {
 					uniqueUid: true,
 					checkBearer: false,
+					sendIdTokenHint: true,
 				},
 			},
 			showNewProvider: false,

src/components/SettingsForm.vue

@@ -108,6 +108,12 @@
 		<p class="settings-hint">
 			{{ t('user_oidc', 'Do you want to allow API calls and WebDav request that are authenticated with an OIDC ID token or access token?') }}
 		</p>
+		<CheckboxRadioSwitch :checked.sync="localProvider.settings.sendIdTokenHint" wrapper-element="div">
+			{{ t('user_oidc', 'Send ID token hint on logout') }}
+		</CheckboxRadioSwitch>
+		<p class="settings-hint">
+			{{ t('user_oidc', 'Should the ID token be included as a parameter (id_token_hint) of the provider\'s logout URL? The user will be redirected to this URL after logout in Nextcloud. Enabling this exposes the ID token to the user agent, which may not be necessary depending on the particular OIDC provider used.') }}
+		</p>
 		<input type="button" :value="t('user_oidc', 'Cancel')" @click="$emit('cancel')">
 		<input type="submit" :value="submitText">
 	</form>

tests/unit/Service/ProviderServiceTest.php

@@ -82,6 +82,7 @@ public function testGetProvidersWithSettings() {
 					'mappingUid' => '1',
 					'uniqueUid' => true,
 					'checkBearer' => true,
+					'sendIdTokenHint' => true,
 					'extraClaims' => '1',
 				],
 			],
@@ -98,6 +99,7 @@ public function testGetProvidersWithSettings() {
 					'mappingUid' => '1',
 					'uniqueUid' => true,
 					'checkBearer' => true,
+					'sendIdTokenHint' => true,
 					'extraClaims' => '1',
 				],
 			],
@@ -112,6 +114,7 @@ public function testSetSettings() {
 			'mappingUid' => 'uid',
 			'uniqueUid' => true,
 			'checkBearer' => false,
+			'sendIdTokenHint' => true,
 			'extraClaims' => 'claim1 claim2',
 		];
 		$this->config->expects(self::any())
@@ -123,6 +126,7 @@ public function testSetSettings() {
 				[Application::APP_ID, 'provider-1-' . ProviderService::SETTING_MAPPING_UID, '', 'uid'],
 				[Application::APP_ID, 'provider-1-' . ProviderService::SETTING_UNIQUE_UID, '', '1'],
 				[Application::APP_ID, 'provider-1-' . ProviderService::SETTING_CHECK_BEARER, '', '0'],
+				[Application::APP_ID, 'provider-1-' . ProviderService::SETTING_SEND_ID_TOKEN_HINT, '', '1'],
 				[Application::APP_ID, 'provider-1-' . ProviderService::SETTING_EXTRA_CLAIMS, '', 'claim1 claim2'],
 			]);
 
@@ -140,6 +144,11 @@ public function testSetSettings() {
 			array_merge($defaults, ['checkBearer' => '1']),
 			$this->providerService->setSettings(1, [ProviderService::SETTING_CHECK_BEARER => '1'])
 		);
+
+		Assert::assertEquals(
+			array_merge($defaults, ['sendIdTokenHint' => '0']),
+			$this->providerService->setSettings(1, [ProviderService::SETTING_SEND_ID_TOKEN_HINT => '0'])
+		);
 	}
 
 	public function testDeleteSettings() {
@@ -188,6 +197,10 @@ public function dataConvertJson() {
 			[ProviderService::SETTING_CHECK_BEARER, true, '1', true],
 			[ProviderService::SETTING_CHECK_BEARER, false, '0', false],
 			[ProviderService::SETTING_CHECK_BEARER, 'test', '1', true],
+			// Setting sendIdTokenHint is a boolean
+			[ProviderService::SETTING_SEND_ID_TOKEN_HINT, true, '1', true],
+			[ProviderService::SETTING_SEND_ID_TOKEN_HINT, false, '0', false],
+			[ProviderService::SETTING_SEND_ID_TOKEN_HINT, 'test', '1', true],
 			// Any other values are just strings
 			[ProviderService::SETTING_MAPPING_EMAIL, false, '', false],
 			[ProviderService::SETTING_MAPPING_EMAIL, true, '1', true],