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: investigate compatibility with CAS login plugin #3810

Closed
1 of 6 tasks
Benunc opened this issue Oct 29, 2018 · 3 comments
Closed
1 of 6 tasks

fix: investigate compatibility with CAS login plugin #3810

Benunc opened this issue Oct 29, 2018 · 3 comments

Comments

@Benunc
Copy link
Member

Benunc commented Oct 29, 2018

Bug Report

User Story

(directly from a ticket)

In our giving site we are using our CAS plugin instead of the Wordpress login. This allows us to have one password for our users (staff and donors) who are using our sites behind this plugin.

From our plugin the user can do all self-service tasks that you would expect, change their email address, first name, last name, password…

When a user changes these things in the plugin we need the changes to email address, first name and last name to migrate over to the Wordpress user and the Give user.

Current Behavior

PHP Fatal error: Uncaught Error: Call to a member function get_donor_by() on null in /var/www/html/wordpress/wp-content/plugins/give/includes/class-give-donor.php:168
Stack trace:
#0 /var/www/html/wordpress/wp-content/plugins/give/includes/admin/admin-actions.php(1099): Give_Donor->__construct(27, true)
#1 /var/www/html/wordpress/wp-includes/class-wp-hook.php(286): give_update_donor_email_on_user_update(27, Object(WP_User))
#2 /var/www/html/wordpress/wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters(27, Array)
#3 /var/www/html/wordpress/wp-includes/plugin.php(453): WP_Hook->do_action(Array)
#4 /var/www/html/wordpress/wp-includes/user.php(1764): do_action('profile_update', 27, Object(WP_User))
#5 /var/www/html/wordpress/wp-includes/user.php(1864): wp_insert_user(Array)
#6 /var/www/html/wordpress/wp-content/mu-plugins/gto-cas-login/src/gto-cas-login.php(283): wp_update_user(Array)
#7 /var/www/html/wordpress/wp-content/mu-plugins/gto-cas-login/vendor/jasig/phpcas/source/CAS/Client.php(1476): GlobalTechnology\CentralAuth in /var/www/html/wordpress/wp-content/plugins/give/includes/class-give-donor.php on line 168

Expected Behavior

The phpCAS plugin should work alongside Give to change donor-related info when WP user infor is changed, without errors.

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. Create a new web site with cas login plugin https://github.com/GlobalTechnology/gto-cas-login
  2. Activate Give plugin (only this plugin, no customizaton for Give)
  3. Create a very simple donation form
  4. Add a WordPress user as a subscriber (this user has already an account with The Key) Login of this user to WordPress (with admin login)
  5. This user makes a gift and so becomes a donor.
  6. Logout of the user
  7. Change name with cas login
  8. New login to Wordpress
  9. 500 error

Possible Solution

From the developer on the project ( @Omicron7 )

First instance

[24-Oct-2018 15:46:00 UTC] https://localhost.cru.org/givetest/wp-admin/
[24-Oct-2018 15:46:03 UTC] instance
[24-Oct-2018 15:46:03 UTC] Give_DB_Donors Object
(
[table_name] => wp_3_give_donors
[min_index_length] => 191
[version] => 1.0
[primary_key] => id
)

Second instance

[24-Oct-2018 15:46:57 UTC] https://localhost.cru.org/givetest/wp-admin/?ticket=ST-435-jWdZzt0I2f-lUFUBNMTRsJfyXkocas2
[24-Oct-2018 15:46:59 UTC] instance
[24-Oct-2018 15:46:59 UTC]
[24-Oct-2018 15:47:02 UTC] instance
[24-Oct-2018 15:47:02 UTC]
[24-Oct-2018 15:47:02 UTC] PHP Fatal error: Uncaught Error: Call to a member function get_donor_by() on null in /var/www/html/wordpress/wp-content/plugins/give/includes/class-give-donor.php:168
Stack trace:
#0 /var/www/html/wordpress/wp-content/plugins/give/includes/admin/admin-actions.php(1099): Give_Donor->__construct(27, true)
#1 /var/www/html/wordpress/wp-includes/class-wp-hook.php(286): give_update_donor_email_on_user_update(27, Object(WP_User))
#2 /var/www/html/wordpress/wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters(27, Array)
#3 /var/www/html/wordpress/wp-includes/plugin.php(453): WP_Hook->do_action(Array)
#4 /var/www/html/wordpress/wp-includes/user.php(1764): do_action('profile_update', 27, Object(WP_User))
#5 /var/www/html/wordpress/wp-includes/user.php(1864): wp_insert_user(Array)
#6 /var/www/html/wordpress/wp-content/mu-plugins/gto-cas-login/src/gto-cas-login.php(283): wp_update_user(Array)
#7 /var/www/html/wordpress/wp-content/mu-plugins/gto-cas-login/vendor/jasig/phpcas/source/CAS/Client.php(1476): GlobalTechnology\CentralAuth in /var/www/html/wordpress/wp-content/plugins/give/includes/class-give-donor.php on line 168

So, the first instance you’re seeing is from the request for the wp-admin, (or protected page) that causes the 302 redirect to CAS to pickup user login.

The second instance, where it seems like it lost the donors object is actually a new request to WordPress after authenticating with CAS. Since this request has a ?ticket in the URL, phpCAS will attempt to authenticate it. On success, it immediately updates the $user object which fires the ‘profile_update’ hook. This is so early in the WordPress lifecycle that Give hasn’t fully initialized the donors object yet.

My change to defer updating the profile until later didn’t work. When phpCAS successfully validates a ticket, it immediately issues a 302 redirect to strip the ?ticket from the url, thus halting WordPress execution. Any deferred hooks/filters or user attributes from CAS are lost at this point unless saved before the redirect.

Would it be possible to initialize the donor object earlier in the WordPress lifecycle, around ‘plugins_loaded’ maybe?

Related

https://secure.helpscout.net/conversation/691768407/0/?folderId=848135

Acceptance Criteria

  • Give works nicely alongside the phpCAS plugin
  • If not, a workaround or fix is suggested for the other plugin
  • The fix here does not affect existing Give users when changing name or other details
@Omicron7
Copy link

I've added the following to our plugin to work around the issue, waiting to hear from the user of Give if this fixes it. I'm also not sure if this would have any side effects on Give.

    /**
     * Pre-populates Give plugin donor and donor_meta properties if the request includes a ticket.
     * This bootstraps Give enough for it to change a donors email address during cas_authenticate.
     */
    public function give_plugin_pre_populate_donors() {
      if (class_exists('\Give') && array_key_exists('ticket', $_REQUEST)) {
        \Give()->donors = new \Give_DB_Donors();
        \Give()->donor_meta = new \Give_DB_Donor_Meta();
      }
    }
    add_action('plugins_loaded', 'give_plugin_pre_populate_donors', 0, 0);

@kevinwhoffman
Copy link
Contributor

@Omicron7 Thanks for helping out here. We'll wait to hear back from the user and then take a closer look if the issue is still unresolved.

@Benunc
Copy link
Member Author

Benunc commented Nov 21, 2018

Confirmed the fix from the user via email

@Benunc Benunc closed this as completed Nov 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants