Fix sessionId handling to allow authentication with sessionId #80

chrbayer · 2019-02-12T12:10:14Z

The sessionId was used manually for some ParseUser communication, but has to be used with all communication with the parse server to be able to access protected resources. It took me quite some time to find me the reason why I was not able to access my classes after I activating CLP for my classes...
Some small code clean-ups regarding storage.

phillwiggins · 2019-02-13T21:55:12Z

Hey, can you check my comments please.

Just a few queries.

Cheers

chrbayer · 2019-02-14T07:35:38Z

Hi, of course, but where can I find your comments? I'm not using github that often :-)

phillwiggins · 2019-02-14T07:39:17Z

Ah okay, they are on GitHub. There seems to be some confusion on when your removing token from one method and when your replacing it elsewhere.

…

On Thu, Feb 14, 2019, 07:35 Christoph Bayer ***@***.*** wrote: Hi, of course, but where can I find your comments? I'm not using github that often :-) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#80 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgn3mNhqKCJDSChNbcqm0fvGllxv4ndks5vNRHKgaJpZM4a2QYz> .

chrbayer · 2019-02-14T07:51:43Z

In the previous version, the sessionId was manually inserted in the client request in three methods of ParseUser: getCurrentUserFromServer(), save() and destroy(). I removed this, instead the the sessionId is now always send in any transaction to the server if available, I included it in ParseHTTPClient.send(). This means that of course the sessionId is still send for these three methods. In a real parse server application I think it is important to restrict the access to any used classes on the server, if not there would be no reason to # or login at all. And if the access is restricted, you need to authenticate each access with your sessionId.

phillwiggins · 2019-02-14T08:27:04Z

I see. So the user token is only sent for user based requests?

…

On Thu, Feb 14, 2019, 07:51 Christoph Bayer ***@***.*** wrote: In the previous version, the sessionId was manually inserted in the client request in three methods of ParseUser: getCurrentUserFromServer(), save() and destroy(). I removed this, instead the the sessionId is now always send in any transaction to the server if available, I included it in ParseHTTPClient.send(). In a real parse server application I think it is important to restrict the access to any used classes on the server, if not there would be no reason to # or login at all. And if the access is restricted, you need to authenticate each access with your sessionId. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#80 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgn3h2GZcotHFNEuqJ5-gWKzSBLsNHnks5vNRWPgaJpZM4a2QYz> .

chrbayer · 2019-02-14T08:52:13Z

No, for all requests. Is not every request somehow user based after login?

phillwiggins

After further reading, I do not think is correct.

It makes sense for user related changes, not all objects. We need to revisit this.

https://docs.parseplatform.org/rest/guide/#validating-session-tokens--retrieving-current-user

In your current setup, do you need a user token for everything you do? This doesn't seem logical.

phillwiggins · 2019-02-12T12:33:36Z

lib/parse_server_sdk.dart

@@ -89,8 +89,6 @@ class Parse {
        sessionId: sessionId,
        securityContext: securityContext);

-    ParseCoreData().initStorage();


Just to confirm, the ParseCoreData class is a singleton, initStorage method creates an instance of ParseCoreData with all the necessary data. If this method is not called here, where else is it called? / Or is not needed to be called?

The init functionality was and still is in getStore(), It is initialized at the first usage:

Future<SharedPreferences> getStore() async { return storage ?? (storage = await SharedPreferences.getInstance()); }

phillwiggins · 2019-02-12T12:35:07Z

lib/src/objects/parse_user.dart

