From 45a9d501108475ea9814f6f30a827dbba614e45d Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Sun, 7 May 2017 12:55:30 -0400 Subject: [PATCH] Skip authData validation if it hasn't changed. (#3783) * Adds test for the new feature * Re-validate authData only if mutated - In case of short-lived tokens (like facebook) this will allow clients to be lax with asking users to re-login --- spec/ParseUser.spec.js | 34 ++++++++++++++++++++++++++++++++++ spec/helper.js | 22 +++++++++++++++++++++- src/RestWrite.js | 18 +++++++++--------- 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 02d90a71..d239802d 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -1696,6 +1696,40 @@ describe('Parse.User testing', () => { }); }); + it('should allow login with old authData token', (done) => { + const provider = { + authData: { + id: '12345', + access_token: 'token' + }, + restoreAuthentication: function() { + return true; + }, + deauthenticate: function() { + provider.authData = {}; + }, + authenticate: function(options) { + options.success(this, provider.authData); + }, + getAuthType: function() { + return "shortLivedAuth"; + } + } + defaultConfiguration.auth.shortLivedAuth.setValidAccessToken('token'); + Parse.User._registerAuthenticationProvider(provider); + Parse.User._logInWith("shortLivedAuth", {}).then(() => { + // Simulate a remotely expired token (like a short lived one) + // In this case, we want success as it was valid once. + // If the client needs an updated one, do lock the user out + defaultConfiguration.auth.shortLivedAuth.setValidAccessToken('otherToken'); + return Parse.User._logInWith("shortLivedAuth", {}); + }).then(() => { + done(); + }, (err) => { + done.fail(err); + }); + }); + it('should properly error when password is missing', (done) => { var provider = getMockFacebookProvider(); Parse.User._registerAuthenticationProvider(provider); diff --git a/spec/helper.js b/spec/helper.js index 7e52f175..f94e6cb5 100644 --- a/spec/helper.js +++ b/spec/helper.js @@ -106,7 +106,8 @@ var defaultConfiguration = { facebook: mockFacebook(), myoauth: { module: path.resolve(__dirname, "myoauth") // relative path as it's run from src - } + }, + shortLivedAuth: mockShortLivedAuth() } }; @@ -369,6 +370,25 @@ function mockFacebook() { return mockFacebookAuthenticator('8675309', 'jenny'); } +function mockShortLivedAuth() { + const auth = {}; + let accessToken; + auth.setValidAccessToken = function(validAccessToken) { + accessToken = validAccessToken; + } + auth.validateAuthData = function(authData) { + if (authData.access_token == accessToken) { + return Promise.resolve(); + } else { + return Promise.reject('Invalid access token'); + } + }; + auth.validateAppId = function() { + return Promise.resolve(); + }; + return auth; +} + // This is polluting, but, it makes it way easier to directly port old tests. global.Parse = Parse; diff --git a/src/RestWrite.js b/src/RestWrite.js index 0cdc88bc..5ad94544 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -277,9 +277,7 @@ RestWrite.prototype.findUsersWithAuthData = function(authData) { RestWrite.prototype.handleAuthData = function(authData) { let results; - return this.handleAuthDataValidation(authData).then(() => { - return this.findUsersWithAuthData(authData); - }).then((r) => { + return this.findUsersWithAuthData(authData).then((r) => { results = r; if (results.length > 1) { // More than 1 user with the passed id's @@ -307,16 +305,20 @@ RestWrite.prototype.handleAuthData = function(authData) { mutatedAuthData[provider] = providerData; } }); - this.response = { response: userResult, location: this.location() }; + // If we didn't change the auth data, just keep going + if (Object.keys(mutatedAuthData).length === 0) { + return; + } // We have authData that is updated on login // that can happen when token are refreshed, // We should update the token and let the user in - if (Object.keys(mutatedAuthData).length > 0) { + // We should only check the mutated keys + return this.handleAuthDataValidation(mutatedAuthData).then(() => { // Assign the new authData in the response Object.keys(mutatedAuthData).forEach((provider) => { this.response.response.authData[provider] = mutatedAuthData[provider]; @@ -324,9 +326,7 @@ RestWrite.prototype.handleAuthData = function(authData) { // Run the DB update directly, as 'master' // Just update the authData part return this.config.database.update(this.className, {objectId: this.data.objectId}, {authData: mutatedAuthData}, {}); - } - return; - + }); } else if (this.query && this.query.objectId) { // Trying to update auth data but users // are different @@ -336,7 +336,7 @@ RestWrite.prototype.handleAuthData = function(authData) { } } } - return; + return this.handleAuthDataValidation(authData); }); }