Fix issue 13454. Allow to query checkout by ID as unauthorized user

[i-so-late]

Description

This pull request relates to issue #13454.

Before the change, the checkout query doesn't allow fetching the checkout by ID when the checkout.user field is set and an unauthorized user sends the request. The logic of this query is as follows: if checkout.user is not set, it returns checkout. If checkout.user is set, checkout is returned only when the currently authenticated user corresponds to checkout.user. Therefore, unauthenticated users cannot query checkout by ID.

Now, according to the instructions in the issue, when the channel is active, checkout will always be returned.

Testing

This pull request utilizes both pytest and manual testing. For manual testing, after running the project locally, I followed the instructions in the documentation to create a token in the GraphQL interface and then created and queried checkouts. The results were as expected.

For pytest, after running the test sets, test_anonymous_client_cant_fetch_checkout_with_attached_user, test_query_customer_checkout_as_anonymous_customer and test_query_other_customer_checkout_as_customer failed, which also was expected since the logic of this function has changed.

Impact

• New migrations
• New/Updated API fields or mutations
• Deprecated API fields or mutations
• Removed API types, fields, or mutations

Docs

• Link to documentation: checkout
  In the #Permissions# section, if we apply this change, we need to modify this description. Because now querying checkout only requires using the ID.

Pull Request Checklist

• Privileged queries and mutations are either absent or guarded by proper permission checks
• Database queries are optimized and the number of queries is constant
• Database migrations are either absent or optimized for zero downtime
• The changes are covered by test cases
• All new fields/inputs/mutations have proper labels added (ADDED_IN_X, PREVIEW_FEATURE, etc.)
• All migrations have proper dependencies
• All indexes are added concurrently in migrations
• All RunSql and RunPython migrations have revert option defined

[CLAassistant]

All committers have signed the CLA.

[kadewu]

Hello @i-so-late thank you for your contribution!
About the PR as you have rightly noticed, the tests are failing which means they should be adjusted to the new logic, as we do not allow to merge changes when tests fails. Please let me know if you can work on that.

[i-so-late]

Hello @i-so-late thank you for your contribution! About the PR as you have rightly noticed, the tests are failing which means they should be adjusted to the new logic, as we do not allow to merge changes when tests fails. Please let me know if you can work on that.

Hello, thank you for your response. Based on my understanding, what I need to do is to modify the three testing functions in pytest to adapt to the new logic. Then I would begin to work on this and submit a new PR.

[kadewu]

@i-so-late
Yes, those tests needs to be adjusted or removed, based on what is the exact scope of these.
Also no need to create new PR, you can push changes to this one here, it's easier to track this way.

[i-so-late]

@i-so-late Yes, those tests needs to be adjusted or removed, based on what is the exact scope of these. Also no need to create new PR, you can push changes to this one here, it's easier to track this way.

Hello,

I've encountered an issue when modifying a pytest test case. After changing the function logic in saleor/graphql/checkout/tests/test_checkout.py::test_anonymous_client_cant_fetch_checkout_with_attached_user, the get_graphql_content function throws an error: AssertionError: [{'message': 'To access this path, you need one of the following permissions: MANAGE_USERS, HANDLE_PAYMENTS, OWNER', 'locations': [{'line': 4, 'column': 12}], 'path': ['checkout', 'user'], 'extensions': {'exception': {'code': 'PermissionDenied'}}}]. However, another test, test_query_customer_checkout_as_anonymous_customer, does not have this issue. I think the logic in these two tests is the same, and I have reviewed the doc, but I still do not understand what is causing this difference in results. Could you please share your thoughts on this?

Thank you.

[kadewu]

Hello @i-so-late , the difference between those tests are different graphql queries. When you try to access Checkout.user.id you will run into permission check, which is of course expected, and thats the reason why the test is failing.

[aniav]

Hey @i-so-late let me know if you intend to finish this PR so we know if we should wait or update it and merge 🙌

[Air-t]

Hello @i-so-late thanks for your contribution to the project! I'm closing this PR since this change was introduced here as a part of 3 separate issues.