Repository: parse-community/Parse-SDK-JS
Pull request #846 (2019-07-03 23:29:14)

Stateless UserController
Closes: https://github.com/parse-community/Parse-SDK-JS/issues/833

There aren't any CurrentUser in a server environment. This PR allows for use of sessionTokens and prevent errors from being thrown.

Let me know if more tests are needed and any feedback

codecov[bot]: # [Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/846?src=pr&el=h1) Report
> Merging [#846](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/846?src=pr&el=desc) into [master](https://codecov.io/gh/parse-community/Parse-SDK-JS/commit/4fcbabe8cacdde79269f95a9a0a460415dc767cc?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/846/graphs/tree.svg?width=650&token=8nCENM0QHj&height=150&src=pr)](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/846?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #846   +/-   ##
=======================================
  Coverage   92.07%   92.07%           
=======================================
  Files          54       54           
  Lines        4933     4933           
  Branches     1097     1098    +1     
=======================================
  Hits         4542     4542           
  Misses        391      391
```


| [Impacted Files](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/846?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/CoreManager.js](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/846/diff?src=pr&el=tree#diff-c3JjL0NvcmVNYW5hZ2VyLmpz) | `100% <ø> (ø)` | :arrow_up: |
| [src/ParseUser.js](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/846/diff?src=pr&el=tree#diff-c3JjL1BhcnNlVXNlci5qcw==) | `82.58% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/846?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/846?src=pr&el=footer). Last update [4fcbabe...32d3f35](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/846?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

dplewis: > My suggestion is to leave the become as is now and create a new function (maybe called "me"?).

`Parse.User.me(sessionToken)` @acinader What do you think?

> Shouldn't we take care of "signUp" and "logIn" as well?

I believe those are already taken care of in a node js environment.
acinader: ugh.  I can't quite wrap my head around this -- probably just soft after my holiday!

Most of the work I've done with the JS-SDK is cloud code, so I am familiar with the territory, but I could use a use case of what we're trying to accomplish (in the form of an integration test or two) to show what the goal of this change is more clearly.

I read #833 twice, but I can't quite piece together what we're solving and why.

davimacedo: @acinader currently, there is no method in the Node.js SDK to log out or validate a session token. That's the problem @dplewis is solving. This PR does two different things:
- Include the option of passing a session token argument in the logOut() method, so it can be used in Node.js;
- Changes the become() method to be used in Node.js - for this one, in my opinion, I think we should create a new method maybe called me();
acinader: ok.  I get the logout fix and I buy it.  Not so sure about the become/me.  

What use case would you use this become/me for?  And where would the session Id be coming from?

When I've wanted server code to be able to 'log in', I have made an auth adapter so I can create a session using login.
davimacedo: Currently the same UserController is used in two different scenarios:
- Running in web browser:
1) Client uses signUp(username, password), logIn(username, password), or become(sessionToken) functions
2) SDK validates credentials/sessionToken in Parse Server and receive back a valid session token and the current user data
3) SDK stores the session token, current user data and automatically uses the session token in any future api call
4) Client uses logOut() to destroy the session token
- Running in server
1) Client calls some kind of signUp or logIn function in a custom Node.js application
2) Node.js application use uses SDK's signUp(username, password) or logIn(username, password) functions
3) SDK validates credentials in Parse Server and receive back a valid session token and the current user data (it does not store it, differently when used in the web browser)
4) Node.js application returns session token and current user data to client
5) Client stores session token and current user data no matter how
6) For a future request (for instance, find() objects), client send to Node.js the session token
7) Node.js calls SDK's find({ sessionToken }) function
8) SDK sends the operation to Parse Server together with sessionToken and receive back the result
9) Node.js receive result from SDK and send to client

