RESTful-Drupal / restful

RESTful best practices for Drupal
https://drupal.org/project/restful
419 stars 173 forks source link

token_auth on multiple devices #977

Open cedatif opened 7 years ago

cedatif commented 7 years ago

Hi,

Can I have multiple access token (and refresh_token) for one user : one account used on multiple devices with the restful_token_auth module ?

Thanks !

tnightingale commented 7 years ago

This doesn't seem to be supported; restful_token_auth makes sure to only allow one access_token/refresh_token pair per user. This functionality also surprised us as (we have an app talking to Drupal via RESTful & RESTful Token) it's not uncommon for a user to want to log into the application with two or more devices.

Here's a very rough idea of the changes required to loosen this requirement:

diff --git a/modules/restful_token_auth/src/Entity/RestfulTokenAuthController.php b/modules/restful_token_auth/src/Entity/RestfulTokenAuthController.php
index a730f07..e136453 100644
--- a/modules/restful_token_auth/src/Entity/RestfulTokenAuthController.php
+++ b/modules/restful_token_auth/src/Entity/RestfulTokenAuthController.php
@@ -57,17 +57,17 @@ class RestfulTokenAuthController extends \EntityAPIController {
    */
   private function generateRefreshToken($uid) {
     // Check if there are other refresh tokens for the user.
-    $query = new \EntityFieldQuery();
-    $results = $query
-      ->entityCondition('entity_type', 'restful_token_auth')
-      ->entityCondition('bundle', 'refresh_token')
-      ->propertyCondition('uid', $uid)
-      ->execute();
-
-    if (!empty($results['restful_token_auth'])) {
-      // Delete the tokens.
-      entity_delete_multiple('restful_token_auth', array_keys($results['restful_token_auth']));
-    }
+    // $query = new \EntityFieldQuery();
+    // $results = $query
+    //   ->entityCondition('entity_type', 'restful_token_auth')
+    //   ->entityCondition('bundle', 'refresh_token')
+    //   ->propertyCondition('uid', $uid)
+    //   ->execute();
+    //
+    // if (!empty($results['restful_token_auth'])) {
+    //   // Delete the tokens.
+    //   entity_delete_multiple('restful_token_auth', array_keys($results['restful_token_auth']));
+    // }

     // Create a new refresh token.
     $values = array(
diff --git a/modules/restful_token_auth/src/Plugin/resource/AccessToken__1_0.php b/modules/restful_token_auth/src/Plugin/resource/AccessToken__1_0.php
index 7782658..f161ff7 100644
--- a/modules/restful_token_auth/src/Plugin/resource/AccessToken__1_0.php
+++ b/modules/restful_token_auth/src/Plugin/resource/AccessToken__1_0.php
@@ -46,7 +46,7 @@ class AccessToken__1_0 extends TokenAuthenticationBase implements ResourceInterf
     return array(
       '' => array(
         // Get or create a new token.
-        RequestInterface::METHOD_GET => 'getOrCreateToken',
+        RequestInterface::METHOD_GET => 'createToken',
         RequestInterface::METHOD_OPTIONS => 'discover',
       ),
     );
@@ -55,43 +55,45 @@ class AccessToken__1_0 extends TokenAuthenticationBase implements ResourceInterf
   /**
    * Create a token for a user, and return its value.
    */
-  public function getOrCreateToken() {
+  public function createToken() {
     $entity_type = $this->getEntityType();
     $account = $this->getAccount();
-    // Check if there is a token that did not expire yet.
-    /* @var DataProviderEntityInterface $data_provider */
-    $data_provider = $this->getDataProvider();
-    $query = $data_provider->EFQObject();
-    $result = $query
-      ->entityCondition('entity_type', $entity_type)
-      ->entityCondition('bundle', 'access_token')
-      ->propertyCondition('uid', $account->uid)
-      ->range(0, 1)
-      ->execute();
-
-    $token_exists = FALSE;

-    if (!empty($result[$entity_type])) {
-      $id = key($result[$entity_type]);
-      $access_token = entity_load_single($entity_type, $id);
+    // TODO: Reimplement token cleanup (but needs to support multiple tokens).

-      $token_exists = TRUE;
-      if (!empty($access_token->expire) && $access_token->expire < REQUEST_TIME) {
-        if (variable_get('restful_token_auth_delete_expired_tokens', TRUE)) {
-          // Token has expired, so we can delete this token.
-          $access_token->delete();
-        }
+    // Check if there is a token that did not expire yet.
+    /* @var DataProviderEntityInterface $data_provider */
+    // $data_provider = $this->getDataProvider();
+    // $query = $data_provider->EFQObject();
+    // $result = $query
+    //   ->entityCondition('entity_type', $entity_type)
+    //   ->entityCondition('bundle', 'access_token')
+    //   ->propertyCondition('uid', $account->uid)
+    //   ->range(0, 1)
+    //   ->execute();
+    //
+    // $token_exists = FALSE;
+    //
+    // if (!empty($result[$entity_type])) {
+    //   $id = key($result[$entity_type]);
+    //   $access_token = entity_load_single($entity_type, $id);
+    //
+    //   $token_exists = TRUE;
+    //   if (!empty($access_token->expire) && $access_token->expire < REQUEST_TIME) {
+    //     if (variable_get('restful_token_auth_delete_expired_tokens', TRUE)) {
+    //       // Token has expired, so we can delete this token.
+    //       $access_token->delete();
+    //     }
+    //
+    //     $token_exists = FALSE;
+    //   }
+    // }

-        $token_exists = FALSE;
-      }
-    }
+    /* @var \Drupal\restful_token_auth\Entity\RestfulTokenAuthController $controller */
+    $controller = entity_get_controller($this->getEntityType());
+    $access_token = $controller->generateAccessToken($account->uid);
+    $id = $access_token->id;

-    if (!$token_exists) {
-      /* @var \Drupal\restful_token_auth\Entity\RestfulTokenAuthController $controller */
-      $controller = entity_get_controller($this->getEntityType());
-      $access_token = $controller->generateAccessToken($account->uid);
-      $id = $access_token->id;
-    }
     $output = $this->view($id);

     return $output;

Note that this is barely tested. It doesn't aggressively clean up expired tokens outside of cron runs in the same way the module currently does. There's likely other stuff I have missed too.

Would be great to hear from the maintainers about the thinking behind why this functions this way.

tnightingale commented 7 years ago

This means the same user can have multiple valid tokens, which is not necessarily a bad thing. https://github.com/RESTful-Drupal/restful/issues/738#issuecomment-155854426

Indicates that current functionality is by-design (though maybe just arbitrarily?).

rhclayto commented 7 years ago

The current method of having one token per user doesn't prevent a user from being logged in on multiple devices. Each device will share the same token. I am able to log in to multiple devices simultaneously as the same user without problems. Does this not work for you guys?

Although I guess the problem is if you log out of one device it logs you out of all devices.

tnightingale commented 7 years ago

Yes it works but only until the token expires and one of the clients attempts to refresh using the refresh token. That client gets a new pair of tokens but the other clients are left with stale access token and a used refresh token. On Thu, Mar 23, 2017 at 5:31 AM rhclayto notifications@github.com wrote:

The current method of having one token per user doesn't prevent a user from being logged in on multiple devices. Each device will use the same token. Does this not work for you guys? I am able to log in to multiple devices simultaneously as the same user without problems.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/RESTful-Drupal/restful/issues/977#issuecomment-288704052, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKWo4jPYI0h1obfs8xzOgYF8PYY7sVRks5romYQgaJpZM4MPnW7 .