From 159670e08c22bcf35ab72017fc65d9665cd6e66f Mon Sep 17 00:00:00 2001 From: Jon Ziebell Date: Mon, 1 Mar 2021 22:30:31 -0500 Subject: [PATCH] Fixed ecobee tokens not always deleting when they are revoked. --- api/cora/exception.php | 11 +++++-- api/cora/request.php | 63 ++++++++++++++++++++++++++++++---------- api/ecobee.php | 13 ++++++--- api/ecobee_token.php | 14 ++++----- api/external_api_log.php | 2 +- api/patreon_token.php | 9 +++--- api/user.php | 28 ++---------------- js/component/header.js | 6 +--- 8 files changed, 77 insertions(+), 69 deletions(-) diff --git a/api/cora/exception.php b/api/cora/exception.php index 22a7926..432457f 100644 --- a/api/cora/exception.php +++ b/api/cora/exception.php @@ -14,9 +14,10 @@ namespace cora; * @author Jon Ziebell */ final class exception extends \Exception { - public function __construct($message, $code, $reportable = true, $extra = null) { + public function __construct($message, $code, $reportable = true, $extra_info = null, $rollback = true) { $this->reportable = $reportable; - $this->extra = $extra; + $this->extra_info = $extra_info; + $this->rollback = $rollback; return parent::__construct($message, $code, null); } @@ -25,6 +26,10 @@ final class exception extends \Exception { } public function getExtraInfo() { - return $this->extra; + return $this->extra_info; + } + + public function getRollback() { + return $this->rollback; } } diff --git a/api/cora/request.php b/api/cora/request.php index 306b815..535add7 100644 --- a/api/cora/request.php +++ b/api/cora/request.php @@ -100,11 +100,11 @@ final class request { private $current_working_directory; /** - * A list of create calls queued up to run at the end of the request. + * A list of database calls to make at the very end of the script. * * @var array */ - private $queued_creates = []; + private $queued_database_actions = []; /** * Save the request variables for use later on. If unset, they are defaulted @@ -439,6 +439,7 @@ final class request { $this->set_error_response( $error_message, $error_code, + true, true ); @@ -468,7 +469,8 @@ final class request { $this->set_error_response( $e->getMessage(), $e->getCode(), - (method_exists($e, 'getReportable') === true ? $e->getReportable() : true) + (method_exists($e, 'getReportable') === true ? $e->getReportable() : true), + (method_exists($e, 'getRollback') === true ? $e->getRollback() : true) ); try { @@ -489,23 +491,26 @@ final class request { * * There are a few places that call this function to set an error response, * so this can't just be done in the exception handler alone. If an error - * occurs, rollback the current transaction. + * occurs, rollback the current transaction unless specified otherwise. * * @param string $error_message The error message. * @param mixed $error_code The supplied error code. * @param array $reportable Whether or not the error is reportable. + * @param array $rollback Whether or not the error should cause a rollback. */ - public function set_error_response($error_message, $error_code, $reportable) { + public function set_error_response($error_message, $error_code, $reportable, $rollback) { $setting = setting::get_instance(); $session = session::get_instance(); // I guess if this fails then things are really bad, but let's at least // protect against additional exceptions if the database connection or // similar fails. - try { - $database = database::get_instance(); - $database->rollback_transaction(); - } catch(\Exception $e) {} + if($rollback === true) { + try { + $database = database::get_instance(); + $database->rollback_transaction(); + } catch(\Exception $e) {} + } $this->response = [ 'success' => false, @@ -595,8 +600,29 @@ final class request { chdir($this->current_working_directory); // Run any queued creates. - foreach($this->queued_creates as $queued_create) { - $database->create($queued_create['resource'], $queued_create['attributes'], 'id'); + foreach($this->queued_database_actions as $queued_database_action) { + switch($queued_database_action['method']) { + case 'create': + $database->create( + $queued_database_action['resource'], + $queued_database_action['attributes'], + 'id' + ); + break; + case 'update': + $database->update( + $queued_database_action['resource'], + $queued_database_action['attributes'], + 'id' + ); + break; + case 'delete': + $database->delete( + $queued_database_action['resource'], + $queued_database_action['attributes'] + ); + break; + } } // If I didn't catch an error/exception with my handlers, look here...this @@ -611,6 +637,7 @@ final class request { $this->set_error_response( $error['message'], $error['type'], + true, true ); @@ -666,7 +693,8 @@ final class request { $this->set_error_response( $e->getMessage(), $e->getCode(), - (method_exists($e, 'getReportable') === true ? $e->getReportable() : true) + (method_exists($e, 'getReportable') === true ? $e->getReportable() : true), + (method_exists($e, 'getRollback') === true ? $e->getRollback() : true) ); try { @@ -684,12 +712,15 @@ final class request { * be used for logging things as these will not be affected by transaction * rollbacks in the event of an exception. * - * @param string $resource - * @param array $attributes + * @param string $resource The database table to act on. + * @param string $method create|update|delete + * @param array $attributes The attributes of the create or update. If + * delete just the ID to delete. */ - public function queue_create($resource, $attributes) { - $this->queued_creates[] = [ + public function queue_database_action($resource, $method, $attributes) { + $this->queued_database_actions[] = [ 'resource' => $resource, + 'method' => $method, 'attributes' => $attributes ]; } diff --git a/api/ecobee.php b/api/ecobee.php index 6c917a1..9897d90 100644 --- a/api/ecobee.php +++ b/api/ecobee.php @@ -164,7 +164,7 @@ class ecobee extends external_api { [] ); if(count($ecobee_tokens) !== 1) { - $this->api('user', 'log_out', ['all' => true]); + $this->api('user', 'log_out'); throw new cora\exception('No ecobee access for this user.', 10501, false); } $ecobee_token = $ecobee_tokens[0]; @@ -227,8 +227,7 @@ class ecobee extends external_api { $this->log_mysql($curl_response, true); } $this->api('ecobee_token', 'delete', $ecobee_token['ecobee_token_id']); - $this->api('user', 'log_out', ['all' => true]); - throw new cora\exception('Ecobee access was revoked by user.', 10500, false); + throw new cora\exception('Ecobee access was revoked by user.', 10500, false, null, false); } else if (isset($response['status']) === true && $response['status']['code'] !== 0) { // Any other error @@ -240,10 +239,16 @@ class ecobee extends external_api { else if (isset($response['error']) === true) { // Authorization errors are a bit different // https://www.ecobee.com/home/developer/api/documentation/v1/auth/auth-req-resp.shtml + + if($response['error'] === 'invalid_grant') { + $ecobee_token = $this->api('ecobee_token', 'read')[0]; + $this->api('ecobee_token', 'delete', $ecobee_token['ecobee_token_id']); + } + if($this::$log_mysql !== 'all') { $this->log_mysql($curl_response, true); } - throw new cora\exception(isset($response['error_description']) === true ? $response['error_description'] : $response['error'], 10505); + throw new cora\exception(isset($response['error_description']) === true ? $response['error_description'] : $response['error'], 10505, true, null, false); } else { return $response; diff --git a/api/ecobee_token.php b/api/ecobee_token.php index 411dd2b..daa40b9 100644 --- a/api/ecobee_token.php +++ b/api/ecobee_token.php @@ -97,9 +97,8 @@ class ecobee_token extends cora\crud { isset($response['refresh_token']) === false ) { $this->delete($ecobee_token['ecobee_token_id']); - $this->api('user', 'log_out', ['all' => true]); $database->release_lock($lock_name); - throw new cora\exception('Could not refresh ecobee token; ecobee returned no token.', 10002); + throw new cora\exception('Could not refresh ecobee token; ecobee returned no token.', 10002, true, null, false); } $ecobee_token = $database->update( @@ -118,19 +117,16 @@ class ecobee_token extends cora\crud { } /** - * Delete an ecobee token. If this happens immediately log out all of this - * user's currently logged in sessions. + * Delete an ecobee token and log the user out. Make sure to delete the + * token prior to logging out so the right permissions are present. * * @param int $id * * @return int */ public function delete($id) { - $database = cora\database::get_transactionless_instance(); - - // Need to delete the token before logging out or else the delete fails. - $return = $database->delete($this->resource, $id); - + $return = parent::delete($id); + $this->api('user', 'log_out'); return $return; } diff --git a/api/external_api_log.php b/api/external_api_log.php index e6845e1..bc96748 100644 --- a/api/external_api_log.php +++ b/api/external_api_log.php @@ -16,7 +16,7 @@ class external_api_log extends cora\crud { */ public function create($attributes) { $attributes['user_id'] = $this->session->get_user_id(); - $this->request->queue_create($this->resource, $attributes); + $this->request->queue_database_action($this->resource, 'create', $attributes); } } diff --git a/api/patreon_token.php b/api/patreon_token.php index 6815836..7e6fa5b 100644 --- a/api/patreon_token.php +++ b/api/patreon_token.php @@ -105,7 +105,7 @@ class patreon_token extends cora\crud { ) { $this->delete($patreon_token['patreon_token_id']); $database->release_lock($lock_name); - throw new cora\exception('Could not refresh Patreon token; Patreon returned no token.', 10102); + throw new cora\exception('Could not refresh Patreon token; Patreon returned no token.', 10102, true, null, false); } $database->update( @@ -122,16 +122,15 @@ class patreon_token extends cora\crud { } /** - * Delete an patreon token. + * Delete a Patreon token and log the user out. Make sure to delete the + * token prior to logging out so the right permissions are present. * * @param int $id * * @return int */ public function delete($id) { - $database = cora\database::get_transactionless_instance(); - $return = $database->delete($this->resource, $id); - return $return; + return parent::delete($id); } } diff --git a/api/user.php b/api/user.php index 1268bf0..e8d9a3e 100644 --- a/api/user.php +++ b/api/user.php @@ -131,37 +131,13 @@ class user extends cora\crud { /** * Logs out the currently logged in user. - * - * @return bool True if it was successfully invalidated. Could return false - * for a non-existant session key or if it was already logged out. In the - * case of multiple sessions, return true if all open sessions were - * successfully deleted, false if not. */ - public function log_out($all) { + public function log_out() { if($this->setting->is_demo() === true) { return; } - if($all === true) { - // Sometimes I need to log out and then throw an exception. Using the - // transactionless instance makes sure that actually works. - $database = cora\database::get_transactionless_instance(); - $sessions = $database->read( - 'cora\session', - [ - 'user_id' => $this->session->get_user_id(), - 'api_user_id' => null - ] - ); - $success = true; - foreach($sessions as $session) { - $success &= $database->delete('cora\session', $session['session_id']); - } - return $success; - } - else { - return $this->session->delete(); - } + return $this->session->delete(); } /** diff --git a/js/component/header.js b/js/component/header.js index 0aaefbf..a1d13dc 100644 --- a/js/component/header.js +++ b/js/component/header.js @@ -192,11 +192,7 @@ beestat.component.header.prototype.decorate_ = function(parent) { .set_callback(function() { window.location.href = window.location.href.replace('app.', ''); }) - .add_call( - 'user', - 'log_out', - {'all': false} - ) + .add_call('user', 'log_out') .send(); })); };