rugk /
xenforo-threema-gateway
Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like ThreemaGateway_Handler_Action_Callback often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use ThreemaGateway_Handler_Action_Callback, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 11 | class ThreemaGateway_Handler_Action_Callback extends ThreemaGateway_Handler_Action_Abstract |
||
| 12 | { |
||
| 13 | /** |
||
| 14 | * @var string used by strtotime to allow messages sent in the future |
||
| 15 | */ |
||
| 16 | const ALLOW_FUTURE_MESSAGE_TIME = '+5 sec'; |
||
| 17 | |||
| 18 | /** |
||
| 19 | * @var XenForo_Input raw parameters |
||
| 20 | */ |
||
| 21 | protected $input; |
||
| 22 | |||
| 23 | /** |
||
| 24 | * @var array filtered parameters |
||
| 25 | */ |
||
| 26 | protected $filtered; |
||
| 27 | |||
| 28 | /** |
||
| 29 | * @var bool whether it has been checked that the message is not used in a |
||
| 30 | * replay attack |
||
| 31 | */ |
||
| 32 | protected $messageReplayChecked = false; |
||
| 33 | |||
| 34 | /** |
||
| 35 | * Initializes handling for processing a request callback. |
||
| 36 | * |
||
| 37 | * @param Zend_Controller_Request_Http $request |
||
| 38 | */ |
||
| 39 | public function initCallbackHandling(Zend_Controller_Request_Http $request) |
||
| 40 | { |
||
| 41 | $this->input = new XenForo_Input($request); |
||
| 42 | |||
| 43 | $this->filtered = $this->input->filter([ |
||
| 44 | 'accesstoken' => XenForo_Input::STRING, |
||
| 45 | 'from' => XenForo_Input::STRING, |
||
| 46 | 'to' => XenForo_Input::STRING, |
||
| 47 | 'messageId' => XenForo_Input::STRING, |
||
| 48 | 'date' => XenForo_Input::DATE_TIME, |
||
| 49 | 'nonce' => XenForo_Input::STRING, |
||
| 50 | 'box' => XenForo_Input::STRING, |
||
| 51 | 'mac' => XenForo_Input::STRING |
||
| 52 | ]); |
||
| 53 | } |
||
| 54 | |||
| 55 | /** |
||
| 56 | * Validates the callback request. In case of failure the Gateway server |
||
| 57 | * should not retry here as it likely would not help anyway. |
||
| 58 | * |
||
| 59 | * This validation is only a basic validation and does not handle with any |
||
| 60 | * potentially secret data to prevent any exposures. |
||
| 61 | * It makes sure malwformed requests can be denied fastly without needing |
||
| 62 | * to check the authentity/security of the message. |
||
| 63 | * |
||
| 64 | * @param string|array $errorString Output error string/array |
||
| 65 | * |
||
| 66 | * @return bool |
||
| 67 | */ |
||
| 68 | public function validatePreConditions(&$errorString) |
||
| 69 | { |
||
| 70 | /** @var XenForo_Options $options */ |
||
| 71 | $options = XenForo_Application::getOptions(); |
||
| 72 | |||
| 73 | // only allow POST requests (unless GET is allowed in ACP) |
||
| 74 | if (!$this->settings->isDebug() || !$options->threema_gateway_allow_get_receive) { |
||
| 75 | // as an exception we access the superglobal directly here as it is |
||
| 76 | // difficult to get the request object |
||
| 77 | if ($_SERVER['REQUEST_METHOD'] !== 'POST') { |
||
| 78 | $errorString = [null, 'No POST request.', '']; |
||
| 79 | return false; |
||
| 80 | }; |
||
| 81 | } |
||
| 82 | |||
| 83 | // special error message to let users know not to forget the access |
||
| 84 | // token |
||
| 85 | if (!$this->input->inRequest('accesstoken')) { |
||
| 86 | $errorString = 'Access token missing'; |
||
| 87 | return false; |
||
| 88 | }; |
||
| 89 | |||
| 90 | // check for other missing parameters |
||
| 91 | if (!$this->input->inRequest('from') || |
||
| 92 | !$this->input->inRequest('to') || |
||
| 93 | !$this->input->inRequest('messageId') || |
||
| 94 | !$this->input->inRequest('date') || |
||
| 95 | !$this->input->inRequest('nonce') || |
||
| 96 | !$this->input->inRequest('box') || |
||
| 97 | !$this->input->inRequest('mac') |
||
| 98 | ) { |
||
| 99 | $errorString = [null, 'Invalid request: parameter missing', 'Invalid request']; |
||
| 100 | return false; |
||
| 101 | }; |
||
| 102 | |||
| 103 | if (!$this->settings->isEndtoEnd()) { |
||
| 104 | $errorString = [null, 'Receiving messages is not supported, end to end mode is not configured (correctly)', 'Receiving messages is not supported']; |
||
| 105 | return false; |
||
| 106 | } |
||
| 107 | |||
| 108 | return true; |
||
| 109 | } |
||
| 110 | |||
| 111 | /** |
||
| 112 | * Validates the callback request's authenticy and integrity. In case of |
||
| 113 | * failure the Gateway server SHOULD retry. |
||
| 114 | * |
||
| 115 | * This validates the integrity of the request and the authentity that the |
||
| 116 | * calling instance actually is the Threema Gateway server. |
||
| 117 | * Retrying is allowed as the secrets, which are used to validate the |
||
| 118 | * request may change at any time and to avoid a loss of messages the |
||
| 119 | * Gateway server is supposed to retry the delivery. |
||
| 120 | * |
||
| 121 | * @param string|array $errorString Output error string/array |
||
| 122 | * |
||
| 123 | * @return bool |
||
| 124 | */ |
||
| 125 | public function validateRequest(&$errorString) |
||
| 126 | { |
||
| 127 | // access token validation (authentication of Gateway server) |
||
| 128 | /** @var XenForo_Options $options */ |
||
| 129 | $options = XenForo_Application::getOptions(); |
||
| 130 | if (!$options->threema_gateway_receivecallback) { |
||
| 131 | $errorString = [null, 'Unverified request: access token is not configured', 'Unverified request']; |
||
| 132 | return false; |
||
| 133 | } |
||
| 134 | |||
| 135 | View Code Duplication | if (!$this->getCryptTool()->stringCompare( |
|
|
|
|||
| 136 | $options->threema_gateway_receivecallback, |
||
| 137 | $this->filtered['accesstoken'] |
||
| 138 | )) { |
||
| 139 | $errorString = [null, 'Unverified request: access token invalid', 'Unverified request']; |
||
| 140 | return false; |
||
| 141 | } |
||
| 142 | |||
| 143 | // HMAC validation (verifies integrity of request) |
||
| 144 | if (!$this->getE2EHelper()->checkMac( |
||
| 145 | $this->filtered['from'], |
||
| 146 | $this->filtered['to'], |
||
| 147 | $this->filtered['messageId'], |
||
| 148 | $this->filtered['date'], |
||
| 149 | $this->filtered['nonce'], |
||
| 150 | $this->filtered['box'], |
||
| 151 | $this->filtered['mac'], |
||
| 152 | $this->settings->getSecret() |
||
| 153 | )) { |
||
| 154 | $errorString = [null, 'Unverified request: HMAC verification failed', 'Unverified request']; |
||
| 155 | return false; |
||
| 156 | } |
||
| 157 | |||
| 158 | return true; |
||
| 159 | } |
||
| 160 | |||
| 161 | /** |
||
| 162 | * Validates the callback request formally. In case of failure the Gateway |
||
| 163 | * server should NOT retry, as it likely would not help anyway. |
||
| 164 | * |
||
| 165 | * This validation is only a formal validation of the request data. The |
||
| 166 | * request should already have been validated ({@see validateRequest()}). |
||
| 167 | * In contrast to the basic validation ({@see validatePreConditions()}) this |
||
| 168 | * validation deals with secret data and furthermore assures that the send |
||
| 169 | * request is valid. |
||
| 170 | * It is used to prevent malformed (but verified) bad requests to get to the |
||
| 171 | * decryption part, which cannot decrypt them anyway. |
||
| 172 | * |
||
| 173 | * @param string|array $errorString Output error string/array |
||
| 174 | * |
||
| 175 | * @return bool |
||
| 176 | */ |
||
| 177 | public function validateFormalities(&$errorString) |
||
| 178 | { |
||
| 179 | // simple, formal validation of Gateway ID |
||
| 180 | View Code Duplication | if (!$this->getCryptTool()->stringCompare($this->filtered['to'], $this->settings->getId())) { |
|
| 181 | $errorString = [null, 'Invalid request: formal verification failed', 'Invalid request']; |
||
| 182 | return false; |
||
| 183 | } |
||
| 184 | |||
| 185 | /** @var XenForo_Options $options */ |
||
| 186 | $options = XenForo_Application::getOptions(); |
||
| 187 | /** @var string $rejectOld the maximum age of a message; default/fallback: 14 days */ |
||
| 188 | $rejectOld = '-14 days'; |
||
| 189 | if ($options->threema_gateway_verify_receive_time && $options->threema_gateway_verify_receive_time['enabled']) { |
||
| 190 | $rejectOld = $options->threema_gateway_verify_receive_time['time']; |
||
| 191 | } |
||
| 192 | |||
| 193 | // discard too old messages |
||
| 194 | if ($this->filtered['date'] < strtotime($rejectOld, XenForo_Application::$time)) { |
||
| 195 | $errorString = [null, 'Message cannot be processed: Message is too old (send at ' . date('Y-m-d H:i:s', $this->filtered['date']) . ', messages older than ' . $rejectOld . ' are rejected)', 'Message cannot be processed']; |
||
| 196 | return false; |
||
| 197 | } |
||
| 198 | |||
| 199 | // discard messages sent in the future |
||
| 200 | // (to handle leap seconds or so we allow some seconds difference) |
||
| 201 | if ($this->filtered['date'] > strtotime(self::ALLOW_FUTURE_MESSAGE_TIME, XenForo_Application::$time)) { |
||
| 202 | $errorString = [null, 'Message cannot be processed: Message is send in the future (send at ' . date('Y-m-d H:i:s', $this->filtered['date']) . ', please check your server clock)', 'Message cannot be processed']; |
||
| 203 | return false; |
||
| 204 | } |
||
| 205 | |||
| 206 | return true; |
||
| 207 | } |
||
| 208 | |||
| 209 | /** |
||
| 210 | * Receive the message, decrypt it and save it. |
||
| 211 | * |
||
| 212 | * @param string $downloadPath The directory where to store received files |
||
| 213 | * @param bool $debugMode Whether debugging information should be returned |
||
| 214 | * (default: false) |
||
| 215 | * |
||
| 216 | * @throws XenForo_Exception |
||
| 217 | * @return string|array the message, which should be shown |
||
| 218 | */ |
||
| 219 | public function processMessage($downloadPath, $debugMode = false) |
||
| 220 | { |
||
| 221 | /** @var string $output */ |
||
| 222 | $output = ''; |
||
| 223 | |||
| 224 | if (!ThreemaGateway_Handler_Validation::checkDir($downloadPath)) { |
||
| 225 | throw new XenForo_Exception('Download dir ' . $downloadPath . ' cannot be accessed.'); |
||
| 226 | } |
||
| 227 | |||
| 228 | try { |
||
| 229 | /** @var Threema\MsgApi\Helpers\ReceiveMessageResult $receiveResult */ |
||
| 230 | $receiveResult = $this->getE2EHelper()->receiveMessage( |
||
| 231 | $this->filtered['from'], |
||
| 232 | $this->filtered['messageId'], |
||
| 233 | $this->getCryptTool()->hex2bin($this->filtered['box']), |
||
| 234 | $this->getCryptTool()->hex2bin($this->filtered['nonce']), |
||
| 235 | $downloadPath |
||
| 236 | ); |
||
| 237 | } catch (Exception $e) { |
||
| 238 | // as XenForo does not allow exception chaining, we better log the exception right now |
||
| 239 | XenForo_Error::logException($e); |
||
| 240 | throw new XenForo_Exception('Message cannot be processed: [' . get_class($e) . '] ' . $e->getMessage()); |
||
| 241 | } |
||
| 242 | |||
| 243 | if (!$receiveResult->isSuccess()) { |
||
| 244 | throw new XenForo_Exception('Message cannot be processed: [ResultErrors] ' . implode('|', $receiveResult->getErrors())); |
||
| 245 | } |
||
| 246 | |||
| 247 | /** @var Threema\MsgApi\Messages\ThreemaMessage $threemaMsg */ |
||
| 248 | $threemaMsg = $receiveResult->getThreemaMessage(); |
||
| 249 | |||
| 250 | // create detailed log when debug mode is enabled |
||
| 251 | if ($debugMode) { |
||
| 252 | $output = $this->getLogData($receiveResult, $threemaMsg); |
||
| 253 | } |
||
| 254 | |||
| 255 | /** @var bool $saveMessage whether to save the message to DB. */ |
||
| 256 | $saveMessage = true; |
||
| 257 | |||
| 258 | XenForo_CodeEvent::fire('threemagw_message_callback_presave', [ |
||
| 259 | $this, |
||
| 260 | $receiveResult, |
||
| 261 | $threemaMsg, |
||
| 262 | &$output, |
||
| 263 | &$saveMessage, |
||
| 264 | $debugMode |
||
| 265 | ], $threemaMsg->getTypeCode()); |
||
| 266 | |||
| 267 | // save message in database |
||
| 268 | try { |
||
| 269 | if ($saveMessage) { |
||
| 270 | $this->saveMessage($receiveResult, $threemaMsg); |
||
| 271 | } else { |
||
| 272 | $this->saveMessageId($receiveResult->getMessageId()); |
||
| 273 | } |
||
| 274 | } catch (Exception $e) { |
||
| 275 | // as XenForo does not allow Exception chaining, we better log the exception right now |
||
| 276 | XenForo_Error::logException($e); |
||
| 277 | throw new XenForo_Exception('Message could not be saved: [' . get_class($e) . '] ' . $e->getMessage()); |
||
| 278 | } |
||
| 279 | |||
| 280 | XenForo_CodeEvent::fire('threemagw_message_callback_postsave', [ |
||
| 281 | $this, |
||
| 282 | $receiveResult, |
||
| 283 | $threemaMsg, |
||
| 284 | &$output, |
||
| 285 | $saveMessage, |
||
| 286 | $debugMode |
||
| 287 | ], $threemaMsg->getTypeCode()); |
||
| 288 | |||
| 289 | return $output; |
||
| 290 | } |
||
| 291 | |||
| 292 | /** |
||
| 293 | * Adds a string to the current log string or array. |
||
| 294 | * |
||
| 295 | * @param array|string $log string or array |
||
| 296 | * @param string $stringToAdd |
||
| 297 | * @param string $stringToAddDetail |
||
| 298 | */ |
||
| 299 | public function addLog(&$log, $stringToAdd, $stringToAddDetail = null) |
||
| 323 | |||
| 324 | /** |
||
| 325 | * Checks whether a message is already saved. If so this may indicate a |
||
| 326 | * replay attack. |
||
| 327 | * |
||
| 328 | * @param string $messageId |
||
| 329 | * |
||
| 330 | * @throws XenForo_Exception |
||
| 331 | */ |
||
| 332 | public function assertNoReplayAttack($messageId) |
||
| 351 | |||
| 352 | /** |
||
| 353 | * Get request data. |
||
| 354 | * |
||
| 355 | * If you obmit the $key parameter you get an array of all request parameters. |
||
| 356 | * If not, you'll get one specific entry. |
||
| 357 | * In case nothing could be found, this returns "null". |
||
| 358 | * |
||
| 359 | * @param string $key |
||
| 360 | * @return string|array|null |
||
| 361 | */ |
||
| 362 | public function getRequest($key = null) |
||
| 374 | |||
| 375 | /** |
||
| 376 | * Returns the original input. |
||
| 377 | * |
||
| 378 | * It is strongly discouraghed to use this and better use {@see getRequest()} |
||
| 379 | * as this data is already filtered. |
||
| 380 | * In general you should have few real reasons to get this RAW data. |
||
| 381 | * |
||
| 382 | * @return XenForo_Input |
||
| 383 | */ |
||
| 384 | public function getInput() |
||
| 388 | |||
| 389 | /** |
||
| 390 | * Returns an array with a not so detailed[2] and a very detailed[1] log |
||
| 391 | * of the received message. |
||
| 392 | * |
||
| 393 | * @param Threema\MsgApi\Helpers\ReceiveMessageResult $receiveResult Threema MsgApi receive result |
||
| 394 | * @param Threema\MsgApi\Messages\ThreemaMessage $threemaMsg Threema MsgApi message |
||
| 395 | * |
||
| 396 | * @return array[null, string, string] |
||
| 397 | */ |
||
| 398 | protected function getLogData( |
||
| 441 | |||
| 442 | /** |
||
| 443 | * Saves a decrypted message in the database. |
||
| 444 | * |
||
| 445 | * @param Threema\MsgApi\Helpers\ReceiveMessageResult $receiveResult Threema MsgApi receive result |
||
| 446 | * @param Threema\MsgApi\Messages\ThreemaMessage $threemaMsg Threema MsgApi message |
||
| 447 | * |
||
| 448 | * @throws XenForo_Exception |
||
| 449 | */ |
||
| 450 | protected function saveMessage( |
||
| 501 | |||
| 502 | /** |
||
| 503 | * Only saves a message ID to the database to prevent replay attacks. |
||
| 504 | * |
||
| 505 | * @param string $messageId |
||
| 506 | * |
||
| 507 | * @throws XenForo_Exception |
||
| 508 | */ |
||
| 509 | protected function saveMessageId($messageId) |
||
| 518 | |||
| 519 | /** |
||
| 520 | * Converts binary data in an array to hex. |
||
| 521 | * |
||
| 522 | * @param array $bin binary array |
||
| 523 | * |
||
| 524 | * @return array |
||
| 525 | */ |
||
| 526 | protected function bin2hexArray($bin) |
||
| 534 | } |
||
| 535 |
Loading history...
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.