Implement OAuth method

[jjunineuro]

New Pull Request Checklist

• I am not disclosing a vulnerability.
• I am creating this PR in reference to an issue.

Issue Description

Related issue: This is the initial version, so you can test it out. Inclusion of routines for OAuth, discussed in #7248

Approach

TODOs before merging

• Add test cases
• Add entry to changelog
• Add changes to documentation (guides, repository pages, in-code descriptions)
• Add security check
• Add new Parse Error codes to Parse JS SDK

[codecov]

Codecov Report

Merging #7257 (d07c17d) into master (f7d2e09) will decrease coverage by 0.18%.
The diff coverage is 86.97%.

❗ Current head d07c17d differs from pull request most recent head 4a0d22a. Consider uploading reports for the commit 4a0d22a to get more accurate results

@@            Coverage Diff             @@
##           master    #7257      +/-   ##
==========================================
- Coverage   94.01%   93.82%   -0.19%     
==========================================
  Files         179      179              
  Lines       13144    13247     +103     
==========================================
+ Hits        12357    12429      +72     
- Misses        787      818      +31     

Impacted Files	Coverage Δ
src/Adapters/Storage/Postgres/PostgresClient.js	70.00% <0.00%> (+3.33%)	⬆️
src/KeyPromiseQueue.js	95.45% <ø> (ø)
src/Options/index.js	100.00% <ø> (ø)
src/PromiseRouter.js	90.58% <ø> (+0.47%)	⬆️
src/Routers/UsersRouter.js	88.37% <74.13%> (-5.38%)	⬇️
src/Controllers/DatabaseController.js	93.70% <79.41%> (-1.77%)	⬇️
src/Auth.js	97.87% <89.74%> (-2.13%)	⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js	95.52% <90.00%> (+0.02%)	⬆️
src/Adapters/Auth/instagram.js	91.66% <100.00%> (ø)
src/Adapters/Cache/RedisCacheAdapter.js	87.71% <100.00%> (ø)
... and 20 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7d2e09...4a0d22a. Read the comment docs.

[dplewis]

Just a few comments. Can you write some test cases? Looks like there is 1 failing test.

[dplewis]

Codecov lets you know which lines of code are missing tests. Please add your new test cases in Auth.spec.js

Here are a few that are missing

• Testing oauth20, oauthTTL, oauthKey configuration works and defaults
• Users are created with accessToken, refreshToken, expires_in fields
• JWT users aren't returned with sessionTokens
• Can refresh JWT Tokens
• Can revoke JWT Tokens

[jjunineuro]

• Testing oauth20, oauthTTL, oauthKey configuration works and defaults

• In the './spec/helper.js' file I added the lines into const defaultConfiguration:
  oauth: true, oauthKey: 'test',
• What I understand is that the file 'Auth.spec.js' executes the commands of 'Auth.js'?

• Users are created with accessToken, refreshToken, expires_in fields
• JWT users aren't returned with sessionTokens

How to include the refresh token and revoke token checks if there is no parameter to enter the URL?

• Can refresh JWT Tokens
• Can revoke JWT Tokens

Good morning, I really need help.
However, when running 'npm test npm test. \ Spec \ Auth.spec.js', it displays only:

Codecov is not a tool that I am aware of and I do not want to let this functionality die because of this barrier. Please help me!

[cbaker6]

However, when running 'npm test npm test. \ Spec \ Auth.spec.js', it displays only:

Did you install mongo runner before running the test and make sure it's version 4.4? npm install -g mongodb-runner;

[dplewis]

I could write the tests myself but I figure its more important to get your testing environment setup. We may need to update our documentation to help other developers.

As a workaround you could install mongodb directly https://docs.mongodb.com/manual/administration/install-community/

$ mongod
$ npm run testonly ./spec/Auth.spec.js

[cbaker6]

@jjunineuro great work!

I have a few questions below:

When refreshing:

curl -X POST
-H "X-Parse-Application-Id: appid"
-H "X-Parse-REST-API-Key: restapi"
-H "Content-Type: application/json"
-d '{"client":"OBJECT_ID_FROM_USER", "code":"REFRESH_TOKEN"}'
https://YOUR.PARSE-SERVER.HERE/parse/users/refresh

• Is the OBJECT_ID_FROM_USER really needed in the body from the client side? Most of the SDKs send the sessionToken in the header when it's available. Can't you just use the header sessionToken in combination with the REFRESH_TOKEN in the body? Or is this OBJECT_ID_FROM_USER different from a Parse objectId? Maybe I don't understand what this is used for.
• If you really do need the objectId, should this endpoint be? https://YOUR.PARSE-SERVER.HERE/parse/users/OBJECT_ID_FROM_USER/refresh or https://YOUR.PARSE-SERVER.HERE/parse/users/me/refresh.
• If you really need the objectId in the body, can it be objectId instead of client?
• Similarly, it seems code can be REFRESH_TOKEN as I didn't see it be anything else in your examples. Are properties like client and code part of the RFC or can be named anyway you want?

