Skip to content
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

fix: Usernames should encode all characters in emails #9541

Open
wants to merge 6 commits into
base: alpha
Choose a base branch
from
Open
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
31 changes: 31 additions & 0 deletions spec/EmailVerificationToken.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,37 @@ describe('Email Verification Token Expiration: ', () => {
});
});

it('should send an HTML or properly escaped plain text password reset email', async () => {
const user = new Parse.User();
let sendEmailOptions;
const emailAdapter = {
sendPasswordResetEmail: (options) => {
sendEmailOptions = options;
},
sendVerificationEmail: async () => {},
sendMail: async () => {},
};

await reconfigureServer({
appName: 'specialCharacterUsernameTest',
publicServerURL: 'http://localhost:8378/1',
emailAdapter: emailAdapter,
});

user.setUsername('hello :)');
Copy link
Member

@mtrezza mtrezza Jan 21, 2025

Choose a reason for hiding this comment

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

Could you add a loop and test a bunch of different chars? See also #9541 (comment).

user.setPassword('password123');
user.set('email', '[email protected]');
await user.signUp();

await Parse.User.requestPasswordReset('[email protected]');

expect(sendEmailOptions).toBeDefined();

const username = sendEmailOptions.link.split('username=')[1];
expect(username).toBe('hello%20%3A%29');
});


it('emailVerified should set to false, if the user does not verify their email before the email verify token expires', done => {
const user = new Parse.User();
let sendEmailOptions;
Expand Down
5 changes: 3 additions & 2 deletions src/Controllers/UserController.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import rest from '../rest';
import Parse from 'parse/node';
import AccountLockout from '../AccountLockout';
import Config from '../Config';
import Utils from '../Utils';

var RestQuery = require('../RestQuery');
var Auth = require('../Auth');
Expand Down Expand Up @@ -173,7 +174,7 @@ export class UserController extends AdaptableController {
if (!shouldSendEmail) {
return;
}
const username = encodeURIComponent(fetchedUser.username);
const username = Utils.encode(fetchedUser.username);

const link = buildEmailLink(this.config.verifyEmailURL, username, token, this.config);
const options = {
Expand Down Expand Up @@ -286,7 +287,7 @@ export class UserController extends AdaptableController {
user = await this.setPasswordResetToken(email);
}
const token = encodeURIComponent(user._perishable_token);
const username = encodeURIComponent(user.username);
const username = Utils.encode(user.username);

const link = buildEmailLink(this.config.requestResetPasswordURL, username, token, this.config);
const options = {
Expand Down
11 changes: 11 additions & 0 deletions src/Utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,17 @@
}
return obj;
}

/**
* Encodes a string to be used in a URL.
* @param {String} input The string to encode.
* @returns {String} The encoded string.
*/
static encode(input) {
return encodeURIComponent(input).replace(/[!'()*]/g, char =>
Copy link
Member

Choose a reason for hiding this comment

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

From here:

But GMail/GitHub/Facebook (etc) require escaping because people often put a closing parenthesis or square bracket or comma or period (etc) right after a URL in a sentence. In almost all cases, the URL didn't end with that punctuation, the punctuation was added by the user after pasting the URL. So the problem is that encodeURIComponent strictly follows the standard instead of dealing with certain non-conforming aspects of reality (that have good reasons to be non-conforming, imho).

How does it handle dot, comma, etc after the URL? Do we need to add these chars to the regex, or add tests for that?

'%' + char.charCodeAt(0).toString(16).toUpperCase()

Check warning on line 410 in src/Utils.js

View check run for this annotation

Codecov / codecov/patch

src/Utils.js#L409-L410

Added lines #L409 - L410 were not covered by tests
);
}
}

module.exports = Utils;
Loading