@@ -63,20 +63,16 @@ class ParseUser extends ParseObject implements ParseCloneable {
  /// fromServer can be called and an updated version of the [User] object will be
  /// returned
  static Future<ParseResponse> getCurrentUserFromServer(
-      {String token, bool debug, ParseHTTPClient client}) async {


This was added recently by another user who needed a token. I understand your hoping to send the token if we have it for every call, but I don't see where you are adding it?

Oh, Ok, I was not aware it was recently added, I will look for a way to use a different token in this function.

phillwiggins · 2019-02-16T06:46:50Z

Okay so after further reading, I think this makes sense when used with ACL. As we don't always want to send a token, please can you change your code so that the user has the option of sending a token. @chrbayer it's good work, but not quite there. This doesn't fit all user requirements. Maybe set a bool in ParseCoreData for now that can determine whether to send token with all requests?

…e_sdk

chrbayer · 2019-02-16T12:46:23Z

Even if the the sessionId is not needed for ACL or CLP, what harm does it make sending it? Nevertheless I can make sending the sessionId optional for not user objects.

phillwiggins · 2019-02-16T12:48:43Z

I see. I'm not sure if there are many side affects of having the user token present when not required, but looking through the docs, it still doesn't look like best practice. On this basis, please do make it an optional. I think somebody is looking into implementing ACL features as we speak and I am hoping they will make the call on this. I really do appreciate your efforts.

…

On Sat, Feb 16, 2019, 12:46 Christoph Bayer ***@***.*** wrote: Even if the the sessionId is not needed for ACL or CLP, what harm does it make sending it? Nevertheless I can make sending the sessionId optional for not user objects. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#80 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgn3vigeR8HgkwVayikICSkTXwJKJUlks5vN_2fgaJpZM4a2QYz> .

phillwiggins · 2019-02-16T14:18:19Z

I've merged this but will need to do some work.

Your still forcing tokens when it's not necessary. The only token that users are going to be sending is their current user token, so checking if a token is present in the headers and if not using the token from ParseCoreData is redundant. It's going to be the same token either way.

chrbayer · 2019-02-16T19:08:29Z

Yeah, I am not ready and still working on making sending the token optional, I was interrupted this afternoon :-)

Checking if a header is present is not completely redundant, because in the getCurrentUserFromServer() function I do insert a user given not necessarily the same SessionId header.

I inform you as soon as my work is done.

Thanks!

Fix sessionId handling to allow authentication with sessionId

Christoph Bayer added 4 commits February 10, 2019 13:07

Small clean-ups for storage.

9e65690

Always send sessionId if available.

241e8ce

Merge remote-tracking branch 'origin/master'

3808441

Remove unused function initStorage.

8c0d423

phillwiggins reviewed Feb 16, 2019

View reviewed changes

Merge branch 'master' of https://github.com/phillwiggins/flutter_pars…

3194cab

…e_sdk

Allow given token in getCurrentUserFromServer.

0e8734c

phillwiggins merged commit 88f0b80 into parse-community:master Feb 16, 2019

phillwiggins mentioned this pull request Jun 18, 2019

Best strategy for testing my app #202

Closed

fischerscode pushed a commit to fischerscode/Parse-SDK-Flutter that referenced this pull request Sep 5, 2020

Merge pull request parse-community#80 from chrbayer/master

b57fd2a

Fix sessionId handling to allow authentication with sessionId

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix sessionId handling to allow authentication with sessionId #80

Fix sessionId handling to allow authentication with sessionId #80

chrbayer commented Feb 12, 2019

phillwiggins commented Feb 13, 2019

chrbayer commented Feb 14, 2019

phillwiggins commented Feb 14, 2019 via email

chrbayer commented Feb 14, 2019 •

edited

Loading

phillwiggins commented Feb 14, 2019 via email

chrbayer commented Feb 14, 2019

phillwiggins left a comment

phillwiggins Feb 12, 2019

chrbayer Feb 16, 2019 •

edited

Loading

phillwiggins Feb 12, 2019

chrbayer Feb 16, 2019

phillwiggins commented Feb 16, 2019

chrbayer commented Feb 16, 2019

phillwiggins commented Feb 16, 2019 via email

phillwiggins commented Feb 16, 2019

chrbayer commented Feb 16, 2019

Fix sessionId handling to allow authentication with sessionId #80

Fix sessionId handling to allow authentication with sessionId #80

Conversation

chrbayer commented Feb 12, 2019

phillwiggins commented Feb 13, 2019

chrbayer commented Feb 14, 2019

phillwiggins commented Feb 14, 2019 via email

chrbayer commented Feb 14, 2019 • edited Loading

phillwiggins commented Feb 14, 2019 via email

chrbayer commented Feb 14, 2019

phillwiggins left a comment

Choose a reason for hiding this comment

phillwiggins Feb 12, 2019

Choose a reason for hiding this comment

chrbayer Feb 16, 2019 • edited Loading

Choose a reason for hiding this comment

phillwiggins Feb 12, 2019

Choose a reason for hiding this comment

chrbayer Feb 16, 2019

Choose a reason for hiding this comment

phillwiggins commented Feb 16, 2019

chrbayer commented Feb 16, 2019

phillwiggins commented Feb 16, 2019 via email

phillwiggins commented Feb 16, 2019

chrbayer commented Feb 16, 2019

chrbayer commented Feb 14, 2019 •

edited

Loading

chrbayer Feb 16, 2019 •

edited

Loading