magento/magento2#: Remove oauthUserContext from app/code/Magento/GraphQl/etc/graphql/di.xml #27373

atwixfirster · 2020-03-20T12:58:56Z

Description (*)

Issue:
With an active customer's cookies and empty Authorization token GraphQl uses cookies to retrieve a type of user and it's ID.

That's incorrect behavior because GraphQl should use an Authorization token ONLY to retrieve a type of user and it's ID.

Technical details:
Magento_Customer/etc/graphql/di.xml adds customerSessionUserContext into \Magento\Authorization\Model\CompositeUserContext. It allows to retrieves a customer ID from the session (cookies) wihtin GraphQl operations.

Related Pull Requests

Fixed Issues (if relevant)

GraphQl. Retrieve customer's shopping cart without an authorization token #28040: GraphQl. Retrieve customer's shopping cart without an authorization token

Manual testing scenarios (*)

Preconditions:

Cloud environment
PWA and Magento installed on the same <Magento_root> folder
PWA and Magento are available for user per configured Magento_UpwardConnector

PWA storefront: login as registered user
PWA storefront: as a customer please add any product into shopping cart
Open a new tab in the same browser window and login as a registered customer in Magento standard storefront
PWA storefront: customer makes a logout
PWA storefront: verify Shopping cart.

Actual result: guest sees a product which customer has been added into shopping cart. Current quote as a masked quote ID of registered customer.

Expected result: a new guest quote has been created after customer's logout.

Questions or comments

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

atwixfirster · 2020-03-25T18:50:09Z

Failed Functional tests are not tie to the PR changes.

dmytro-ch

Hello @atwixfirster, thank you for your contribution!
Due to Magento Definition of Done the changes should be covered by automated tests. Could you please cover this case by API functional test?
Thank you!

atwixfirster · 2020-04-29T14:55:05Z

Could you please cover this case by API functional test?

done

Thank you, @dmytro-ch

m2-assistant · 2020-04-30T15:49:45Z

Hi @atwixfirster, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

m2-assistant · 2020-04-30T15:50:08Z

Hi @atwixfirster. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

lenaorobei

Please see comment #28040 (comment)

Could you please remove oauthUserContext from the ce/app/code/Magento/GraphQl/etc/graphql/di.xml:18 instead.

…thorization\Model\CompositeUserContext in GraphQl operations

atwixfirster · 2020-05-03T17:07:08Z

Could you please remove oauthUserContext from the ce/app/code/Magento/GraphQl/etc/graphql/di.xml:18 instead.

@lenaorobei ,

done

…text

magento-engcom-team · 2020-05-04T17:18:22Z

Hi @lenaorobei, thank you for the review.
ENGCOM-7512 has been created to process this Pull Request
✳️ @lenaorobei, could you please add one of the following labels to the Pull Request?

Label	Description
`Auto-Tests: Covered`	All changes in Pull Request is covered by auto-tests
`Auto-Tests: Not Covered`	Changes in Pull Request requires coverage by auto-tests
`Auto-Tests: Not Required`	Changes in Pull Request does not require coverage by auto-tests

…/Magento/GraphQl/etc/graphql/di.xml #27373

m2-assistant · 2020-06-05T15:43:46Z

Hi @atwixfirster, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

nrkapoor · 2020-06-10T23:01:20Z

@danielrenaud @lenaorobei @paliarush Will this PR break the staging functionality in GraphQL?

ghost added the Progress: pending review label Mar 20, 2020

magento-engcom-team added Component: Customer Release Line: 2.4 Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Mar 20, 2020

dmytro-ch self-assigned this Mar 20, 2020

atwixfirster force-pushed the graphql-remove-customerSessionUserContext branch 2 times, most recently from b8d9aba to e601c74 Compare March 25, 2020 19:46

dmytro-ch requested changes Mar 29, 2020

View reviewed changes

ghost added Progress: needs update and removed Progress: pending review labels Mar 29, 2020

atwixfirster mentioned this pull request Apr 29, 2020

GraphQl. Retrieve customer's shopping cart without an authorization token #28040

Closed

atwixfirster force-pushed the graphql-remove-customerSessionUserContext branch from 5c57d52 to 7361a79 Compare April 29, 2020 14:54

ghost added Severity: S1 Affects critical data or functionality and forces users to employ a workaround. Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. labels Apr 29, 2020

lenaorobei closed this Apr 30, 2020

