diff --git a/app/Access/Oidc/OidcProviderSettings.php b/app/Access/Oidc/OidcProviderSettings.php index 49ccab6f0..616bf1012 100644 --- a/app/Access/Oidc/OidcProviderSettings.php +++ b/app/Access/Oidc/OidcProviderSettings.php @@ -18,7 +18,6 @@ class OidcProviderSettings public string $issuer; public string $clientId; public string $clientSecret; - public ?string $redirectUri; public ?string $authorizationEndpoint; public ?string $tokenEndpoint; public ?string $endSessionEndpoint; @@ -38,7 +37,7 @@ class OidcProviderSettings /** * Apply an array of settings to populate setting properties within this class. */ - protected function applySettingsFromArray(array $settingsArray) + protected function applySettingsFromArray(array $settingsArray): void { foreach ($settingsArray as $key => $value) { if (property_exists($this, $key)) { @@ -52,7 +51,7 @@ class OidcProviderSettings * * @throws InvalidArgumentException */ - protected function validateInitial() + protected function validateInitial(): void { $required = ['clientId', 'clientSecret', 'redirectUri', 'issuer']; foreach ($required as $prop) { @@ -74,12 +73,20 @@ class OidcProviderSettings public function validate(): void { $this->validateInitial(); + $required = ['keys', 'tokenEndpoint', 'authorizationEndpoint']; foreach ($required as $prop) { if (empty($this->$prop)) { throw new InvalidArgumentException("Missing required configuration \"{$prop}\" value"); } } + + $endpointProperties = ['tokenEndpoint', 'authorizationEndpoint', 'userinfoEndpoint']; + foreach ($endpointProperties as $prop) { + if (is_string($this->$prop) && !str_starts_with($this->$prop, 'https://')) { + throw new InvalidArgumentException("Endpoint value for \"{$prop}\" must start with https://"); + } + } } /** @@ -87,7 +94,7 @@ class OidcProviderSettings * * @throws OidcIssuerDiscoveryException */ - public function discoverFromIssuer(ClientInterface $httpClient, Repository $cache, int $cacheMinutes) + public function discoverFromIssuer(ClientInterface $httpClient, Repository $cache, int $cacheMinutes): void { try { $cacheKey = 'oidc-discovery::' . $this->issuer; @@ -180,9 +187,9 @@ class OidcProviderSettings /** * Get the settings needed by an OAuth provider, as a key=>value array. */ - public function arrayForProvider(): array + public function arrayForOAuthProvider(): array { - $settingKeys = ['clientId', 'clientSecret', 'redirectUri', 'authorizationEndpoint', 'tokenEndpoint', 'userinfoEndpoint']; + $settingKeys = ['clientId', 'clientSecret', 'authorizationEndpoint', 'tokenEndpoint', 'userinfoEndpoint']; $settings = []; foreach ($settingKeys as $setting) { $settings[$setting] = $this->$setting; diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php index 467e31417..00ac2b6dc 100644 --- a/app/Access/Oidc/OidcService.php +++ b/app/Access/Oidc/OidcService.php @@ -91,7 +91,6 @@ class OidcService 'issuer' => $config['issuer'], 'clientId' => $config['client_id'], 'clientSecret' => $config['client_secret'], - 'redirectUri' => url('/oidc/callback'), 'authorizationEndpoint' => $config['authorization_endpoint'], 'tokenEndpoint' => $config['token_endpoint'], 'endSessionEndpoint' => is_string($config['end_session_endpoint']) ? $config['end_session_endpoint'] : null, @@ -130,7 +129,10 @@ class OidcService */ protected function getProvider(OidcProviderSettings $settings): OidcOAuthProvider { - $provider = new OidcOAuthProvider($settings->arrayForProvider(), [ + $provider = new OidcOAuthProvider([ + ...$settings->arrayForOAuthProvider(), + 'redirectUri' => url('/oidc/callback'), + ], [ 'httpClient' => $this->http->buildClient(5), 'optionProvider' => new HttpBasicAuthOptionProvider(), ]);