diff --git a/AUTHORS.md b/AUTHORS.md index cac0acad..7cf6cfb8 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -40,3 +40,4 @@ - Thibault Coupin - Tobias Wolter - Vincent Petry +- Elvis Yerel Roman Concha \ No newline at end of file diff --git a/README.md b/README.md index e9e205f9..fe60ff00 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,51 @@ OpenID Connect user backend for Nextcloud See [Nextcloud and OpenID-Connect](https://web.archive.org/web/20240412121655/https://www.schiessle.org/articles/2023/07/04/nextcloud-and-openid-connect/) for a proper jumpstart. +--- + +## `user_oidc.httpclient.allowselfsigned` + +```php +'user_oidc' => [ + 'httpclient.allowselfsigned' => true, +] +``` + +This configuration allows Nextcloud to **trust self-signed SSL certificates** when making HTTP requests through the internal HTTP client. +It is especially useful when your OAuth2 or OIDC provider is hosted locally or uses a self-signed certificate not recognized by public CAs. + +* **true**: Disables SSL certificate verification (adds the `verify => false` option to the actual HTTP client) +* **false** (default): SSL verification remains enabled and strict + +> ⚠️ Use with caution in production environments, as disabling certificate verification can introduce security risks. + +--- + +## `user_oidc.prompt` + +```php +'user_oidc' => [ + 'prompt' => 'internal' +] +``` + +This option allows customizing the `prompt` parameter sent in the OAuth2/OIDC authorization request. + +Supported values include: + +* `none` +* `login` +* `consent` +* `internal` (custom) + +The `internal` prompt is specific to **[OAuth2 Passport Server](https://github.com/elyerr/oauth2-passport-server)** and is designed to enable seamless login +for private or internal applications without requiring user consent or interaction. + +Documentation for all supported prompt values is available here: +[Oauth2 passport server prompts-supported](https://gitlab.com/elyerr/oauth2-passport-server/-/wikis/home/prompts-supported) + +--- + ### User IDs The OpenID Connect backend will ensure that user ids are unique even when multiple providers would report the same user diff --git a/composer.lock b/composer.lock index 9cd1c0f6..39cfd606 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "6ec5b146c0205f89df6c9883eab66a38", + "content-hash": "1879aca685c182293d8b9e124cfc0946", "packages": [ { "name": "bamarni/composer-bin-plugin", @@ -460,21 +460,21 @@ }, { "name": "nextcloud/coding-standard", - "version": "v1.3.2", + "version": "v1.4.0", "source": { "type": "git", "url": "https://github.com/nextcloud/coding-standard.git", - "reference": "9c719c4747fa26efc12f2e8b21c14a9a75c6ba6d" + "reference": "8e06808c1423e9208d63d1bd205b9a38bd400011" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/nextcloud/coding-standard/zipball/9c719c4747fa26efc12f2e8b21c14a9a75c6ba6d", - "reference": "9c719c4747fa26efc12f2e8b21c14a9a75c6ba6d", + "url": "https://api.github.com/repos/nextcloud/coding-standard/zipball/8e06808c1423e9208d63d1bd205b9a38bd400011", + "reference": "8e06808c1423e9208d63d1bd205b9a38bd400011", "shasum": "" }, "require": { "kubawerlos/php-cs-fixer-custom-fixers": "^3.22", - "php": "^7.3|^8.0", + "php": "^8.0", "php-cs-fixer/shim": "^3.17" }, "type": "library", @@ -494,11 +494,14 @@ } ], "description": "Nextcloud coding standards for the php cs fixer", + "keywords": [ + "dev" + ], "support": { "issues": "https://github.com/nextcloud/coding-standard/issues", - "source": "https://github.com/nextcloud/coding-standard/tree/v1.3.2" + "source": "https://github.com/nextcloud/coding-standard/tree/v1.4.0" }, - "time": "2024-10-14T16:49:05+00:00" + "time": "2025-06-19T12:27:27+00:00" }, { "name": "nextcloud/ocp", @@ -2696,8 +2699,5 @@ "prefer-lowest": false, "platform": {}, "platform-dev": {}, - "platform-overrides": { - "php": "8.0" - }, "plugin-api-version": "2.6.0" } diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 3f400e9e..4ab20f8e 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -20,6 +20,7 @@ use OCA\UserOIDC\Db\ProviderMapper; use OCA\UserOIDC\Db\SessionMapper; use OCA\UserOIDC\Event\TokenObtainedEvent; +use OCA\UserOIDC\Helper\HttpClientHelper; use OCA\UserOIDC\Service\DiscoveryService; use OCA\UserOIDC\Service\LdapService; use OCA\UserOIDC\Service\OIDCService; @@ -40,7 +41,6 @@ use OCP\Authentication\Token\IToken; use OCP\DB\Exception; use OCP\EventDispatcher\IEventDispatcher; -use OCP\Http\Client\IClientService; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; @@ -72,7 +72,7 @@ public function __construct( private LdapService $ldapService, private ISecureRandom $random, private ISession $session, - private IClientService $clientService, + private HttpClientHelper $clientService, private IURLGenerator $urlGenerator, private IUserSession $userSession, private IUserManager $userManager, @@ -260,6 +260,8 @@ public function login(int $providerId, ?string $redirectUrl = null) { } } + $oidcConfig = $this->config->getSystemValue('user_oidc', []); + $data += [ 'client_id' => $provider->getClientId(), 'response_type' => 'code', @@ -268,11 +270,16 @@ public function login(int $providerId, ?string $redirectUrl = null) { 'claims' => json_encode($claims), 'state' => $state, 'nonce' => $nonce, + 'prompt' => $oidcConfig['prompt'] ?? 'consent' ]; + + if ($isPkceEnabled) { $data['code_challenge'] = $this->toCodeChallenge($code_verifier); $data['code_challenge_method'] = 'S256'; } + + $authorizationUrl = $this->discoveryService->buildAuthorizationUrl($discovery['authorization_endpoint'], $data); $this->logger->debug('Redirecting user to: ' . $authorizationUrl); @@ -356,7 +363,6 @@ public function code(string $state = '', string $code = '', string $scope = '', $isPkceSupported = in_array('S256', $discovery['code_challenge_methods_supported'] ?? [], true); $isPkceEnabled = $isPkceSupported && ($oidcSystemConfig['use_pkce'] ?? true); - $client = $this->clientService->newClient(); try { $requestBody = [ 'code' => $code, @@ -370,10 +376,12 @@ public function code(string $state = '', string $code = '', string $scope = '', $headers = []; $tokenEndpointAuthMethod = 'client_secret_post'; // Use Basic only if client_secret_post is not available as supported by the endpoint - if (array_key_exists('token_endpoint_auth_methods_supported', $discovery) && - is_array($discovery['token_endpoint_auth_methods_supported']) && - in_array('client_secret_basic', $discovery['token_endpoint_auth_methods_supported']) && - !in_array('client_secret_post', $discovery['token_endpoint_auth_methods_supported'])) { + if ( + array_key_exists('token_endpoint_auth_methods_supported', $discovery) + && is_array($discovery['token_endpoint_auth_methods_supported']) + && in_array('client_secret_basic', $discovery['token_endpoint_auth_methods_supported']) + && !in_array('client_secret_post', $discovery['token_endpoint_auth_methods_supported']) + ) { $tokenEndpointAuthMethod = 'client_secret_basic'; } @@ -388,12 +396,10 @@ public function code(string $state = '', string $code = '', string $scope = '', $requestBody['client_secret'] = $providerClientSecret; } - $result = $client->post( + $body = $this->clientService->post( $discovery['token_endpoint'], - [ - 'body' => $requestBody, - 'headers' => $headers, - ] + $requestBody, + $headers ); } catch (ClientException|ServerException $e) { $response = $e->getResponse(); @@ -417,7 +423,7 @@ public function code(string $state = '', string $code = '', string $scope = '', return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, [], false); } - $data = json_decode($result->getBody(), true); + $data = json_decode($body, true); $this->logger->debug('Received code response: ' . json_encode($data, JSON_THROW_ON_ERROR)); $this->eventDispatcher->dispatchTyped(new TokenObtainedEvent($data, $provider, $discovery)); @@ -676,7 +682,8 @@ public function singleLogoutService() { $endSessionEndpoint .= '&client_id=' . $provider->getClientId(); $shouldSendIdToken = $this->providerService->getSetting( $provider->getId(), - ProviderService::SETTING_SEND_ID_TOKEN_HINT, '0' + ProviderService::SETTING_SEND_ID_TOKEN_HINT, + '0' ) === '1'; $idToken = $this->session->get(self::ID_TOKEN); if ($shouldSendIdToken && $idToken) { @@ -826,7 +833,9 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok * @return JSONResponse */ private function getBackchannelLogoutErrorResponse( - string $error, string $description, array $throttleMetadata = [], + string $error, + string $description, + array $throttleMetadata = [], ): JSONResponse { $this->logger->debug('Backchannel logout error. ' . $error . ' ; ' . $description); return new JSONResponse( diff --git a/lib/Helper/HttpClientHelper.php b/lib/Helper/HttpClientHelper.php index d1b2cfb6..86197881 100644 --- a/lib/Helper/HttpClientHelper.php +++ b/lib/Helper/HttpClientHelper.php @@ -8,32 +8,47 @@ namespace OCA\UserOIDC\Helper; -use OCP\Http\Client\IClientService; +use OCP\IConfig; require_once __DIR__ . '/../../vendor/autoload.php'; use Id4me\RP\HttpClient; +use OCP\Http\Client\IClientService; class HttpClientHelper implements HttpClient { public function __construct( private IClientService $clientService, + private IConfig $config, ) { } - public function get($url, array $headers = []) { + public function get($url, array $headers = [], array $options = []) { + $oidcConfig = $this->config->getSystemValue('user_oidc', []); + $client = $this->clientService->newClient(); - return $client->get($url, [ - 'headers' => $headers, - ])->getBody(); + if (isset($oidcConfig['httpclient.allowselfsigned']) + && !in_array($oidcConfig['httpclient.allowselfsigned'], [false, 'false', 0, '0'], true)) { + $options['verify'] = false; + } + + return $client->get($url, $options)->getBody(); } public function post($url, $body, array $headers = []) { + $oidcConfig = $this->config->getSystemValue('user_oidc', []); $client = $this->clientService->newClient(); - return $client->post($url, [ + $options = [ 'headers' => $headers, 'body' => $body, - ])->getBody(); + ]; + + if (isset($oidcConfig['httpclient.allowselfsigned']) + && !in_array($oidcConfig['httpclient.allowselfsigned'], [false, 'false', 0, '0'], true)) { + $options['verify'] = false; + } + + return $client->post($url, $options)->getBody(); } } diff --git a/lib/Service/DiscoveryService.php b/lib/Service/DiscoveryService.php index fcf781c7..2261008c 100644 --- a/lib/Service/DiscoveryService.php +++ b/lib/Service/DiscoveryService.php @@ -10,9 +10,9 @@ namespace OCA\UserOIDC\Service; use OCA\UserOIDC\Db\Provider; +use OCA\UserOIDC\Helper\HttpClientHelper; use OCA\UserOIDC\Vendor\Firebase\JWT\JWK; use OCA\UserOIDC\Vendor\Firebase\JWT\JWT; -use OCP\Http\Client\IClientService; use OCP\ICache; use OCP\ICacheFactory; use Psr\Log\LoggerInterface; @@ -39,7 +39,7 @@ class DiscoveryService { public function __construct( private LoggerInterface $logger, - private IClientService $clientService, + private HttpClientHelper $clientService, private ProviderService $providerService, ICacheFactory $cacheFactory, ) { @@ -53,9 +53,7 @@ public function obtainDiscovery(Provider $provider): array { $url = $provider->getDiscoveryEndpoint(); $this->logger->debug('Obtaining discovery endpoint: ' . $url); - $client = $this->clientService->newClient(); - $response = $client->get($url); - $cachedDiscovery = $response->getBody(); + $cachedDiscovery = $this->clientService->get($url); $this->cache->set($cacheKey, $cachedDiscovery, self::INVALIDATE_DISCOVERY_CACHE_AFTER_SECONDS); } @@ -77,8 +75,7 @@ public function obtainJWK(Provider $provider, string $tokenToDecode, bool $useCa $rawJwks = json_decode($rawJwks, true); } else { $discovery = $this->obtainDiscovery($provider); - $client = $this->clientService->newClient(); - $responseBody = $client->get($discovery['jwks_uri'])->getBody(); + $responseBody = $this->clientService->get($discovery['jwks_uri']); $rawJwks = json_decode($responseBody, true); // cache jwks $this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE, $responseBody); @@ -99,8 +96,8 @@ public function obtainJWK(Provider $provider, string $tokenToDecode, bool $useCa public function buildAuthorizationUrl(string $authorizationEndpoint, array $extraGetParameters = []): string { $parsedUrl = parse_url($authorizationEndpoint); - $urlWithoutParams = - (isset($parsedUrl['scheme']) ? $parsedUrl['scheme'] . '://' : '') + $urlWithoutParams + = (isset($parsedUrl['scheme']) ? $parsedUrl['scheme'] . '://' : '') . ($parsedUrl['host'] ?? '') . (isset($parsedUrl['port']) ? ':' . strval($parsedUrl['port']) : '') . ($parsedUrl['path'] ?? ''); diff --git a/lib/Service/OIDCService.php b/lib/Service/OIDCService.php index abb96696..ede6ff6f 100644 --- a/lib/Service/OIDCService.php +++ b/lib/Service/OIDCService.php @@ -10,7 +10,7 @@ namespace OCA\UserOIDC\Service; use OCA\UserOIDC\Db\Provider; -use OCP\Http\Client\IClientService; +use OCA\UserOIDC\Helper\HttpClientHelper; use OCP\Security\ICrypto; use Psr\Log\LoggerInterface; use Throwable; @@ -20,7 +20,7 @@ class OIDCService { public function __construct( private DiscoveryService $discoveryService, private LoggerInterface $logger, - private IClientService $clientService, + private HttpClientHelper $clientService, private ICrypto $crypto, ) { } @@ -31,7 +31,6 @@ public function userinfo(Provider $provider, string $accessToken): array { return []; } - $client = $this->clientService->newClient(); $this->logger->debug('Fetching user info endpoint'); $options = [ 'headers' => [ @@ -39,7 +38,7 @@ public function userinfo(Provider $provider, string $accessToken): array { ], ]; try { - return json_decode($client->get($url, $options)->getBody(), true); + return json_decode($this->clientService->get($url, [], $options), true); } catch (Throwable $e) { return []; } @@ -57,18 +56,18 @@ public function introspection(Provider $provider, string $accessToken): array { return []; } - $client = $this->clientService->newClient(); $this->logger->debug('Fetching user info endpoint'); - $options = [ - 'headers' => [ - 'Authorization' => base64_encode($provider->getClientId() . ':' . $providerClientSecret), - ], - 'body' => [ - 'token' => $accessToken, - ], - ]; + try { - return json_decode($client->post($url, $options)->getBody(), true); + $body = $this->clientService->post( + $url, + ['token' => $accessToken], + [ + 'Authorization' => base64_encode($provider->getClientId() . ':' . $providerClientSecret), + ] + ); + + return json_decode($body, true); } catch (Throwable $e) { return []; } diff --git a/lib/Service/TokenService.php b/lib/Service/TokenService.php index f47101dc..1acaf0aa 100644 --- a/lib/Service/TokenService.php +++ b/lib/Service/TokenService.php @@ -13,6 +13,7 @@ use OCA\UserOIDC\AppInfo\Application; use OCA\UserOIDC\Db\ProviderMapper; use OCA\UserOIDC\Exception\TokenExchangeFailedException; +use OCA\UserOIDC\Helper\HttpClientHelper; use OCA\UserOIDC\Model\Token; use OCA\UserOIDC\Vendor\Firebase\JWT\JWT; use OCP\App\IAppManager; @@ -20,7 +21,6 @@ use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\EventDispatcher\IEventDispatcher; use OCP\Http\Client\IClient; -use OCP\Http\Client\IClientService; use OCP\IConfig; use OCP\IRequest; use OCP\ISession; @@ -42,7 +42,7 @@ class TokenService { private IClient $client; public function __construct( - IClientService $clientService, + public HttpClientHelper $clientService, private ISession $session, private IUserSession $userSession, private IConfig $config, @@ -55,7 +55,7 @@ public function __construct( private DiscoveryService $discoveryService, private ProviderMapper $providerMapper, ) { - $this->client = $clientService->newClient(); + } public function storeToken(array $tokenData): Token { @@ -169,15 +169,13 @@ public function refresh(Token $token): Token { } } $this->logger->debug('[TokenService] Refreshing the token: ' . $discovery['token_endpoint']); - $result = $this->client->post( + $body = $this->clientService->post( $discovery['token_endpoint'], [ - 'body' => [ - 'client_id' => $oidcProvider->getClientId(), - 'client_secret' => $clientSecret, - 'grant_type' => 'refresh_token', - 'refresh_token' => $token->getRefreshToken(), - ], + 'client_id' => $oidcProvider->getClientId(), + 'client_secret' => $clientSecret, + 'grant_type' => 'refresh_token', + 'refresh_token' => $token->getRefreshToken(), ] ); $this->logger->debug('[TokenService] Token refresh request params', [ @@ -186,7 +184,7 @@ public function refresh(Token $token): Token { 'grant_type' => 'refresh_token', // 'refresh_token' => $token->getRefreshToken(), ]); - $body = $result->getBody(); + $bodyArray = json_decode(trim($body), true, 512, JSON_THROW_ON_ERROR); $this->logger->debug('[TokenService] ---- Refresh token success'); return $this->storeToken( @@ -256,23 +254,22 @@ public function getExchangedToken(string $targetAudience, array $extraScopes = [ } $this->logger->debug('[TokenService] Exchanging the token: ' . $discovery['token_endpoint']); // more in https://www.keycloak.org/securing-apps/token-exchange - $result = $this->client->post( + $body = $this->clientService->post( $discovery['token_endpoint'], [ - 'body' => [ - 'client_id' => $oidcProvider->getClientId(), - 'client_secret' => $clientSecret, - 'grant_type' => 'urn:ietf:params:oauth:grant-type:token-exchange', - 'subject_token' => $loginToken->getAccessToken(), - 'subject_token_type' => 'urn:ietf:params:oauth:token-type:access_token', - // can also be - // urn:ietf:params:oauth:token-type:access_token - // or urn:ietf:params:oauth:token-type:id_token - // this one will get us an access token and refresh token within the response - 'requested_token_type' => 'urn:ietf:params:oauth:token-type:refresh_token', - 'audience' => $targetAudience, - 'scope' => $scope, - ], + 'client_id' => $oidcProvider->getClientId(), + 'client_secret' => $clientSecret, + 'grant_type' => 'urn:ietf:params:oauth:grant-type:token-exchange', + 'subject_token' => $loginToken->getAccessToken(), + 'subject_token_type' => 'urn:ietf:params:oauth:token-type:access_token', + // can also be + // urn:ietf:params:oauth:token-type:access_token + // or urn:ietf:params:oauth:token-type:id_token + // this one will get us an access token and refresh token within the response + 'requested_token_type' => 'urn:ietf:params:oauth:token-type:refresh_token', + 'audience' => $targetAudience, + 'scope' => $scope, + 'prompt' => $this->config->getSystemValue('user_oidc.prompt', 'consent') // none,consent,login and internal for oauth2 passport server ] ); $this->logger->debug('[TokenService] Token exchange request params', [ @@ -284,7 +281,7 @@ public function getExchangedToken(string $targetAudience, array $extraScopes = [ 'requested_token_type' => 'urn:ietf:params:oauth:token-type:refresh_token', 'audience' => $targetAudience, ]); - $body = $result->getBody(); + $bodyArray = json_decode(trim($body), true, 512, JSON_THROW_ON_ERROR); $this->logger->debug('[TokenService] Token exchange success: "' . trim($body) . '"'); $tokenData = array_merge( diff --git a/tests/unit/Service/DiscoveryServiceTest.php b/tests/unit/Service/DiscoveryServiceTest.php index a5943d88..1e36c1ea 100644 --- a/tests/unit/Service/DiscoveryServiceTest.php +++ b/tests/unit/Service/DiscoveryServiceTest.php @@ -8,9 +8,9 @@ declare(strict_types=1); +use OCA\UserOIDC\Helper\HttpClientHelper; use OCA\UserOIDC\Service\DiscoveryService; use OCA\UserOIDC\Service\ProviderService; -use OCP\Http\Client\IClientService; use OCP\ICacheFactory; use PHPUnit\Framework\Assert; use PHPUnit\Framework\MockObject\MockObject; @@ -24,9 +24,9 @@ class DiscoveryServiceTest extends TestCase { */ private $logger; /** - * @var IClientService|MockObject + * @var HttpClientHelper|MockObject */ - private $clientService; + private $clientHelper; /** * @var ProviderService|MockObject */ @@ -43,10 +43,10 @@ class DiscoveryServiceTest extends TestCase { public function setUp(): void { parent::setUp(); $this->logger = $this->createMock(LoggerInterface::class); - $this->clientService = $this->createMock(IClientService::class); + $this->clientHelper = $this->createMock(HttpClientHelper::class); $this->providerService = $this->createMock(ProviderService::class); $this->cacheFactory = $this->createMock(ICacheFactory::class); - $this->discoveryService = new DiscoveryService($this->logger, $this->clientService, $this->providerService, $this->cacheFactory); + $this->discoveryService = new DiscoveryService($this->logger, $this->clientHelper, $this->providerService, $this->cacheFactory); } public function testBuildAuthorizationUrl() {