diff --git a/lib/Controller/ConsentController.php b/lib/Controller/ConsentController.php index 8f79e168..67c2c98d 100644 --- a/lib/Controller/ConsentController.php +++ b/lib/Controller/ConsentController.php @@ -201,11 +201,22 @@ public function grant(): RedirectResponse { // Update the scope in session to use granted scopes instead of requested $this->session->set('oidc_scope', $grantedScopes); - // Build authorize URL BEFORE closing session (need to read oidc_client_id) - $authorizeUrl = $this->urlGenerator->linkToRoute('oidc.LoginRedirector.authorize', [ + // Build authorize URL with ALL OAuth params to avoid session dependency. + // Previously only client_id and scope were passed, relying on session + // fallback in LoginRedirectorController. This caused intermittent 500 + // errors when session values were lost between the redirect and the + // subsequent GET request (race condition with session->close()). + $authorizeUrl = $this->urlGenerator->linkToRoute('oidc.LoginRedirector.authorize', array_filter([ 'client_id' => $this->session->get('oidc_client_id'), 'scope' => $grantedScopes, - ]); + 'state' => $this->session->get('oidc_state'), + 'response_type' => $this->session->get('oidc_response_type'), + 'redirect_uri' => $this->session->get('oidc_redirect_uri'), + 'nonce' => $this->session->get('oidc_nonce'), + 'resource' => $this->session->get('oidc_resource'), + 'code_challenge' => $this->session->get('oidc_code_challenge'), + 'code_challenge_method' => $this->session->get('oidc_code_challenge_method'), + ])); // IMPORTANT: Close the session to commit changes before redirecting // Without this, the authorize endpoint won't see the updated session values diff --git a/lib/Controller/LoginRedirectorController.php b/lib/Controller/LoginRedirectorController.php index 453df55d..adf4a395 100644 --- a/lib/Controller/LoginRedirectorController.php +++ b/lib/Controller/LoginRedirectorController.php @@ -213,18 +213,39 @@ public function authorize( $scopeFromParam = $scope ?? 'null'; $this->logger->debug('[SCOPE DEBUG] Scope from URL parameter: ' . $scopeFromParam); - if (empty($client_id)) { - $client_id = $this->session->get('oidc_client_id'); - $this->logger->debug('[CLIENT DEBUG] Client ID from session fallback: ' . ($client_id ?? 'null')); - $state = $this->session->get('oidc_state'); - $response_type = $this->session->get('oidc_response_type'); - $redirect_uri = $this->session->get('oidc_redirect_uri'); - $scope = $this->session->get('oidc_scope'); + if (empty($client_id) || empty($state) || empty($response_type) || empty($redirect_uri)) { + if (empty($client_id)) { + $client_id = $this->session->get('oidc_client_id'); + $this->logger->debug('[CLIENT DEBUG] Client ID from session fallback: ' . ($client_id ?? 'null')); + } + if (empty($state)) { + $state = $this->session->get('oidc_state'); + } + if (empty($response_type)) { + $response_type = $this->session->get('oidc_response_type'); + } + if (empty($redirect_uri)) { + $redirect_uri = $this->session->get('oidc_redirect_uri'); + } + $scope = $scope ?? $this->session->get('oidc_scope'); $this->logger->debug('[SCOPE DEBUG] Scope from session fallback: ' . ($scope ?? 'null')); - $nonce = $this->session->get('oidc_nonce'); - $resource = $this->session->get('oidc_resource'); - $code_challenge = $this->session->get('oidc_code_challenge'); - $code_challenge_method = $this->session->get('oidc_code_challenge_method'); + $nonce = $nonce ?? $this->session->get('oidc_nonce'); + $resource = $resource ?? $this->session->get('oidc_resource'); + $code_challenge = $code_challenge ?? $this->session->get('oidc_code_challenge'); + $code_challenge_method = $code_challenge_method ?? $this->session->get('oidc_code_challenge_method'); + } + + // Guard: if critical OAuth params are still missing after session fallback, + // return a meaningful error instead of letting downstream code crash with a 500 + // (e.g. matchRedirectUri(null, ...) or trim(null) in PHP 8.4). + if (empty($state) || empty($response_type) || empty($redirect_uri)) { + $this->logger->error('Missing critical OAuth params after session fallback: ' + . 'state=' . var_export($state, true) . ', ' + . 'response_type=' . var_export($response_type, true) . ', ' + . 'redirect_uri=' . var_export($redirect_uri, true)); + return new TemplateResponse('core', '400', [ + 'message' => $this->l->t('Authorization session expired. Please try again.'), + ], 'error'); } // Set default scope if scope is not set at all