The #2946: Login fails and no warning is issued if cookies are not enabled causes TrustedRedirectResponse::isSafe() to return false when formerly it always returned true for the same configuration.
The website encountered an unexpected error. Please try again later.
InvalidArgumentException: It is not safe to redirect to https://site.com?check_logged_in=1 in Drupal\Component\HttpFoundation\SecuredRedirectResponse->setTargetUrl() (line 58 of core/lib/Drupal/Component/HttpFoundation/SecuredRedirectResponse.php).
Drupal\user\Authentication\Provider\Cookie->addCheckToUrl(Object, 'kernel.response', Object)
call_user_func(Array, Object, 'kernel.response', Object) (Line: 142)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.response') (Line: 191)
Symfony\Component\HttpKernel\HttpKernel->filterResponse(Object, Object, 1) (Line: 179)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 80)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 191)
Drupal\page_cache\StackMiddleware\PageCache->fetch(Object, 1, 1) (Line: 128)
Drupal\page_cache\StackMiddleware\PageCache->lookup(Object, 1, 1) (Line: 82)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 49)
Asm89\Stack\Cors->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
Configuration:
session.storage.options.cookie_domain = .site.comtrusted_host_patterns = ['^api\\.site\\.com$', '^site\\.com$']
The login process (SSO):
- https://site.com - visited by a user.
- https://REDACTED.auth.eu-west-1.amazoncognito.com/oauth2/authorize?state... - visited by a user by hitting a login link.
- https://api.site.com/auth/cognito/frontend?code=CODE&state=STATE - redirected automatically after signing in.
- https://site.com - redirected by the
TrustedRedirectResponsewithin a custom module that handles Cognito authorization.
On the final step, the TrustedRedirectResponse::$trustedUrls has ['https://site.com'] while the redirect URL is modified to https://site.com?check_logged_in=1. This causes it to be considered unsafe and not local because the Drupal instance lives on https://api.site.com.

| Comment | File | Size | Author |
|---|---|---|---|
| #5 | interdiff_3-5.txt | 1.22 KB | murilohp |
| #5 | 3253889-5.patch | 3.76 KB | murilohp |
| #3 | 3253889-3.patch | 3.76 KB | br0ken |
Comments
Comment #2
br0kenComment #3
br0kenComment #4
catchPatch looks good to me, bumping to major.
Comment #5
murilohp commentedThe patch looks good! It applied without any problems, I just have one minor suggestion, inside the UserAuthTest, you've used static calls to valid the assert and the expect for the mocks, I think for this scenario, it's a good idea to use "$this" instead of "static::", this way we keep the consistency of the code, since all the methods are using "$this->{method}".
The following patch is the suggestion that I've mentioned, I'll keep to needs to review to see if anyone has any other ideas.
Thanks!
Comment #6
longwaveThis also looks good to me - nice fix, test is good too. Agree with #5 that we use
$this->for PHPUnit even though the methods are static.Comment #9
catchLooks good here too. Committed/pushed to 10.0.x and cherry-picked to 9.4.x and 9.3.x, thanks!