diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 9842067f..b24a24a3 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -1,6 +1,7 @@ setName('user_oidc:provider:delete') ->setDescription('Delete an OpenId connect provider') @@ -39,26 +40,35 @@ protected function configure() { parent::configure(); } - protected function execute(InputInterface $input, OutputInterface $output) { + protected function execute(InputInterface $input, OutputInterface $output): int { try { $identifier = $input->getArgument('identifier'); + try { $provider = $this->providerMapper->findProviderByIdentifier($identifier); } catch (DoesNotExistException $e) { $output->writeln('Provider not found.'); return -1; } + $helper = $this->getHelper('question'); - $question = new ConfirmationQuestion('Are you sure you want to delete OpenID Provider "' . $provider->getIdentifier() . '"? It may invalidate all associated user accounts [y/N] ', false); - if ($input->getOption('force') || $helper->ask($input, $output, $question)) { - $this->providerMapper->delete($provider); - $this->providerService->deleteSettings($provider->getId()); - $output->writeln('"' . $provider->getIdentifier() . '" has been deleted.'); + $question = new ConfirmationQuestion( + 'Are you sure you want to delete OpenID Provider "' . $provider->getIdentifier() . '"? It may invalidate all associated user accounts [y/N] ', + false + ); + + if (!$input->getOption('force') && !$helper->ask($input, $output, $question)) { + return 0; } + + $this->providerMapper->delete($provider); + $this->providerService->deleteSettings($provider->getId()); + $output->writeln('"' . $provider->getIdentifier() . '" has been deleted.'); + + return 0; } catch (Exception $e) { - $output->writeln($e->getMessage()); + $output->writeln('' . $e->getMessage() . ''); return -1; } - return 0; } } diff --git a/lib/Command/ListProviders.php b/lib/Command/ListProviders.php index 02966341..3fe81aff 100644 --- a/lib/Command/ListProviders.php +++ b/lib/Command/ListProviders.php @@ -1,6 +1,7 @@ setName('user_oidc:providers') ->setDescription('List all providers and print their configuration') @@ -35,7 +36,7 @@ protected function configure() { parent::configure(); } - protected function execute(InputInterface $input, OutputInterface $output) { + protected function execute(InputInterface $input, OutputInterface $output): int { $outputFormat = $input->getOption('output') ?? 'json_pretty'; $sensitive = $input->getOption('sensitive'); @@ -44,6 +45,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { $providersWithSettings = array_map(function ($provider) use ($sensitive) { $providerSettings = $this->providerService->getSettings($provider->getId()); $serializedProvider = $provider->jsonSerialize(); + if ($sensitive) { $serializedProvider['clientId'] = '********'; $serializedProvider['clientSecret'] = '********'; @@ -51,35 +53,31 @@ protected function execute(InputInterface $input, OutputInterface $output) { $discoveryDomainName = parse_url($serializedProvider['discoveryEndpoint'], PHP_URL_HOST); $serializedProvider['discoveryEndpoint'] = str_replace($discoveryDomainName, '********', $serializedProvider['discoveryEndpoint']); } catch (\Exception|\Throwable) { + // Failed to parse URL, keep original endpoint } } else { $serializedProvider['clientSecret'] = $this->crypto->decrypt($provider->getClientSecret()); } + return array_merge($serializedProvider, ['settings' => $providerSettings]); }, $providers); - if ($outputFormat === 'json') { - foreach ($providersWithSettings as $provider) { - $output->writeln(json_encode($provider, JSON_THROW_ON_ERROR)); - } - return 0; - } + $jsonFlags = match ($outputFormat) { + 'json' => JSON_THROW_ON_ERROR, + 'json_pretty' => JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT, + default => null, + }; - if ($outputFormat === 'json_pretty') { + if ($jsonFlags !== null) { foreach ($providersWithSettings as $provider) { - $output->writeln(json_encode($provider, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT)); + $output->writeln(json_encode($provider, $jsonFlags)); } return 0; } - $output->writeln( - 'Only "' . self::OUTPUT_FORMAT_JSON . '" and "' . self::OUTPUT_FORMAT_JSON_PRETTY . '" output formats are supported.', - ); + $output->writeln('Only "' . self::OUTPUT_FORMAT_JSON . '" and "' . self::OUTPUT_FORMAT_JSON_PRETTY . '" output formats are supported.'); + $output->writeln('Use "--output=' . self::OUTPUT_FORMAT_JSON . '" or "--output=' . self::OUTPUT_FORMAT_JSON_PRETTY . '" (default format is "' . self::OUTPUT_FORMAT_JSON_PRETTY . '")'); - $output->writeln( - 'Use "--output=' . self::OUTPUT_FORMAT_JSON . '" or "--output=' . self::OUTPUT_FORMAT_JSON_PRETTY . '" ' - . '(default format is "' . self::OUTPUT_FORMAT_JSON_PRETTY . '")', - ); return 0; } } diff --git a/lib/Command/UpsertProvider.php b/lib/Command/UpsertProvider.php index f6690046..d3574245 100644 --- a/lib/Command/UpsertProvider.php +++ b/lib/Command/UpsertProvider.php @@ -1,6 +1,7 @@ setName('user_oidc:provider') ->setDescription('Create, show or update a OpenId connect provider config given the identifier of a provider') @@ -188,10 +189,14 @@ protected function configure() { parent::configure(); } - protected function execute(InputInterface $input, OutputInterface $output) { + protected function execute(InputInterface $input, OutputInterface $output): int { $outputFormat = $input->getOption('output') ?? 'table'; - $identifier = $input->getArgument('identifier'); + + if ($identifier === null) { + return $this->listProviders($input, $output); + } + $clientid = $input->getOption('clientid'); $clientsecret = $input->getOption('clientsecret'); if ($clientsecret !== null) { @@ -202,108 +207,137 @@ protected function execute(InputInterface $input, OutputInterface $output) { $postLogoutUri = $input->getOption('postlogouturi'); $scope = $input->getOption('scope'); - if ($identifier === null) { - return $this->listProviders($input, $output); - } - - // check if any option for updating is provided + // Check if any option for updating is provided $updateOptions = array_filter($input->getOptions(), static function ($value, $option) { return in_array($option, [ 'identifier', 'clientid', 'clientsecret', 'discoveryuri', 'endsessionendpointuri', 'postlogouturi', 'scope', ...array_keys(self::EXTRA_OPTIONS), - ]) && $value !== null; + ], true) && $value !== null; }, ARRAY_FILTER_USE_BOTH); if (count($updateOptions) === 0) { - try { - $provider = $this->providerMapper->findProviderByIdentifier($identifier); - } catch (DoesNotExistException $e) { - $output->writeln('Provider not found'); - return -1; - } - $provider = $this->providerService->getProviderWithSettings($provider->getId()); - if ($outputFormat === 'json') { - $output->writeln(json_encode($provider, JSON_THROW_ON_ERROR)); - return 0; - } + return $this->displayProvider($identifier, $outputFormat, $output); + } - if ($outputFormat === 'json_pretty') { - $output->writeln(json_encode($provider, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT)); - return 0; - } + return $this->updateOrCreateProvider($input, $output, $identifier, $clientid, $clientsecret, $discoveryuri, $scope, $endsessionendpointuri, $postLogoutUri); + } + + private function displayProvider(string $identifier, string $outputFormat, OutputInterface $output): int { + try { + $provider = $this->providerMapper->findProviderByIdentifier($identifier); + } catch (DoesNotExistException) { + $output->writeln('Provider not found'); + return 1; + } + + $provider = $this->providerService->getProviderWithSettings($provider->getId()); - $provider['settings'] = json_encode($provider['settings']); - $table = new Table($output); - $table->setHeaders(['ID', 'Identifier', 'Client ID', 'Discovery endpoint', 'End session endpoint', 'Advanced settings']); - $table->addRow($provider); - $table->render(); + $jsonFlags = match ($outputFormat) { + 'json' => JSON_THROW_ON_ERROR, + 'json_pretty' => JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT, + default => null, + }; + + if ($jsonFlags !== null) { + $output->writeln(json_encode($provider, $jsonFlags)); return 0; } + $provider['settings'] = json_encode($provider['settings']); + $table = new Table($output); + $table->setHeaders(['ID', 'Identifier', 'Client ID', 'Discovery endpoint', 'End session endpoint', 'Advanced settings']); + $table->addRow($provider); + $table->render(); + + return 0; + } + + private function updateOrCreateProvider( + InputInterface $input, + OutputInterface $output, + string $identifier, + ?string $clientid, + ?string $clientsecret, + ?string $discoveryuri, + ?string $scope, + ?string $endsessionendpointuri, + ?string $postLogoutUri, + ): int { $provider = $this->providerService->getProviderByIdentifier($identifier); + if ($provider !== null) { - // existing provider, keep values that are not set, the scope has to be set anyway - $scope = $scope ?? $provider->getScope(); + // Existing provider, keep values that are not set, the scope has to be set anyway + $scope ??= $provider->getScope(); } else { - // new provider default scope value - $scope = $scope ?? 'openid email profile'; + // New provider default scope value + $scope ??= 'openid email profile'; } + try { $provider = $this->providerMapper->createOrUpdateProvider( $identifier, $clientid, $clientsecret, $discoveryuri, $scope, $endsessionendpointuri, $postLogoutUri ); - // invalidate JWKS cache (even if it was just created) + + // Invalidate JWKS cache (even if it was just created) $this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE, ''); $this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE_TIMESTAMP, ''); } catch (DoesNotExistException|MultipleObjectsReturnedException $e) { $output->writeln('' . $e->getMessage() . ''); - return -1; + return 1; } + foreach (self::EXTRA_OPTIONS as $name => $option) { - if (($value = $input->getOption($name)) !== null) { - if (array_key_exists($option['setting_key'], ProviderService::BOOLEAN_SETTINGS_DEFAULT_VALUES)) { - $value = (string)$value === '0' ? '0' : '1'; - } else { - $value = (string)$value; - } - $this->providerService->setSetting($provider->getId(), $option['setting_key'], $value); + $value = $input->getOption($name); + if ($value === null) { + continue; + } + + if (array_key_exists($option['setting_key'], ProviderService::BOOLEAN_SETTINGS_DEFAULT_VALUES)) { + $value = (string)$value === '0' ? '0' : '1'; + } else { + $value = (string)$value; } + + $this->providerService->setSetting($provider->getId(), $option['setting_key'], $value); } + return 0; } - private function listProviders(InputInterface $input, OutputInterface $output) { + private function listProviders(InputInterface $input, OutputInterface $output): int { $outputFormat = $input->getOption('output') ?? 'table'; $providers = $this->providerMapper->getProviders(); - if ($outputFormat === 'json') { - $output->writeln(json_encode($providers, JSON_THROW_ON_ERROR)); + if (count($providers) === 0) { + $output->writeln('No providers configured'); return 0; } - if ($outputFormat === 'json_pretty') { - $output->writeln(json_encode($providers, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT)); - return 0; - } + $jsonFlags = match ($outputFormat) { + 'json' => JSON_THROW_ON_ERROR, + 'json_pretty' => JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT, + default => null, + }; - if (count($providers) === 0) { - $output->writeln('No providers configured'); + if ($jsonFlags !== null) { + $output->writeln(json_encode($providers, $jsonFlags)); return 0; } $table = new Table($output); $table->setHeaders(['ID', 'Identifier', 'Discovery endpoint', 'End session endpoint', 'Client ID']); - $providers = array_map(function ($provider) { - return [ + $table->setRows(array_map( + fn ($provider) => [ $provider->getId(), $provider->getIdentifier(), $provider->getDiscoveryEndpoint(), $provider->getEndSessionEndpoint(), $provider->getClientId() - ]; - }, $providers); - $table->setRows($providers); + ], + $providers + )); $table->render(); + return 0; } } diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index fcfea413..c57032b5 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1,6 +1,7 @@ setQuota($quota); } - $userFolder = $this->root->getUserFolder($user->getUID()); + $uid = $user->getUID(); + $userFolder = $this->root->getUserFolder($uid); try { // copy skeleton - \OC_Util::copySkeleton($user->getUID(), $userFolder); + \OC_Util::copySkeleton($uid, $userFolder); } catch (NotPermittedException $ex) { // read only uses } - return new DataResponse(['user_id' => $user->getUID()]); + return new DataResponse(['user_id' => $uid]); } /** @@ -77,7 +79,7 @@ public function createUser(int $providerId, string $userId, ?string $displayName #[NoCSRFRequired] public function deleteUser(string $userId): DataResponse { $user = $this->userManager->get($userId); - if (is_null($user) || $user->getBackendClassName() !== Application::APP_ID) { + if ($user === null || $user->getBackendClassName() !== Application::APP_ID) { return new DataResponse(['message' => 'User not found'], Http::STATUS_NOT_FOUND); } diff --git a/lib/Controller/BaseOidcController.php b/lib/Controller/BaseOidcController.php index 619e0e56..3358b58c 100644 --- a/lib/Controller/BaseOidcController.php +++ b/lib/Controller/BaseOidcController.php @@ -1,5 +1,7 @@ config->getSystemValueBool('debug', false); } - /** - * @param string $message - * @param int $statusCode - * @param array $throttleMetadata - * @param bool|null $throttle - * @return TemplateResponse - */ - protected function buildErrorTemplateResponse(string $message, int $statusCode, array $throttleMetadata = [], ?bool $throttle = null): TemplateResponse { + protected function buildErrorTemplateResponse( + string $message, + int $statusCode, + array $throttleMetadata = [], + ?bool $throttle = null, + ): TemplateResponse { $params = [ 'message' => $message, 'title' => $this->l->t('Error'), ]; + return $this->buildFailureTemplateResponse($params, $statusCode, $throttleMetadata, $throttle); } - /** - * @param string $message - * @param int $statusCode - * @param array $throttleMetadata - * @param bool|null $throttle - * @return TemplateResponse - */ - protected function build403TemplateResponse(string $message, int $statusCode, array $throttleMetadata = [], ?bool $throttle = null): TemplateResponse { + protected function build403TemplateResponse( + string $message, + int $statusCode, + array $throttleMetadata = [], + ?bool $throttle = null, + ): TemplateResponse { $params = [ 'message' => $message, 'title' => $this->l->t('Access forbidden'), ]; + return $this->buildFailureTemplateResponse($params, $statusCode, $throttleMetadata, $throttle); } - /** - * @param array $params - * @param int $statusCode - * @param array $throttleMetadata - * @param bool|null $throttle - * @return TemplateResponse - */ protected function buildFailureTemplateResponse( - array $params, int $statusCode, array $throttleMetadata = [], ?bool $throttle = null, + array $params, + int $statusCode, + array $throttleMetadata = [], + ?bool $throttle = null, ): TemplateResponse { $response = new TemplateResponse( Application::APP_ID, @@ -80,55 +73,50 @@ protected function buildFailureTemplateResponse( TemplateResponse::RENDER_AS_ERROR ); $response->setStatus($statusCode); + // if not specified, throttle if debug mode is off if (($throttle === null && !$this->isDebugModeEnabled()) || $throttle) { $response->throttle($throttleMetadata); } + return $response; } // TODO: use the following methods only when 32 is the min supported version // as it includes the "back to NC" button - - /** - * @param string $message - * @param int $statusCode - * @param array $throttleMetadata - * @param bool|null $throttle - * @return TemplateResponse - */ - protected function buildCoreErrorTemplateResponse(string $message, int $statusCode, array $throttleMetadata = [], ?bool $throttle = null): TemplateResponse { + protected function buildCoreErrorTemplateResponse( + string $message, + int $statusCode, + array $throttleMetadata = [], + ?bool $throttle = null, + ): TemplateResponse { $params = [ 'errors' => [ ['error' => $message], ], ]; + return $this->buildCoreFailureTemplateResponse('', 'error', $params, $statusCode, $throttleMetadata, $throttle); } - /** - * @param string $message - * @param int $statusCode - * @param array $throttleMetadata - * @param bool|null $throttle - * @return TemplateResponse - */ - protected function buildCore403TemplateResponse(string $message, int $statusCode, array $throttleMetadata = [], ?bool $throttle = null): TemplateResponse { + protected function buildCore403TemplateResponse( + string $message, + int $statusCode, + array $throttleMetadata = [], + ?bool $throttle = null, + ): TemplateResponse { $params = ['message' => $message]; return $this->buildCoreFailureTemplateResponse('core', '403', $params, $statusCode, $throttleMetadata, $throttle); } - /** - * @param string $appName - * @param string $templateName - * @param array $params - * @param int $statusCode - * @param array $throttleMetadata - * @param bool|null $throttle - * @return TemplateResponse - */ - protected function buildCoreFailureTemplateResponse(string $appName, string $templateName, array $params, int $statusCode, - array $throttleMetadata = [], ?bool $throttle = null): TemplateResponse { + protected function buildCoreFailureTemplateResponse( + string $appName, + string $templateName, + array $params, + int $statusCode, + array $throttleMetadata = [], + ?bool $throttle = null, + ): TemplateResponse { $response = new TemplateResponse( $appName, $templateName, @@ -136,10 +124,12 @@ protected function buildCoreFailureTemplateResponse(string $appName, string $tem TemplateResponse::RENDER_AS_ERROR ); $response->setStatus($statusCode); + // if not specified, throttle if debug mode is off if (($throttle === null && !$this->isDebugModeEnabled()) || $throttle) { $response->throttle($throttleMetadata); } + return $response; } } diff --git a/lib/Controller/Id4meController.php b/lib/Controller/Id4meController.php index f2c247ba..1e2be589 100644 --- a/lib/Controller/Id4meController.php +++ b/lib/Controller/Id4meController.php @@ -1,6 +1,7 @@ id4MeService->getID4ME()) { $message = $this->l10n->t('ID4Me is disabled'); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, [], false); @@ -90,20 +91,15 @@ public function showLogin() { $csp = new Http\ContentSecurityPolicy(); $csp->addAllowedFormActionDomain('*'); - $response->setContentSecurityPolicy($csp); return $response; } - /** - * @param string $domain - * @return RedirectResponse|TemplateResponse - */ #[PublicPage] #[UseSession] #[BruteForceProtection(action: 'userOidcId4MeLogin')] - public function login(string $domain) { + public function login(string $domain): RedirectResponse|TemplateResponse { if (!$this->id4MeService->getID4ME()) { $message = $this->l10n->t('ID4Me is disabled'); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, [], false); @@ -150,6 +146,7 @@ public function login(string $domain) { ]; $url = $openIdConfig->getAuthorizationEndpoint() . '?' . http_build_query($data); + return new RedirectResponse($url); } @@ -166,17 +163,13 @@ private function registerClient(string $authorityName, OpenIdConfig $openIdConfi } /** - * @param string $state - * @param string $code - * @param string $scope - * @return JSONResponse|RedirectResponse|TemplateResponse * @throws \Exception */ #[PublicPage] #[NoCSRFRequired] #[UseSession] #[BruteForceProtection(action: 'userOidcId4MeCode')] - public function code(string $state = '', string $code = '', string $scope = '') { + public function code(string $state = '', string $code = '', string $scope = ''): JSONResponse|RedirectResponse|TemplateResponse { if (!$this->id4MeService->getID4ME()) { $message = $this->l10n->t('ID4Me is disabled'); return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, [], false); diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index a9e4f951..1763b21a 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -7,8 +7,6 @@ /** @noinspection AdditionOperationOnArraysInspection */ -declare(strict_types=1); - namespace OCA\UserOIDC\Controller; use GuzzleHttp\Exception\ClientException; @@ -100,30 +98,20 @@ public function __construct( parent::__construct($request, $config, $l10n); } - /** - * @return bool - */ private function isSecure(): bool { // no restriction in debug mode return $this->isDebugModeEnabled() || $this->request->getServerProtocol() === 'https'; } - /** - * @param bool|null $throttle - * @return TemplateResponse - */ private function buildProtocolErrorResponse(?bool $throttle = null): TemplateResponse { $params = [ 'message' => $this->l10n->t('You must access Nextcloud with HTTPS to use OpenID Connect.'), ]; $throttleMetadata = ['reason' => 'insecure connection']; + return $this->buildFailureTemplateResponse($params, Http::STATUS_NOT_FOUND, $throttleMetadata, $throttle); } - /** - * @param string|null $redirectUrl - * @return RedirectResponse - */ private function getRedirectResponse(?string $redirectUrl = null): RedirectResponse { if ($redirectUrl === null) { return new RedirectResponse($this->urlGenerator->getBaseUrl()); @@ -140,16 +128,11 @@ private function getRedirectResponse(?string $redirectUrl = null): RedirectRespo return new RedirectResponse($filtered); } - /** - * @param int $providerId - * @param string|null $redirectUrl - * @return DataDisplayResponse|RedirectResponse|TemplateResponse - */ #[PublicPage] #[NoCSRFRequired] #[UseSession] #[BruteForceProtection(action: 'userOidcLogin')] - public function login(int $providerId, ?string $redirectUrl = null) { + public function login(int $providerId, ?string $redirectUrl = null): DataDisplayResponse|RedirectResponse|TemplateResponse { if ($this->userSession->isLoggedIn()) { return $this->getRedirectResponse($redirectUrl); } @@ -310,12 +293,6 @@ public function login(int $providerId, ?string $redirectUrl = null) { } /** - * @param string $state - * @param string $code - * @param string $scope - * @param string $error - * @param string $error_description - * @return JSONResponse|RedirectResponse|TemplateResponse * @throws DoesNotExistException * @throws MultipleObjectsReturnedException * @throws SessionNotAvailableException @@ -325,7 +302,13 @@ public function login(int $providerId, ?string $redirectUrl = null) { #[NoCSRFRequired] #[UseSession] #[BruteForceProtection(action: 'userOidcCode')] - public function code(string $state = '', string $code = '', string $scope = '', string $error = '', string $error_description = '') { + public function code( + string $state = '', + string $code = '', + string $scope = '', + string $error = '', + string $error_description = '', + ): JSONResponse|RedirectResponse|TemplateResponse { if (!$this->isSecure()) { return $this->buildProtocolErrorResponse(); } @@ -459,6 +442,7 @@ public function code(string $state = '', string $code = '', string $scope = '', $idTokenRaw = $data['id_token']; $jwks = $this->discoveryService->obtainJWK($provider, $idTokenRaw); JWT::$leeway = 60; + try { $idTokenPayload = JWT::decode($idTokenRaw, $jwks); } catch (UnexpectedValueException $e) { @@ -661,7 +645,6 @@ public function code(string $state = '', string $code = '', string $scope = '', /** * Endpoint called by NC to logout in the IdP before killing the current session * - * @return RedirectResponse|TemplateResponse * @throws Exception * @throws SessionNotAvailableException * @throws \JsonException @@ -671,7 +654,7 @@ public function code(string $state = '', string $code = '', string $scope = '', #[NoCSRFRequired] #[UseSession] #[BruteForceProtection(action: 'userOidcSingleLogout')] - public function singleLogoutService() { + public function singleLogoutService(): RedirectResponse|TemplateResponse { // TODO throttle in all failing cases $oidcSystemConfig = $this->config->getSystemValue('user_oidc', []); $targetUrl = $this->urlGenerator->getAbsoluteURL('/'); @@ -739,6 +722,7 @@ public function singleLogoutService() { // make sure we clear the session to avoid messing with Backend::isSessionActive $this->session->clear(); + return new RedirectResponse($targetUrl); } @@ -748,9 +732,6 @@ public function singleLogoutService() { * which leads to the auth token that we can invalidate * Implemented according to https://openid.net/specs/openid-connect-backchannel-1_0.html * - * @param string $providerIdentifier - * @param string $logout_token - * @return JSONResponse * @throws Exception * @throws \JsonException */ @@ -889,11 +870,6 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok /** * Generate an error response according to the OIDC standard * Log the error - * - * @param string $error - * @param string $description - * @param array $throttleMetadata - * @return JSONResponse */ private function getBackchannelLogoutErrorResponse( string $error, @@ -901,6 +877,7 @@ private function getBackchannelLogoutErrorResponse( array $throttleMetadata = [], ): JSONResponse { $this->logger->debug('Backchannel logout error. ' . $error . ' ; ' . $description); + return new JSONResponse( [ 'error' => $error, @@ -917,6 +894,7 @@ private function toCodeChallenge(string $data): string { $s = explode('=', $s)[0]; // Remove any trailing '='s $s = str_replace('+', '-', $s); // 62nd char of encoding $s = str_replace('/', '_', $s); // 63rd char of encoding + return $s; } } diff --git a/lib/Controller/OcsApiController.php b/lib/Controller/OcsApiController.php index fbabc5ff..a29fd6b2 100644 --- a/lib/Controller/OcsApiController.php +++ b/lib/Controller/OcsApiController.php @@ -1,6 +1,7 @@ userMapper->getOrCreate($providerId, $userId); $user = $this->userManager->get($backendUser->getUserId()); @@ -57,28 +55,26 @@ public function createUser(int $providerId, string $userId, ?string $displayName $user->setQuota($quota); } - $userFolder = $this->root->getUserFolder($user->getUID()); + $uid = $user->getUID(); + $userFolder = $this->root->getUserFolder($uid); try { // copy skeleton - \OC_Util::copySkeleton($user->getUID(), $userFolder); + \OC_Util::copySkeleton($uid, $userFolder); } catch (NotPermittedException $ex) { // read only uses } - return new DataResponse(['user_id' => $user->getUID()]); + return new DataResponse(['user_id' => $uid]); } - /** - * @param string $userId - * @return DataResponse - */ public function deleteUser(string $userId): DataResponse { $user = $this->userManager->get($userId); - if (is_null($user) || $user->getBackendClassName() !== Application::APP_ID) { + if ($user === null || $user->getBackendClassName() !== Application::APP_ID) { return new DataResponse(['message' => 'User not found'], Http::STATUS_NOT_FOUND); } $user->delete(); + return new DataResponse(['user_id' => $userId], Http::STATUS_OK); } } diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 004edc04..4d045afb 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -1,6 +1,7 @@ false, 'missingFields' => [], @@ -53,20 +54,27 @@ public function isDiscoveryEndpointValid($url) { $body = $response->getBody(); // Check if the request was successful - if ($httpCode === Http::STATUS_OK && !empty($body)) { - $result['isReachable'] = true; - $data = json_decode($body, true); - - // Check for required fields as defined in: https://openid.net/specs/openid-connect-discovery-1_0.html - $requiredFields = [ - 'issuer', 'authorization_endpoint', 'token_endpoint', 'jwks_uri', - 'response_types_supported', 'subject_types_supported', 'id_token_signing_alg_values_supported', - ]; - - foreach ($requiredFields as $field) { - if (!isset($data[$field])) { - $result['missingFields'][] = $field; - } + if ($httpCode !== Http::STATUS_OK || empty($body)) { + return $result; + } + + $result['isReachable'] = true; + $data = json_decode($body, true, JSON_THROW_ON_ERROR); + + // Check for required fields as defined in: https://openid.net/specs/openid-connect-discovery-1_0.html + $requiredFields = [ + 'issuer', + 'authorization_endpoint', + 'token_endpoint', + 'jwks_uri', + 'response_types_supported', + 'subject_types_supported', + 'id_token_signing_alg_values_supported', + ]; + + foreach ($requiredFields as $field) { + if (!isset($data[$field])) { + $result['missingFields'][] = $field; } } } catch (Exception $e) { @@ -77,9 +85,16 @@ public function isDiscoveryEndpointValid($url) { } #[PasswordConfirmationRequired] - public function createProvider(string $identifier, string $clientId, string $clientSecret, string $discoveryEndpoint, - array $settings = [], string $scope = 'openid email profile', ?string $endSessionEndpoint = null, - ?string $postLogoutUri = null): JSONResponse { + public function createProvider( + string $identifier, + string $clientId, + string $clientSecret, + string $discoveryEndpoint, + array $settings = [], + string $scope = 'openid email profile', + ?string $endSessionEndpoint = null, + ?string $postLogoutUri = null, + ): JSONResponse { if ($this->providerService->getProviderByIdentifier($identifier) !== null) { return new JSONResponse(['message' => 'Provider with the given identifier already exists'], Http::STATUS_CONFLICT); } @@ -110,22 +125,39 @@ public function createProvider(string $identifier, string $clientId, string $cli } #[PasswordConfirmationRequired] - public function updateProvider(int $providerId, string $identifier, string $clientId, string $discoveryEndpoint, ?string $clientSecret = null, - array $settings = [], string $scope = 'openid email profile', ?string $endSessionEndpoint = null, - ?string $postLogoutUri = null): JSONResponse { + public function updateProvider( + int $providerId, + string $identifier, + string $clientId, + string $discoveryEndpoint, + ?string $clientSecret = null, + array $settings = [], + string $scope = 'openid email profile', + ?string $endSessionEndpoint = null, + ?string $postLogoutUri = null, + ): JSONResponse { $provider = $this->providerMapper->getProvider($providerId); if ($this->providerService->getProviderByIdentifier($identifier) === null) { - return new JSONResponse(['message' => 'Provider with the given identifier does not exist'], Http::STATUS_NOT_FOUND); + return new JSONResponse( + ['message' => 'Provider with the given identifier does not exist'], + Http::STATUS_NOT_FOUND + ); } $result = $this->isDiscoveryEndpointValid($discoveryEndpoint); if (!$result['isReachable']) { $message = 'The discovery endpoint is not reachable.'; - return new JSONResponse(['message' => $message], Http::STATUS_BAD_REQUEST); + return new JSONResponse( + ['message' => $message], + Http::STATUS_BAD_REQUEST + ); } elseif (!empty($result['missingFields'])) { $message = 'Invalid discovery endpoint. Missing fields: ' . implode(', ', $result['missingFields']); - return new JSONResponse(['message' => $message], Http::STATUS_BAD_REQUEST); + return new JSONResponse( + ['message' => $message], + Http::STATUS_BAD_REQUEST + ); } $provider->setIdentifier($identifier); @@ -141,6 +173,7 @@ public function updateProvider(int $providerId, string $identifier, string $clie $provider = $this->providerMapper->update($provider); $providerSettings = $this->providerService->setSettings($providerId, $settings); + // invalidate JWKS cache $this->providerService->setSetting($providerId, ProviderService::SETTING_JWKS_CACHE, ''); $this->providerService->setSetting($providerId, ProviderService::SETTING_JWKS_CACHE_TIMESTAMP, ''); diff --git a/lib/Controller/TimezoneController.php b/lib/Controller/TimezoneController.php index f672cd94..0d3de7ef 100644 --- a/lib/Controller/TimezoneController.php +++ b/lib/Controller/TimezoneController.php @@ -1,6 +1,7 @@ config->setUserValue($this->userId, 'core', 'timezone', $timezone); $this->session->set('timezone', $timezoneOffset); diff --git a/lib/Cron/CleanupSessions.php b/lib/Cron/CleanupSessions.php index 6de75f52..17bc7f23 100644 --- a/lib/Cron/CleanupSessions.php +++ b/lib/Cron/CleanupSessions.php @@ -1,5 +1,7 @@ setInterval(24 * 60 * 60); if (method_exists($this, 'setTimeSensitivity')) { $this->setTimeSensitivity(IJob::TIME_INSENSITIVE); } } - /** - * @param $argument - * @return void - */ protected function run($argument): void { $nowTimestamp = (new DateTime())->getTimestamp(); $configSessionLifetime = $this->config->getSystemValueInt('session_lifetime', 60 * 60 * 24); $configCookieLifetime = $this->config->getSystemValueInt('remember_login_cookie_lifetime', 60 * 60 * 24 * 15); $since = $nowTimestamp - max($configSessionLifetime, $configCookieLifetime); + $this->sessionMapper->cleanupSessions($since); } } diff --git a/lib/Db/Id4Me.php b/lib/Db/Id4Me.php index de401b89..cdab1ae9 100644 --- a/lib/Db/Id4Me.php +++ b/lib/Db/Id4Me.php @@ -1,6 +1,7 @@ scope ?: ' '; } diff --git a/lib/Db/ProviderMapper.php b/lib/Db/ProviderMapper.php index c3dd55fb..7b2f0578 100644 --- a/lib/Db/ProviderMapper.php +++ b/lib/Db/ProviderMapper.php @@ -1,6 +1,7 @@ db->getQueryBuilder(); $qb->select('*') @@ -78,21 +75,19 @@ public function getProviders() { /** * Create or update provider settings * - * @param string $identifier - * @param string|null $clientid - * @param string|null $clientsecret - * @param string|null $discoveryuri - * @param string $scope - * @param string|null $endsessionendpointuri - * @param string|null $postLogoutUri - * @return Provider|Entity * @throws DoesNotExistException * @throws Exception * @throws MultipleObjectsReturnedException */ - public function createOrUpdateProvider(string $identifier, ?string $clientid = null, - ?string $clientsecret = null, ?string $discoveryuri = null, string $scope = 'openid email profile', - ?string $endsessionendpointuri = null, ?string $postLogoutUri = null) { + public function createOrUpdateProvider( + string $identifier, + ?string $clientid = null, + ?string $clientsecret = null, + ?string $discoveryuri = null, + string $scope = 'openid email profile', + ?string $endsessionendpointuri = null, + ?string $postLogoutUri = null, + ): Provider|Entity { try { $provider = $this->findProviderByIdentifier($identifier); } catch (DoesNotExistException $eNotExist) { @@ -101,7 +96,7 @@ public function createOrUpdateProvider(string $identifier, ?string $clientid = n if ($provider === null) { $provider = new Provider(); - if (($clientid === null) || ($clientsecret === null) || ($discoveryuri === null)) { + if ($clientid === null || $clientsecret === null || $discoveryuri === null) { throw new DoesNotExistException('Provider must be created. All provider parameters required.'); } $provider->setIdentifier($identifier); @@ -111,6 +106,7 @@ public function createOrUpdateProvider(string $identifier, ?string $clientid = n $provider->setEndSessionEndpoint($endsessionendpointuri); $provider->setPostLogoutUri($postLogoutUri); $provider->setScope($scope); + return $this->insert($provider); } else { if ($clientid !== null) { @@ -125,10 +121,13 @@ public function createOrUpdateProvider(string $identifier, ?string $clientid = n if ($endsessionendpointuri !== null) { $provider->setEndSessionEndpoint($endsessionendpointuri ?: null); } - if ($postLogoutUri !== null) { - $provider->setPostLogoutUri($postLogoutUri ?: null); + if ($postLogoutUri !== null && $postLogoutUri !== '') { + $provider->setPostLogoutUri($postLogoutUri); + } else { + $provider->setPostLogoutUri(null); } $provider->setScope($scope); + return $this->update($provider); } } diff --git a/lib/Db/Session.php b/lib/Db/Session.php index 42e6e605..b896f0b0 100644 --- a/lib/Db/Session.php +++ b/lib/Db/Session.php @@ -1,6 +1,7 @@ addType('sid', Types::STRING); diff --git a/lib/Db/SessionMapper.php b/lib/Db/SessionMapper.php index c7e3c192..2e5ce8ca 100644 --- a/lib/Db/SessionMapper.php +++ b/lib/Db/SessionMapper.php @@ -1,6 +1,7 @@ where( $qb->expr()->eq('nc_session_id', $qb->createNamedParameter($ncSessionId, IQueryBuilder::PARAM_STR)) ); + return $qb->executeStatement(); } /** - * @param int $minCreationTimestamp * @throws Exception */ public function cleanupSessions(int $minCreationTimestamp): void { @@ -153,6 +140,7 @@ public function cleanupSessions(int $minCreationTimestamp): void { ->where( $qb->expr()->lt('created_at', $qb->createNamedParameter($minCreationTimestamp, IQueryBuilder::PARAM_INT)) ); + $qb->executeStatement(); } @@ -165,21 +153,18 @@ public function cleanupSessions(int $minCreationTimestamp): void { * * In short: If there are multiple Nextcloud logins using the same IdP session, we only store the last one * - * @param string $sid - * @param string $sub - * @param string $iss - * @param int $authtokenId - * @param string $ncSessionId - * @param string $idToken - * @param string $userId - * @param int $providerId - * @param bool $idpSessionClosed - * @return Session|null * @throws Exception */ public function createOrUpdateSession( - string $sid, string $sub, string $iss, int $authtokenId, string $ncSessionId, - string $idToken, string $userId, int $providerId, bool $idpSessionClosed = false, + string $sid, + string $sub, + string $iss, + int $authtokenId, + string $ncSessionId, + string $idToken, + string $userId, + int $providerId, + bool $idpSessionClosed = false, ): ?Session { $createdAt = (new DateTime())->getTimestamp(); @@ -195,6 +180,7 @@ public function createOrUpdateSession( $existingSession->setUserId($userId); $existingSession->setProviderId($providerId); $existingSession->setIdpSessionClosed($idpSessionClosed ? 1 : 0); + return $this->update($existingSession); } catch (MultipleObjectsReturnedException $e) { // this can't happen @@ -213,6 +199,7 @@ public function createOrUpdateSession( $session->setUserId($userId); $session->setProviderId($providerId); $session->setIdpSessionClosed($idpSessionClosed ? 1 : 0); + return $this->insert($session); } } diff --git a/lib/Db/User.php b/lib/Db/User.php index 03126e00..97e054f8 100644 --- a/lib/Db/User.php +++ b/lib/Db/User.php @@ -1,6 +1,7 @@ addType('userId', Types::STRING); diff --git a/lib/Db/UserMapper.php b/lib/Db/UserMapper.php index ace1651c..ad08eb20 100644 --- a/lib/Db/UserMapper.php +++ b/lib/Db/UserMapper.php @@ -1,6 +1,7 @@ findEntity($qb); $this->userCache->set($uid, $user); + return $user; } @@ -154,7 +154,6 @@ public function userExists(string $uid): bool { public function getOrCreate(int $providerId, string $sub, bool $id4me = false): User { $userId = $this->idService->getId($providerId, $sub, $id4me); - if (strlen($userId) > 64) { $userId = hash('sha256', $userId); } @@ -167,6 +166,7 @@ public function getOrCreate(int $providerId, string $sub, bool $id4me = false): $user = new User(); $user->setUserId($userId); + return $this->insert($user); } } diff --git a/lib/Event/AttributeMappedEvent.php b/lib/Event/AttributeMappedEvent.php index 09c8e90d..e137af2b 100644 --- a/lib/Event/AttributeMappedEvent.php +++ b/lib/Event/AttributeMappedEvent.php @@ -1,13 +1,12 @@ claims; } - public function hasValue() : bool { + public function hasValue(): bool { return ($this->value != null); } diff --git a/lib/Event/ExchangedTokenRequestedEvent.php b/lib/Event/ExchangedTokenRequestedEvent.php index 87e7a5ec..6f9f4edc 100644 --- a/lib/Event/ExchangedTokenRequestedEvent.php +++ b/lib/Event/ExchangedTokenRequestedEvent.php @@ -1,6 +1,7 @@ config->getSystemValue('user_oidc', []); - $client = $this->clientService->newClient(); $debugModeEnabled = $this->config->getSystemValueBool('debug', false); - if ($debugModeEnabled - || (isset($oidcConfig['httpclient.allowselfsigned']) - && !in_array($oidcConfig['httpclient.allowselfsigned'], [false, 'false', 0, '0'], true))) { + $allowSelfSigned = !in_array($oidcConfig['httpclient.allowselfsigned'] ?? false, [false, 'false', 0, '0'], true); + + if ($debugModeEnabled || $allowSelfSigned) { $options['verify'] = false; } @@ -46,8 +46,8 @@ public function post($url, $body, array $headers = []) { 'body' => $body, ]; - if (isset($oidcConfig['httpclient.allowselfsigned']) - && !in_array($oidcConfig['httpclient.allowselfsigned'], [false, 'false', 0, '0'], true)) { + $allowSelfSigned = !in_array($oidcConfig['httpclient.allowselfsigned'] ?? false, [false, 'false', 0, '0'], true); + if ($allowSelfSigned) { $options['verify'] = false; } diff --git a/lib/Listener/ExchangedTokenRequestedListener.php b/lib/Listener/ExchangedTokenRequestedListener.php index dc6eb273..dca8efe2 100644 --- a/lib/Listener/ExchangedTokenRequestedListener.php +++ b/lib/Listener/ExchangedTokenRequestedListener.php @@ -1,6 +1,7 @@ tokenService->getExchangedToken($targetAudience, $extraScopes); + $event->setToken($token); } } diff --git a/lib/Listener/ExternalTokenRequestedListener.php b/lib/Listener/ExternalTokenRequestedListener.php index 277a00a0..a1d43c3d 100644 --- a/lib/Listener/ExternalTokenRequestedListener.php +++ b/lib/Listener/ExternalTokenRequestedListener.php @@ -1,6 +1,7 @@ tokenService->getToken(); + $event->setToken($token); } } diff --git a/lib/Listener/InternalTokenRequestedListener.php b/lib/Listener/InternalTokenRequestedListener.php index c80453a0..336e9dfe 100644 --- a/lib/Listener/InternalTokenRequestedListener.php +++ b/lib/Listener/InternalTokenRequestedListener.php @@ -1,6 +1,7 @@ logger->debug('[TokenInvalidatedListener] Failed to request the end_session_endpoint', ['exception' => $e]); } + // we know this oidc session is not useful anymore, we can delete it $this->sessionMapper->delete($oidcSession); } diff --git a/lib/Migration/Version00001Date20200322114947.php b/lib/Migration/Version00001Date20200322114947.php index d07d26a9..e0e8b934 100644 --- a/lib/Migration/Version00001Date20200322114947.php +++ b/lib/Migration/Version00001Date20200322114947.php @@ -30,18 +30,20 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); - $table = $schema->createTable('user_oidc'); - $table->addColumn('id', 'integer', [ - 'autoincrement' => true, - 'notnull' => true, - 'length' => 4, - ]); - $table->addColumn('user_id', 'string', [ - 'notnull' => true, - 'length' => 64, - ]); - $table->setPrimaryKey(['id']); - $table->addUniqueIndex(['user_id'], 'user_oidc_uid'); + if (!$schema->hasTable('user_oidc')) { + $table = $schema->createTable('user_oidc'); + $table->addColumn('id', 'integer', [ + 'autoincrement' => true, + 'notnull' => true, + 'length' => 4, + ]); + $table->addColumn('user_id', 'string', [ + 'notnull' => true, + 'length' => 64, + ]); + $table->setPrimaryKey(['id']); + $table->addUniqueIndex(['user_id'], 'user_oidc_uid'); + } return $schema; } diff --git a/lib/Migration/Version00003Date20200420120107.php b/lib/Migration/Version00003Date20200420120107.php index 83b0eed5..33990172 100644 --- a/lib/Migration/Version00003Date20200420120107.php +++ b/lib/Migration/Version00003Date20200420120107.php @@ -27,25 +27,27 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); - $table = $schema->createTable('user_oidc_id4me'); - $table->addColumn('id', 'integer', [ - 'autoincrement' => true, - 'notnull' => true, - 'length' => 4, - ]); - $table->addColumn('identifier', 'string', [ - 'notnull' => true, - 'length' => 128, - ]); - $table->addColumn('client_id', 'string', [ - 'notnull' => true, - 'length' => 64, - ]); - $table->addColumn('client_secret', 'string', [ - 'notnull' => true, - 'length' => 64, - ]); - $table->setPrimaryKey(['id']); + if (!$schema->hasTable('user_oidc_id4me')) { + $table = $schema->createTable('user_oidc_id4me'); + $table->addColumn('id', 'integer', [ + 'autoincrement' => true, + 'notnull' => true, + 'length' => 4, + ]); + $table->addColumn('identifier', 'string', [ + 'notnull' => true, + 'length' => 128, + ]); + $table->addColumn('client_id', 'string', [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('client_secret', 'string', [ + 'notnull' => true, + 'length' => 64, + ]); + $table->setPrimaryKey(['id']); + } return $schema; } diff --git a/lib/Migration/Version00004Date20200428102743.php b/lib/Migration/Version00004Date20200428102743.php index 9f5f7b37..b964244d 100644 --- a/lib/Migration/Version00004Date20200428102743.php +++ b/lib/Migration/Version00004Date20200428102743.php @@ -30,29 +30,31 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); - $table = $schema->createTable('user_oidc_providers'); - $table->addColumn('id', 'integer', [ - 'autoincrement' => true, - 'notnull' => true, - 'length' => 4, - ]); - $table->addColumn('identifier', 'string', [ - 'notnull' => true, - 'length' => 128, - ]); - $table->addColumn('client_id', 'string', [ - 'notnull' => true, - 'length' => 64, - ]); - $table->addColumn('client_secret', 'string', [ - 'notnull' => true, - 'length' => 64, - ]); - $table->addColumn('discovery_endpoint', 'string', [ - 'notnull' => false, - 'length' => 255, - ]); - $table->setPrimaryKey(['id']); + if (!$schema->hasTable('user_oidc_providers')) { + $table = $schema->createTable('user_oidc_providers'); + $table->addColumn('id', 'integer', [ + 'autoincrement' => true, + 'notnull' => true, + 'length' => 4, + ]); + $table->addColumn('identifier', 'string', [ + 'notnull' => true, + 'length' => 128, + ]); + $table->addColumn('client_id', 'string', [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('client_secret', 'string', [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('discovery_endpoint', 'string', [ + 'notnull' => false, + 'length' => 255, + ]); + $table->setPrimaryKey(['id']); + } return $schema; } diff --git a/lib/Migration/Version00005Date20200428123958.php b/lib/Migration/Version00005Date20200428123958.php index 8bcf0a1c..c6f08051 100644 --- a/lib/Migration/Version00005Date20200428123958.php +++ b/lib/Migration/Version00005Date20200428123958.php @@ -29,12 +29,14 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); - $table = $schema->getTable('user_oidc'); - $table->addColumn('display_name', 'string', [ - 'length' => 255, - 'default' => '', - 'notnull' => false, - ]); + if ($schema->hasTable('user_oidc')) { + $table = $schema->getTable('user_oidc'); + $table->addColumn('display_name', 'string', [ + 'length' => 255, + 'default' => '', + 'notnull' => false, + ]); + } return $schema; } diff --git a/lib/Migration/Version00006Date20210630120251.php b/lib/Migration/Version00006Date20210630120251.php index 5defa59d..68acb7e5 100644 --- a/lib/Migration/Version00006Date20210630120251.php +++ b/lib/Migration/Version00006Date20210630120251.php @@ -19,8 +19,10 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); - $table = $schema->getTable('user_oidc_providers'); - $table->addUniqueIndex(['identifier'], 'user_oidc_prov_idtf'); + if ($schema->hasTable('user_oidc_providers')) { + $table = $schema->getTable('user_oidc_providers'); + $table->addUniqueIndex(['identifier'], 'user_oidc_prov_idtf'); + } return $schema; } diff --git a/lib/Migration/Version00007Date20210730170713.php b/lib/Migration/Version00007Date20210730170713.php index d6d6018b..16729cd8 100644 --- a/lib/Migration/Version00007Date20210730170713.php +++ b/lib/Migration/Version00007Date20210730170713.php @@ -19,12 +19,14 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); - $table = $schema->getTable('user_oidc_providers'); - $table->addColumn('scope', 'string', [ - 'length' => 128, - 'default' => 'openid email profile', - 'notnull' => true, - ]); + if ($schema->hasTable('user_oidc_providers')) { + $table = $schema->getTable('user_oidc_providers'); + $table->addColumn('scope', 'string', [ + 'length' => 128, + 'default' => 'openid email profile', + 'notnull' => true, + ]); + } return $schema; } diff --git a/lib/Migration/Version01021Date20220719114355.php b/lib/Migration/Version01021Date20220719114355.php index 98cb8f31..4f7d8a50 100644 --- a/lib/Migration/Version01021Date20220719114355.php +++ b/lib/Migration/Version01021Date20220719114355.php @@ -31,42 +31,44 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); - $table = $schema->createTable('user_oidc_sessions'); - $table->addColumn('id', Types::INTEGER, [ - 'autoincrement' => true, - 'notnull' => true, - 'length' => 4, - ]); - // https://openid.net/specs/openid-connect-core-1_0.html#IDToken - $table->addColumn('sid', Types::STRING, [ - 'notnull' => true, - 'length' => 256, - ]); - $table->addColumn('sub', Types::STRING, [ - 'notnull' => true, - 'length' => 256, - ]); - $table->addColumn('iss', Types::STRING, [ - 'notnull' => true, - 'length' => 512, - ]); - $table->addColumn('authtoken_id', Types::INTEGER, [ - 'notnull' => true, - 'length' => 4, - ]); - $table->addColumn('nc_session_id', Types::STRING, [ - 'notnull' => true, - 'length' => 200, - ]); - $table->addColumn('created_at', Types::BIGINT, [ - 'notnull' => true, - 'length' => 20, - 'unsigned' => true, - ]); - $table->setPrimaryKey(['id']); - $table->addIndex(['created_at'], 'user_oidc_sess_crat'); - $table->addUniqueIndex(['sid'], 'user_oidc_sess_sid'); - $table->addUniqueIndex(['nc_session_id'], 'user_oidc_sess_sess_id'); + if (!$schema->hasTable('user_oidc_sessions')) { + $table = $schema->createTable('user_oidc_sessions'); + $table->addColumn('id', Types::INTEGER, [ + 'autoincrement' => true, + 'notnull' => true, + 'length' => 4, + ]); + // https://openid.net/specs/openid-connect-core-1_0.html#IDToken + $table->addColumn('sid', Types::STRING, [ + 'notnull' => true, + 'length' => 256, + ]); + $table->addColumn('sub', Types::STRING, [ + 'notnull' => true, + 'length' => 256, + ]); + $table->addColumn('iss', Types::STRING, [ + 'notnull' => true, + 'length' => 512, + ]); + $table->addColumn('authtoken_id', Types::INTEGER, [ + 'notnull' => true, + 'length' => 4, + ]); + $table->addColumn('nc_session_id', Types::STRING, [ + 'notnull' => true, + 'length' => 200, + ]); + $table->addColumn('created_at', Types::BIGINT, [ + 'notnull' => true, + 'length' => 20, + 'unsigned' => true, + ]); + $table->setPrimaryKey(['id']); + $table->addIndex(['created_at'], 'user_oidc_sess_crat'); + $table->addUniqueIndex(['sid'], 'user_oidc_sess_sid'); + $table->addUniqueIndex(['nc_session_id'], 'user_oidc_sess_sess_id'); + } return $schema; } diff --git a/lib/Migration/Version010303Date20230602125945.php b/lib/Migration/Version010303Date20230602125945.php index 2f9a02c5..5c8d803e 100644 --- a/lib/Migration/Version010303Date20230602125945.php +++ b/lib/Migration/Version010303Date20230602125945.php @@ -18,25 +18,13 @@ use OCP\Security\ICrypto; class Version010303Date20230602125945 extends SimpleMigrationStep { - - /** - * @var IDBConnection - */ - private $connection; - /** - * @var ICrypto - */ - private $crypto; - public function __construct( - IDBConnection $connection, - ICrypto $crypto, + private IDBConnection $connection, + private ICrypto $crypto, ) { - $this->connection = $connection; - $this->crypto = $crypto; } - public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) { + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); @@ -54,7 +42,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt return null; } - public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { // update secrets in user_oidc_providers and user_oidc_id4me foreach (['user_oidc_providers', 'user_oidc_id4me'] as $tableName) { $qbUpdate = $this->connection->getQueryBuilder(); @@ -68,6 +56,7 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $qbSelect->select('id', 'client_secret') ->from($tableName); $req = $qbSelect->executeQuery(); + while ($row = $req->fetch()) { $id = $row['id']; $secret = $row['client_secret']; @@ -76,6 +65,7 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $qbUpdate->setParameter('updateId', $id, IQueryBuilder::PARAM_INT); $qbUpdate->executeStatement(); } + $req->closeCursor(); } } diff --git a/lib/Migration/Version010304Date20231121102449.php b/lib/Migration/Version010304Date20231121102449.php index 60bdf8e2..77ecdf81 100644 --- a/lib/Migration/Version010304Date20231121102449.php +++ b/lib/Migration/Version010304Date20231121102449.php @@ -18,24 +18,13 @@ class Version010304Date20231121102449 extends SimpleMigrationStep { - /** - * @var IDBConnection - */ - private $connection; - /** - * @var ICrypto - */ - private $crypto; - public function __construct( - IDBConnection $connection, - ICrypto $crypto, + private IDBConnection $connection, + private ICrypto $crypto, ) { - $this->connection = $connection; - $this->crypto = $crypto; } - public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) { + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); @@ -58,6 +47,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt if ($schemaChanged) { return $schema; } + return null; } } diff --git a/lib/Migration/Version010304Date20231130104459.php b/lib/Migration/Version010304Date20231130104459.php index a813a02f..64b0d9a7 100644 --- a/lib/Migration/Version010304Date20231130104459.php +++ b/lib/Migration/Version010304Date20231130104459.php @@ -19,12 +19,6 @@ * Auto-generated migration step: Please modify to your needs! */ class Version010304Date20231130104459 extends SimpleMigrationStep { - /** - * @param IOutput $output - * @param Closure $schemaClosure - * @param array $options - * @return null|ISchemaWrapper - */ public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); diff --git a/lib/Migration/Version070400Date20250820141709.php b/lib/Migration/Version070400Date20250820141709.php index ec997c6b..80e2dcf2 100644 --- a/lib/Migration/Version070400Date20250820141709.php +++ b/lib/Migration/Version070400Date20250820141709.php @@ -16,12 +16,6 @@ use OCP\Migration\SimpleMigrationStep; class Version070400Date20250820141709 extends SimpleMigrationStep { - /** - * @param IOutput $output - * @param Closure $schemaClosure - * @param array $options - * @return null|ISchemaWrapper - */ public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); diff --git a/lib/Migration/Version070500Date20250515141105.php b/lib/Migration/Version070500Date20250515141105.php index 61a02eaa..2754074b 100644 --- a/lib/Migration/Version070500Date20250515141105.php +++ b/lib/Migration/Version070500Date20250515141105.php @@ -15,16 +15,7 @@ use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; -/** - * Auto-generated migration step: Please modify to your needs! - */ class Version070500Date20250515141105 extends SimpleMigrationStep { - /** - * @param IOutput $output - * @param Closure $schemaClosure - * @param array $options - * @return null|ISchemaWrapper - */ public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); diff --git a/lib/Migration/Version080101Date20251201121749.php b/lib/Migration/Version080101Date20251201121749.php index d8dcf471..996fa4ec 100644 --- a/lib/Migration/Version080101Date20251201121749.php +++ b/lib/Migration/Version080101Date20251201121749.php @@ -25,18 +25,14 @@ public function __construct( ) { } - /** - * @param IOutput $output - * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` - * @param array $options - */ - public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { // make admin settings lazy $keys = [ 'store_login_token', 'id4me_enabled', 'allow_multiple_user_backends', ]; + foreach ($keys as $key) { try { if ($this->appConfig->hasKey(Application::APP_ID, $key)) { @@ -49,6 +45,7 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array // make all provider settings lazy $providers = $this->providerMapper->getProviders(); + // equivalent of $this->providerService->getSupportedSettings() $supportedSettingKeys = [ ProviderService::SETTING_MAPPING_DISPLAYNAME, @@ -87,8 +84,10 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array ProviderService::SETTING_RESTRICT_LOGIN_TO_GROUPS, ProviderService::SETTING_RESOLVE_NESTED_AND_FALLBACK_CLAIMS_MAPPING, ]; + $supportedSettingKeys[] = ProviderService::SETTING_JWKS_CACHE; $supportedSettingKeys[] = ProviderService::SETTING_JWKS_CACHE_TIMESTAMP; + foreach ($supportedSettingKeys as $key) { foreach ($providers as $provider) { // equivalent of $this->providerService->getSettingsKey($provider->getId(), $key) diff --git a/lib/Model/Token.php b/lib/Model/Token.php index f5f66c00..35b6ad56 100644 --- a/lib/Model/Token.php +++ b/lib/Model/Token.php @@ -1,6 +1,7 @@ createdAt + $this->refreshExpiresIn; + return $refreshExpiresAt - time(); } @@ -82,6 +84,7 @@ public function refreshIsExpired(): bool { if ($this->refreshExpiresIn === null) { return false; } + return time() > ($this->createdAt + $this->refreshExpiresIn); } @@ -90,10 +93,11 @@ public function refreshIsExpiring(): bool { if ($this->refreshExpiresIn === null) { return false; } + return time() > ($this->createdAt + (int)($this->refreshExpiresIn / 2)); } - public function getCreatedAt() { + public function getCreatedAt(): int { return $this->createdAt; } diff --git a/lib/Service/DiscoveryService.php b/lib/Service/DiscoveryService.php index 1e810528..ccc881c5 100644 --- a/lib/Service/DiscoveryService.php +++ b/lib/Service/DiscoveryService.php @@ -1,12 +1,12 @@ cache = $cacheFactory->createDistributed('user_oidc'); } + /** + * @throws \JsonException + */ public function obtainDiscovery(Provider $provider): array { $cacheKey = 'discovery-2-' . $provider->getDiscoveryEndpoint(); $cachedDiscovery = $this->cache->get($cacheKey); @@ -67,41 +70,36 @@ public function obtainDiscovery(Provider $provider): array { } /** - * @param Provider $provider * @param string $tokenToDecode This is used to potentially fix the missing alg in - * @param bool $useCache - * @return array * @throws \JsonException */ public function obtainJWK(Provider $provider, string $tokenToDecode, bool $useCache = true): array { - $lastJwksRefresh = $this->providerService->getSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE_TIMESTAMP); + $providerId = $provider->getId(); + $lastJwksRefresh = $this->providerService->getSetting($providerId, ProviderService::SETTING_JWKS_CACHE_TIMESTAMP); if ($lastJwksRefresh !== '' && $useCache && (int)$lastJwksRefresh > time() - self::INVALIDATE_JWKS_CACHE_AFTER_SECONDS) { - $rawJwks = $this->providerService->getSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE); - $rawJwks = json_decode($rawJwks, true); + $rawJwks = $this->providerService->getSetting($providerId, ProviderService::SETTING_JWKS_CACHE); + $rawJwks = json_decode($rawJwks, true, JSON_THROW_ON_ERROR); $this->logger->debug('[obtainJWK] jwks cache content', ['jwks_cache' => $rawJwks]); } else { $discovery = $this->obtainDiscovery($provider); $responseBody = $this->clientService->get($discovery['jwks_uri']); - $rawJwks = json_decode($responseBody, true); + $rawJwks = json_decode($responseBody, true, JSON_THROW_ON_ERROR); $this->logger->debug('[obtainJWK] getting fresh jwks', ['jwks' => $rawJwks]); // cache jwks - $this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE, $responseBody); + $this->providerService->setSetting($providerId, ProviderService::SETTING_JWKS_CACHE, $responseBody); $this->logger->debug('[obtainJWK] setting cache', ['jwks_cache' => $responseBody]); - $this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE_TIMESTAMP, strval(time())); + $this->providerService->setSetting($providerId, ProviderService::SETTING_JWKS_CACHE_TIMESTAMP, strval(time())); } $fixedJwks = $this->fixJwksAlg($rawJwks, $tokenToDecode); $this->logger->debug('[obtainJWK] fixed jwks', ['fixed_jwks' => $fixedJwks]); $jwks = JWK::parseKeySet($fixedJwks, 'RS256'); + $this->logger->debug('Parsed the jwks'); + return $jwks; } - /** - * @param string $authorizationEndpoint - * @param array $extraGetParameters - * @return string - */ public function buildAuthorizationUrl(string $authorizationEndpoint, array $extraGetParameters = []): string { $parsedUrl = parse_url($authorizationEndpoint); @@ -175,14 +173,12 @@ private function validateKeyStrength(array $key, string $alg): void { /** * Inspired by https://github.com/snake/moodle/compare/880462a1685...MDL-77077-master * - * @param array $jwks The JSON Web Key Set - * @param string $jwt The JWT token - * @return array The modified JWKS + * @throws \JsonException * @throws \RuntimeException if no matching key is found or algorithm is unsupported */ public function fixJwksAlg(array $jwks, string $jwt): array { $jwtParts = explode('.', $jwt, 3); - $header = json_decode(JWT::urlsafeB64Decode($jwtParts[0]), true); + $header = json_decode(JWT::urlsafeB64Decode($jwtParts[0]), true, JSON_THROW_ON_ERROR); $kid = $header['kid'] ?? null; $alg = $header['alg'] ?? null; diff --git a/lib/Service/ID4MeService.php b/lib/Service/ID4MeService.php index e4961722..bf4ed6bd 100644 --- a/lib/Service/ID4MeService.php +++ b/lib/Service/ID4MeService.php @@ -1,6 +1,7 @@ appConfig->getValueString(Application::APP_ID, 'id4me_enabled', '0', lazy: true) === '1'; } + /** + * @throws \JsonException + */ public function obtainJWK(string $jwkUri, string $tokenToDecode, bool $useCache = true): array { $cacheKey = 'jwks-' . $jwkUri; $cachedJwks = $this->cache->get($cacheKey); @@ -55,6 +59,7 @@ public function obtainJWK(string $jwkUri, string $tokenToDecode, bool $useCache $this->logger->debug('[ID4ME-obtainJWK] fixed jwks', ['fixed_jwks' => $fixedJwks]); $jwks = JWK::parseKeySet($fixedJwks, 'RS256'); $this->logger->debug('Parsed the jwks'); + return $jwks; } } diff --git a/lib/Service/LdapService.php b/lib/Service/LdapService.php index efccf683..ca954803 100644 --- a/lib/Service/LdapService.php +++ b/lib/Service/LdapService.php @@ -1,12 +1,12 @@ providerService->getSetting($providerId, ProviderService::SETTING_UNIQUE_UID, '1') === '1' || $id4me) { $newId = strval($providerId) . '_'; - - if ($id4me) { - $newId .= '1_'; - } else { - $newId .= '0_'; - } - + $newId .= $id4me ? '1_' : '0_'; $newId .= $id; $newId = hash('sha256', $newId); } elseif ($this->providerService->getSetting($providerId, ProviderService::SETTING_PROVIDER_BASED_ID, '0') === '1') { diff --git a/lib/Service/OIDCService.php b/lib/Service/OIDCService.php index ede6ff6f..a5b2d063 100644 --- a/lib/Service/OIDCService.php +++ b/lib/Service/OIDCService.php @@ -1,12 +1,12 @@ clientService->get($url, [], $options), true); + return json_decode($this->clientService->get($url, [], $options), true, JSON_THROW_ON_ERROR); } catch (Throwable $e) { return []; } @@ -51,6 +51,7 @@ public function introspection(Provider $provider, string $accessToken): array { $this->logger->error('Failed to decrypt the client secret', ['exception' => $e]); return []; } + $url = $this->discoveryService->obtainDiscovery($provider)['introspection_endpoint'] ?? null; if ($url === null) { return []; @@ -66,8 +67,7 @@ public function introspection(Provider $provider, string $accessToken): array { 'Authorization' => base64_encode($provider->getClientId() . ':' . $providerClientSecret), ] ); - - return json_decode($body, true); + return json_decode($body, true, JSON_THROW_ON_ERROR); } catch (Throwable $e) { return []; } diff --git a/lib/Service/ProviderService.php b/lib/Service/ProviderService.php index bfda134b..76d137be 100644 --- a/lib/Service/ProviderService.php +++ b/lib/Service/ProviderService.php @@ -1,13 +1,12 @@ getSetting($providerId, $setting); $result[$setting] = $this->convertToJSON($setting, $value); } + return $result; } @@ -113,6 +113,7 @@ public function setSettings(int $providerId, array $settings): array { $this->setSetting($providerId, $setting, $this->convertFromJSON($setting, $value)); $storedSettings[$setting] = $value; } + return $storedSettings; } @@ -133,6 +134,7 @@ public function getSetting(int $providerId, string $key, string $default = ''): if ($value === '') { return $default; } + return $value; } @@ -184,6 +186,7 @@ private function convertFromJSON(string $key, $value): string { if (array_key_exists($key, self::BOOLEAN_SETTINGS_DEFAULT_VALUES)) { return $value ? '1' : '0'; } + return (string)$value; } @@ -194,6 +197,7 @@ private function convertToJSON(string $key, $value) { } return $value === '1'; } + return (string)$value; } } diff --git a/lib/Service/ProvisioningService.php b/lib/Service/ProvisioningService.php index c78ee7e1..43524a87 100644 --- a/lib/Service/ProvisioningService.php +++ b/lib/Service/ProvisioningService.php @@ -1,5 +1,7 @@ eventDispatcher->dispatchTyped($event); $this->logger->debug('Displayname mapping event dispatched'); if ($event->hasValue() && $event->getValue() !== null && $event->getValue() !== '') { @@ -436,11 +436,6 @@ public function provisionUser(string $tokenUserId, int $providerId, object $idTo ]; } - /** - * @param string $userId - * @param string $avatarAttribute - * @return void - */ private function setUserAvatar(string $userId, string $avatarAttribute): void { $avatarContent = null; if (filter_var($avatarAttribute, FILTER_VALIDATE_URL)) { @@ -456,13 +451,13 @@ private function setUserAvatar(string $userId, string $avatarAttribute): void { /** @psalm-suppress RedundantCast */ $contentType = (string)$ct; } - $contentType = strtolower(trim(explode(';', $contentType)[0])); if (!in_array($contentType, ['image/jpeg', 'image/png', 'image/gif'], true)) { $this->logger->warning('Avatar response is not an image', ['content_type' => $contentType]); return; } + $avatarContent = $response->getBody(); if (is_resource($avatarContent)) { $avatarContent = stream_get_contents($avatarContent); @@ -527,7 +522,10 @@ private function setUserAvatar(string $userId, string $avatarAttribute): void { } } - public function getSyncGroupsOfToken(int $providerId, object $idTokenPayload) { + /** + * @return object[]|null + */ + public function getSyncGroupsOfToken(int $providerId, object $idTokenPayload): ?array { $groupsAttribute = $this->providerService->getSetting($providerId, ProviderService::SETTING_MAPPING_GROUPS, 'groups'); $groupsData = $this->getClaimValues($idTokenPayload, $groupsAttribute, $providerId); @@ -577,7 +575,6 @@ public function getSyncGroupsOfToken(int $providerId, object $idTokenPayload) { } $group->gid = $this->idService->getId($providerId, $group->gid); - $syncGroups[] = $group; } @@ -591,7 +588,6 @@ public function provisionUserGroups(IUser $user, int $providerId, object $idToke $groupsWhitelistRegex = $this->getGroupWhitelistRegex($providerId); $syncGroups = $this->getSyncGroupsOfToken($providerId, $idTokenPayload); - if ($syncGroups === null) { return null; } diff --git a/lib/Service/SettingsService.php b/lib/Service/SettingsService.php index ffd75d34..2e64945c 100644 --- a/lib/Service/SettingsService.php +++ b/lib/Service/SettingsService.php @@ -1,13 +1,12 @@ session->set(self::SESSION_TOKEN_KEY, json_encode($token, JSON_THROW_ON_ERROR)); $this->logger->debug('[TokenService] Store token in the session', ['session_id' => $this->session->getId()]); + return $token; } @@ -81,7 +84,6 @@ public function storeToken(array $tokenData): Token { * Get the token stored in the session * If it has expired: try to refresh it * - * @param bool $refreshIfExpired * @return Token|null Return a token only if it is valid or has been successfully refreshed * @throws \JsonException */ @@ -108,13 +110,13 @@ public function getToken(bool $refreshIfExpired = true): ?Token { } $this->logger->debug('[TokenService] getToken: return a token that has not been refreshed'); + return $token; } /** * Check to make sure the login token is still valid * - * @return void * @throws \JsonException * @throws PreConditionNotMetException */ @@ -175,21 +177,23 @@ public function checkLoginToken(): void { $this->logger->debug('[TokenService] checkLoginToken: all good'); } - public function reauthenticate(int $providerId) { + public function reauthenticate(int $providerId): RedirectResponse { + $this->logger->debug('[TokenService] Starting reauthentication', ['providerId' => $providerId]); + // Logout the user and redirect to the oidc login flow to gather a fresh token $this->userSession->logout(); + $redirectUrl = $this->urlGenerator->linkToRouteAbsolute(Application::APP_ID . '.login.login', [ 'providerId' => $providerId, 'redirectUrl' => $this->request->getRequestUri(), ]); - header('Location: ' . $redirectUrl); - $this->logger->debug('[TokenService] reauthenticate', ['redirectUrl' => $redirectUrl]); - exit(); + + $this->logger->debug('[TokenService] Redirecting for reauthentication', ['redirectUrl' => $redirectUrl]); + + return new RedirectResponse($redirectUrl); } /** - * @param Token $token - * @return Token * @throws \JsonException * @throws DoesNotExistException * @throws MultipleObjectsReturnedException @@ -276,19 +280,21 @@ public function refresh(Token $token): Token { } } + /** + * @throws \JsonException + */ public function decodeIdToken(Token $token): array { $provider = $this->providerMapper->getProvider($token->getProviderId()); $jwks = $this->discoveryService->obtainJWK($provider, $token->getIdToken()); JWT::$leeway = 60; $idTokenObject = JWT::decode($token->getIdToken(), $jwks); - return json_decode(json_encode($idTokenObject), true); + + return json_decode(json_encode($idTokenObject), true, JSON_THROW_ON_ERROR); } /** * Exchange the login token for another audience (client ID) * - * @param string $targetAudience - * @return Token * @throws DoesNotExistException * @throws MultipleObjectsReturnedException * @throws TokenExchangeFailedException @@ -390,7 +396,7 @@ public function getExchangedToken(string $targetAudience, array $extraScopes = [ $e, ); } - } catch (\Exception|\Throwable $e) { + } catch (\Throwable $e) { $this->logger->error('[TokenService] Failed to exchange token ', ['exception' => $e]); throw new TokenExchangeFailedException('Failed to exchange token, error in the exchange request', 0, $e); } @@ -398,12 +404,13 @@ public function getExchangedToken(string $targetAudience, array $extraScopes = [ /** * Try to get a token from the Oidc provider app for a user and a specific audience (client ID) - * - * @param string $userId - * @param string $targetAudience - * @return Token|null */ - public function getTokenFromOidcProviderApp(string $userId, string $targetAudience, array $extraScopes = [], string $resource = ''): ?Token { + public function getTokenFromOidcProviderApp( + string $userId, + string $targetAudience, + array $extraScopes = [], + string $resource = '', + ): ?Token { if (!class_exists(\OCA\OIDCIdentityProvider\AppInfo\Application::class)) { $this->logger->warning('[TokenService] Failed to get token from Oidc provider app, oidc app is not installed'); return null; diff --git a/lib/Settings/AdminSettings.php b/lib/Settings/AdminSettings.php index 332caa6e..efb77be0 100644 --- a/lib/Settings/AdminSettings.php +++ b/lib/Settings/AdminSettings.php @@ -30,7 +30,7 @@ public function __construct( ) { } - public function getForm() { + public function getForm(): TemplateResponse { $this->initialStateService->provideInitialState( 'id4meState', $this->Id4MeService->getID4ME() @@ -53,11 +53,11 @@ public function getForm() { return new TemplateResponse(Application::APP_ID, 'admin-settings'); } - public function getSection() { + public function getSection(): string { return Application::APP_ID; } - public function getPriority() { + public function getPriority(): int { return 90; } } diff --git a/lib/Settings/Section.php b/lib/Settings/Section.php index 87e4f435..11d25c6b 100644 --- a/lib/Settings/Section.php +++ b/lib/Settings/Section.php @@ -22,28 +22,19 @@ public function __construct( ) { } - /** - * {@inheritdoc} - */ - public function getID() { + public function getID(): string { return Application::APP_ID; } - /** - * {@inheritdoc} - */ - public function getName() { + public function getName(): string { return $this->l->t('OpenID Connect'); } - /** - * {@inheritdoc} - */ - public function getPriority() { + public function getPriority(): int { return 75; } - public function getIcon() { + public function getIcon(): string { return $this->urlGenerator->imagePath(Application::APP_ID, 'app-dark.svg'); } } diff --git a/lib/User/Backend.php b/lib/User/Backend.php index 548e0350..fd87e030 100644 --- a/lib/User/Backend.php +++ b/lib/User/Backend.php @@ -1,6 +1,7 @@ getUserId(); }, $this->userMapper->find($search, $limit, $offset)); @@ -99,6 +101,7 @@ public function userExists($uid): bool { if (!is_string($uid)) { return false; } + return $this->userMapper->userExists($uid); } @@ -122,6 +125,7 @@ public function getDisplayNames($search = '', $limit = null, $offset = null): ar ) { return []; } + return $this->userMapper->findDisplayNames($search, $limit, $offset); } @@ -157,9 +161,6 @@ public function isSessionActive(): bool { return $headerToken !== '' || $this->session->get(LoginController::PROVIDERID) !== null; } - /** - * {@inheritdoc} - */ public function getLogoutUrl(): string { return $this->urlGenerator->linkToRouteAbsolute( 'user_oidc.login.singleLogoutService', @@ -216,7 +217,6 @@ private function formatUserData(int $providerId, array $attributes): array { /** * Return the id of the current user - * @return string * @since 6.0.0 */ public function getCurrentUserId(): string { @@ -357,14 +357,13 @@ public function getCurrentUserId(): string { } $this->logger->debug('Could not find unique token validation'); + return ''; } /** * Inspired by lib/private/User/Session.php::prepareUserLogin() * - * @param string $userId - * @return bool * @throws NotFoundException */ private function checkFirstLogin(string $userId): bool { @@ -392,24 +391,22 @@ private function checkFirstLogin(string $userId): bool { // $this->eventDispatcher->dispatchTyped(new UserFirstTimeLoggedInEvent($user)); } $user->updateLastLoginTimestamp(); + return $firstLogin; } /** * Triggers user provisioning based on the provided strategy - * - * @param string $provisioningStrategyClass - * @param Provider $provider - * @param string $tokenUserId - * @param string $headerToken - * @param IUser|null $existingUser - * @return IUser|null */ private function provisionUser( - string $provisioningStrategyClass, Provider $provider, string $tokenUserId, string $headerToken, + string $provisioningStrategyClass, + Provider $provider, + string $tokenUserId, + string $headerToken, ?IUser $existingUser, ): ?IUser { $provisioningStrategy = \OC::$server->get($provisioningStrategyClass); + return $provisioningStrategy->provisionUser($provider, $tokenUserId, $headerToken, $existingUser); } } diff --git a/lib/User/Provisioning/IProvisioningStrategy.php b/lib/User/Provisioning/IProvisioningStrategy.php index 79210e10..00a792bd 100644 --- a/lib/User/Provisioning/IProvisioningStrategy.php +++ b/lib/User/Provisioning/IProvisioningStrategy.php @@ -1,5 +1,7 @@ provisioningService->provisionUser($tokenUserId, $provider->getId(), $payload, $userFromOtherBackend); + return $provisioningResult['user']; } } diff --git a/lib/User/Validator/IBearerTokenValidator.php b/lib/User/Validator/IBearerTokenValidator.php index b130dac0..6289f540 100644 --- a/lib/User/Validator/IBearerTokenValidator.php +++ b/lib/User/Validator/IBearerTokenValidator.php @@ -1,12 +1,12 @@ get(ProviderService::class); + $providerService = \OCP\Server::get(ProviderService::class); $providerId = $provider->getId(); $uidAttribute = $providerService->getSetting($providerId, ProviderService::SETTING_MAPPING_UID, ProviderService::SETTING_MAPPING_UID_DEFAULT); @@ -77,6 +77,7 @@ public function isValidBearerToken(Provider $provider, string $bearerToken): ?st // find the user ID $uid = $this->provisioningService->getClaimValue($payload, $uidAttribute, $providerId); + return $uid ?: null; } diff --git a/lib/User/Validator/UserInfoValidator.php b/lib/User/Validator/UserInfoValidator.php index fd36a08c..2af77599 100644 --- a/lib/User/Validator/UserInfoValidator.php +++ b/lib/User/Validator/UserInfoValidator.php @@ -1,12 +1,12 @@ userInfoService->userinfo($provider, $bearerToken); $providerId = $provider->getId(); $uidAttribute = $this->providerService->getSetting($providerId, ProviderService::SETTING_MAPPING_UID, ProviderService::SETTING_MAPPING_UID_DEFAULT); + // find the user ID $uid = $this->provisioningService->getClaimValue($userInfo, $uidAttribute, $providerId); + return $uid ?: null; }