diff --git a/.gitignore b/.gitignore
index 4f03131e2..3b293ef24 100644
--- a/.gitignore
+++ b/.gitignore
@@ -31,6 +31,7 @@ includes/**/tests/
build/
coverage/
phpunit.xml
+.phpunit.result.cache
tests/yourls-tests-config.php
tests/vendor/
tests/data/auth/config-test-auth-hashed.php
diff --git a/admin/admin-ajax.php b/admin/admin-ajax.php
index 77f68ac2b..103cf1c82 100644
--- a/admin/admin-ajax.php
+++ b/admin/admin-ajax.php
@@ -40,11 +40,6 @@
echo json_encode(array('success'=>$query));
break;
- case 'logout':
- // unused for the moment
- yourls_logout();
- break;
-
default:
yourls_do_action( 'yourls_ajax_'.$action );
diff --git a/includes/functions-auth.php b/includes/functions-auth.php
index 09c8f1a41..3a6f04943 100644
--- a/includes/functions-auth.php
+++ b/includes/functions-auth.php
@@ -20,6 +20,7 @@ function yourls_maybe_require_auth() {
/**
* Check for valid user via login form or stored cookie. Returns true or an error message
*
+ * @return bool|string|mixed true if valid user, error message otherwise. Can also call yourls_die() or redirect to login page. Oh my.
*/
function yourls_is_valid_user() {
// Allow plugins to short-circuit the whole function
@@ -32,7 +33,9 @@ function yourls_is_valid_user() {
$unfiltered_valid = false;
// Logout request
- if( isset( $_GET['action'] ) && $_GET['action'] == 'logout' ) {
+ if( isset( $_GET['action'] ) && $_GET['action'] == 'logout' && isset( $_REQUEST['nonce'] ) ) {
+ // The logout nonce is associated to fake user 'logout' since at this point we don't know the real user
+ yourls_verify_nonce('admin_logout', $_REQUEST['nonce'], 'logout');
yourls_do_action( 'logout' );
yourls_store_cookie( null );
return yourls__( 'Logged out successfully' );
@@ -444,6 +447,8 @@ function yourls_store_cookie( $user = null ) {
if ( $domain == 'localhost' )
$domain = '';
+ yourls_do_action( 'pre_setcookie', $user, $time, $path, $domain, $secure, $httponly );
+
if ( !headers_sent( $filename, $linenum ) ) {
yourls_setcookie( yourls_cookie_name(), yourls_cookie_value( $user ), $time, $path, $domain, $secure, $httponly );
} else {
@@ -576,7 +581,7 @@ function yourls_salt( $string ) {
*
*/
function yourls_create_nonce( $action, $user = false ) {
- if( false == $user )
+ if( false === $user )
$user = defined( 'YOURLS_USER' ) ? YOURLS_USER : '-1';
$tick = yourls_tick();
$nonce = substr( yourls_salt($tick . $action . $user), 0, 10 );
@@ -612,24 +617,25 @@ function yourls_nonce_url( $action, $url = false, $name = 'nonce', $user = false
*
*/
function yourls_verify_nonce( $action, $nonce = false, $user = false, $return = '' ) {
- // get user
- if( false == $user )
- $user = defined( 'YOURLS_USER' ) ? YOURLS_USER : '-1';
+ // Get user
+ if( false === $user ) {
+ $user = defined('YOURLS_USER') ? YOURLS_USER : '-1';
+ }
- // get current nonce value
- if( false == $nonce && isset( $_REQUEST['nonce'] ) )
- $nonce = $_REQUEST['nonce'];
+ // Get nonce value from $_REQUEST if not specified
+ if( false === $nonce && isset( $_REQUEST['nonce'] ) ) {
+ $nonce = $_REQUEST['nonce'];
+ }
// Allow plugins to short-circuit the rest of the function
- $valid = yourls_apply_filter( 'verify_nonce', false, $action, $nonce, $user, $return );
- if ($valid) {
+ if (yourls_apply_filter( 'verify_nonce', false, $action, $nonce, $user, $return ) === true) {
return true;
}
- // what nonce should be
+ // What nonce should be
$valid = yourls_create_nonce( $action, $user );
- if( $nonce == $valid ) {
+ if( $nonce === $valid ) {
return true;
} else {
if( $return )
diff --git a/includes/functions-html.php b/includes/functions-html.php
index 4de8891f1..c88880a93 100644
--- a/includes/functions-html.php
+++ b/includes/functions-html.php
@@ -750,7 +750,11 @@ function yourls_html_menu() {
// Build menu links
if( defined( 'YOURLS_USER' ) ) {
- $logout_link = yourls_apply_filter( 'logout_link', sprintf( yourls__('Hello %s'), YOURLS_USER ) . ' (' . yourls__( 'Logout' ) . ')' );
+ // Create a logout link with a nonce associated to fake user 'logout' : the user is not yet defined
+ // when the logout check is done -- see yourls_is_valid_user()
+ $logout_url = yourls_nonce_url( 'admin_logout',
+ yourls_add_query_arg(['action' => 'logout'], yourls_admin_url('index.php')), 'nonce', 'logout');
+ $logout_link = yourls_apply_filter('logout_link', sprintf( yourls__('Hello %s'), YOURLS_USER ) . ' (' . yourls__( 'Logout' ) . ')' );
} else {
$logout_link = yourls_apply_filter( 'logout_link', '' );
}
diff --git a/tests/tests/auth/login_cookie.php b/tests/tests/auth/login_cookie.php
index 7cba9e744..050671da1 100644
--- a/tests/tests/auth/login_cookie.php
+++ b/tests/tests/auth/login_cookie.php
@@ -11,15 +11,20 @@ class Auth_Login_Cookie_Tests extends PHPUnit\Framework\TestCase {
protected $cookie;
protected $request;
+ protected $backup_yourls_actions;
protected function setUp(): void {
$this->cookie = $_COOKIE;
$this->request = $_REQUEST;
+ global $yourls_actions;
+ $this->backup_yourls_actions = $yourls_actions;
}
protected function tearDown(): void {
$_COOKIE = $this->cookie;
$_REQUEST = $this->request;
+ global $yourls_actions;
+ $yourls_actions = $this->backup_yourls_actions;
}
public static function setUpBeforeClass(): void {
@@ -52,7 +57,7 @@ public function test_cookie_life() {
}
/**
- * Test login with valid cookie
+ * Test login with valid cookie - also check that cookie is set
*/
public function test_login_valid_cookie() {
global $yourls_user_passwords;
@@ -60,19 +65,23 @@ public function test_login_valid_cookie() {
$_COOKIE[yourls_cookie_name()] = yourls_cookie_value( $random_user );
unset($_REQUEST);
+ $this->assertSame( 0, yourls_did_action('pre_setcookie') );
$this->assertTrue(yourls_check_auth_cookie());
$this->assertTrue(yourls_is_valid_user());
+ $this->assertSame( 1, yourls_did_action('pre_setcookie') );
}
/**
- * Test login with invalid cookie
+ * Test login with invalid cookie - also check that no cookie is set
*/
public function test_login_invalid_cookie() {
$_COOKIE[yourls_cookie_name()] = yourls_cookie_value( rand_str() );
unset($_REQUEST);
+ $this->assertSame( 0, yourls_did_action('pre_setcookie') );
$this->assertFalse(yourls_check_auth_cookie());
$this->assertNotTrue(yourls_is_valid_user());
+ $this->assertSame( 0, yourls_did_action('pre_setcookie') );
}
}
diff --git a/tests/tests/auth/logout.php b/tests/tests/auth/logout.php
index 5de8c84a6..a7153d9e2 100644
--- a/tests/tests/auth/logout.php
+++ b/tests/tests/auth/logout.php
@@ -9,10 +9,12 @@ class Logout_Func_Tests extends PHPUnit\Framework\TestCase {
protected $backup_get;
protected $backup_request;
+ private static $user;
protected function setUp(): void {
$this->backup_get = $_GET;
$this->backup_request = $_REQUEST;
+ self::$user = false;
}
protected function tearDown(): void {
@@ -20,33 +22,47 @@ protected function tearDown(): void {
$_REQUEST = $this->backup_request;
}
+ public static function setUpBeforeClass(): void {
+ yourls_add_action( 'pre_setcookie', function ($in) {
+ self::$user = $in[0]; // $in[0] is the user ID passed to yourls_setcookie()
+ } );
+ }
+
+ public static function tearDownAfterClass(): void {
+ yourls_remove_all_actions('pre_setcookie');
+ }
+
/**
- * Check logout procedure - phase 1
+ * Check logout procedure - phase 1 - we're logging in
*/
public function test_logout_user_is_logged_in() {
$_REQUEST['nonce'] = yourls_create_nonce('admin_login');
$valid = yourls_is_valid_user();
$this->assertTrue($valid);
+ $this->assertSame(self::$user, 'yourls');
}
/**
- * Check logout procedure - phase 2
+ * Check logout procedure - phase 2 - we're logging out and checking that cookie was reset
* @depends test_logout_user_is_logged_in
*/
public function test_logout_user_logs_out() {
$_GET['action'] = 'logout';
+ $_REQUEST['nonce'] = yourls_create_nonce('admin_logout', 'logout');
$invalid = yourls_is_valid_user();
$this->assertNotTrue( $invalid );
+ $this->assertSame(self::$user, null);
}
/**
- * Check logout procedure - phase 3
+ * Check logout procedure - phase 3 - check we can log in again
* @depends test_logout_user_logs_out
*/
public function test_logout_user_is_logged_in_back() {
$_REQUEST['nonce'] = yourls_create_nonce('admin_login');
$valid = yourls_is_valid_user();
$this->assertTrue( $valid );
+ $this->assertSame(self::$user, 'yourls');
}
}
diff --git a/tests/tests/auth/misc.php b/tests/tests/auth/misc.php
index b753fe4fd..11a616969 100644
--- a/tests/tests/auth/misc.php
+++ b/tests/tests/auth/misc.php
@@ -11,4 +11,6 @@ public function test_yourls_skip_password_hashing_is_bool() {
$this->assertIsBool(yourls_skip_password_hashing());
}
+
+
}
diff --git a/tests/tests/auth/nonces.php b/tests/tests/auth/nonces.php
index 21dc9f428..4c295806e 100644
--- a/tests/tests/auth/nonces.php
+++ b/tests/tests/auth/nonces.php
@@ -47,7 +47,6 @@ public function test_create_nonce_field() {
public function test_create_nonce_url() {
$url = yourls_nonce_url( rand_str(), rand_str(), rand_str(), rand_str() );
$this->assertTrue( is_string($url) );
- // $this->assertIsString($url);
}
/**
@@ -77,4 +76,23 @@ public function test_invalid_nonce() {
$this->assertTrue(yourls_verify_nonce(rand_str(), rand_str(), rand_str()));
}
+ /**
+ * Check nonces are different for different actions & users
+ */
+ public function test_nonce_different_for_different_actions_and_users() {
+ $action1 = rand_str();
+ $action2 = rand_str();
+ $user1 = rand_str();
+ $user2 = rand_str();
+
+ $nonce_a1 = yourls_create_nonce($action1);
+ $nonce_a2 = yourls_create_nonce($action2);
+ $nonce_a1_u1 = yourls_create_nonce($action1, $user1);
+ $nonce_a1_u2 = yourls_create_nonce($action1, $user2);
+
+ $this->assertNotEquals($nonce_a1, $nonce_a2);
+ $this->assertNotEquals($nonce_a1_u1, $nonce_a1_u2);
+ $this->assertNotEquals($nonce_a1, $nonce_a1_u1);
+ }
+
}