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

WEBAPI: PHP session is always started 2.1.2 #7213

Closed
boldhedgehog opened this issue Oct 27, 2016 · 7 comments · Fixed by #26032
Closed

WEBAPI: PHP session is always started 2.1.2 #7213

boldhedgehog opened this issue Oct 27, 2016 · 7 comments · Fixed by #26032
Assignees
Labels
bug report Component: Framework/Webapi USE ONLY for FRAMEWORK RELATED BUG! E.g If bug related to Catalog WEB API use just Catalog Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround.

Comments

@boldhedgehog
Copy link

boldhedgehog commented Oct 27, 2016

Although REST API should be stateless for anonymous calls, PHP session is always created. This is caused by the fact that session_start() is called implicitly from '\Magento\Framework\Session\SessionManager' constructor.

There are 2 issues with this: Spammed PHP session which will never be used, if remote address validation is enabled for sessions, clients with dynamic IP address will get 302 redirect instead of REST API result, and this is undesirable.

Preconditions

  1. Magento 2.4-develop

Steps to reproduce

  1. Make a REST request, for example from Swagger UI, but it can be a request from any client. For example, call /V1/directory/countries

Expected result

  1. Received JSON response with countries;
  2. No PHP session is started;
  3. No PHPSESSID in the response cookies.

Actual result

  1. PHP session is started and is perhaps never used (because the request is anonymous)

Why?

The reason for this is that in di.xml \Magento\Authorization\Model\CompositeUserContext is fed with userContexts argument, and at least 2 of them will start PHP session: customerSessionUserContext and adminSessionUserContext.

How to fix

My PoC solution was to modify vendor/magento/module-customer/etc/webapi_rest/di.xml and vendor/magento/module-user/etc/webapi_rest/di.xml so that types for userContext would be Proxies, and they would be created on-demand.

<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
     <type name="Magento\Framework\Authorization">
        <plugin name="customerAuthorization" type="Magento\Customer\Model\Plugin\CustomerAuthorization" />
    </type>
    <type name="Magento\Authorization\Model\CompositeUserContext">
        <arguments>
            <argument name="userContexts" xsi:type="array">
                <item name="customerSessionUserContext" xsi:type="array">
 <!-- *********************** LET IT BE A PROXY ************************** -->
                    <item name="type" xsi:type="object">Magento\Customer\Model\Authorization\CustomerSessionUserContext\Proxy</item>
                    <item name="sortOrder" xsi:type="string">20</item>
                </item>
                <item name="adminSessionUserContext" xsi:type="array">
 <!-- *********************** LET IT BE A PROXY ************************** -->
                    <item name="type" xsi:type="object">Magento\User\Model\Authorization\AdminSessionUserContext\Proxy</item>
                    <item name="sortOrder" xsi:type="string">30</item>
                </item>
            </argument>
        </arguments>
    </type>
</config>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
    <type name="Magento\Authorization\Model\CompositeUserContext">
        <arguments>
            <argument name="userContexts" xsi:type="array">
                <item name="adminSessionUserContext" xsi:type="array">
 <!-- *********************** LET IT BE A PROXY ************************** -->
                    <item name="type" xsi:type="object">Magento\User\Model\Authorization\AdminSessionUserContext\Proxy</item>
                    <item name="sortOrder" xsi:type="string">30</item>
                </item>
            </argument>
        </arguments>
    </type>
</config>

I do not know which one of the changes fixed the issue, but I achieved the expected result.

@veloraven veloraven added bug report Component: Framework/Webapi USE ONLY for FRAMEWORK RELATED BUG! E.g If bug related to Catalog WEB API use just Catalog 2.1.x labels Oct 27, 2016
@magento-engcom-team magento-engcom-team added bug report Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Component: Framework/Webapi USE ONLY for FRAMEWORK RELATED BUG! E.g If bug related to Catalog WEB API use just Catalog labels Sep 11, 2017
@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Oct 10, 2017
@magento-engcom-team
Copy link
Contributor

