nexedi / erp5

added 6 commits

• 535e1c35 - oauth_google_login: reuse setAuthCookie
• 3ab994fe - oauth_google_login: inline getGoogleClientIdAndSecretKey
• 1ad9cf49 - oauth_facebook_login: minor BT definition fixes
• 6f20134d - oauth_facebook_login: reuse setAuthCookie
• 3d585385 - oauth_facebook_login: inline getFacebookClientIdAndSecretKey
• d2d4b8c4 - core: set SameSite=Lax on authentication cookie

Compare with previous version

added 8 commits

• cd2a8367 - testERP5Security: test how ERP5 sets cookie attributes
• b2123e0e - oauth_google_login: BT definition fixes
• c79e5e64 - oauth_google_login: reuse setAuthCookie
• 0453a1a0 - oauth_google_login: inline getGoogleClientIdAndSecretKey
• 62feb4d9 - oauth_facebook_login: minor BT definition fixes
• 992b660b - oauth_facebook_login: reuse setAuthCookie
• 839d8549 - oauth_facebook_login: inline getFacebookClientIdAndSecretKey
• e030adfc - core: set SameSite=Lax on authentication cookie

Compare with previous version

added 3 commits

• 03072ffc - oauth_facebook_login: reuse setAuthCookie
• 05deeb26 - oauth_facebook_login: inline getFacebookClientIdAndSecretKey
• a68d12eb - core: set SameSite=Lax on authentication cookie

Compare with previous version

@jerome keep in mind that you must keep the compatibility with officeJS apps, which will not be possible anymore if you add the lax parameter.

MR 138 was also trying to use it, but it was also making cross domain cookie more robust.

Thanks for feedback, @cedric.leninivin also explained that this was something that was initially planned as part of !138.

How to reproduce this problem with officejs app ? erp5_officejs_support_request_ui_test testLogin is passing... After thinking a bit, I think I understand the problematic scenario, it's when using an officejs app on one origin and using jio connector to access an ERP5 from another origin (which we don't cover in tests currently as they all use same origin). I did not consider this scenario.

BTW, I forgot to mention also @rafael who can be interested in the facebook/google fixes and @yusei and @vpelletier - we talked together about this.

@cedric.leninivin zrote a test in his merge request. I do not remember what was the status of the implementation, but it is for sure a good starting point.

To progress faster and because only jerome/erp5@a68d12eb seem to have officejs incompatibilities I extracted everything else in !803

I have not fully understood !138 yet but I start to understand why this made sense. I guess this ( !802 ) alone cannot work without breaking officejs.

The story here is that it seemed like an easy improvement, so I though "let's fix this problem quickly", but I completely missed this scenario. Now I don't see how we can improve this quickly. I cannot allocate the time to continue !138 or work on any other "more integrated login from officejs", so maybe we can close this for now.

BTW, about the testing, I have been thinking that we could add a scenario of cross origin usage of ERP5/JIO in a "normal" functional test. We could use an office js app ( with just some static html js ) hosted somewhere, for example https://test-remote-erp5.app.officejs.com/ . The test could be a selenium test doing something like:

1. create something in the runUnitTest instance to be synchronized
2. logs out the browser from the runUnitTest instance
3. open https://test-remote-erp5.app.officejs.com/
4. choose synchronize from ERP5
5. enter the url of the runUnitTest instance
   
6. login (which currently fail with this branch)
7. proceed with synchronization
8. inspect https://test-remote-erp5.app.officejs.com/ and check "something" was synchronized.
9. ... we can also check that another origin app such as https://test-another-remote-erp5.app.officejs.com/ cannot use. ( which currently fail on master )

does it make sense ? is it worth making a silly app just for testing ( we could use an existing app) ?

All OfficeJS application functionnalities can't be tested with our test framework currently, and what you're describing look more like an integration/monitoring test on the OfficeJS urls: check that all storages (erp5, dropbox, etc) work as expected, check that site is not down, etc.

Maybe, this can be done with the scalability testing framework (no idea if it is possible). And duplicating the Zelenium macros to Selenium to reach somehow stable tests will take time.

This would be an ERP5 test (not OfficeJS test), checking that ERP5 uses authentication cookies that are compatible with cross origin access like OfficeJS does.

I don't really like it, but I felt it would be better than no test.

added 90 commits

• a68d12eb...ee05c9c4 - 89 commits from branch nexedi:master
• 65882e59 - core: set SameSite=Lax on authentication cookie

Compare with previous version

marked as a Work In Progress

Mozilla announced that they are implement SameSite=lax by default. I guess chromium will follow as the proposal came from them. So this MR will soon be useless (and current officejs login will be broken).

I guess we can close this.

closed