Implement OAuth method

[jjunineuro]

New Pull Request Checklist

• [x] I am not disclosing a vulnerability.
• [x] 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 https://github.com/parse-community/parse-server/issues/7248

Approach

TODOs before merging

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

[codecov[bot]]

Codecov Report

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

:exclamation: 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%)	:arrow_up:
src/KeyPromiseQueue.js	95.45% <ø> (ø)
src/Options/index.js	100.00% <ø> (ø)
src/PromiseRouter.js	90.58% <ø> (+0.47%)	:arrow_up:
src/Routers/UsersRouter.js	88.37% <74.13%> (-5.38%)	:arrow_down:
src/Controllers/DatabaseController.js	93.70% <79.41%> (-1.77%)	:arrow_down:
src/Auth.js	97.87% <89.74%> (-2.13%)	:arrow_down:
...dapters/Storage/Postgres/PostgresStorageAdapter.js	95.52% <90.00%> (+0.02%)	:arrow_up:
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]

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

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

[jjunineuro]

• [x] 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.

• [x] Add test cases
• [x] 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"?
• [x] 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 https://github.com/parse-community/parse-server/pull/7226#issuecomment-797814131?

[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.

[jjunineuro]

I closed it, wrong, sorry.

[cbaker6]

I added a client side implementation that you can use to test (it looks like you have a local server with your changes on it) here: https://github.com/parse-community/Parse-Swift/pull/91

Of course, this is based on my interpretation of this PR, so there can easily be things I misinterpreted. You can see my interpretations by looking at the client tests I added to replicate the server here... You can easily use this to test in Swift Playgrounds (assuming you have a Mac) by changing the Parse initialization settings here: https://github.com/parse-community/Parse-Swift/blob/main/ParseSwift.playground/Sources/Common.swift

The Swift playground files you care about to test OAuth are: https://github.com/parse-community/Parse-Swift/tree/main/ParseSwift.playground/Pages/3%20-%20User%20-%20Sign%20Up.xcplaygroundpage

and https://github.com/parse-community/Parse-Swift/tree/main/ParseSwift.playground/Pages/4%20-%20User%20-%20Continued.xcplaygroundpage

Of course, this doesn't circumvent the test cases @dplewis mention need to be added to the test suite, but can help you determine if your server code as working as intended with a real ParseClient

[jjunineuro]

@cbaker6 Thank you very much for your help, unfortunately I don't deal with the Swift language. I will prepare a public url to make it available for you to test this functionality. I develop with Flutter, a method in which I will also prepare to receive the data. Then I made a print of the calls via CURL with the view of the Parse Dashboard, all functional.

[cbaker6]

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.

Yes, but I thought the server sends the authToken in it's place? Then, when further communication is needed, the client sends the authToken using the sessionToken header, X-Parse-Session-Token. If this is true, then the use of authToken and sessionToken can be agnostic to the client as long as they follow the design pattern I used in ParseSwift PR. Basically, the client doesn't care if the server sends an older style sessionToken or an authToken, it treats them both the same (they are of same type, string). The client knows the difference when the refreshToken and expiresAt key/values are sent, this means oauth.

[cbaker6]

@cbaker6 Thank you very much for your help, unfortunately I don't deal with the Swift language. I will prepare a public url to make it available for you to test this functionality. I develop with Flutter, a method in which I will also prepare to receive the data. Then I made a print of the calls via CURL with the view of the Parse Dashboard, all functional.

Not a problem, I can use your public link. CURL is your best friend here as most of the SDKs are using REST anyways, so if you got it right with REST, technically it should work on the client SDKs.

[jjunineuro]

Yes, but I thought the server sends the authToken in it's place? Then, when further communication is needed, the client sends the authToken using the sessionToken header, X-Parse-Session-Token. If this is true, then the use of authToken and sessionToken can be agnostic to the client as long as they follow the design pattern I used in ParseSwift PR. Basically, the client doesn't care if the server sends an older style sessionToken or an authToken, it treats them both the same (they are of same type, string). The client knows the difference when the refreshToken and expiresAt key/values are sent, this means oauth.

The "oauth" field was removed at the suggestion of @mtrezza https://github.com/parse-community/parse-server/issues/7248#issuecomment-795719236

[mtrezza]

The client knows the difference when the refreshToken and expiresAt key/values are sent, this means oauth

Not sure about this. We'll likely run into trouble once there is another version of oauth coming along or someone wants to implement something else. If the SDK determines the auth method by the presence of fields that can be ambiguous. Hence I suggested an explicit indication of the auth method to the client in https://github.com/parse-community/parse-server/issues/7248#issuecomment-795738886.

[cbaker6]

Not sure about this. We'll likely run into trouble once there is another version of oauth coming along or someone wants to implement something else. If the SDK determines the auth method by the presence of fields that can be ambiguous.

A boolean oauth doesn't solve this either. If this is a concern, it should probably be a unique string so the client can determine the type and use it in an enum

[jjunineuro]

For this reason, to suggest previously for the SDK´s, when starting the Parse Server, add the field "oauth20 = true", thus, you will understand that you must treat the return fields: "accessToken", "refreshToken" and "expiresAt" . Just as I did for Parse Server, I will prepare a schema for the SDK's.

[mtrezza]

@cbaker6 That's what I suggested, see my comment. A boolean would solve the issue, and be still better than using the presence of fields to determine the auth method, but a string would be more versatile.

Otherwise we'll end up with some fields along the way, "oauth20: true", "oauth30: true", "somethingelse: true" in the future. Using a string "auth: oauth20"is just better code quality and more descriptive I think.

[cbaker6]

That's what I suggested, see my comment.

It looks like you added the link to your comment through an edit after I replied. It didn't show in your original response...

This unique string and switch will future proof, but only becomes relevant when 1) Other auth methods use the same field names but require a different type for those fields or 2) The client needs to do something different for a specific authType with the same field

