joe/kami-parse-server
Public Access
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Case insensitive signup (#5634)

Browse Source 
* Always delete data after each, even for mongo.

* Add failing simple case test

* run all tests

* 1. when validating username be case insensitive

2. add _auth_data_anonymous to specialQueryKeys...whatever that is!

* More case sensitivity

1. also make email validation case insensitive
2. update comments to reflect what this change does

* wordsmithery and grammar

* first pass at a preformant case insensitive query.  mongo only so far.

* change name of parameter from insensitive to
caseInsensitive

* Postgres support

* properly handle auth data null

* wip

* use 'caseInsensitive' instead of 'insensitive' in all places.

* update commenet to reclect current plan

* skip the mystery test for now

* create case insensitive indecies for
mongo to support case insensitive
checks for email and username

* remove unneeded specialKey

* pull collation out to a function.

* not sure what i planned
to do with this test.
removing.

* remove typo

* remove another unused flag

* maintain order

* maintain order of params

* boil the ocean on param sequence
i like having explain last cause it seems
like something you would
change/remove after getting what you want
from the explain?

* add test to verify creation
and use of caseInsensitive index

* add no op func to prostgress

* get collation object from mongocollection
make flow lint happy by declaring things Object.

* fix typo

* add changelog

* kick travis

* properly reference static method

* add a test to confirm that anonymous users with
unique username that do collide when compared
insensitively can still be created.

* minot doc nits

* add a few tests to make sure our spy is working as expected
wordsmith the changelog

Co-authored-by: Diamond Lewis <findlewis@gmail.com>
This commit is contained in:
Arthur Cinader
2020-02-14 09:44:51 -08:00
committed by GitHub
parent 1ea3f864a8
commit fd0b535159
10 changed files with 413 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -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)

32
spec/MongoStorageAdapter.spec.js
View File

@@ -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' &&

123
spec/ParseUser.spec.js
View File

@@ -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');

8
spec/helper.js
View File

@@ -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

39
src/Adapters/Storage/Mongo/MongoCollection.js
View File

@@ -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);
}

53
src/Adapters/Storage/Mongo/MongoStorageAdapter.js
View File

@@ -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<any> {
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<any> {
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

109
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js
View File

@@ -254,7 +254,12 @@ interface WhereClause {
sorts: Array<any>;
}
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<void> {
async createIndexes(
className: string,
indexes: any,
conn: ?any
): Promise<void> {
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<void> {
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<void> {
@@ -2444,6 +2512,11 @@ export class PostgresStorageAdapter implements StorageAdapter {
);
return result;
}
// TODO: implement?
ensureIndex(): Promise<void> {
return Promise.resolve();
}
}
function convertPolygonToSQL(polygon) {

8
src/Adapters/Storage/StorageAdapter.js
View File

@@ -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<any>;
ensureUniqueness(
className: string,
schema: SchemaType,

38
src/Controllers/DatabaseController.js
View File

@@ -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,

37
src/RestWrite.js
View File

@@ -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
)