diff --git a/CHANGELOG.md b/CHANGELOG.md index 625becbbdc..1488539480 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### master [Full Changelog](https://github.com/parse-community/parse-server/compare/3.10.0...master) +- FIX: FIX: Prevent new usernames or emails that clash with existing users' email or username if it only differs by case. For example, don't allow a new user with the name 'Jane' if we already have a user 'jane'. [#5634](https://github.com/parse-community/parse-server/pull/5634). Thanks to [Arthur Cinader](https://github.com/acinader) ### 3.10.0 [Full Changelog](https://github.com/parse-community/parse-server/compare/3.9.0...3.10.0) diff --git a/spec/MongoStorageAdapter.spec.js b/spec/MongoStorageAdapter.spec.js index 8a5d42dd77..45c7068341 100644 --- a/spec/MongoStorageAdapter.spec.js +++ b/spec/MongoStorageAdapter.spec.js @@ -318,6 +318,38 @@ describe_only_db('mongo')('MongoStorageAdapter', () => { ); }); + it('should use index for caseInsensitive query', async () => { + const user = new Parse.User(); + user.set('username', 'Bugs'); + user.set('password', 'Bunny'); + await user.signUp(); + + const database = Config.get(Parse.applicationId).database; + const preIndexPlan = await database.find( + '_User', + { username: 'bugs' }, + { caseInsensitive: true, explain: true } + ); + + const schema = await new Parse.Schema('_User').get(); + + await database.adapter.ensureIndex( + '_User', + schema, + ['username'], + 'case_insensitive_username', + true + ); + + const postIndexPlan = await database.find( + '_User', + { username: 'bugs' }, + { caseInsensitive: true, explain: true } + ); + expect(preIndexPlan.executionStats.executionStages.stage).toBe('COLLSCAN'); + expect(postIndexPlan.executionStats.executionStages.stage).toBe('FETCH'); + }); + if ( process.env.MONGODB_VERSION === '4.0.4' && process.env.MONGODB_TOPOLOGY === 'replicaset' && diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 805ef8f3e0..8484cb4b17 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -12,6 +12,7 @@ const MongoStorageAdapter = require('../lib/Adapters/Storage/Mongo/MongoStorageA const request = require('../lib/request'); const passwordCrypto = require('../lib/password'); const Config = require('../lib/Config'); +const cryptoUtils = require('../lib/cryptoUtils'); function verifyACL(user) { const ACL = user.getACL(); @@ -2244,6 +2245,128 @@ describe('Parse.User testing', () => { ); }); + describe('case insensitive signup not allowed', () => { + it('signup should fail with duplicate case insensitive username with basic setter', async () => { + const user = new Parse.User(); + user.set('username', 'test1'); + user.set('password', 'test'); + await user.signUp(); + + const user2 = new Parse.User(); + user2.set('username', 'Test1'); + user2.set('password', 'test'); + await expectAsync(user2.signUp()).toBeRejectedWith( + new Parse.Error( + Parse.Error.USERNAME_TAKEN, + 'Account already exists for this username.' + ) + ); + }); + + it('signup should fail with duplicate case insensitive username with field specific setter', async () => { + const user = new Parse.User(); + user.setUsername('test1'); + user.setPassword('test'); + await user.signUp(); + + const user2 = new Parse.User(); + user2.setUsername('Test1'); + user2.setPassword('test'); + await expectAsync(user2.signUp()).toBeRejectedWith( + new Parse.Error( + Parse.Error.USERNAME_TAKEN, + 'Account already exists for this username.' + ) + ); + }); + + it('signup should fail with duplicate case insensitive email', async () => { + const user = new Parse.User(); + user.setUsername('test1'); + user.setPassword('test'); + user.setEmail('test@example.com'); + await user.signUp(); + + const user2 = new Parse.User(); + user2.setUsername('test2'); + user2.setPassword('test'); + user2.setEmail('Test@Example.Com'); + await expectAsync(user2.signUp()).toBeRejectedWith( + new Parse.Error( + Parse.Error.EMAIL_TAKEN, + 'Account already exists for this email address.' + ) + ); + }); + + it('edit should fail with duplicate case insensitive email', async () => { + const user = new Parse.User(); + user.setUsername('test1'); + user.setPassword('test'); + user.setEmail('test@example.com'); + await user.signUp(); + + const user2 = new Parse.User(); + user2.setUsername('test2'); + user2.setPassword('test'); + user2.setEmail('Foo@Example.Com'); + await user2.signUp(); + + user2.setEmail('Test@Example.Com'); + await expectAsync(user2.save()).toBeRejectedWith( + new Parse.Error( + Parse.Error.EMAIL_TAKEN, + 'Account already exists for this email address.' + ) + ); + }); + + describe('anonymous users', () => { + beforeEach(() => { + const insensitiveCollisions = [ + 'abcdefghijklmnop', + 'Abcdefghijklmnop', + 'ABcdefghijklmnop', + 'ABCdefghijklmnop', + 'ABCDefghijklmnop', + 'ABCDEfghijklmnop', + 'ABCDEFghijklmnop', + 'ABCDEFGhijklmnop', + 'ABCDEFGHijklmnop', + 'ABCDEFGHIjklmnop', + 'ABCDEFGHIJklmnop', + 'ABCDEFGHIJKlmnop', + 'ABCDEFGHIJKLmnop', + 'ABCDEFGHIJKLMnop', + 'ABCDEFGHIJKLMnop', + 'ABCDEFGHIJKLMNop', + 'ABCDEFGHIJKLMNOp', + 'ABCDEFGHIJKLMNOP', + ]; + + // need a bunch of spare random strings per api request + spyOn(cryptoUtils, 'randomString').and.returnValues( + ...insensitiveCollisions + ); + }); + + it('should not fail on case insensitive matches', async () => { + const user1 = await Parse.AnonymousUtils.logIn(); + const username1 = user1.get('username'); + + const user2 = await Parse.AnonymousUtils.logIn(); + const username2 = user2.get('username'); + + expect(username1).not.toBeUndefined(); + expect(username2).not.toBeUndefined(); + expect(username1.toLowerCase()).toBe('abcdefghijklmnop'); + expect(username2.toLowerCase()).toBe('abcdefghijklmnop'); + expect(username2).not.toBe(username1); + expect(username2.toLowerCase()).toBe(username1.toLowerCase()); // this is redundant :). + }); + }); + }); + it('user cannot update email to existing user', done => { const user = new Parse.User(); user.set('username', 'test1'); diff --git a/spec/helper.js b/spec/helper.js index 724751ec4a..16d25ba1b8 100644 --- a/spec/helper.js +++ b/spec/helper.js @@ -207,13 +207,7 @@ afterEach(function(done) { 'There were open connections to the server left after the test finished' ); } - on_db( - 'postgres', - () => { - TestUtils.destroyAllDataPermanently(true).then(done, done); - }, - done - ); + TestUtils.destroyAllDataPermanently(true).then(done, done); }; Parse.Cloud._removeAllHooks(); databaseAdapter diff --git a/src/Adapters/Storage/Mongo/MongoCollection.js b/src/Adapters/Storage/Mongo/MongoCollection.js index 7cabc522cd..8ab24fc29b 100644 --- a/src/Adapters/Storage/Mongo/MongoCollection.js +++ b/src/Adapters/Storage/Mongo/MongoCollection.js @@ -15,7 +15,17 @@ export default class MongoCollection { // idea. Or even if this behavior is a good idea. find( query, - { skip, limit, sort, keys, maxTimeMS, readPreference, hint, explain } = {} + { + skip, + limit, + sort, + keys, + maxTimeMS, + readPreference, + hint, + caseInsensitive, + explain, + } = {} ) { // Support for Full Text Search - $text if (keys && keys.$score) { @@ -30,6 +40,7 @@ export default class MongoCollection { maxTimeMS, readPreference, hint, + caseInsensitive, explain, }).catch(error => { // Check for "no geoindex" error @@ -60,6 +71,7 @@ export default class MongoCollection { maxTimeMS, readPreference, hint, + caseInsensitive, explain, }) ) @@ -67,9 +79,26 @@ export default class MongoCollection { }); } + /** + * Collation to support case insensitive queries + */ + static caseInsensitiveCollation() { + return { locale: 'en_US', strength: 2 }; + } + _rawFind( query, - { skip, limit, sort, keys, maxTimeMS, readPreference, hint, explain } = {} + { + skip, + limit, + sort, + keys, + maxTimeMS, + readPreference, + hint, + caseInsensitive, + explain, + } = {} ) { let findOperation = this._mongoCollection.find(query, { skip, @@ -83,6 +112,12 @@ export default class MongoCollection { findOperation = findOperation.project(keys); } + if (caseInsensitive) { + findOperation = findOperation.collation( + MongoCollection.caseInsensitiveCollation() + ); + } + if (maxTimeMS) { findOperation = findOperation.maxTimeMS(maxTimeMS); } diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index e60551bdb0..b6902fd764 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -620,7 +620,16 @@ export class MongoStorageAdapter implements StorageAdapter { className: string, schema: SchemaType, query: QueryType, - { skip, limit, sort, keys, readPreference, hint, explain }: QueryOptions + { + skip, + limit, + sort, + keys, + readPreference, + hint, + caseInsensitive, + explain, + }: QueryOptions ): Promise { schema = convertParseSchemaToMongoSchema(schema); const mongoWhere = transformWhere(className, query, schema); @@ -653,6 +662,7 @@ export class MongoStorageAdapter implements StorageAdapter { maxTimeMS: this._maxTimeMS, readPreference, hint, + caseInsensitive, explain, }) ) @@ -667,6 +677,47 @@ export class MongoStorageAdapter implements StorageAdapter { .catch(err => this.handleError(err)); } + ensureIndex( + className: string, + schema: SchemaType, + fieldNames: string[], + indexName: ?string, + caseInsensitive: boolean = false + ): Promise { + schema = convertParseSchemaToMongoSchema(schema); + const indexCreationRequest = {}; + const mongoFieldNames = fieldNames.map(fieldName => + transformKey(className, fieldName, schema) + ); + mongoFieldNames.forEach(fieldName => { + indexCreationRequest[fieldName] = 1; + }); + + const defaultOptions: Object = { background: true, sparse: true }; + const indexNameOptions: Object = indexName ? { name: indexName } : {}; + const caseInsensitiveOptions: Object = caseInsensitive + ? { collation: MongoCollection.caseInsensitiveCollation() } + : {}; + const indexOptions: Object = { + ...defaultOptions, + ...caseInsensitiveOptions, + ...indexNameOptions, + }; + + return this._adaptiveCollection(className) + .then( + collection => + new Promise((resolve, reject) => + collection._mongoCollection.createIndex( + indexCreationRequest, + indexOptions, + error => (error ? reject(error) : resolve()) + ) + ) + ) + .catch(err => this.handleError(err)); + } + // Create a unique index. Unique indexes on nullable fields are not allowed. Since we don't // currently know which fields are nullable and which aren't, we ignore that criteria. // As such, we shouldn't expose this function to users of parse until we have an out-of-band diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 2ce4d92f48..0fc5d4c820 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -254,7 +254,12 @@ interface WhereClause { sorts: Array; } -const buildWhereClause = ({ schema, query, index }): WhereClause => { +const buildWhereClause = ({ + schema, + query, + index, + caseInsensitive, +}): WhereClause => { const patterns = []; let values = []; const sorts = []; @@ -276,10 +281,24 @@ const buildWhereClause = ({ schema, query, index }): WhereClause => { } } - if (fieldName.indexOf('.') >= 0) { + const authDataMatch = fieldName.match(/^_auth_data_([a-zA-Z0-9_]+)$/); + if (authDataMatch) { + // TODO: Handle querying by _auth_data_provider, authData is stored in authData field + continue; + } else if ( + caseInsensitive && + (fieldName === 'username' || fieldName === 'email') + ) { + patterns.push(`LOWER($${index}:name) = LOWER($${index + 1})`); + values.push(fieldName, fieldValue); + index += 2; + } else if (fieldName.indexOf('.') >= 0) { let name = transformDotField(fieldName); if (fieldValue === null) { - patterns.push(`${name} IS NULL`); + patterns.push(`$${index}:raw IS NULL`); + values.push(name); + index += 1; + continue; } else { if (fieldValue.$in) { name = transformDotFieldToComponents(fieldName).join('->'); @@ -325,7 +344,12 @@ const buildWhereClause = ({ schema, query, index }): WhereClause => { const clauses = []; const clauseValues = []; fieldValue.forEach(subQuery => { - const clause = buildWhereClause({ schema, query: subQuery, index }); + const clause = buildWhereClause({ + schema, + query: subQuery, + index, + caseInsensitive, + }); if (clause.pattern.length > 0) { clauses.push(clause.pattern); clauseValues.push(...clause.values); @@ -464,10 +488,16 @@ const buildWhereClause = ({ schema, query, index }): WhereClause => { } }; if (fieldValue.$in) { - createConstraint(_.flatMap(fieldValue.$in, elt => elt), false); + createConstraint( + _.flatMap(fieldValue.$in, elt => elt), + false + ); } if (fieldValue.$nin) { - createConstraint(_.flatMap(fieldValue.$nin, elt => elt), true); + createConstraint( + _.flatMap(fieldValue.$nin, elt => elt), + true + ); } } else if (typeof fieldValue.$in !== 'undefined') { throw new Parse.Error(Parse.Error.INVALID_JSON, 'bad $in value'); @@ -1437,7 +1467,12 @@ export class PostgresStorageAdapter implements StorageAdapter { debug('deleteObjectsByQuery', className, query); const values = [className]; const index = 2; - const where = buildWhereClause({ schema, index, query }); + const where = buildWhereClause({ + schema, + index, + query, + caseInsensitive: false, + }); values.push(...where.values); if (Object.keys(query).length === 0) { where.pattern = 'TRUE'; @@ -1744,7 +1779,12 @@ export class PostgresStorageAdapter implements StorageAdapter { } } - const where = buildWhereClause({ schema, index, query }); + const where = buildWhereClause({ + schema, + index, + query, + caseInsensitive: false, + }); values.push(...where.values); const whereClause = @@ -1795,13 +1835,24 @@ export class PostgresStorageAdapter implements StorageAdapter { className: string, schema: SchemaType, query: QueryType, - { skip, limit, sort, keys }: QueryOptions + { skip, limit, sort, keys, caseInsensitive }: QueryOptions ) { - debug('find', className, query, { skip, limit, sort, keys }); + debug('find', className, query, { + skip, + limit, + sort, + keys, + caseInsensitive, + }); const hasLimit = limit !== undefined; const hasSkip = skip !== undefined; let values = [className]; - const where = buildWhereClause({ schema, query, index: 2 }); + const where = buildWhereClause({ + schema, + query, + index: 2, + caseInsensitive, + }); values.push(...where.values); const wherePattern = @@ -2027,7 +2078,12 @@ export class PostgresStorageAdapter implements StorageAdapter { ) { debug('count', className, query, readPreference, estimate); const values = [className]; - const where = buildWhereClause({ schema, query, index: 2 }); + const where = buildWhereClause({ + schema, + query, + index: 2, + caseInsensitive: false, + }); values.push(...where.values); const wherePattern = @@ -2080,7 +2136,12 @@ export class PostgresStorageAdapter implements StorageAdapter { schema.fields[fieldName] && schema.fields[fieldName].type === 'Pointer'; const values = [field, column, className]; - const where = buildWhereClause({ schema, query, index: 4 }); + const where = buildWhereClause({ + schema, + query, + index: 4, + caseInsensitive: false, + }); values.push(...where.values); const wherePattern = @@ -2364,7 +2425,11 @@ export class PostgresStorageAdapter implements StorageAdapter { }); } - async createIndexes(className: string, indexes: any, conn: ?any): Promise { + async createIndexes( + className: string, + indexes: any, + conn: ?any + ): Promise { return (conn || this._client).tx(t => t.batch( indexes.map(i => { @@ -2384,10 +2449,13 @@ export class PostgresStorageAdapter implements StorageAdapter { type: any, conn: ?any ): Promise { - await (conn || this._client).none( - 'CREATE INDEX $1:name ON $2:name ($3:name)', - [fieldName, className, type] - ); + await ( + conn || this._client + ).none('CREATE INDEX $1:name ON $2:name ($3:name)', [ + fieldName, + className, + type, + ]); } async dropIndexes(className: string, indexes: any, conn: any): Promise { @@ -2444,6 +2512,11 @@ export class PostgresStorageAdapter implements StorageAdapter { ); return result; } + + // TODO: implement? + ensureIndex(): Promise { + return Promise.resolve(); + } } function convertPolygonToSQL(polygon) { diff --git a/src/Adapters/Storage/StorageAdapter.js b/src/Adapters/Storage/StorageAdapter.js index 6d2aba1060..ca4b406b37 100644 --- a/src/Adapters/Storage/StorageAdapter.js +++ b/src/Adapters/Storage/StorageAdapter.js @@ -16,6 +16,7 @@ export type QueryOptions = { readPreference?: ?string, hint?: ?mixed, explain?: Boolean, + caseInsensitive?: boolean, action?: string, addsField?: boolean, }; @@ -86,6 +87,13 @@ export interface StorageAdapter { query: QueryType, options: QueryOptions ): Promise<[any]>; + ensureIndex( + className: string, + schema: SchemaType, + fieldNames: string[], + indexName?: string, + caseSensitive?: boolean + ): Promise; ensureUniqueness( className: string, schema: SchemaType, diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index ee0f7d4954..4b69d05e29 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -1299,6 +1299,7 @@ class DatabaseController { // acl restrict this operation with an ACL for the provided array // of user objectIds and roles. acl: null means no user. // when this field is not present, don't do anything regarding ACLs. + // caseInsensitive make string comparisons case insensitive // TODO: make userIds not needed here. The db adapter shouldn't know // anything about users, ideally. Then, improve the format of the ACL // arg to work like the others. @@ -1317,6 +1318,7 @@ class DatabaseController { pipeline, readPreference, hint, + caseInsensitive = false, explain, }: any = {}, auth: any = {}, @@ -1368,6 +1370,7 @@ class DatabaseController { keys, readPreference, hint, + caseInsensitive, explain, }; Object.keys(sort).forEach(fieldName => { @@ -1723,6 +1726,24 @@ class DatabaseController { throw error; }); + const usernameCaseInsensitiveIndex = userClassPromise + .then(() => + this.adapter.ensureIndex( + '_User', + requiredUserFields, + ['username'], + 'case_insensitive_username', + true + ) + ) + .catch(error => { + logger.warn( + 'Unable to create case insensitive username index: ', + error + ); + throw error; + }); + const emailUniqueness = userClassPromise .then(() => this.adapter.ensureUniqueness('_User', requiredUserFields, ['email']) @@ -1735,6 +1756,21 @@ class DatabaseController { throw error; }); + const emailCaseInsensitiveIndex = userClassPromise + .then(() => + this.adapter.ensureIndex( + '_User', + requiredUserFields, + ['email'], + 'case_insensitive_email', + true + ) + ) + .catch(error => { + logger.warn('Unable to create case insensitive email index: ', error); + throw error; + }); + const roleUniqueness = roleClassPromise .then(() => this.adapter.ensureUniqueness('_Role', requiredRoleFields, ['name']) @@ -1752,7 +1788,9 @@ class DatabaseController { }); return Promise.all([ usernameUniqueness, + usernameCaseInsensitiveIndex, emailUniqueness, + emailCaseInsensitiveIndex, roleUniqueness, adapterInit, indexPromise, diff --git a/src/RestWrite.js b/src/RestWrite.js index c87a49ab7e..38a795bb4d 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -704,13 +704,21 @@ RestWrite.prototype._validateUserName = function() { } return Promise.resolve(); } - // We need to a find to check for duplicate username in case they are missing the unique index on usernames - // TODO: Check if there is a unique index, and if so, skip this query. + /* + Usernames should be unique when compared case insensitively + + Users should be able to make case sensitive usernames and + login using the case they entered. I.e. 'Snoopy' should preclude + 'snoopy' as a valid username. + */ return this.config.database .find( this.className, - { username: this.data.username, objectId: { $ne: this.objectId() } }, - { limit: 1 }, + { + username: this.data.username, + objectId: { $ne: this.objectId() }, + }, + { limit: 1, caseInsensitive: true }, {}, this.validSchemaController ) @@ -725,6 +733,18 @@ RestWrite.prototype._validateUserName = function() { }); }; +/* + As with usernames, Parse should not allow case insensitive collisions of email. + unlike with usernames (which can have case insensitive collisions in the case of + auth adapters), emails should never have a case insensitive collision. + + This behavior can be enforced through a properly configured index see: + https://docs.mongodb.com/manual/core/index-case-insensitive/#create-a-case-insensitive-index + which could be implemented instead of this code based validation. + + Given that this lookup should be a relatively low use case and that the case sensitive + unique index will be used by the db for the query, this is an adequate solution. +*/ RestWrite.prototype._validateEmail = function() { if (!this.data.email || this.data.email.__op === 'Delete') { return Promise.resolve(); @@ -738,12 +758,15 @@ RestWrite.prototype._validateEmail = function() { ) ); } - // Same problem for email as above for username + // Case insensitive match, see note above function. return this.config.database .find( this.className, - { email: this.data.email, objectId: { $ne: this.objectId() } }, - { limit: 1 }, + { + email: this.data.email, + objectId: { $ne: this.objectId() }, + }, + { limit: 1, caseInsensitive: true }, {}, this.validSchemaController )