Skip to content

Improve email verification #3681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 117 additions & 0 deletions spec/ParseUser.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2784,4 +2784,121 @@ describe('Parse.User testing', () => {
done();
});
});

it('should not allow updates to emailVerified', done => {
var emailAdapter = {
sendVerificationEmail: () => {},
sendPasswordResetEmail: () => Promise.resolve(),
sendMail: () => Promise.resolve()
}

const user = new Parse.User();
user.set({
username: 'hello',
password: 'world',
email: "[email protected]"
})

reconfigureServer({
appName: 'unused',
verifyUserEmails: true,
emailAdapter: emailAdapter,
publicServerURL: "http://localhost:8378/1"
}).then(() => {
return user.signUp();
}).then(() => {
return Parse.User.current().set('emailVerified', true).save();
}).then(() => {
fail("Should not be able to update emailVerified");
done();
}).catch((err) => {
expect(err.message).toBe("Clients aren't allowed to manually update email verification.");
done();
});
});

it('should not retrieve hidden fields', done => {

var emailAdapter = {
sendVerificationEmail: () => {},
sendPasswordResetEmail: () => Promise.resolve(),
sendMail: () => Promise.resolve()
}

const user = new Parse.User();
user.set({
username: 'hello',
password: 'world',
email: "[email protected]"
})

reconfigureServer({
appName: 'unused',
verifyUserEmails: true,
emailAdapter: emailAdapter,
publicServerURL: "http://localhost:8378/1"
}).then(() => {
return user.signUp();
}).then(() => rp({
method: 'GET',
url: 'http://localhost:8378/1/users/me',
json: true,
headers: {
'X-Parse-Application-Id': Parse.applicationId,
'X-Parse-Session-Token': Parse.User.current().getSessionToken(),
'X-Parse-REST-API-Key': 'rest'
},
})).then((res) => {
expect(res.emailVerified).toBe(false);
expect(res._email_verify_token).toBeUndefined();
done()
}).then(() => rp({
method: 'GET',
url: 'http://localhost:8378/1/users/' + Parse.User.current().id,
json: true,
headers: {
'X-Parse-Application-Id': Parse.applicationId,
'X-Parse-REST-API-Key': 'rest'
},
})).then((res) => {
expect(res.emailVerified).toBe(false);
expect(res._email_verify_token).toBeUndefined();
done()
}).catch((err) => {
fail(JSON.stringify(err));
done();
});
});

it('should not allow updates to hidden fields', done => {
var emailAdapter = {
sendVerificationEmail: () => {},
sendPasswordResetEmail: () => Promise.resolve(),
sendMail: () => Promise.resolve()
}

const user = new Parse.User();
user.set({
username: 'hello',
password: 'world',
email: "[email protected]"
})

reconfigureServer({
appName: 'unused',
verifyUserEmails: true,
emailAdapter: emailAdapter,
publicServerURL: "http://localhost:8378/1"
}).then(() => {
return user.signUp();
}).then(() => {
return Parse.User.current().set('_email_verify_token', 'bad').save();
}).then(() => {
fail("Should not be able to update email verification token");
done();
}).catch((err) => {
expect(err).toBeDefined();
done();
});
});
});
5 changes: 5 additions & 0 deletions src/RestWrite.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,11 @@ RestWrite.prototype.transformUser = function() {
return promise;
}

if (!this.auth.isMaster && "emailVerified" in this.data) {
const error = `Clients aren't allowed to manually update email verification.`
throw new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, error);
}

if (this.query) {
// If we're updating a _User object, we need to clear out the cache for that user. Find all their
// session tokens, and remove them from the cache.
Expand Down
11 changes: 11 additions & 0 deletions src/Routers/UsersRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,17 @@ export class UsersRouter extends ClassesRouter {
const user = response.results[0].user;
// Send token back on the login, because SDKs expect that.
user.sessionToken = sessionToken;

// Remove hidden properties.
for (var key in user) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a quick question,
Those fields get stripped only when calling /me, but not when let's say we make an unauthenticated query?

Could we add a test for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What sort of unauthenticated query would we want these fields to be available?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the point, we won't. But those fields would only be stripped here, not everywhere we need them to be stripped

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There does seem to be handling somewhere that removes hidden fields (see updated test where users/ is retrieved without hidden field).

The general case where all hidden fields are removed from all unauthenticated queries is likely better to be talked in its own issue, rather than tacked on to this PR.

Does this current version require any further changes?

if (user.hasOwnProperty(key)) {
// Regexp comes from Parse.Object.prototype.validate
if (key !== "__type" && !(/^[A-Za-z][0-9A-Za-z_]*$/).test(key)) {
delete user[key];
}
}
}

return { response: user };
}
});
Expand Down