Skip to content

Commit

Permalink
CS improvements, fix unwanted output in tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@alecpl
alecpl committed Dec 17, 2023
1 parent ca8b17d commit f363481
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 12 deletions.
28 changes: 16 additions & 12 deletions program/include/rcmail_oauth.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ public function jwt_decode($jwt)
throw new RuntimeException('Failed to validate JWT: expired message');
}

$this->log_debug('jwt: %s', json_encode($body));
$this->log_debug("'jwt: %s", json_encode($body));

return $body;
}
Expand Down Expand Up @@ -480,7 +480,7 @@ public function login_redirect()
'state' => $_SESSION['oauth_state'],
] + (array) $this->options['auth_parameters']);

$this->log_debug('requesting authorization with scope: %s', $this->options['scope']);
$this->log_debug("requesting authorization with scope: %s", $this->options['scope']);

$this->last_error = null; // clean last error
$this->rcmail->output->redirect($this->options['auth_uri'] . $delimiter . $query); // exit
Expand Down Expand Up @@ -541,11 +541,13 @@ public function request_access_token($auth_code, $state = null)

// validate state parameter against $_SESSION['oauth_state']
if (!isset($_SESSION['oauth_state']) || ($_SESSION['oauth_state'] !== $state)) {
throw new RuntimeException('state parameter mismatch');
throw new RuntimeException("state parameter mismatch");
}

$this->rcmail->session->remove('oauth_state');

$this->log_debug('requesting a grant_type=authorization_code to %s', $oauth_token_uri);
$this->log_debug("requesting a grant_type=authorization_code to %s", $oauth_token_uri);

$response = $this->http_client->post($oauth_token_uri, [
'form_params' => [
'grant_type' => 'authorization_code',
Expand All @@ -555,6 +557,7 @@ public function request_access_token($auth_code, $state = null)
'redirect_uri' => $this->get_redirect_uri(),
],
]);

$data = json_decode($response->getBody(), true);

$authorization = $this->parse_tokens('authorization_code', $data);
Expand Down Expand Up @@ -582,7 +585,6 @@ public function request_access_token($auth_code, $state = null)
$this->log_debug("fetched identity: %s", json_encode($fetched_identity, true));

if (!empty($fetched_identity)) {

$identity = $fetched_identity;

foreach ($this->options['identity_fields'] as $field) {
Expand Down Expand Up @@ -663,7 +665,8 @@ public function refresh_access_token(array $token)

// send token request to get a real access token for the given auth code
try {
$this->log_debug('requesting a grant_type=refresh_token to %s', $oauth_token_uri);
$this->log_debug("requesting a grant_type=refresh_token to %s", $oauth_token_uri);

$response = $this->http_client->post($oauth_token_uri, [
'form_params' => [
'grant_type' => 'refresh_token',
Expand Down Expand Up @@ -779,7 +782,8 @@ protected function is_token_revoked($token)
protected function parse_tokens($grant_type, &$data, $previous_data=null)
{
// TODO move it into to log_info ?
$this->log_debug('received tokens from a grant request "%s": session: %s with scope %s, access_token type %s exp in %ss, refresh_token exp in %ss, id_token present: %s, not-before-policy: %s',
$this->log_debug("received tokens from a grant request %s: session: %s with scope %s, "
. "access_token type %s exp in %ss, refresh_token exp in %ss, id_token present: %s, not-before-policy: %s",
$grant_type,
$data['session_state'], $data['scope'],
$data['token_type'], $data['expires_in'],
Expand All @@ -790,7 +794,7 @@ protected function parse_tokens($grant_type, &$data, $previous_data=null)

if (is_array($previous_data)) {
$this->log_debug(
'changes: session_state: %s, access_token: %s, refresh_token: %s, id_token: %s',
"changes: session_state: %s, access_token: %s, refresh_token: %s, id_token: %s",
isset($previous_data['session_state']) ? $previous_data['session_state'] !== $data['session_state'] : null,
isset($previous_data['access_token']) ? $previous_data['access_token'] !== $data['access_token'] : null,
isset($previous_data['refresh_token']) ? $previous_data['refresh_token'] !== $data['refresh_token'] : null,
Expand All @@ -800,17 +804,17 @@ protected function parse_tokens($grant_type, &$data, $previous_data=null)

// sanity check, check that payload correctly contains access_token
if (empty($data['access_token'])) {
throw new RuntimeException('access_token missing ins answer, error from server');
throw new RuntimeException("access_token missing ins answer, error from server");
}

// sanity check, check that payload correctly contains access_token
if (empty($data['refresh_token'])) {
throw new RuntimeException('refresh_token missing ins answer, error from server');
throw new RuntimeException("refresh_token missing ins answer, error from server");
}

// (> 0, it means that all token generated before this timestamp date are compromisd and that we need to download a new version of JWKS)
if (!empty($data['not-before-policy']) && $data['not-before-policy'] > 0) {
$this->log_debug('all tokens generated before %s timestmp are compromised', $data['not-before-policy']);
$this->log_debug("all tokens generated before %s timestmp are compromised", $data['not-before-policy']);
// TODO
}

Expand Down Expand Up @@ -1085,7 +1089,7 @@ public function handle_logout()
}

$this->logout_redirect_url = $this->options['logout_uri'] . '?' . http_build_query($params);
$this->log_debug('creating logout call: %s', $this->logout_redirect_url);
$this->log_debug("creating logout call: %s", $this->logout_redirect_url);
}

/**
Expand Down
5 changes: 5 additions & 0 deletions tests/Rcmail/Oauth.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,15 @@ function test_request_access_token_with_wrong_state()
$oauth->init();

$_SESSION['oauth_state'] = "random-state";

StderrMock::start();
$response = $oauth->request_access_token('fake-code', 'mismatch-state');
StderrMock::stop();

// should be false as state do not match
$this->assertFalse($response);

$this->assertSame("ERROR: OAuth token request failed: state parameter mismatch", trim(StderrMock::$output));
}

/**
Expand Down

0 comments on commit f363481

Please sign in to comment.