The flow works, except:
- There is no convenient method for logging out, because current SDK's logOut() function does not have sessionToken option - I understand this point we've already pacified
- There is no convenient method for validating a session token and receiving back the user data (take a look in this thread here: https://community.parseplatform.org/t/session-token-validation-with-nodejs/87/27) - This is the become/me function; I agree with @dplewis that it should exist, I am not so comfortable with the name become() though, because it can cause some confusion to the developers as they may think that, after using this function, the next api calls will use the sessionToken automatically (and that's not true for the Node.js SDK). I suggested me, because it is actually the name of the REST endpoint that performs the job in the Parse Server side (https://github.com/parse-community/parse-server/blob/master/src/Routers/UsersRouter.js#L424).

I agree with you that we could have two controllers but I'd tackle this on a separate issue.
acinader: @davimacedo 

whew.  Thank you for laying that out so crystal clearly.  

the only real purpose of me/become in the node environment would be to validate a sessionToken? or is there any other use case?

If that's the only use, I'm good with it.  My preference is also `me` over become now that I understand you rational @davimacedo 

davimacedo: > the only real purpose of me/become in the node environment would be to validate a sessionToken? or is there any other use case? I can't think of one.

I think it is the only one

> On the two controllers, just asking for your view. After reading your response, the thread you cite and playing with some code, I don't think that breaking into an abstract class with two subclasses would really be worth overhead it would take to figure out what is going on. Though I don't have a strong opinion either way :).

In my personal opinion, I think it is easier to understand if we go with the subclasses instead of the current approach in which I have to read the whole code and understand the "if" clauses. I think it is a low priority issue though.

