Implemented auth0 adapater. #7502
Conversation
There was a problem hiding this comment.
Thanks for opening this PR!
In general, I wonder why this (or any other) auth adapter is part of the Parse Server repository and not a repository of their own, like additional database adapters. That would be more in line with the modular structure of Parse Server.
| try { | ||
| await auth0.validateAuthData( | ||
| { id: 'the_user_id', id_token: 'the_token' }, | ||
| { tenantId: 'example.eu.auth0.com', clientId: 'secret' } |
There was a problem hiding this comment.
Is this a URI that is made available for development use per policy?
There was a problem hiding this comment.
Not, as far as I know, I just choose a value for the tests. Since the tenant is not really used in the tests, it does not really matter. Another option would be to create an auth0 account and register a tenant, but then someone from the Parse team would have to manage this account.
There was a problem hiding this comment.
Is there any benefit regarding test efficacy if we create an auth0 account? If not, I suggest we comply with RFC 2606 and replace the URLs in the tests with example.com.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## alpha #7502 +/- ##
==========================================
- Coverage 94.37% 93.95% -0.43%
==========================================
Files 185 182 -3
Lines 14761 13314 -1447
==========================================
- Hits 13931 12509 -1422
+ Misses 830 805 -25
☔ View full report in Codecov by Sentry. |
|
|
||
| it('Should throw error with missing id_token', async () => { | ||
| try { | ||
| await auth0.validateAuthData({}, { tenantId: 'example.eu.auth0.com', clientId: 'secret' }); |
There was a problem hiding this comment.
Prefer
await expectAsync(auth0.validateAuthData({}, { tenantId: 'example.eu.auth0.com', clientId: 'secret' })).toBeRejectedWith(new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'id token is invalid for this user.'));
| const fakeDecodedToken = { header: { kid: '123', alg: 'RS256' } }; | ||
| spyOn(jwt, 'decode').and.callFake(() => fakeDecodedToken); | ||
| spyOn(jwt, 'verify').and.callFake(() => fakeClaim); | ||
| const fakeGetSigningKeyAsyncFunction = () => { |
There was a problem hiding this comment.
Could probably simplify this to:
const fakeGetSigningKeyAsyncFunction = () => ({ kid: '123', rsaPublicKey: 'the_rsa_public_key' })| // tenantId: the auth0 tenant, e.g. example.eu.auth0.com (NO HTTP / HTTPS) | ||
| // clientId: the auth0 client id, can be found in the auth0 console. | ||
|
|
||
| var Parse = require('parse/node').Parse; |
| const jwt = require('jsonwebtoken'); | ||
| const util = require('util'); | ||
|
|
||
| const getAuth0KeyByKeyId = async (keyId, cacheMaxEntries, cacheMaxAge, tenantId) => { |
There was a problem hiding this comment.
Some functions are arrow syntax others are regular function syntax - I would recommend consistency
|
|
||
| let key; | ||
| try { | ||
| key = await asyncGetSigningKeyFunction(keyId); |
There was a problem hiding this comment.
Prefer return await asyncGetSigningKeyFunction(keyId);, no need to define key
| const { kid: keyId, alg: algorithm } = getHeaderFromToken(token); | ||
| let jwtClaims; | ||
| const ONE_HOUR_IN_MS = 3600000; | ||
| cacheMaxAge = cacheMaxAge || ONE_HOUR_IN_MS; |
There was a problem hiding this comment.
Can you use default parameters on the object destructure?
| } | ||
|
|
||
| module.exports = { | ||
| validateAppId: validateAppId, |
There was a problem hiding this comment.
can use destructuring assignment here, i.e:
module.exports = {
validateAppId,
validateAuthData
}
|
@lukasreichart would you want to make the suggested changes, so we can merge this? |
|
@mtrezza Is there any updates on this PR? |
New Pull Request Checklist
Issue Description
Implements #7478
Approach
I followed a similar approach to the apple authentication adapter.
Documentation PR: parse-community/docs#843
TODOs before merging