When revoking:

curl -X POST
-H "X-Parse-Application-Id: appid"
-H "X-Parse-REST-API-Key: restapi"
-H "Content-Type: application/json"
-d '{"code":"REFRESH_TOKEN"}'
https://YOUR.PARSE-SERVER.HERE/parse/users/revoke

• It seems revoke should be at the same level as logout login. Should the endpoint be https://YOUR.PARSE-SERVER.HERE/parse/revoke?
• code should be refreshToken

It seems the difference between sessionToken and accessToken can be agnostic to the client side if I'm reading everything correctly as the client treats the session token the same regardless. It seems the way the client knows if oauth is being used if it sees a refreshToken and expires_in.

Last question, can expires_in be expiresIn or is the former way required? It seems like expiresAt would fit better with Parse (I say this because of createdAt and updatedAt). I ask this because none of the other keys for this are written with underscores.

[jjunineuro]

Hello @cbaker6, sorry for the delay in responding, but I was busy with work without time for open source.

I made the changes you suggested:

• Change revocation path to: "parse/revoke";
• Change the parameters from "code" to "refreshToken" on renewal and revocation;
• Removed "clientId" parameter from the token renewal method;
• Change the return to "expiresIn".

I took the parameters based on the guidelines of RFC 6749.

• Add test cases
• Add entry to changelog
• Add changes to documentation (guides, repository pages, in-code descriptions)
• Should I include the OAuth 2.0 model specifications in "README.md"?
• Add security check
• If it is the tests performed when sending the commit, this is ok.
• Add new Parse Error codes to Parse JS SDK
• When changing the description of a new error, should a new error code be created?

I am also preparing the SDK template to respond to the use of OAuth. It will be something like:

ParseSwift.initialize(applicationId: "xxxxxxxxxx", clientKey: "xxxxxxxxxx", serverURL: URL(string: "https://example.com")!, liveQueryServerURL: URL(string: "https://example.com")!, oauth20: true, authentication: ((URLAuthenticationChallenge,
(URLSession.AuthChallengeDisposition,
URLCredential?) -> Void) -> Void))

When the SDK starts Parse Server and has "oauth20 = true" as a parameter:

• The "signup" and "login" methods must receive as return: "accessToken", "refreshToken" and "expiresIn";
• Highly recommended to store information on Keychain (iOS) or KeyStore (Android);
• The SDK should check the "expiresIn" with the current timestamp to find out if the "accessToken" is still valid;
• If it is expired, you must request a new "accessToken" requesting the renewal of the key:

-H "X-Parse-Application-Id: appid"
-H "X-Parse-REST-API-Key: restapi"
-H "Content-Type: application/json"
-d '{"refreshToken":"REFRESH_TOKEN"}'
https://YOUR.PARSE-SERVER.HERE/parse/users/refresh
PS: When refresh the "accessToken" it must be stored together with the new "refreshToken", since the previous one will be discarded.

• All requests must send the "accessToken" in the "X-Parse-Session-Token" header instead of sending the "sessionToken";
• Instead of using the logout method, the revocation method should be used:
  curl -X POST

-H "X-Parse-Application-Id: appid"
-H "X-Parse-REST-API-Key: restapi"
-H "Content-Type: application/json"
-d '{"refreshToken":"REFRESH_TOKEN"}'
https://YOUR.PARSE-SERVER.HERE/parse/revoke

I hope that this project can proceed successfully, as it will be a big increase for both Mobile and Web.

I am experiencing great difficulty with the case tests. @dplewis listed what I should change in "spec / Auth.spec.js", but I don't know the "codecov" I spent all morning, I searched for tutorials, Google searches but without success.

If this is the barrier to being able to contribute to the community, unfortunately I will be very sad, because I do not know the "codedov" and I do not know how to create in the new case tests.

[dplewis]

@jjunineuro Ignore code coverage for right now since it doesn't effect the actual test themselves. Can you run the test suite? npm run test

Can you give me access to your fork?

[mtrezza]

@davimacedo so you have the same concerns here that you mentioned in #7226 (comment)?

[cbaker6]

Change the return to "expiresIn".

What do you all think about expiresAt instead of expiresIn? I say this because Parse already uses createdAt and updatedAt and it looks like, expiresAt will also show in the dashboard. expiresAt seems like it will be more consistent. Unless expiresIn means a number of seconds/hours then I can see how expiresIn makes more sense

[jjunineuro]

@jjunineuro Ignore code coverage for right now since it doesn't effect the actual test themselves. Can you run the test suite? npm run test

