Skip to content

Commit a065ba6

Browse files
committed
Refactor authData handling: streamline validation, enhance security, and improve efficiency
- Refactor authData processing to remove redundant logic and simplify code - Replace deep-diff with native isDeepStrictEqual for better maintainability - Enhance security: add strict provider name validation to prevent prototype pollution and NoSQL injection - Harden diffAuthData against injection attacks with null-prototype objects - Improve validation logic: remove unnecessary checks for unlink-only operations - Fix dead code branches and redundant conditions - Consolidate mockFetch calls in tests for better maintainability - Update @semantic-release/github dependency (11.0.2 -> 11.0.3) - Translate all Cyrillic comments to English - Add comprehensive test coverage for edge cases and security scenarios
1 parent cd745b1 commit a065ba6

File tree

7 files changed

+679
-375
lines changed

7 files changed

+679
-375
lines changed

package-lock.json

Lines changed: 258 additions & 221 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@
7878
"@semantic-release/changelog": "6.0.3",
7979
"@semantic-release/commit-analyzer": "13.0.1",
8080
"@semantic-release/git": "10.0.1",
81-
"@semantic-release/github": "11.0.2",
81+
"@semantic-release/github": "11.0.3",
8282
"@semantic-release/npm": "12.0.1",
8383
"@semantic-release/release-notes-generator": "14.0.3",
8484
"all-node-versions": "13.0.1",

spec/RestWrite.handleAuthData.spec.js

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,12 @@ describe('RestWrite.handleAuthData', () => {
3434
gpgames: {
3535
clientId: 'validClientId',
3636
clientSecret: 'validClientSecret',
37-
}
37+
},
38+
instagram: {
39+
clientId: 'validClientId',
40+
clientSecret: 'validClientSecret',
41+
redirectUri: 'https://example.com/callback',
42+
},
3843
},
3944
});
4045
};
@@ -51,17 +56,54 @@ describe('RestWrite.handleAuthData', () => {
5156
const sessionToken = user.getSessionToken();
5257

5358
await user.fetch({ sessionToken });
54-
const currentAuthData = user.get('authData') || {};
59+
const currentAuthData = user.get('authData');
60+
expect(currentAuthData).toBeDefined();
61+
62+
// Add another provider to ensure gpgames removal doesn't delete all authData
63+
mockFetch([
64+
{
65+
url: 'https://api.instagram.com/oauth/access_token',
66+
method: 'POST',
67+
response: {
68+
ok: true,
69+
json: () => Promise.resolve({ access_token: 'ig_token' }),
70+
},
71+
},
72+
{
73+
url: 'https://graph.instagram.com/me?fields=id&access_token=ig_token',
74+
method: 'GET',
75+
response: {
76+
ok: true,
77+
json: () => Promise.resolve({ id: 'I1' }),
78+
},
79+
},
80+
]);
5581

5682
user.set('authData', {
5783
...currentAuthData,
84+
instagram: { id: 'I1', code: 'IC1' },
85+
});
86+
await user.save(null, { sessionToken });
87+
88+
await user.fetch({ sessionToken });
89+
const authDataWithInstagram = user.get('authData');
90+
expect(authDataWithInstagram).toBeDefined();
91+
expect(authDataWithInstagram.gpgames).toBeDefined();
92+
expect(authDataWithInstagram.instagram).toBeDefined();
93+
94+
// Now unlink gpgames
95+
user.set('authData', {
96+
...authDataWithInstagram,
5897
gpgames: null,
5998
});
6099
await user.save(null, { sessionToken });
61100

62101
const updatedUser = await new Parse.Query(Parse.User).get(user.id, { useMasterKey: true });
63-
const finalAuthData = updatedUser.get('authData') || {};
102+
const finalAuthData = updatedUser.get('authData');
64103

104+
expect(finalAuthData).toBeDefined();
65105
expect(finalAuthData.gpgames).toBeUndefined();
106+
expect(finalAuthData.instagram).toBeDefined();
107+
expect(finalAuthData.instagram.id).toBe('I1');
66108
});
67109
});

spec/Users.authdata.security.spec.js

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1856,6 +1856,211 @@ describe('AuthData Security Tests', () => {
18561856
expect(error.code).toBeDefined();
18571857
}
18581858
});
1859+
1860+
it('should reject provider names with prototype pollution attempts', async () => {
1861+
mockFetch(mockGpgamesLogin());
1862+
const user = await Parse.User.logInWith('gpgames', {
1863+
authData: { id: MOCK_USER_ID, code: 'C1' },
1864+
});
1865+
const sessionToken = user.getSessionToken();
1866+
1867+
// Try to use constructor as provider name (enumerable by default)
1868+
try {
1869+
await user.save(
1870+
{ authData: { constructor: { id: 'test' } } },
1871+
{ sessionToken }
1872+
);
1873+
// If it doesn't throw, verify that constructor was not stored as malicious data
1874+
await user.fetch({ sessionToken });
1875+
const authData = user.get('authData');
1876+
// Constructor should not be in authData with our test data
1877+
// (either rejected or not configured, or stored but not as our test object)
1878+
if (authData && authData.constructor) {
1879+
const isOurTestData = typeof authData.constructor === 'object' &&
1880+
authData.constructor.id === 'test';
1881+
expect(isOurTestData).toBe(false);
1882+
}
1883+
} catch (error) {
1884+
// Should reject with INVALID_KEY_NAME (either from validateAuthData or diffAuthData)
1885+
// If provider is not configured, may throw UNSUPPORTED_SERVICE, which is also acceptable
1886+
expect([Parse.Error.INVALID_KEY_NAME, Parse.Error.UNSUPPORTED_SERVICE]).toContain(error.code);
1887+
if (error.code === Parse.Error.INVALID_KEY_NAME) {
1888+
expect(error.message).toContain('Invalid provider name');
1889+
}
1890+
}
1891+
1892+
// Try to use prototype as provider name
1893+
try {
1894+
await user.save(
1895+
{ authData: { prototype: { id: 'test' } } },
1896+
{ sessionToken }
1897+
);
1898+
// If it doesn't throw, verify that prototype was not stored as malicious data
1899+
await user.fetch({ sessionToken });
1900+
const authData = user.get('authData');
1901+
// Prototype should not be in authData with our test data
1902+
// (either rejected or not configured, or stored but not as our test object)
1903+
if (authData && authData.prototype) {
1904+
const isOurTestData = typeof authData.prototype === 'object' &&
1905+
authData.prototype.id === 'test';
1906+
expect(isOurTestData).toBe(false);
1907+
}
1908+
} catch (error) {
1909+
// Should reject with INVALID_KEY_NAME (either from validateAuthData or diffAuthData)
1910+
// If provider is not configured, may throw UNSUPPORTED_SERVICE, which is also acceptable
1911+
expect([Parse.Error.INVALID_KEY_NAME, Parse.Error.UNSUPPORTED_SERVICE]).toContain(error.code);
1912+
if (error.code === Parse.Error.INVALID_KEY_NAME) {
1913+
expect(error.message).toContain('Invalid provider name');
1914+
}
1915+
}
1916+
1917+
// Note: __proto__ is not enumerable by default in Object.keys(),
1918+
// so it won't be processed by diffAuthData. However, if it were
1919+
// enumerable (via Object.defineProperty with enumerable: true),
1920+
// it would be rejected by our validation. This is acceptable because
1921+
// normal object literals don't have enumerable __proto__.
1922+
});
1923+
1924+
it('should reject provider names with NoSQL injection attempts', async () => {
1925+
mockFetch(mockGpgamesLogin());
1926+
const user = await Parse.User.logInWith('gpgames', {
1927+
authData: { id: MOCK_USER_ID, code: 'C1' },
1928+
});
1929+
const sessionToken = user.getSessionToken();
1930+
1931+
// Try to use dots in provider name (NoSQL injection)
1932+
const maliciousProviders = [
1933+
'gpgames.id',
1934+
'gpgames.access_token',
1935+
'provider.name.field',
1936+
'test.test',
1937+
];
1938+
1939+
for (const maliciousProvider of maliciousProviders) {
1940+
try {
1941+
await user.save(
1942+
{ authData: { [maliciousProvider]: { id: 'test' } } },
1943+
{ sessionToken }
1944+
);
1945+
fail(`Should have rejected provider name with dots: ${maliciousProvider}`);
1946+
} catch (error) {
1947+
expect(error.code).toBe(Parse.Error.INVALID_KEY_NAME);
1948+
expect(error.message).toContain('Invalid provider name');
1949+
}
1950+
}
1951+
});
1952+
1953+
it('should reject provider names with special characters', async () => {
1954+
mockFetch(mockGpgamesLogin());
1955+
const user = await Parse.User.logInWith('gpgames', {
1956+
authData: { id: MOCK_USER_ID, code: 'C1' },
1957+
});
1958+
const sessionToken = user.getSessionToken();
1959+
1960+
// Try various special characters
1961+
const maliciousProviders = [
1962+
'provider-name', // hyphen
1963+
'provider name', // space
1964+
'provider@name', // @
1965+
'provider#name', // #
1966+
'provider$name', // $
1967+
'provider[name]', // brackets
1968+
'provider{name}', // braces
1969+
'provider/name', // slash
1970+
'provider\\name', // backslash
1971+
'provider.name', // dot
1972+
'123provider', // starts with number
1973+
'', // empty string
1974+
];
1975+
1976+
for (const maliciousProvider of maliciousProviders) {
1977+
try {
1978+
await user.save(
1979+
{ authData: { [maliciousProvider]: { id: 'test' } } },
1980+
{ sessionToken }
1981+
);
1982+
fail(`Should have rejected invalid provider name: ${maliciousProvider}`);
1983+
} catch (error) {
1984+
expect(error.code).toBe(Parse.Error.INVALID_KEY_NAME);
1985+
expect(error.message).toContain('Invalid provider name');
1986+
}
1987+
}
1988+
});
1989+
1990+
it('should accept valid provider names', async () => {
1991+
mockFetch(mockGpgamesLogin());
1992+
const user = await Parse.User.logInWith('gpgames', {
1993+
authData: { id: MOCK_USER_ID, code: 'C1' },
1994+
});
1995+
const sessionToken = user.getSessionToken();
1996+
1997+
// Valid provider names should work
1998+
const validProviders = [
1999+
'gpgames',
2000+
'instagram',
2001+
'provider_name',
2002+
'providerName',
2003+
'ProviderName',
2004+
'provider123',
2005+
'p',
2006+
'provider_name_123',
2007+
];
2008+
2009+
for (const validProvider of validProviders) {
2010+
// Skip if provider is not configured (will fail with different error)
2011+
if (validProvider === 'gpgames' || validProvider === 'instagram') {
2012+
continue;
2013+
}
2014+
2015+
try {
2016+
await user.save(
2017+
{ authData: { [validProvider]: { id: 'test' } } },
2018+
{ sessionToken }
2019+
);
2020+
// Should not throw INVALID_KEY_NAME error
2021+
// May throw UNSUPPORTED_SERVICE if provider not configured, which is expected
2022+
} catch (error) {
2023+
// Should not be INVALID_KEY_NAME for valid provider names
2024+
expect(error.code).not.toBe(Parse.Error.INVALID_KEY_NAME);
2025+
}
2026+
}
2027+
});
2028+
2029+
it('should prevent injection when linking multiple providers', async () => {
2030+
mockFetch(mockGpgamesLogin());
2031+
const user = await Parse.User.logInWith('gpgames', {
2032+
authData: { id: MOCK_USER_ID, code: 'C1' },
2033+
});
2034+
const sessionToken = user.getSessionToken();
2035+
2036+
// Try to link malicious provider name
2037+
// Use constructor which should be enumerable
2038+
try {
2039+
await user.save(
2040+
{
2041+
authData: {
2042+
constructor: { id: 'test' },
2043+
},
2044+
},
2045+
{ sessionToken }
2046+
);
2047+
// If it doesn't throw, verify that constructor was not stored
2048+
await user.fetch({ sessionToken });
2049+
const authData = user.get('authData');
2050+
// Constructor should not be in authData (either rejected or not configured)
2051+
const hasConstructor = authData && authData.constructor &&
2052+
typeof authData.constructor === 'object' &&
2053+
authData.constructor.id === 'test';
2054+
expect(hasConstructor).toBe(false);
2055+
} catch (error) {
2056+
// Should reject with INVALID_KEY_NAME (either from validateAuthData or diffAuthData)
2057+
// If provider is not configured, may throw UNSUPPORTED_SERVICE, which is also acceptable
2058+
expect([Parse.Error.INVALID_KEY_NAME, Parse.Error.UNSUPPORTED_SERVICE]).toContain(error.code);
2059+
if (error.code === Parse.Error.INVALID_KEY_NAME) {
2060+
expect(error.message).toContain('Invalid provider name');
2061+
}
2062+
}
2063+
});
18592064
});
18602065
});
18612066

spec/Users.authdata.spec.js

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ describe('AuthData Delta Behavior', () => {
7878
expect(authData.gpgames).toBeUndefined();
7979
});
8080

81-
// Level 1.1: Дополнительные способы создания пользователя
81+
// Level 1.1: Additional user creation methods
8282
it('should create user with code only (without id)', async () => {
8383
// Note: gpgames adapter requires id for getUserFromAccessToken URL construction
8484
// This test documents that code-only login is not fully supported by gpgames adapter
@@ -191,7 +191,7 @@ describe('AuthData Delta Behavior', () => {
191191
expect(authData.instagram.id).toBe('I1');
192192
});
193193

194-
// Level 1.2: Тесты на удаление провайдеров
194+
// Level 1.2: Provider unlinking tests
195195
it('should unlink provider when it\'s the only one', async () => {
196196
const user = await Parse.User.logInWith('gpgames', {
197197
authData: { id: MOCK_USER_ID, code: 'C1' },
@@ -305,7 +305,7 @@ describe('AuthData Delta Behavior', () => {
305305
expect(authData.gpgames && authData.gpgames.id).toBe(MOCK_USER_ID);
306306
});
307307

308-
// Level 2.1: Множественные провайдеры (3+)
308+
// Level 2.1: Multiple providers (3+)
309309
it('should handle three providers: add, update, unlink', async () => {
310310
const user = await Parse.User.logInWith('gpgames', {
311311
authData: { id: MOCK_USER_ID, code: 'C1' },
@@ -398,7 +398,7 @@ describe('AuthData Delta Behavior', () => {
398398
expect(authData.other).toBeDefined();
399399
});
400400

401-
// Level 2.2: Частичные обновления
401+
// Level 2.2: Partial updates
402402
it('should update only one provider when multiple exist', async () => {
403403
const user = await Parse.User.logInWith('gpgames', {
404404
authData: { id: MOCK_USER_ID, code: 'C1' },
@@ -646,7 +646,7 @@ describe('AuthData Delta Behavior', () => {
646646
expect(authData.instagram && authData.instagram.id).toBe('I1');
647647
});
648648

649-
// Level 4.1: Комбинации code/id
649+
// Level 4.1: code/id combinations
650650
it('should handle code without id (merge from baseAuthData)', async () => {
651651
const user = await Parse.User.logInWith('gpgames', {
652652
authData: { id: MOCK_USER_ID, code: 'C1' },
@@ -780,7 +780,7 @@ describe('AuthData Delta Behavior', () => {
780780
mockHappyPath();
781781
});
782782

783-
// Level 4.2: Граничные случаи с пустым authData
783+
// Level 4.2: Edge cases with empty authData
784784
it('should handle empty authData object', async () => {
785785
const user = await Parse.User.signUp(TEST_USERNAME, TEST_PASSWORD);
786786
const sessionToken = user.getSessionToken();
@@ -854,7 +854,7 @@ describe('AuthData Delta Behavior', () => {
854854
expect(authData.instagram).toBeUndefined();
855855
});
856856

857-
// Level 4.3: Последовательные обновления
857+
// Level 4.3: Sequential updates
858858
it('should preserve unchanged providers across updates', async () => {
859859
const user = await Parse.User.logInWith('gpgames', {
860860
authData: { id: MOCK_USER_ID, code: 'C1' },
@@ -916,7 +916,7 @@ describe('AuthData Delta Behavior', () => {
916916
expect(authData.instagram).toBeDefined();
917917
});
918918

919-
// Level 4.4: Ошибки валидации
919+
// Level 4.4: Validation errors
920920
it('should reject invalid code during update', async () => {
921921
const user = await Parse.User.logInWith('gpgames', {
922922
authData: { id: MOCK_USER_ID, code: 'C1' },

0 commit comments

Comments
 (0)