diff --git a/program/include/rcmail.php b/program/include/rcmail.php index 64031a4a1..1fb68e392 100644 --- a/program/include/rcmail.php +++ b/program/include/rcmail.php @@ -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. diff --git a/program/include/rcmail_oauth.php b/program/include/rcmail_oauth.php index e570c6165..648959fab 100644 --- a/program/include/rcmail_oauth.php +++ b/program/include/rcmail_oauth.php @@ -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 " " - * `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; } /** diff --git a/tests/Rcmail/Oauth.php b/tests/Rcmail/Oauth.php index 98e179237..198a36b06 100644 --- a/tests/Rcmail/Oauth.php +++ b/tests/Rcmail/Oauth.php @@ -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',