Refactorize request_access_token() returning a simple boolean (#9299)

This commit is contained in:
Edouard Vanbelle
2024-01-06 08:34:12 +01:00
committed by GitHub
parent a797873ef5
commit 77aca18bd5
3 changed files with 24 additions and 33 deletions

View File

@@ -33,7 +33,7 @@ class rcmail extends rcube
*
* @var array
*/
public static $main_tasks = ['mail', 'settings', 'addressbook', 'login', 'logout', 'utils', 'oauth', 'dummy'];
public static $main_tasks = ['mail', 'settings', 'addressbook', 'login', 'logout', 'utils', 'dummy'];
/**
* Current task.

View File

@@ -365,7 +365,6 @@ class rcmail_oauth
return $this->last_error;
}
/**
* Callback for `loginform_content` hook
*
@@ -395,7 +394,7 @@ class rcmail_oauth
return $form_content;
}
// TODO: move it into an helper class
protected static function base64url_decode($encoded)
{
return base64_decode(strtr($encoded, '-_', '+/'), true);
@@ -449,7 +448,6 @@ class rcmail_oauth
}
// FIXME depends on body type: ID, Logout, Bearer, Refresh,
if (isset($body['azp']) && $body['azp'] !== $this->options['client_id']) {
throw new RuntimeException('Failed to validate JWT: invalid azp value');
} elseif (isset($body['aud']) && !in_array($this->options['client_id'], (array) $body['aud'])) {
@@ -582,10 +580,7 @@ class rcmail_oauth
* @param string $auth_code
* @param string $state
*
* @return array Authorization data as hash array with entries
* `username` as the authentication user name
* `authorization` as the oauth authorization string "<type> <access-token>"
* `token` as the complete oauth response to be stored in session
* @return bool true on access token, false on error
*
* @see https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.3
*/
@@ -684,7 +679,7 @@ class rcmail_oauth
// store crypted code_verifier because session is going to be killed
$this->login_phase['code_verifier'] = $_SESSION['oauth_code_verifier'];
}
return $this->login_phase;
return true;
} catch (RequestException $e) {
$this->last_error = 'OAuth token request failed: ' . $e->getMessage();
$this->no_redirect = true;
@@ -696,7 +691,6 @@ class rcmail_oauth
'line' => __LINE__,
], true, false);
return false;
} catch (Exception $e) {
$this->last_error = 'OAuth token request failed: ' . $e->getMessage();
$this->no_redirect = true;
@@ -707,8 +701,8 @@ class rcmail_oauth
'line' => __LINE__,
], true, false);
return false;
}
return false;
}
/**

View File

@@ -4,14 +4,6 @@ use GuzzleHttp\Handler\MockHandler;
use GuzzleHttp\HandlerStack;
use GuzzleHttp\Psr7\Response;
class rcmail_oauth_test extends rcmail_oauth
{
public function forge_login_phase($data)
{
$this->login_phase = $data;
}
}
/**
* Test class to test rcmail_oauth class
*/
@@ -299,11 +291,14 @@ class Rcmail_RcmailOauth extends ActionTestCase
$_SESSION['oauth_nonce'] = 'fake-nonce';
$response = $oauth->request_access_token('fake-code', 'random-state');
$this->assertTrue(is_array($response));
$this->assertSame('Bearer FAKE-ACCESS-TOKEN', $response['authorization']);
$this->assertSame($this->identity['email'], $response['username']);
$this->assertTrue(isset($response['token']));
$this->assertFalse(isset($response['token']['access_token']));
$this->assertTrue($response);
$login_phase = getProperty($oauth, 'login_phase');
$this->assertSame('Bearer FAKE-ACCESS-TOKEN', $login_phase['authorization']);
$this->assertSame($this->identity['email'], $login_phase['username']);
$this->assertTrue(isset($login_phase['token']));
$this->assertFalse(isset($login_phase['token']['access_token']));
}
/**
@@ -338,11 +333,13 @@ class Rcmail_RcmailOauth extends ActionTestCase
$_SESSION['oauth_nonce'] = 'fake-nonce'; // ensure nonce identiquals
$response = $oauth->request_access_token('fake-code', 'random-state');
$this->assertTrue(is_array($response));
$this->assertSame('Bearer FAKE-ACCESS-TOKEN', $response['authorization']);
$this->assertSame($this->identity['email'], $response['username']);
$this->assertTrue(isset($response['token']));
$this->assertFalse(isset($response['token']['access_token']));
$this->assertTrue($response);
$login_phase = getProperty($oauth, 'login_phase');
$this->assertSame('Bearer FAKE-ACCESS-TOKEN', $login_phase['authorization']);
$this->assertSame($this->identity['email'], $login_phase['username']);
$this->assertTrue(isset($login_phase['token']));
$this->assertFalse(isset($login_phase['token']['access_token']));
}
/**
@@ -350,11 +347,11 @@ class Rcmail_RcmailOauth extends ActionTestCase
*/
public function test_valid_user_create()
{
$oauth = new rcmail_oauth_test();
$oauth = new rcmail_oauth();
$oauth->init();
// fake identity
$oauth->forge_login_phase([
setProperty($oauth, 'login_phase', [
'token' => [
'identity' => [
'email' => 'jdoe@faké.dômain',
@@ -377,11 +374,11 @@ class Rcmail_RcmailOauth extends ActionTestCase
*/
public function test_invalid_user_create()
{
$oauth = new rcmail_oauth_test();
$oauth = new rcmail_oauth();
$oauth->init();
// fake identity
$oauth->forge_login_phase([
setProperty($oauth, 'login_phase', [
'token' => [
'identity' => [
'email' => 'bad-domain',