Skip to content

Feat/reader activation tweaks #1780

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.

Sign up for GitHub

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

Merged
adekbadek merged 3 commits into from
Jul 15, 2022
Merged

Feat/reader activation tweaks #1780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2efc925
refactor: CSS classnames, logging, unused variables
adekbadek Jul 13, 2022
9d352c4
feat(reader-activation): set client ID cookie
adekbadek Jul 13, 2022
fc7fdff
fix(reader-activation): handle localization scripts in AMP Plus mode
adekbadek Jul 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion assets/blocks/reader-registration/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default function ReaderRegistrationEdit( {
</PanelBody>
</InspectorControls>
<div { ...blockProps }>
<div className="newspack-reader-registration">
<div className="newspack-registration">
<form onSubmit={ ev => ev.preventDefault() }>
<input type="email" placeholder={ placeholder } />
<input type="submit" value={ label } />
Expand Down
2 changes: 1 addition & 1 deletion assets/blocks/reader-registration/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
@use './style';

.newspack-reader-registration {
.newspack-registration {
form {
input[type='email'] {
background: #fff;
Expand Down
4 changes: 2 additions & 2 deletions assets/blocks/reader-registration/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ function render_block( $attrs ) {
// phpcs:enable
ob_start();
?>
<div class="newspack-reader-registration <?php echo esc_attr( get_block_classes( $attrs ) ); ?>">
<div class="newspack-registration <?php echo esc_attr( get_block_classes( $attrs ) ); ?>">
<?php if ( $registered ) : ?>
<p class="message"><?php echo \esc_html( $message ); ?></p>
<?php else : ?>
Expand All @@ -89,7 +89,7 @@ function render_block( $attrs ) {
<input type="email" name="email" autocomplete="email" placeholder="<?php echo \esc_attr( $attrs['placeholder'] ); ?>" />
<input type="submit" value="<?php echo \esc_attr( $attrs['label'] ); ?>" />
</form>
<div class="newspack-newsletters-registration-response">
<div class="newspack-registration__response">
<?php if ( ! empty( $message ) ) : ?>
<p><?php echo \esc_html( $message ); ?></p>
<?php endif; ?>
Expand Down
2 changes: 1 addition & 1 deletion assets/blocks/reader-registration/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.newspack-reader-registration {
.newspack-registration {
form {
width: 100%;
display: flex;
Expand Down
43 changes: 22 additions & 21 deletions assets/blocks/reader-registration/view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,49 +7,50 @@ import './style.scss';
if ( ! readerActivation ) {
return;
}
[ ...document.querySelectorAll( '.newspack-reader-registration' ) ].forEach( container => {
[ ...document.querySelectorAll( '.newspack-registration' ) ].forEach( container => {
const form = container.querySelector( 'form' );
if ( ! form ) {
return;
}
const messageContainer = container.querySelector(
'.newspack-newsletters-registration-response'
);
const submit = form.querySelector( 'input[type="submit"]' );
const messageElement = container.querySelector( '.newspack-registration__response' );
const submitElement = form.querySelector( 'input[type="submit"]' );
readerActivation.on( 'reader', ( { detail: { email } } ) => {
if ( email ) {
form.style.display = 'none';
}
} );

const endLoginFlow = ( message, status, data ) => {
const messageNode = document.createElement( 'p' );
messageNode.innerHTML = message;
messageNode.className = `message status-${ status }`;
if ( status === 200 ) {
container.replaceChild( messageNode, form );
if ( data?.email ) {
readerActivation.setReader( data.email );
}
} else {
messageElement.appendChild( messageNode );
}
};

form.addEventListener( 'submit', ev => {
ev.preventDefault();
const body = new FormData( form );
if ( ! body.has( 'email' ) || ! body.get( 'email' ) ) {
return;
}
submit.disabled = true;
messageContainer.innerHTML = '';
submitElement.disabled = true;
messageElement.innerHTML = '';
fetch( form.getAttribute( 'action' ) || window.location.pathname, {
method: 'POST',
headers: {
Accept: 'application/json',
},
body,
} ).then( res => {
submit.disabled = false;
res.json().then( ( { message, data } ) => {
const messageNode = document.createElement( 'p' );
messageNode.innerHTML = message;
messageNode.className = `message status-${ res.status }`;
if ( res.status === 200 ) {
container.replaceChild( messageNode, form );
if ( data?.email ) {
readerActivation.setReader( data.email );
}
} else {
messageContainer.appendChild( messageNode );
}
} );
submitElement.disabled = false;
res.json().then( ( { message, data } ) => endLoginFlow( message, res.status, data ) );
} );
} );
} );
Expand Down
20 changes: 20 additions & 0 deletions assets/reader-activation/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,19 @@ function getCookie( name ) {
if ( parts.length === 2 ) return decodeURIComponent( parts.pop().split( ';' ).shift() );
}

/**
* Set a cookie.
*
* @param {string} name Cookie name.
* @param {string} value Cookie value.
* @param {number} expirationDays Expiration in days from now.
*/
function setCookie( name, value, expirationDays = 365 ) {
const date = new Date();
date.setTime( date.getTime() + expirationDays * 24 * 60 * 60 * 1000 );
document.cookie = `${ name }=${ value }; expires=${ date.toUTCString() }; path=/`;
}

/**
* Initialize store data.
*/
Expand Down Expand Up @@ -135,4 +148,11 @@ export function getReader() {
const readerActivation = { on, off, setReader, getReader };
window.newspackReaderActivation = readerActivation;

const clientIDCookieName = window.newspack_reader_activation_data.cid_cookie;
if ( ! getCookie( clientIDCookieName ) ) {
// If entropy is an issue, https://www.npmjs.com/package/nanoid can be used.
const getShortStringId = () => Math.floor( Math.random() * 999999999 ).toString( 36 );
setCookie( clientIDCookieName, `${ getShortStringId() }${ getShortStringId() }` );
}

export default readerActivation;
2 changes: 0 additions & 2 deletions assets/wizards/connections/views/main/google.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const getURLParams = () => {
};

const GoogleOAuth = ( { setError, onInit, onSuccess } ) => {
const [ initialized, setInitialized ] = useState( false );
const [ authState, setAuthState ] = useState( {} );

const userBasicInfo = authState.user_basic_info;
Expand Down Expand Up @@ -73,7 +72,6 @@ const GoogleOAuth = ( { setError, onInit, onSuccess } ) => {
handleError( err );
} )
.finally( () => {
setInitialized( true );
setInFlight( false );
if ( typeof onInit === 'function' ) {
onInit( error );
Expand Down
4 changes: 4 additions & 0 deletions includes/class-amp-enhancements.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ public static function amp_validation_error_sanitized( $is_sanitized, $error ) {
if ( 0 === strpos( $error['node_attributes']['id'], 'wp-' ) ) {
return false;
}
// Allow any Reader Activation (e.g. localizations) scripts.
if ( 0 === strpos( $error['node_attributes']['id'], Reader_Activation::SCRIPT_HANDLE ) && Reader_Activation::is_enabled() ) {
return false;
}
// Allow Complianz plugin (complianz-gdpr) scripts, unless its AMP integration is enabled.
// If it is enabled, `amp-consent` will be used and allowing the script would duplicate the cookie prompt.
if ( function_exists( 'cmplz_is_integration_enabled' ) && ! cmplz_is_integration_enabled( 'amp' ) ) {
Expand Down
1 change: 1 addition & 0 deletions includes/class-magic-link.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ public static function send_email( $user, $redirect_to = '' ) {
$email['message'],
$email['headers']
);
Logger::log( 'Sending magic link to ' . $email['to'] );

return $sent;
}
Expand Down
1 change: 1 addition & 0 deletions includes/class-newspack.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ private function define_constants() {
}
define( 'NEWSPACK_ACTIVATION_TRANSIENT', '_newspack_activation_redirect' );
define( 'NEWSPACK_NRH_CONFIG', 'newspack_nrh_config' );
define( 'NEWSPACK_CLIENT_ID_COOKIE_NAME', 'newspack-cid' );
}

/**
Expand Down
15 changes: 10 additions & 5 deletions includes/class-reader-activation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
final class Reader_Activation {

const AUTH_INTENTION_COOKIE = 'np_auth_intention';
const SCRIPT_HANDLE = 'newspack-reader-activation';

/**
* Reader user meta keys.
Expand All @@ -41,9 +42,8 @@ public static function init() {
* Enqueue front-end scripts.
*/
public static function enqueue_scripts() {
$handle = 'newspack-reader-activation';
\wp_register_script(
$handle,
self::SCRIPT_HANDLE,
Newspack::plugin_url() . '/dist/reader-activation.js',
[],
NEWSPACK_PLUGIN_VERSION,
Expand All @@ -54,15 +54,17 @@ public static function enqueue_scripts() {
$reader_email = \wp_get_current_user()->user_email;
}
\wp_localize_script(
$handle,
self::SCRIPT_HANDLE,
'newspack_reader_activation_data',
[
'auth_intention_cookie' => self::AUTH_INTENTION_COOKIE,
'cid_cookie' => NEWSPACK_CLIENT_ID_COOKIE_NAME,
'nonce' => wp_create_nonce( 'wp_rest' ),
Copy link
Member

@miguelpeixe miguelpeixe Jul 15, 2022

Choose a reason for hiding this comment

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

@adekbadek I overlooked this line. What is the purpose of this nonce? It won't behave as expected for cached pages.

Copy link
Member Author

@adekbadek adekbadek Jul 19, 2022

Choose a reason for hiding this comment

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

This is necessary to make an authenticated request from the front-end. Not actually used in this PR, but in the Google login PR (which was branched off this PR's branch, initially).

It won't behave as expected for cached pages.

Can you elaborate on that?

Copy link
Member

@miguelpeixe miguelpeixe Jul 19, 2022

Choose a reason for hiding this comment

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

The nonce value will be cached as well, defeating the purpose of a number used once.

Do we need a nonce for /wp-json/newspack/v1/login/google/register? Why would that be susceptible to a CSRF attack?

Copy link
Member Author

@adekbadek adekbadek Jul 21, 2022

Choose a reason for hiding this comment

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

(discussion moved to #1781 (comment))

'reader_email' => $reader_email,
]
);
\wp_script_add_data( $handle, 'async', true );
\wp_script_add_data( $handle, 'amp-plus', true );
\wp_script_add_data( self::SCRIPT_HANDLE, 'async', true );
\wp_script_add_data( self::SCRIPT_HANDLE, 'amp-plus', true );
}

/**
Expand Down Expand Up @@ -284,6 +286,7 @@ public static function set_current_reader( $user_id ) {
\wp_set_current_user( $user->ID );
\wp_set_auth_cookie( $user->ID, true );
\do_action( 'wp_login', $user->user_login, $user );
Logger::log( 'Logged in user ' . $user->ID );

return $user;
}
Expand Down Expand Up @@ -363,6 +366,8 @@ public static function register_reader( $email, $display_name = '', $authenticat
\update_user_meta( $user_id, self::READER, true );
\update_user_meta( $user_id, self::EMAIL_VERIFIED, false );

Logger::log( 'Created new reader user with ID ' . $user_id );

if ( $authenticate ) {
self::set_current_reader( $user_id );
}
Expand Down
2 changes: 1 addition & 1 deletion includes/reader-revenue/class-woocommerce-connection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public static function create_transaction( $order_data ) {
$order->add_meta_data( '_stripe_currency', $order_data['currency'] );

if ( ! empty( $order_data['client_id'] ) ) {
$order->add_meta_data( 'newspack-cid', $order_data['client_id'] );
$order->add_meta_data( NEWSPACK_CLIENT_ID_COOKIE_NAME, $order_data['client_id'] );
}

$has_user_id = ! empty( $order_data['user_id'] );
Expand Down