For example, authToken and sessionToken can be used in this PR interchangeably with no issue.

In any case whether current or future proof, the boolean adds little to no value. If you want to add something, it should be a unique string.

[cbaker6]

@jjunineuro I'm testing out your oauth with ParseSwift. Here's what I see so far. Note that what I mention are not suggested changes, but things you will want to get the Parse server team to comment on.

When signing in with ouauth20 == true, the server sends back:

0 : 2 elements
      - key : "createdAt"
      - value : "2021-03-15T14:24:02.761Z"
    ▿ 1 : 2 elements
      - key : "objectId"
      - value : "VpW7mIIAFUeLsOvukODghepAWrdCUOL5"
    ▿ 2 : 2 elements
      - key : "expiresAt"
      - value : 1615820042
    ▿ 3 : 2 elements
      - key : "accessToken"
      - value : "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJyOjNmMDNiNzQ3OGJiYTQxNzlkZjRlYTZmYzZhZjRiY2JjIiwiaWF0IjoxNjE1ODE4MjQyLCJleHAiOjE2MTU4MjAwNDJ9.P3epA1eUSOhWsmzOZQLht-Mb5EB7xyaiu03_b-3IvJA"
    ▿ 4 : 2 elements
      - key : "refreshToken"
      - value : "82f762490bb0091c75b128ac19dc4b7c51bcb98b8eca6263b844ce365a67e458"

Note that there's no sessionToken. In ParseSwift, this will cause a decoding error because sessionToken is missing and is a required key. The other client SDK's "might" require this key also. If so, there are two possible fixes here:

1. send the accessToken as the sessionToken. This doesn't require a client SDK modification. This is what I assumed was happening, because if you are going to send the accessToken from the client using the sessionToken header key, why not receive this way also?
2. all client SDKs modify and check for either a sessionToken or accessToken

[dplewis]

Just FYI there already is a oauth2 adapter already built in. Its for Token Introspection - RFC 7662 PR: https://github.com/parse-community/parse-server/pull/4910

This PR introduces JSON Web Token - RFC 7519. Since this isn't an adapter I believe oauth2jwt=true or similar should suffice.

send the accessToken as the sessionToken.

Why not send both?

[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?