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 user profile saving #573

Merged
merged 2 commits into from
Oct 25, 2018
Merged

Fix user profile saving #573

merged 2 commits into from
Oct 25, 2018

Conversation

joshcanhelp
Copy link
Contributor

Changes

  • Add wp_slash() processing before saving the user profile from Auth0
  • Simplified get user profile functions in WP_Auth0.php
  • Add tests to confirm broken behavior and fix
  • Add sample test suite tests/suiteTemplate.php

References

Testing

Tests were added before changes to confirm all current behavior as well as broken behavior.

  • This change adds unit test coverage
  • This change has been tested on the latest version of the platform

Checklist

  • All existing and new tests complete without errors
  • All code quality tools/guidelines in the Contribution guide have been run/followed
  • All active GitHub CI checks have passed

@joshcanhelp joshcanhelp added this to the 3.8.0 milestone Oct 24, 2018
}

return false;
return $profile ? WP_Auth0_Serializer::unserialize( $profile ) : false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplifying the return here.


$currentauth0_user = get_auth0userinfo( $current_user->ID );

$currentauth0_user = get_auth0userinfo( get_current_user_id() );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just need the ID here so switched to get_current_user_id().

$data->auth0_id = WP_Auth0_UsersRepo::get_meta( $current_user->ID, 'auth0_id' );

return $data;
return (object) array(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Type juggling to simplify object creation.


return $data;
return (object) array(
'auth0_obj' => get_auth0userinfo( get_current_user_id() ),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just need the ID here so switched to get_current_user_id().

Also changed object get to get_auth0userinfo() so all these functions pass through a single one (was also doing the same job here).

public function update_auth0_object( $user_id, $userinfo ) {
global $wpdb;
update_user_meta( $user_id, $wpdb->prefix . 'auth0_id', ( isset( $userinfo->user_id ) ? $userinfo->user_id : $userinfo->sub ) );
update_user_meta( $user_id, $wpdb->prefix . 'auth0_obj', WP_Auth0_Serializer::serialize( $userinfo ) );
$auth0_user_id = isset( $userinfo->user_id ) ? $userinfo->user_id : $userinfo->sub;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplify auth0_user_id setting.

$auth0_user_id = isset( $userinfo->user_id ) ? $userinfo->user_id : $userinfo->sub;
update_user_meta( $user_id, $wpdb->prefix . 'auth0_id', $auth0_user_id );
$userinfo_encoded = WP_Auth0_Serializer::serialize( $userinfo );
$userinfo_encoded = wp_slash( $userinfo_encoded );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actual fix mentioned in the Changes section of the description.

@@ -0,0 +1,87 @@
<?php
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No called anywhere, just a helper for creating new tests.

@joshcanhelp joshcanhelp force-pushed the fix-user-profile-escaping branch from 7fd479b to 4ba4b26 Compare October 24, 2018 22:38
$userinfo = $this->getUserinfo();

// Specially-encoded characters: ¥ £ € ¢ ₡ ₢ ₣ ₤ ₥ ₦ ₪ ₯
$userinfo->encodedValue1 = '\u00a5 \u00a3 \u20ac \u00a2 \u20a1 \u20a2 \u20a3 \u20a4 \u20a5 \u20a6 \u20aa \u20af';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actual problematic data here. Others before are just to confirm that things are stored and retrieved as expected.

@joshcanhelp joshcanhelp force-pushed the fix-user-profile-escaping branch 2 times, most recently from 852663a to 7f818d5 Compare October 24, 2018 23:16
@joshcanhelp joshcanhelp force-pushed the fix-user-profile-escaping branch from 7f818d5 to 427ccf0 Compare October 24, 2018 23:19
@codecov-io
Copy link

Codecov Report

Merging #573 into master will increase coverage by 0.11%.
The diff coverage is 90%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #573      +/-   ##
============================================
+ Coverage     21.15%   21.27%   +0.11%     
  Complexity     1313     1313              
============================================
  Files            51       51              
  Lines          4282     4278       -4     
============================================
+ Hits            906      910       +4     
+ Misses         3376     3368       -8
Impacted Files Coverage Δ Complexity Δ
lib/WP_Auth0_UsersRepo.php 48% <100%> (+1.6%) 41 <0> (ø) ⬇️
WP_Auth0.php 21.54% <80%> (+0.99%) 64 <0> (ø) ⬇️

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 6c1a898...427ccf0. Read the comment docs.

Copy link
Member

@cocojoe cocojoe left a comment

Choose a reason for hiding this comment

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

Some nice clean up

@joshcanhelp joshcanhelp merged commit 6affd80 into master Oct 25, 2018
@joshcanhelp joshcanhelp deleted the fix-user-profile-escaping branch October 25, 2018 14:36
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants