Skip to content

Commit

Permalink
fix: authentication adapter app ID validation may be circumvented; th…
Browse files Browse the repository at this point in the history
…is fixes a vulnerability that affects configurations which allow users to authenticate using the Parse Server authentication adapter for *Facebook* or *Spotify* and where the server-side authentication adapter configuration `appIds` is set as a string (e.g. `abc`) instead of an array of strings (e.g. `["abc"]`) ([GHSA-r657-33vp-gp22](GHSA-r657-33vp-gp22)) [skip release] (#8187)
  • Loading branch information
mtrezza authored Sep 20, 2022
1 parent 37fed30 commit 8c8ec71
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 16 deletions.
23 changes: 23 additions & 0 deletions spec/AuthenticationAdapters.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,29 @@ describe('AuthenticationProviders', function () {
expect(providerOptions).toEqual(options.facebook);
});

it('should throw error when Facebook request appId is wrong data type', async () => {
const httpsRequest = require('../lib/Adapters/Auth/httpsRequest');
spyOn(httpsRequest, 'get').and.callFake(() => {
return Promise.resolve({ id: 'a' });
});
const options = {
facebook: {
appIds: 'abcd',
appSecret: 'secret_sauce',
},
};
const authData = {
access_token: 'badtoken',
};
const { adapter, appIds, providerOptions } = authenticationLoader.loadAuthAdapter(
'facebook',
options
);
await expectAsync(adapter.validateAppId(appIds, authData, providerOptions)).toBeRejectedWith(
new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'appIds must be an array.')
);
});

it('should handle Facebook appSecret for validating appIds', async () => {
const httpsRequest = require('../lib/Adapters/Auth/httpsRequest');
spyOn(httpsRequest, 'get').and.callFake(() => {
Expand Down
19 changes: 10 additions & 9 deletions src/Adapters/Auth/facebook.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,23 @@ function validateGraphToken(authData, options) {
});
}

function validateGraphAppId(appIds, authData, options) {
async function validateGraphAppId(appIds, authData, options) {
var access_token = authData.access_token;
if (process.env.TESTING && access_token === 'test') {
return Promise.resolve();
return;
}
if (!Array.isArray(appIds)) {
throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'appIds must be an array.');
}
if (!appIds.length) {
throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'Facebook auth is not configured.');
}
return graphRequest(
'app?access_token=' + access_token + getAppSecretPath(authData, options)
).then(data => {
if (data && appIds.indexOf(data.id) != -1) {
return;
}
const data = await graphRequest(
`app?access_token=${access_token}${getAppSecretPath(authData, options)}`
);
if (!data || !appIds.includes(data.id)) {
throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'Facebook auth is invalid for this user.');
});
}
}

const getFacebookKeyByKeyId = async (keyId, cacheMaxEntries, cacheMaxAge) => {
Expand Down
15 changes: 8 additions & 7 deletions src/Adapters/Auth/spotify.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,18 @@ function validateAuthData(authData) {
}

// Returns a promise that fulfills if this app id is valid.
function validateAppId(appIds, authData) {
var access_token = authData.access_token;
async function validateAppId(appIds, authData) {
const access_token = authData.access_token;
if (!Array.isArray(appIds)) {
throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'appIds must be an array.');
}
if (!appIds.length) {
throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'Spotify auth is not configured.');
}
return request('me', access_token).then(data => {
if (data && appIds.indexOf(data.id) != -1) {
return;
}
const data = await request('me', access_token);
if (!data || !appIds.includes(data.id)) {
throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'Spotify auth is invalid for this user.');
});
}
}

// A promisey wrapper for Spotify API requests.
Expand Down

0 comments on commit 8c8ec71

Please sign in to comment.