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

fix: give session cookie prefix #3819

Closed
mathetos opened this issue Oct 31, 2018 · 5 comments

Comments

Projects
None yet
4 participants
@mathetos
Copy link
Member

commented Oct 31, 2018

Bug Report

User Story

As a web host like Pantheon, we exclude sessions cookies from caching when they are prefixed according to the WP Codex cookie naming convention of wp-.

Current Behavior

Give currently names its cookies with an underscore, as in wp_give_session. This means that Give users on Pantheon cannot see their donation confirmation receipt successfully but see the email access or login form instead.

$this->cookie_name = apply_filters(
'give_session_cookie',
'wp_give_session_' . COOKIEHASH, // Cookie name.
'session' // Cookie type.
);

Expected Behavior

I expect the Give cookies to follow the WP Codex naming convention so that cookies are named more predictably for hosts.

Bug Type

  • This bug describes functionality that once worked as expected in version X.X.X.
  • This bug describes functionality that never worked as expected.
  • I am not sure whether this functionality ever worked as expected.

Steps to Reproduce

  1. Attempt a donation on this page hosted on Pantheon: http://dev-givetesting.pantheonsite.io/donations/first-form/ -- Credentials to the site can be provided to Give devteam via Slack.
  2. Note that the donation receipt does not appear correctly.
  3. Login to the site and change the cookie name to start with wp-
  4. Attempt a new donation and note that it now DOES appear correctly.

Possible Solution

Change the cookie name with backward compatibility.

Acceptance Criteria

  • The new cookie name is reflected every time the session is created successfully
  • There are not any other dependencies on that cookie name in the plugin that might negatively impact the functioning of Give or its Addons

Environment

### WordPress Environment ###

Home URL: http://dev-givetesting.pantheonsite.io
Site URL: http://dev-givetesting.pantheonsite.io
WP Version: 4.9.8
WP Multisite: –
WP Memory Limit: 256 MB
WP Debug Mode: –
WP Cron: ✔
Language: en_US
Permalink Structure: /%year%/%monthnum%/%day%/%postname%/
Show on Front: posts
Table Prefix Length: wp_
Table Prefix Length: 3
Table Prefix Status: Acceptable
Admin AJAX: Accessible
Registered Post Statuses: publish, future, draft, pending, private, trash, auto-draft, inherit, request-pending, request-confirmed, request-failed, request-completed, refunded, failed, revoked, cancelled, abandoned, processing, preapproval

Server Environment

Hosting Provider: DBH: 10.128.0.124:16302, SRV: appserver-26b699e6.c.pantheon-dmz.internal
TLS Connection: Connection uses TLS 1.2
TLS Connection: Probably Okay
Server Info: nginx/1.8.1
PHP Version: 7.2.11
PHP Post Max Size: 100 MB
PHP Time Limit: 120
PHP Max Input Vars: 10000
PHP Max Upload Size: 100 MB
cURL Version: 7.40.0, NSS/3.21 Basic ECC
SUHOSIN Installed: –
Default Timezone is UTC: ✔
fsockopen/cURL: ✔
SoapClient: ✔
DOMDocument: ✔
gzip: ✔
GD Graphics Library: ✔
Multibyte String: ✔
Remote Post: ✔
Remote Get: ✔

Give Configuration

Give Version: 2.3.0
Give Cache: Enabled
Database Updates: All DB Updates Completed.
Give Cache: Enabled
Give Cache: ✔New Donation✔Donation Receipt✔New Offline Donation✔Offline Donation Instructions✔New User Registration✔User Registration Information✔Donor Note✔Email access
Upgraded From: –
Test Mode: Enabled
Currency Code: USD
Currency Position: Before
Decimal Separator: .
Thousands Separator: ,
Success Page: http://dev-givetesting.pantheonsite.io/donation-confirmation/
Failure Page: http://dev-givetesting.pantheonsite.io/donation-failed/
Donation History Page: http://dev-givetesting.pantheonsite.io/donation-history/
Give Forms Slug: /donations/
Enabled Payment Gateways: Test Donation, Offline Donation
Default Payment Gateway: Test Donation
PayPal IPN Verification: Enabled
PayPal IPN Notifications: N/A
Donor Email Access: Enabled

Active Give Add-ons

Other Active Plugins

Inactive Plugins

Akismet: by Automattic – 3.1.5
Hello Dolly: by Matt Mullenweg – 1.6
My Custom Functions: by Space X-Chimp – 4.30
Native PHP Sessions for WordPress: by Pantheon – 0.6.9
WP Rollback: by WordImpress – 1.5.1

Active MU Plugins

Pantheon: by Pantheon – 0.1

Theme

Name: Twenty Seventeen
Version: 1.0
Author URL: https://wordpress.org/
Child Theme: No – If you're modifying Give on a parent theme you didn't build personally, then we recommend using a child theme. See: How to Create a Child Theme

Browser
  • Name: Chrome | Firefox | Safari | IE | Edge
  • Version: X.X.X
WordPress System Info
@kevinwhoffman

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

@mehul0810 Please look into this issue on the Pantheon server. If you need to manipulate the cookie name, you can change the coookie name on this line using the Dashboard > Plugins > Edit screen.

  • See if you can recreate the issue and fix it by using wp- instead of wp_.
  • Report whether there would be any issue with changing the cookie name to use wp- permanently going forward.
  • Report whether there are other cookies used by Give that should also be updated.
@mathetos

This comment has been minimized.

Copy link
Member Author

commented Oct 31, 2018

As a reference, this is the snippet we sent to customers as a workaround for this issue:

add_filter('give_session_cookie', 'my_custom_give_cookie_prefix', 99, 2);

function my_custom_give_cookie_prefix( $cookie, $session ) {
    $cookie = 'wp-give_session_' . COOKIEHASH; // Cookie name.

    return $cookie;
}
@ravinderk

This comment has been minimized.

Copy link
Collaborator

commented Nov 1, 2018

@mathetos @kevinwhoffman We already addressed this issue because Pantheon only allow cookies with specific prefix: #3705 (comment)

I will suggest adding support for Pantheon host in core if a user coming so often. Pantheon host defines PANTHEON_ENVIRONMENT global variable on basis of that we can set our logic: https://pantheon.io/docs/settings-php/

@mehul0810

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2018

Slack Call Summary

Participants: @mehul0810 @ravinderk
Topic: Discussion about cookies for a pantheon host
Result: I've noticed that changing cookie prefix from wp_ to wp- will make donation receipt working on pantheon. I'll make those changes in PHP as well as JS files and test it again to ensure it is working fine.

@mehul0810 mehul0810 referenced this issue Nov 1, 2018

Merged

fix: give session cookie prefix #3819 #3822

3 of 3 tasks complete
@mehul0810

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2018

Slack Call Summary

Participants: @mehul0810 @ravinderk
Topic: Discussion about making the cookie names dynamic
Result: Ravinder suggested me to make the cookie names dynamic as using session anyone can easily change the name and there is hardcoded cookie names implemented earlier which need to be dynamic.

@mathetos mathetos referenced this issue Nov 1, 2018

Closed

Update cookies.md #4214

0 of 4 tasks complete

ravinderk added a commit that referenced this issue Nov 2, 2018

Merge pull request #3822 from impress-org/issue/3819
fix: give session cookie prefix #3819
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.