lenaorobei reopened this Apr 30, 2020

ghost added Progress: pending review and removed Progress: needs update labels Apr 30, 2020

ghost unassigned dmytro-ch Apr 30, 2020

lenaorobei requested changes Apr 30, 2020

View reviewed changes

ghost assigned lenaorobei Apr 30, 2020

ghost added Progress: needs update and removed Progress: pending review labels Apr 30, 2020

magento/magento2#: Remove customerSessionUserContext from \Magento\Au…

b96c8e9

…thorization\Model\CompositeUserContext in GraphQl operations

atwixfirster force-pushed the graphql-remove-customerSessionUserContext branch from 429aad8 to b96c8e9 Compare May 2, 2020 17:05

atwixfirster changed the title ~~magento/magento2#: Remove customerSessionUserContext from \Magento\Authorization\Model\CompositeUserContext in GraphQl operations~~ magento/magento2#: Remove oauthUserContext from app/code/Magento/GraphQl/etc/graphql/di.xml May 2, 2020

Merge branch '2.4-develop' into graphql-remove-customerSessionUserCon…

41f6a6e

…text

lenaorobei approved these changes May 4, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels May 4, 2020

magento-engcom-team added Component: GraphQL GraphQL and removed Component: Customer labels May 4, 2020

dmytro-ch added Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests Award: category of expertise labels May 6, 2020

nrkapoor requested review from cpartica and dthampy May 8, 2020 21:57

dthampy approved these changes May 13, 2020

View reviewed changes

ghost assigned dthampy May 13, 2020

lenaorobei added Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests and removed Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests labels Jun 2, 2020

ghost added Progress: testing in progress Progress: accept and removed Progress: ready for testing Progress: testing in progress labels Jun 2, 2020

lenaorobei added this to the 2.4.1 milestone Jun 2, 2020

magento-engcom-team added Component: GraphQL GraphQL and removed Component: GraphQL GraphQL labels Jun 2, 2020

magento-engcom-team pushed a commit that referenced this pull request Jun 5, 2020

ENGCOM-7512: magento/magento2#: Remove oauthUserContext from app/code…

0733d8c

…/Magento/GraphQl/etc/graphql/di.xml #27373

magento-engcom-team merged commit 9381f01 into magento:2.4-develop Jun 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

magento/magento2#: Remove oauthUserContext from app/code/Magento/GraphQl/etc/graphql/di.xml #27373

magento/magento2#: Remove oauthUserContext from app/code/Magento/GraphQl/etc/graphql/di.xml #27373

Uh oh!

atwixfirster commented Mar 20, 2020 •

edited

Loading

Uh oh!

atwixfirster commented Mar 25, 2020 •

edited

Loading

Uh oh!

dmytro-ch left a comment

Uh oh!

atwixfirster commented Apr 29, 2020

Uh oh!

m2-assistant bot commented Apr 30, 2020

Uh oh!

m2-assistant bot commented Apr 30, 2020

Uh oh!

lenaorobei left a comment

Uh oh!

atwixfirster commented May 3, 2020

Uh oh!

magento-engcom-team commented May 4, 2020

Uh oh!

m2-assistant bot commented Jun 5, 2020

Uh oh!

nrkapoor commented Jun 10, 2020

Uh oh!

Uh oh!

magento/magento2#: Remove oauthUserContext from app/code/Magento/GraphQl/etc/graphql/di.xml #27373

magento/magento2#: Remove oauthUserContext from app/code/Magento/GraphQl/etc/graphql/di.xml #27373

Uh oh!

Conversation

atwixfirster commented Mar 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Uh oh!

atwixfirster commented Mar 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmytro-ch left a comment

Choose a reason for hiding this comment

Uh oh!

atwixfirster commented Apr 29, 2020

Uh oh!

m2-assistant bot commented Apr 30, 2020

Uh oh!

m2-assistant bot commented Apr 30, 2020

Uh oh!

lenaorobei left a comment

Choose a reason for hiding this comment

Uh oh!

atwixfirster commented May 3, 2020

Uh oh!

magento-engcom-team commented May 4, 2020

Uh oh!

m2-assistant bot commented Jun 5, 2020

Uh oh!

nrkapoor commented Jun 10, 2020

Uh oh!

Uh oh!

atwixfirster commented Mar 20, 2020 •

edited

Loading

atwixfirster commented Mar 25, 2020 •

edited

Loading