Can you give me access to your fork?

I sent the invite to access to fork.

[jjunineuro]

Change the return to "expiresIn".

What do you all think about expiresAt instead of expiresIn? I say this because Parse already uses createdAt and updatedAt and it looks like, expiresAt will also show in the dashboard. expiresAt seems like it will be more consistent. Unless expiresIn means a number of seconds/hours then I can see how expiresIn makes more sense

I can change yes ..

[cbaker6]

Change the return to "expiresIn".

What do you all think about expiresAt instead of expiresIn? I say this because Parse already uses createdAt and updatedAt and it looks like, expiresAt will also show in the dashboard. expiresAt seems like it will be more consistent. Unless expiresIn means a number of seconds/hours then I can see how expiresIn makes more sense

I can change yes ..

I just read your RFC link. It looks like it's:

Unless expiresIn means a number of seconds/hours then I can see how expiresIn makes more sense

Which makes sense why they called in expires_in, but, if you are adding this delta time (expires_in) onto the time of the server clock, then it's expiresAt. I'm assuming this is what you are doing?

If you do it the way I mentioned above, current_server_time + expires_in, then the client doesn't have to save as much info locally (like the time the server created the new tokens). They can simply compare their local relative time to the expiresAt time.

[jjunineuro]

Which makes sense why they called in expires_in, but, if you are adding this delta time (expires_in) onto the time of the server clock, then it's expiresAt. I'm assuming this is what you are doing?

Yes @cbaker6, I am using the server clock,

[cbaker6]

I am also preparing the SDK template to respond to the use of OAuth. It will be something like:
ParseSwift.initialize(applicationId: "xxxxxxxxxx", clientKey: "xxxxxxxxxx", serverURL: URL(string: "https://example.com")!, liveQueryServerURL: URL(string: "https://example.com")!, oauth20: true, authentication: ((URLAuthenticationChallenge,
(URLSession.AuthChallengeDisposition,
URLCredential?) -> Void) -> Void))

In this scenario is your intention for the developer to only use oauth? If so, this may have some other implications and "might" cause other log in methods on the client side to not work, depending on the implementation. My understanding from earlier is that oauth is enabled at the server level and the client should be able to deal with either. Is that correct?

[jjunineuro]

That's right, "oauth" is defined on the server side. However, when it is true, it will no longer send the "sessionToken" as a response to Login / Signup.

[cbaker6]

Why not send both?

This could work, but I was under the impression that either the sessionToken or the accessToken was being produced. I could have misinterpreted this. If both are produced, but 1 is for oauth, that could work. Does this mean the client will still send the accessToken using the sessionToken header key when communicating with the server?

[dplewis]

Let me add some test cases and clean up so we can see whats going on.

[dplewis]

Does this mean the client will still send the accessToken using the sessionToken header key when communicating with the server?

We could create a new header. This would allow out of the box configurations since we know that an accessToken would be jwt.

@jjunineuro I added some test cases. Can you try to run them locally?

[cbaker6]

A small change you might want to consider is the server is returning expiresAt as an Int type, which is what the RFC states, but since you are adding a delta to a date, the clients will expect the type to be a Date (a dictionary with an iso key) for decoding purposes. You can look at how createdAt (which is passed in the same response) is made for this since Parse is particular about Date types.

[cbaker6]

Switched expiresAt to send as a date and modified related test cases. Feel free to revert if necessary

[cbaker6]

Update on testing on a Swift client:

• Sign up seems to work
• Tokens look and expiration data (+30 mins in time) look fine:

Successfully signed up user _User ({"createdAt":{"__type":"Date","iso":"2021-03-16T21:11:17.056Z"},"username":"hello","objectId":"W5FjmCjZt4utDiUUGkxgt6KdG8YYld8m"})
The users' sessionToken is: nil
The users' accessToken is: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJyOmVmZTU3MGRjNmUyYzQ2NDI0OGRjZDI5YzBlMjBkYWYwIiwiaWF0IjoxNjE1OTI5MDc3LCJleHAiOjE2MTU5MzA4Nzd9.SSHmwcRHkf_KGV4ATSJfNAc13_zOl9bzZnEq4heZ5c4
The users' refreshToken is: 7de19f4ad79905ad03f88b874c824330dea2123d699975e9762749953c18f647
The users' token expires at: 2021-03-17 01:41:17 +0000

Problems I ran into:

• The expiresAt == createdAt, doesn't seem save to the DB correctly. The date saved isn't the same sent to the client
• Not able to refresh, probably because the above time is expired

Things to think about:

• The endpoints, /users/refresh and /users/revoke are not at the level of /login, /logout, should they be? I think you made some changes to move this, but the client and test cases can currently only pass by using /users/refresh and /users/revoke

