Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CartLoadRoute is called with unauthenticated SalesChannelContext via CacheResponseSubscriber after LoginRoute #3666

Open
nyze2oo9 opened this issue Apr 13, 2024 · 4 comments
Labels

Comments

@nyze2oo9
Copy link

PHP Version

8.3

Shopware Version

6.5.8.8

Expected behaviour

After the initial login (when there's no entry for a specific customer in sales_channel_api_context), the SalesChannelContext passed to CartLoadRoute->load() from CacheResponseSubscriber->setResponseCache() should be "re-fetched"/"rebuilt" before calling $cart = $this->cartService->getCart($context->getToken(), $context); to ensure that the correct authenticated SalesChannelContext (with customer and customerId correctly set) is available in CartLoadRoute->load().

Maybe it could also be feasible to relocate the check below, preceding the call to $this->cartService->getCart(), but I'm uncertain about the potential performance implications this may entail.

// We need to allow it on login, otherwise the state is wrong
if (!($route === 'frontend.account.login' || $request->getMethod() === Request::METHOD_GET)) {
  return;
}

Actual behaviour

Currently, after the initial login (when there's no entry in sales_channel_api_context), the SalesChannelContext passed to CartLoadRoute->load() from CacheResponseSubscriber->setResponseCache() does not include information about the newly logged-in customer. From the second login onwards (when an entry in sales_channel_api_context exists), the passed SalesChannelContext contains the accurate information about the currently logged-in customer.

How to reproduce

  1. Remove all entries from sales_channel_api_context for a specific customer.
  2. Add $context->ensureLoggedIn(false) to a RouteDecorator for store-api.checkout.cart.read.
  3. Make a request to store-api.account.login.
  4. The first request will fail with code store-api.account.login (because of the missing customer and customerId).
  5. From the second request onward the request will succeed.
@nyze2oo9 nyze2oo9 added the Bug label Apr 13, 2024
@shyim
Copy link
Member

shyim commented Apr 13, 2024

Maybe we need to move this into the Login Route? 🤔

$newContext = $this->salesChannelContextService->get(
new SalesChannelContextServiceParameters(
$context->getSalesChannelId(),
$token,
$context->getLanguageId(),
$context->getCurrencyId(),
$context->getDomainId(),
$context->getContext()
)
);
// Update the sales channel context for CacheResponseSubscriber
$request->attributes->set(PlatformRequest::ATTRIBUTE_SALES_CHANNEL_CONTEXT_OBJECT, $newContext);

@nyze2oo9
Copy link
Author

I can confirm, that it work with a RouteDecorator for the LoginRoute

  #[Route(path: '/store-api/account/login', name: 'store-api.account.login', methods: ['POST'])]
  public function login(RequestDataBag $data, SalesChannelContext $context): ContextTokenResponse
  {
    $result = $this->decorated->login($data, $context);

    $newContext = $this->salesChannelContextService->get(
      new SalesChannelContextServiceParameters(
        $context->getSalesChannelId(),
        $result->getToken(),
        $context->getLanguageId(),
        $context->getCurrencyId(),
        $context->getDomainId(),
        $context->getContext()
      )
    );
    $request = $this->requestStack->getCurrentRequest();
    $request->attributes->set(PlatformRequest::ATTRIBUTE_SALES_CHANNEL_CONTEXT_OBJECT, $newContext);

    return $result;
  }

One thing I don't like is the fact that I had to inject the RequestStack into the decorator because the login method only gets passed the RequestDataBag.

If desired, I can work on a PR in the next few days to get this fix into the original route and get rid of the decorator?

@shyim
Copy link
Member

shyim commented Apr 15, 2024

sounds good :)

I would also check that getCurrentRequest is not null, before using it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants