From bb044f53941907795a66e27edc0c3598a48f3a79 Mon Sep 17 00:00:00 2001
From: sam marshall <s.marshall@open.ac.uk>
Date: Mon, 25 Mar 2019 16:31:46 +0000
Subject: [PATCH] MDL-65179 Web service: Token last access is updated too
 frequently

---
 lib/datalib.php               |  3 ++
 webservice/lib.php            | 33 ++++++++++++++++++--
 webservice/tests/lib_test.php | 57 +++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 2 deletions(-)

diff --git a/lib/datalib.php b/lib/datalib.php
index ad64468ad48..f2eb8f2791d 100644
--- a/lib/datalib.php
+++ b/lib/datalib.php
@@ -45,6 +45,9 @@ define('MAX_COURSE_CATEGORIES', 10000);
  *
  * We allow overwrites from config.php, useful to ensure coherence in performance
  * tests results.
+ *
+ * Note: For web service requests in the external_tokens field, we use a different constant
+ * webservice::TOKEN_LASTACCESS_UPDATE_SECS.
  */
 if (!defined('LASTACCESS_UPDATE_SECS')) {
     define('LASTACCESS_UPDATE_SECS', 60);
diff --git a/webservice/lib.php b/webservice/lib.php
index fedfbc8e46b..3126b60a8be 100644
--- a/webservice/lib.php
+++ b/webservice/lib.php
@@ -48,6 +48,14 @@ define('WEBSERVICE_AUTHMETHOD_SESSION_TOKEN', 2);
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class webservice {
+    /**
+     * Only update token last access once per this many seconds. (This constant controls update of
+     * the external tokens last access field. There is a similar define LASTACCESS_UPDATE_SECS
+     * which controls update of the web site last access fields.)
+     *
+     * @var int
+     */
+    const TOKEN_LASTACCESS_UPDATE_SECS = 60;
 
     /**
      * Authenticate user (used by download/upload file scripts)
@@ -209,11 +217,32 @@ class webservice {
         }
 
         // log token access
-        $DB->set_field('external_tokens', 'lastaccess', time(), array('id' => $token->id));
+        self::update_token_lastaccess($token);
 
         return array('user' => $user, 'token' => $token, 'service' => $service);
     }
 
+    /**
+     * Updates the last access time for a token.
+     *
+     * @param \stdClass $token Token object (must include id, lastaccess fields)
+     * @param int $time Time of access (0 = use current time)
+     * @throws dml_exception If database error
+     */
+    public static function update_token_lastaccess($token, int $time = 0) {
+        global $DB;
+
+        if (!$time) {
+            $time = time();
+        }
+
+        // Only update the field if it is a different time from previous request,
+        // so as not to waste database effort.
+        if ($time >= $token->lastaccess + self::TOKEN_LASTACCESS_UPDATE_SECS) {
+            $DB->set_field('external_tokens', 'lastaccess', $time, array('id' => $token->id));
+        }
+    }
+
     /**
      * Allow user to call a service
      *
@@ -1109,7 +1138,7 @@ abstract class webservice_server implements webservice_server_interface {
         $user = $DB->get_record('user', array('id'=>$token->userid), '*', MUST_EXIST);
 
         // log token access
-        $DB->set_field('external_tokens', 'lastaccess', time(), array('id'=>$token->id));
+        webservice::update_token_lastaccess($token);
 
         return $user;
 
diff --git a/webservice/tests/lib_test.php b/webservice/tests/lib_test.php
index 5fd86802fee..59d741c7415 100644
--- a/webservice/tests/lib_test.php
+++ b/webservice/tests/lib_test.php
@@ -136,6 +136,63 @@ class webservice_test extends advanced_testcase {
         }
     }
 
+    /**
+     * Tests update_token_lastaccess() function.
+     *
+     * @throws dml_exception
+     */
+    public function test_update_token_lastaccess() {
+        global $DB;
+
+        $this->resetAfterTest(true);
+
+        // Set current user.
+        $this->setAdminUser();
+
+        // Add a web service.
+        $webservice = new stdClass();
+        $webservice->name = 'Test web service';
+        $webservice->enabled = true;
+        $webservice->restrictedusers = false;
+        $webservice->component = 'moodle';
+        $webservice->timecreated = time();
+        $webservice->downloadfiles = true;
+        $webservice->uploadfiles = true;
+        $DB->insert_record('external_services', $webservice);
+
+        // Add token.
+        $tokenstr = external_create_service_token($webservice->name, context_system::instance()->id);
+        $token = $DB->get_record('external_tokens', ['token' => $tokenstr]);
+
+        // Trigger last access once (at current time).
+        webservice::update_token_lastaccess($token);
+
+        // Check last access.
+        $token = $DB->get_record('external_tokens', ['token' => $tokenstr]);
+        $this->assertLessThan(5, abs(time() - $token->lastaccess));
+
+        // Try setting it to +1 second. This should not update yet.
+        $before = (int)$token->lastaccess;
+        webservice::update_token_lastaccess($token, $before + 1);
+        $token = $DB->get_record('external_tokens', ['token' => $tokenstr]);
+        $this->assertEquals($before, $token->lastaccess);
+
+        // To -1000 seconds. This should not update.
+        webservice::update_token_lastaccess($token, $before - 1000);
+        $token = $DB->get_record('external_tokens', ['token' => $tokenstr]);
+        $this->assertEquals($before, $token->lastaccess);
+
+        // To +59 seconds. This should also not quite update.
+        webservice::update_token_lastaccess($token, $before + 59);
+        $token = $DB->get_record('external_tokens', ['token' => $tokenstr]);
+        $this->assertEquals($before, $token->lastaccess);
+
+        // Finally to +60 seconds, where it should update.
+        webservice::update_token_lastaccess($token, $before + 60);
+        $token = $DB->get_record('external_tokens', ['token' => $tokenstr]);
+        $this->assertEquals($before + 60, $token->lastaccess);
+    }
+
     /**
      * Utility method that tests the parameter type of a method info's input/output parameter.
      *