X Tutup
Skip to content

Set samesite=strict coockies - will work starting php 7.3#37442

Merged
DeepDiver1975 merged 1 commit intomasterfrom
feature/strict-same-site-cookies
May 27, 2020
Merged

Set samesite=strict coockies - will work starting php 7.3#37442
DeepDiver1975 merged 1 commit intomasterfrom
feature/strict-same-site-cookies

Conversation

@DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented May 25, 2020 •
edited
Loading

Description

Starting with php7.3 samesite cookies can be set. This PR sets the session cookie to be samesite=stritc.

Related Issue

https://github.com/owncloud/enterprise/issues/3890

How Has This Been Tested?

🤖

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@DeepDiver1975 DeepDiver1975 force-pushed the feature/strict-same-site-cookies branch 2 times, most recently from 6e4ee95 to fad251e Compare May 26, 2020 07:17
@codecov
Copy link

codecov bot commented May 26, 2020 •
edited
Loading

Codecov Report

Merging #37442 into master will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #37442      +/-   ##
============================================
- Coverage     64.68%   64.67%   -0.01%     
- Complexity    19331    19332       +1     
============================================
  Files          1277     1277              
  Lines         75514    75521       +7     
  Branches       1331     1331              
============================================
  Hits          48846    48846              
- Misses        26276    26283       +7     
  Partials        392      392
Flag Coverage Δ Complexity Δ
#javascript 54.14% <ø> (ø) 0.00 <ø> (ø)
#phpunit 65.84% <0.00%> (-0.01%) 19332.00 <0.00> (+1.00) ⬇️
Impacted Files Coverage Δ Complexity Δ
lib/private/Session/CryptoWrapper.php 33.33% <0.00%> (-13.73%) 7.00 <0.00> (+1.00) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ef3bad...7429797. Read the comment docs.

@DeepDiver1975 DeepDiver1975 requested a review from C0rby May 26, 2020 09:49
@DeepDiver1975 DeepDiver1975 self-assigned this May 26, 2020
@DeepDiver1975 DeepDiver1975 force-pushed the feature/strict-same-site-cookies branch from fad251e to e3b6780 Compare May 26, 2020 09:51
@DeepDiver1975 DeepDiver1975 marked this pull request as ready for review May 26, 2020 09:51
phil-davis
phil-davis reviewed May 26, 2020
View reviewed changes
@DeepDiver1975 DeepDiver1975 force-pushed the feature/strict-same-site-cookies branch from e3b6780 to 7dbbfee Compare May 26, 2020 10:35
C0rby
C0rby approved these changes May 26, 2020
View reviewed changes
Copy link
Contributor

@C0rby C0rby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 looks good to me.

@owncloud owncloud deleted a comment from update-docs bot May 26, 2020
@phil-davis
Copy link
Contributor

phil-davis commented May 26, 2020

@DeepDiver1975 override codecov results?

I noticed that codecov is not being collected for unit test runs on PHP 7.3 and 7.4 - so that is probably why codecov failed here. I raised PR #37450 to fix that.

@phil-davis phil-davis force-pushed the feature/strict-same-site-cookies branch from 7dbbfee to 2dd6d55 Compare May 27, 2020 03:18
@phil-davis
Copy link
Contributor

phil-davis commented May 27, 2020

PR #37450 was merged. I rebased this - that will be a test of if the extra coverage data makes a different to the codecov results.

@DeepDiver1975 DeepDiver1975 force-pushed the feature/strict-same-site-cookies branch from 2dd6d55 to 7429797 Compare May 27, 2020 06:49
@DeepDiver1975
Copy link
Member Author

DeepDiver1975 commented May 27, 2020

From my pov we can ignore the code coverage on this PR. Ready to be merged from my pov.

@phil-davis
Copy link
Contributor

phil-davis commented May 27, 2020

Fine with me - someone with privs can override and merge.

@DeepDiver1975 DeepDiver1975 merged commit c5b037d into master May 27, 2020
@delete-merged-branch delete-merged-branch bot deleted the feature/strict-same-site-cookies branch May 27, 2020 09:11
@davitol davitol mentioned this pull request Jun 25, 2020
Closed
65 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@phil-davis phil-davis phil-davis approved these changes

+1 more reviewer

@C0rby C0rby C0rby approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@DeepDiver1975 DeepDiver1975

Labels

4 - To release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DeepDiver1975 @phil-davis @C0rby
X Tutup