@boldhedgehog, thank you for your report.
We've created internal ticket(s) MAGETWO-81390 to track progress on the issue.

@magento-engcom-team magento-engcom-team added 2.2.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Oct 11, 2017
@magento-team
Copy link
Contributor

Hi @boldhedgehog. Thank you for your report.
The issue has been fixed in magento-engcom/magento2ce#1247 by @serhii-balko in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming patch release.

@magento-team magento-team added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Jan 22, 2018
magento-team pushed a commit that referenced this issue Jan 22, 2018
…1247

 - Merge Pull Request magento-engcom/magento2ce#1247 from serhii-balko/magento2:github-7213
 - Merged commits:
   1. a282973
   2. 6a58610
@maqlec
Copy link
Contributor

maqlec commented Dec 13, 2019

Dear @magento-team, you have merged something wrong because this fix is in 2.2 but in 2.3 and later is not.

maqlec pushed a commit to maqlec/magento2 that referenced this issue May 2, 2020
@ihor-sviziev ihor-sviziev reopened this May 20, 2020
@ghost ghost removed the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label May 20, 2020
@ghost ghost removed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels May 20, 2020
@ghost ghost assigned maqlec May 20, 2020
@ghost ghost added this to PR In Progress in Community Backlog May 20, 2020
@ihor-sviziev
Copy link
Contributor

I re-opened this task. Issue was fixed in 2.2.x, but not in 2.3.x and 2.4.x release lines. #26032 fixes this issue

@ihor-sviziev ihor-sviziev added the Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround. label May 20, 2020
@VladimirZaets VladimirZaets added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Jun 17, 2020
@engcom-Delta engcom-Delta self-assigned this Jul 1, 2020
@m2-assistant
Copy link

m2-assistant bot commented Jul 1, 2020

Hi @engcom-Delta. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

  • 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).

    DetailsIf the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.

  • 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.

  • 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

  • 4. Verify that the issue is reproducible on 2.4-develop branch

    Details- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
    - If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

  • 5. Add label Issue: Confirmed once verification is complete.

  • 6. Make sure that automatic system confirms that report has been added to the backlog.

@engcom-Delta engcom-Delta added Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed and removed Fixed in 2.2.x The issue has been fixed in 2.2 release line labels Jul 1, 2020
@ghost ghost unassigned engcom-Delta Jul 1, 2020
@magento-engcom-team
Copy link
Contributor

✅ Confirmed by @engcom-Delta
Thank you for verifying the issue. Based on the provided information internal tickets MC-35585 were created

Issue Available: @engcom-Delta, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

@magento-engcom-team magento-engcom-team added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Jul 1, 2020
@slavvka slavvka added the Fixed in 2.4.x The issue has been fixed in 2.4-develop branch label Aug 1, 2020
@slavvka slavvka closed this as completed Aug 1, 2020
@ghost ghost moved this from PR In Progress to Done (last 30 days) in Community Backlog Aug 1, 2020
magento-engcom-team added a commit that referenced this issue Aug 1, 2020
…6032

 - Merge Pull Request #26032 from maqlec/magento2:issue-7213
 - Merged commits:
   1. 859307b
   2. 8aed39f
   3. 4a05e78
   4. cfa889e
   5. b219c4e
   6. c5ad40b
   7. fe0bd17
   8. ac0dea5
   9. ea10595
   10. b4a02dd
   11. 4f20656
   12. 192bc7e
   13. f149763
   14. ac0f4e6
   15. 3e833b3
   16. c1f5509
   17. edd0ccd
   18. 4dac67d
   19. 97b235d
   20. 16797c3
magento-engcom-team pushed a commit that referenced this issue Nov 12, 2021
[Platform Health] Updates for PHP8.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Component: Framework/Webapi USE ONLY for FRAMEWORK RELATED BUG! E.g If bug related to Catalog WEB API use just Catalog Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround.
Projects
No open projects
Community Backlog
  
Done (last 30 days)
Development

Successfully merging a pull request may close this issue.

9 participants