[jjunineuro]

Hi guys, I'm sorry to be absent this week. I had some problems which made time for the open community impossible.

@dplewis, thank you very much for committing the case tests. I will test them all now.

@cbaker6, I just made a correction now to change the fixed value 1800 to the variable "oauthTTL" and also put the same return response for the current user method.

The methods of "refresh" and "revoke" were defined as follows:

• /users/refresh
• /users/revoke
  Just like the current user method is in "/ users"

[jjunineuro]

If there are still any more tasks, please inform.
I'm going to start adding the method to the flutter SDK to support the OAuth feature. I believe that the documentation is still missing.

I ask, should this be done in the README.md file?

[jjunineuro]

• O expiresAt== createdAt, não parece ser salvo no banco de dados corretamente. A data guardada não é a mesma enviada ao cliente

@cbaker6, the expiresAt is not saved in the database nor is the accessToken. So createAt may not be the same as createdAt. That is why in the Parse Server options the parameter "expireInactiveSessions" must be set to false.

The validity period of the token is defined in your body, in the payload parameter "exp" and this information does not need to be saved in a bank.

For example, you have logged in now 2021-03-21 19:27:13 and in an hour, you decide to request a list from the bank. In this case, the device will verify by the "expiresAt" that the token has expired, so before requesting the list, it must request the refresh of the accessToken, and only then, continue requesting the list.

[cbaker6]

For example, you have logged in now 2021-03-21 19:27:13 and in an hour, you decide to request a list from the bank. In this case, the device will verify by the "expiresAt" that the token has expired, so before requesting the list, it must request the refresh of the accessToken, and only then, continue requesting the list.

What happens if the device decides to ignore expiresAt and requests lists after the hour when the token has already expired?

[jjunineuro]

What happens if the device decides to ignore expiresAt and requests lists after the hour when the token has already expired?

will return the following exception: throw new Parse.Error(Parse.Error.INVALID_SESSION_TOKEN, 'Invalid session token');

[lsmilek1]

May I ask what is the status on this PR and if this is planned to be released with next server version release? Thank you!

[bdevore17]

@jjunineuro what is the status here?

[lsmilek1]

As this was not implemented in the 4.10.0 i would like to rise the question whether it is planned to include in 5.0. or not? I see there is one requested change and I assume these are missing tests cases, but then also "All conversations are resolved"?

[mtrezza]

@lsmilek1 @bdevore17 Thanks for your questions. Release 4.10.0 was a security release, it was not supposed to include any features. I assume and hope that this feature will be merged on time to be part of Parse Server 5.0. We will likely do a feature freeze for version 5.0 later this year, so there are still some months left to merge this PR.

The reason I removed the requested review is that there are conflicting files that need to be resolved to review this further. Once the conflict is resolved, the review process will continue.

[mtrezza]

⚠️ Important change for merging PRs from Parse Server 5.0 onwards!

We are planning to release the first beta version of Parse Server 5.0 in October 2021.

If a PR contains a breaking change and is not merged before the beta release of Parse Server 5.0, it cannot be merged until the end of 2022. Instead it has to follow the Deprecation Policy and phase-in breaking changes to be merged during the course of 2022.

One of the most voiced community feedbacks was the demand for predictability in breaking changes to make it easy to upgrade Parse Server. We have made a first step towards this by introducing the Deprecation Policy in February 2021 that assists to phase-in breaking changes, giving developers time to adapt. We will follow-up with the introduction of Release Automation and a branch model that will allow breaking changes only with a new major release, scheduled for the beginning of each calendar year.

We understand that some PRs are a long time in the making and we very much appreciate your contribution. We want to make it easy for PRs that contain a breaking change and were created before the introduction of the Deprecation Policy. These PRs can be merged with a breaking change without being phased-in before the beta release of Parse Server 5.0. We are making this exception because we appreciate that this is a time of transition that requires additional effort from contributors to adapt. We encourage everyone to prepare their PRs until the end of September and account for review time and possible adaptions.

If a PR contains a breaking change and should be merged before the beta release, please mention @parse-community/server-maintenance and we will coordinate with you to merge the PR.

Thanks for your contribution and support during this transition to Parse Server release automation!

[bdevore17]

@mtrezza Did this miss the deadline?

[mtrezza]

@bdevore17 We have moved the deadline to November, so if you or anyone else wants to pick this up, please do. I should say that it's always possible to add this feature anytime after the deadline, but if it contains a breaking change, we'd have to add a deprecation and let the developer turn it on optionally. That shouldn't be a big deal, we are doing this already with some other features.

[bdevore17]

@mtrezza Is this just waiting on a rebase?

[mtrezza]

There is another large auth related PR #7079 that we are currently focusing on. @Moumouls how would you suggest we proceed with this PR in context of #7079?