Patch:
diff --git a/src/CoreManager.js b/src/CoreManager.js
index 9a5881d..75cad7c 100644
--- a/src/CoreManager.js
+++ b/src/CoreManager.js
@@ -121,7 +121,8 @@ type UserController = {
   logIn: (user: ParseUser, options: RequestOptions) => Promise;
   become: (options: RequestOptions) => Promise;
   hydrate: (userJSON: AttributeMap) => Promise;
-  logOut: () => Promise;
+  logOut: (options: RequestOptions) => Promise;
+  me: (options: RequestOptions) => Promise;
   requestPasswordReset: (email: string, options: RequestOptions) => Promise;
   updateUserOnDisk: (user: ParseUser) => Promise;
   upgradeToRevocableSession: (user: ParseUser, options: RequestOptions) => Promise;
@@ -378,6 +379,7 @@ module.exports = {
       'logIn',
       'become',
       'logOut',
+      'me',
       'requestPasswordReset',
       'upgradeToRevocableSession',
       'linkWith',
diff --git a/src/ParseUser.js b/src/ParseUser.js
index 7cbefa9..7f1d03e 100644
--- a/src/ParseUser.js
+++ b/src/ParseUser.js
@@ -651,6 +651,25 @@ class ParseUser extends ParseObject {
     return controller.become(becomeOptions);
   }
 
+  /**
+   * Retrieves a user with a session token.
+   *
+   * @param {String} sessionToken The sessionToken to get user with.
+   * @param {Object} options
+   * @static
+   * @return {Promise} A promise that is fulfilled with the user is fetched.
+   */
+  static me(sessionToken: string, options?: RequestOptions = {}) {
+    const controller = CoreManager.getUserController();
+    const meOptions: RequestOptions = {
+      sessionToken: sessionToken
+    };
+    if (options.useMasterKey) {
+      meOptions.useMasterKey = options.useMasterKey;
+    }
+    return controller.me(meOptions);
+  }
+
   /**
    * Logs in a user with a session token. On success, this saves the session
    * to disk, so you can retrieve the currently logged in user using
@@ -674,20 +693,15 @@ class ParseUser extends ParseObject {
    * Logs out the currently logged in user session. This will remove the
    * session from disk, log out of linked services, and future calls to
    * <code>current</code> will return <code>null</code>.
-
+   *
+   * @param {Object} options
    * @static
    * @return {Promise} A promise that is resolved when the session is
    *   destroyed on the server.
    */
-  static logOut() {
-    if (!canUseCurrentUser) {
-      throw new Error(
-        'There is no current user on a node.js server environment.'
-      );
-    }
-
+  static logOut(options: RequestOptions = {}) {
     const controller = CoreManager.getUserController();
-    return controller.logOut();
+    return controller.logOut(options);
   }
 
   /**
@@ -998,11 +1012,28 @@ const DefaultController = {
     }
   },
 
-  logOut(): Promise<ParseUser> {
+  me(options: RequestOptions): Promise<ParseUser> {
+    const RESTController = CoreManager.getRESTController();
+    return RESTController.request(
+      'GET', 'users/me', {}, options
+    ).then((response) => {
+      const user = new ParseUser();
+      user._finishFetch(response);
+      user._setExisted(true);
+      return user;
+    });
+  },
+
+  logOut(options: RequestOptions): Promise<ParseUser> {
+    const RESTController = CoreManager.getRESTController();
+    if (options.sessionToken) {
+      return RESTController.request(
+        'POST', 'logout', {}, options
+      );
+    }
     return DefaultController.currentUserAsync().then((currentUser) => {
       const path = Storage.generatePath(CURRENT_USER_KEY);
       let promise = Storage.removeItemAsync(path);
-      const RESTController = CoreManager.getRESTController();
       if (currentUser !== null) {
         const isAnonymous = AnonymousUtils.isLinked(currentUser);
         const currentSession = currentUser.getSessionToken();


Test patch:
diff --git a/src/__tests__/ParseUser-test.js b/src/__tests__/ParseUser-test.js
index ef0fea9..7ff1903 100644
--- a/src/__tests__/ParseUser-test.js
+++ b/src/__tests__/ParseUser-test.js
@@ -137,9 +137,6 @@ describe('ParseUser', () => {
     expect(ParseUser.become.bind(null, 'token')).toThrow(
       'It is not memory-safe to become a user in a server environment'
     );
-    expect(ParseUser.logOut).toThrow(
-      'There is no current user on a node.js server environment.'
-    );
   });
 
   it('can sign up a new user', (done) => {
@@ -675,6 +672,71 @@ describe('ParseUser', () => {
     });
   });
 
+  it('can retreive a user with sessionToken (me)', async () => {
+    ParseUser.disableUnsafeCurrentUser();
+    ParseUser._clearCache();
+    CoreManager.setRESTController({
+      request(method, path, body, options) {
+        expect(method).toBe('GET');
+        expect(path).toBe('users/me');
+        expect(options.sessionToken).toBe('123abc');
+        return Promise.resolve({
+          objectId: 'uid3',
+          username: 'username',
+          sessionToken: '123abc'
+        }, 200);
+      },
+      ajax() {}
+    });
+
+    const user = await ParseUser.me('123abc');
+    expect(user.id).toBe('uid3');
+    expect(user.isCurrent()).toBe(false);
+    expect(user.existed()).toBe(true);
+  });
+
+  it('can retreive a user with sessionToken and masterKey(me)', async () => {
+    ParseUser.disableUnsafeCurrentUser();
+    ParseUser._clearCache();
+    CoreManager.setRESTController({
+      request(method, path, body, options) {
+        expect(method).toBe('GET');
+        expect(path).toBe('users/me');
+        expect(options.sessionToken).toBe('123abc');
+        expect(options.useMasterKey).toBe(true);
+        return Promise.resolve({
+          objectId: 'uid3',
+          username: 'username',
+          sessionToken: '123abc'
+        }, 200);
+      },
+      ajax() {}
+    });
+
+    const user = await ParseUser.me('123abc', { useMasterKey: true });
+    expect(user.id).toBe('uid3');
+    expect(user.isCurrent()).toBe(false);
+    expect(user.existed()).toBe(true);
+  });
+
+  it('can logout user with sessionToken', async () => {
+    ParseUser.disableUnsafeCurrentUser();
+    ParseUser._clearCache();
+    Storage._clear();
+    const RESTController = {
+      request() {
+        return Promise.resolve({}, 200);
+      },
+      ajax() {}
+    };
+    jest.spyOn(RESTController, 'request');
+    CoreManager.setRESTController(RESTController);
+
+    await ParseUser.logOut({ sessionToken: '1234' });
+
+    expect(RESTController.request).toHaveBeenCalledWith('POST', 'logout', {}, { sessionToken: '1234' });
+  });
+
   it('can get error when recursive _linkWith call fails', (done) => {
     CoreManager.setRESTController({
       request(method, path, body) {
diff --git a/src/__tests__/RESTController-test.js b/src/__tests__/RESTController-test.js
index 4d6ce9b..3d48b35 100644
--- a/src/__tests__/RESTController-test.js
+++ b/src/__tests__/RESTController-test.js
@@ -263,6 +263,7 @@ describe('RESTController', () => {
       logIn() {},
       become() {},
       logOut() {},
+      me() {},
       requestPasswordReset() {},
       upgradeToRevocableSession() {},
       linkWith() {},
@@ -298,6 +299,7 @@ describe('RESTController', () => {
       logIn() {},
       become() {},
       logOut() {},
+      me() {},
       requestPasswordReset() {},
       upgradeToRevocableSession() {},
       linkWith() {},