thenewinquiry / tni-core-functionality

Contains the site's core functionality.
GNU General Public License v2.0
2 stars 0 forks source link

Remote authentication - Subscriber authentication #4

Closed misfist closed 7 years ago

misfist commented 7 years ago

From: https://github.com/thenewinquiry/tni-theme/issues/36

we're trying to figure out how to integrate the subscriber system with the WP site. As it stands, subscribers can create a user account with the subscription system (which will be located at members.thenewinquiry.com). The subscription system has a cookie-based JWT authentication system so users can login via other sites.

Here's what I had in mind, it would be great to get your thoughts on it as well:

GET to https://members.thenewinquiry.com/auth/ok, which tells us if the user is authenticated or not
if not authenticated
    We show a "Login" menu item at the top of the site
        When clicked, it shows a login modal, asking for email & password
        The email & password is POSTed to https://members.thenewinquiry.com/auth/login which will setup the necessary cookies for the user
    We show the "SUBSCRIBE" menu item at the top of the site
if authenticated
    The "Login" menu item is instead "Manage"
    The "SUBSCRIBE" menu item at the top becomes "LIBRARY"
when a user visits the library page (e.g. https://thenewinquiry.com/library)
    check for authentication
    if not authenticated, show e.g. "The library is available to subscribers only. Click here to subscribe"
    if authenticated, show list of past issues with links to PDFs

One thing I can't quite figure out here is how to fetch the library past issues after checking authentication, rather than rendering it as part of the page but just hiding it in the DOM.

What do you think?

misfist commented 7 years ago

The script needs to be enqueued. ( https://developer.wordpress.org/reference/functions/wp_enqueue_script/ ).

JQuery needs to be used in compatibility mode or need to map JQuery to $. (https://premium.wpmudev.org/blog/adding-jquery-scripts-wordpress/).

And the data needs to be made available to the theme. This can be done using the wp_head function. https://codex.wordpress.org/Plugin_API/Action_Reference/wp_head . Some practical examples: http://wordpress.stackexchange.com/questions/119573/is-it-possible-to-use-wp-localize-script-to-create-global-js-variables-without-a/119736

misfist commented 7 years ago

I've added a new branch, which begins to set things up.

https://github.com/thenewinquiry/tni-core-functionality/tree/1.0.9Authentication

If we want to run some JS when we load a page and pass info back so that we can decide whether or not the user can view the page, then we probably need to hook into wp_head.

If we want to make any data from WP available to the script, we would need to use wp_localize_script. https://codex.wordpress.org/Function_Reference/wp_localize_script

frnsys commented 7 years ago

Great, thank you.

I think we only need to use it in two places:

Header

If you are not logged in, you see:

image

Here, login will bring up a modal:

image

which, on submit, calls the login function

if you are logged in, you instead see:

image

when "Logout" is clicked, it calls the logout function (which I'll add soon).

"My Account" goes to members.thenewinquiry.com/manage.

Magazine issue page

If not logged in:

image

If logged in:

image

So I believe the only data we need to fetch from wordpress is the PDF link of the magazine?

frnsys commented 7 years ago

Looks like we also need to make a separate "Library" page which lists all past issues with their download links, e.g.:

image

misfist commented 7 years ago

We'll need to add a field to attach the PDF to the magazine issue.

misfist commented 7 years ago

Looks like we also need to make a separate "Library" page which lists all past issues with their download links.

I'm not sure how/why this would be a separate page from past issues. Why not just add the download link to the single issue?

frnsys commented 7 years ago

That's a good question...the main reason is symbolic, so subscribers get a different experience (i.e. the page will be designed differently). Aside from that, it is kind of redundant :\

misfist commented 7 years ago

I think that might add more complexity than necessary.

The page can be styled differently without creating a separate page. We can add a class to the body and/or use conditionals to display content.

In single.php, we would have something like:

if( 'magazines' === get_post_type() && is_subscriber() ) : 
   get_template_part( 'template-parts/content-single-magazines', 'subscriber' );
else : 
   get_template_part( 'template-parts/content-single', get_post_type() );

is_subscriber() would use the JS functions to return true/false.

frnsys commented 7 years ago

Ok that sounds good -- can you set that up? I'm having trouble figuring out how to do it myself...

misfist commented 7 years ago

Can you add a JS function for checking is a user is a logged in subscriber that we can call in the head so we can pass it to WP?

frnsys commented 7 years ago

this function will do that: https://github.com/thenewinquiry/tni-core-functionality/blob/1.0.9Authentication/assets/js/auth.js#L56

it takes a callback - do asynchronous functions need to be handled differently?

I also just realized that maybe we can do the authentication checks directly in php?

misfist commented 7 years ago

I also just realized that maybe we can do the authentication checks directly in php?

Yes, we could probably use wp_remote_get();

https://codex.wordpress.org/Function_Reference/wp_remote_get

frnsys commented 7 years ago

It just occurred to me that we need access to cookies to properly check authentication...as far as I know that isn't possible with PHP, or is it?

misfist commented 7 years ago

$_COOKIE['{cookiename}']

This shows PHP and JS equivalents: http://www.pontikis.net/blog/create-cookies-php-javascript

misfist commented 7 years ago

Since some of this ticket is related to the theme, I've added a separate ticket for present-related stuff.

https://github.com/thenewinquiry/tni-theme/issues/39

frnsys commented 7 years ago

I'm making some good progress on this, but still not clear on how I access the functions defining in auth.js from the theme? In particular, the login and logout functions.

frnsys commented 7 years ago

I'm not sure the best place to place this code, but here's what I have so far. I tested it with hardcoded values for the CSRF and access/refresh tokens; once I get the login and logout javascript functions working I can test the entire flow.

$BASEURL = 'https://members.thenewinquiry.com';

function refreshAuth() {
    global $BASEURL;
    $url = $BASEURL . '/auth/refresh';
    $headers = array( 'X-CSRF-TOKEN' => $_COOKIE['csrf_refresh_token'] );
    $cookies = array( new WP_Http_Cookie( array( 'name' => 'refresh_token_cookie', 'value' => $_COOKIE['refresh_token_cookie'] ) ) );

    $resp = wp_remote_post( $url, array( 'cookies' => $cookies, 'headers' => $headers ) );
    if( is_wp_error( $resp ) ) {
        echo $resp->get_error_message();
        return false;
    } else {
        $code = wp_remote_retrieve_response_code( $resp );
        return $code == 200;
    }
}

function checkAuth() {
    global $BASEURL;
    $url = $BASEURL . '/auth/ok';
    $headers = array( 'X-CSRF-TOKEN' => $_COOKIE['csrf_access_token'] );
    $cookies = array( new WP_Http_Cookie( array( 'name' => 'access_token_cookie', 'value' => $_COOKIE['access_token_cookie'] ) ) );

    $resp = wp_remote_get( $url, array( 'cookies' => $cookies, 'headers' => $headers ) );
    if( is_wp_error( $resp ) ) {
        echo $resp->get_error_message();
        return false;
    } else {
        $code = wp_remote_retrieve_response_code( $resp );
        if( $code == 200 ) {
            return true;
        } else if ( $code == 401 ) {
            // try to refresh token if expired
            $data = json_decode( $resp['body'] );
            if ( $data->msg == 'Token has expired' ) {
                return refreshAuth();
            }
        }
        return false;
    }
}

if ( checkAuth() ) {
    echo 'authorized';
} else {
    echo 'unauthorized';
}
misfist commented 7 years ago

I've added these to the core plugin.

See: https://github.com/thenewinquiry/tni-core-functionality/commit/3706a32215cbe0493131e3ef10341a1f6400b732

See: https://github.com/thenewinquiry/tni-core-functionality/commit/fd21fc64e61bb7905b68ee7f67f2380f48db2c09

We'd call the function wherever needed (check it exists first in case it's not available for some reason [like the core plugin isn't active]).

if( function_exists( 'tni_core_check_auth' ) ) {
  if ( tni_core_check_auth() ) {
     echo 'authorized';
  } else {
     echo 'unauthorized';
  }
}
frnsys commented 7 years ago

Awesome, thank you for cleaning that up. I'll work on getting the login/logout functionality in now. The way I was thinking was that the auth.js script would look for and bind to elements .js-login and .js-logout, e.g.:

$('.js-login').on('click', function() {
  var form = $('<form class="login-modal"><input type="text"></form>'); // for example
  $(body).append(form);
  form.on('submit', function() {
    login( ... ); // get login info from form
    form.remove();
  });
});

does that make sense, or is there a best practice around this?

misfist commented 7 years ago

We can add a class to a nav item, but it is added to the <li> tag not the link itself. So, it'd be .js-login a.

<li id="menu-item-75292" class="js-login menu-item menu-item-type-custom menu-item-object-custom menu-item-75292 login">
     <a href="#">Login</a>
</li>

If login is successful, we'll need to change the class to .js-logout and the text to "Log Out".

frnsys commented 7 years ago

I'm running to an issue with $_COOKIE that I can't quite figure out.

After logging in, the response looks correct:

Access-Control-Allow-Credentials:true
Access-Control-Allow-Origin:http://tni.vagrant.test
Content-Length:20
Content-Type:application/json
Date:Fri, 14 Apr 2017 19:41:56 GMT
Server:Werkzeug/0.12 Python/3.5.2
Set-Cookie:access_token_cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1c2VyX2NsYWltcyI6e30sImV4cCI6MTQ5MjE5OTgxNiwiaWRlbnRpdHkiOiJ0ZXN0QHRlc3QuY29tIiwiY3NyZiI6ImY0YWFiZjM0LThlYzUtNDUwYS05NWFkLThhZmY0MGZlNThmMyIsImlhdCI6MTQ5MjE5ODkxNiwidHlwZSI6ImFjY2VzcyIsIm5iZiI6MTQ5MjE5ODkxNiwiZnJlc2giOmZhbHNlLCJqdGkiOiJiYjgyM2I5YS1jNjEwLTRjYTctODI2ZC03ZTllYTM4MmYzNjcifQ.FOAyjJEXwhBGViG8bpXhMmqCw5ppuWqwhmAl-ZunLL4; HttpOnly; Path=/
Set-Cookie:csrf_access_token=f4aabf34-8ec5-450a-95ad-8aff40fe58f3; Path=/
Set-Cookie:refresh_token_cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE0OTQ3OTA5MTYsImlkZW50aXR5IjoidGVzdEB0ZXN0LmNvbSIsImNzcmYiOiI1NzU1ZWNjMS1kZDI5LTRjMzctYTA5ZS1kOTk5ZjgwYzUxNjQiLCJpYXQiOjE0OTIxOTg5MTYsInR5cGUiOiJyZWZyZXNoIiwibmJmIjoxNDkyMTk4OTE2LCJqdGkiOiJhZmM0MDc5NC01MTNiLTRmNjgtOWIyNy1jNzQ0ZjAzOGJjZGIifQ.LZgmn8lzggRTkWGNZI-kv8xSYb16lo6T1qGTSYoxvkI; HttpOnly; Path=/auth/refresh
Set-Cookie:csrf_refresh_token=5755ecc1-dd29-4c37-a09e-d999f80c5164; Path=/
Vary:Origin

but the cookies aren't actually set. The PHP $_COOKIE variable doesn't contain any of those.

Though javascript requests do seem to have the cookies properly set in their request headers:

Cookie:access_token_cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1c2VyX2NsYWltcyI6e30sImV4cCI6MTQ5MjE5OTQ3OSwiaWRlbnRpdHkiOiJ0ZXN0QHRlc3QuY29tIiwiY3NyZiI6ImM2M2RlODA5LTk3YzAtNDU5Yi05NGY5LTMwZTFmNjNhNTAyZCIsImlhdCI6MTQ5MjE5ODU3OSwidHlwZSI6ImFjY2VzcyIsIm5iZiI6MTQ5MjE5ODU3OSwiZnJlc2giOmZhbHNlLCJqdGkiOiIzY2U0YWVjOS00MWE4LTRiZWEtODRiNi1mZmRlYzZiOTAzNWIifQ.M89b2JreC7Wwjws0Ts5AEaw8EgiO0FfsEsKO0cxGLME; csrf_access_token=c63de809-97c0-459b-94f9-30e1f63a502d; csrf_refresh_token=f295e60f-7e97-4b62-afd9-909d6b9d9ed4

any ideas what could be going wrong?

frnsys commented 7 years ago

I don't have a great understanding of cookies but after some digging it seems like it's because I'm running my development authentication server on localhost and my development wordpress instance at tni.vagrant.test, causing a cookie domain issue

misfist commented 7 years ago

A couple of questions:

How are you testing this? Where is the cookie actually set (on which domain)?

frnsys commented 7 years ago

I have a development authentication server running at localhost:5001, and my development wordpress instance is running on vagrant with a hostname of tni.vagrant.test.

After I successfully login from tni.vagrant.test (via AJAX to localhost:5001, so I take it that means the cookie domain is localhost, which is why this isn't working?), I refresh tni.vagrant.test to run the tni_core_check_auth function and see if it detects my authentication, but it doesn't because $_COOKIE does not have those cookies set (because they're for localhost?).

I'm trying to figure out a way to setup my dev environment so they all are on the same domain

misfist commented 7 years ago

Maybe this is relevant?

http://stackoverflow.com/a/25861695/2079618

misfist commented 7 years ago

Also, this...

http://subinsb.com/set-same-cookie-on-different-domains

frnsys commented 7 years ago

It might be simplest to just do the check in javascript...did you say there was a way to call a js function before rendering the rest of the page?

misfist commented 7 years ago

Here is some info:

http://stackoverflow.com/questions/1917576/how-to-pass-javascript-variables-to-php

frnsys commented 7 years ago

Good news - I managed to get this working from PHP.

Is there a way to conditionally show Login/Logout using the tni_core_check_auth function?

frnsys commented 7 years ago

Also -- I realized that the BASEURL constant will need to vary depending on environment...e.g. for staging it should point to subscriptions.staging.darkinquiry.com instead of members.thenewinquiry.com so the proper cookie domain is set.

One more thing - in assets/js/auth.js there is a javascript variable BASE_URL that is hardcoded. Is there anyway we can use the TNI_Core_Authorization::BASEURL variable there as well?

misfist commented 7 years ago

Is there a way to conditionally show Login/Logout using the tni_core_check_auth function?

Yes, something like this:

add_filter( 'wp_nav_menu_items', 'tni_add_logout_link', 10, 2 );

function tni_add_logout_link( $items, $args ) {
    if( 'primary' == $args->theme_location ) {
        if( tni_core_check_auth() ) {
          $items = sprintf( '<li class="js-login"><a href="%ss">%s</a></li>',
            esc_url( {url} ),
            __( 'Log Out' )
         );
        } else {
          $items = sprintf( '<li class="js-logout"><a href="%ss">%s</a></li>',
            esc_url( {url} ),
            __( 'Log In' )
         );
        }
    }

    return $items;
}

I realized that the BASEURL constant will need to vary depending on environment.

OK. Should we add an setting in the admin?

One more thing - in assets/js/auth.js there is a javascript variable BASE_URL that is hardcoded. Is there anyway we can use the TNI_Core_Authorization::BASEURL variable there as well?

Yes, we can use wp_localize_script to add variables that are accessible to JS.

https://codex.wordpress.org/Function_Reference/wp_localize_script

misfist commented 7 years ago

Good news - I managed to get this working from PHP.

:+1: Nice work! You starting to love PHP yet? :D

frnsys commented 7 years ago

hah, well...it's not as bad as I thought!

misfist commented 7 years ago
$js_authorization = array(
    'baseURL' => TNI_Core_Authorization::BASEURL
);
wp_localize_script( 'tni-core-authentication', 'jsAuthorization', $js_authorization );

What that does is put something like this on the page:

<script type="text/javascript">
/* <![CDATA[ */
var jsAuthorization = {"baseURL":"{url}"};
/* ]]> */
</script>
misfist commented 7 years ago

(Obviously, the variables can be named whatever you'd like them to be).

frnsys commented 7 years ago

Forgot to reply to your earlier question - an admin setting would be perfect

misfist commented 7 years ago

OK. I'll add an "Authorization" settings page in the Settings menu.

misfist commented 7 years ago

I added a settings page for authorization in Settings > Authorization Settings and added method for retrieving the URL.

frnsys commented 7 years ago

closed via #6