Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Don't trample over existing sessions when verifying email addresses #2768

Merged
merged 2 commits into from
Mar 8, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
28 changes: 23 additions & 5 deletions src/Lifecycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,17 @@ export async function loadSession(opts) {
}
}

/**
* Gets the user ID of the persisted session, if one exists. This does not validate
* that the user's credentials still work, just that they exist and that a user ID
* is associated with them. The session is not loaded.
* @returns {String} The persisted session's owner, if an owner exists. Null otherwise.
*/
export function getStoredSessionOwner() {
const {hsUrl, userId, accessToken} = _getLocalstorageSessionVars();
return hsUrl && userId && accessToken ? userId : null;
}

/**
* @param {Object} queryParams string->string map of the
* query-parameters extracted from the real query-string of the starting
Expand Down Expand Up @@ -214,6 +225,16 @@ function _registerAsGuest(hsUrl, isUrl, defaultDeviceDisplayName) {
});
}

function _getLocalstorageSessionVars() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Perhaps _getLocalStorage... with capital S?

const hsUrl = localStorage.getItem("mx_hs_url");
const isUrl = localStorage.getItem("mx_is_url") || 'https://matrix.org';
const accessToken = localStorage.getItem("mx_access_token");
const userId = localStorage.getItem("mx_user_id");
const deviceId = localStorage.getItem("mx_device_id");

return {hsUrl, isUrl, accessToken, userId, deviceId};
}

// returns a promise which resolves to true if a session is found in
// localstorage
//
Expand All @@ -228,11 +249,8 @@ async function _restoreFromLocalStorage() {
if (!localStorage) {
return false;
}
const hsUrl = localStorage.getItem("mx_hs_url");
const isUrl = localStorage.getItem("mx_is_url") || 'https://matrix.org';
const accessToken = localStorage.getItem("mx_access_token");
const userId = localStorage.getItem("mx_user_id");
const deviceId = localStorage.getItem("mx_device_id");

const {hsUrl, isUrl, accessToken, userId, deviceId} = _getLocalstorageSessionVars();

let isGuest;
if (localStorage.getItem("mx_is_guest") !== null) {
Expand Down
77 changes: 58 additions & 19 deletions src/components/structures/MatrixChat.js
Original file line number Diff line number Diff line change
Expand Up @@ -357,25 +357,7 @@ export default React.createClass({
return;
}

// the extra Promise.resolve() ensures that synchronous exceptions hit the same codepath as
// asynchronous ones.
return Promise.resolve().then(() => {
return Lifecycle.loadSession({
fragmentQueryParams: this.props.startingFragmentQueryParams,
enableGuest: this.props.enableGuest,
guestHsUrl: this.getCurrentHsUrl(),
guestIsUrl: this.getCurrentIsUrl(),
defaultDeviceDisplayName: this.props.defaultDeviceDisplayName,
});
}).then((loadedSession) => {
if (!loadedSession) {
// fall back to showing the welcome screen
dis.dispatch({action: "view_welcome_page"});
}
});
// Note we don't catch errors from this: we catch everything within
// loadSession as there's logic there to ask the user if they want
// to try logging out.
return this._loadSession();
});

if (SettingsStore.getValue("showCookieBar")) {
Expand All @@ -389,6 +371,28 @@ export default React.createClass({
}
},

_loadSession: function() {
// the extra Promise.resolve() ensures that synchronous exceptions hit the same codepath as
// asynchronous ones.
return Promise.resolve().then(() => {
return Lifecycle.loadSession({
fragmentQueryParams: this.props.startingFragmentQueryParams,
enableGuest: this.props.enableGuest,
guestHsUrl: this.getCurrentHsUrl(),
guestIsUrl: this.getCurrentIsUrl(),
defaultDeviceDisplayName: this.props.defaultDeviceDisplayName,
});
}).then((loadedSession) => {
if (!loadedSession) {
// fall back to showing the welcome screen
dis.dispatch({action: "view_welcome_page"});
}
});
// Note we don't catch errors from this: we catch everything within
// loadSession as there's logic there to ask the user if they want
// to try logging out.
},

componentWillUnmount: function() {
Lifecycle.stopMatrixClient();
dis.unregister(this.dispatcherRef);
Expand Down Expand Up @@ -1684,6 +1688,41 @@ export default React.createClass({
// XXX: This should be in state or ideally store(s) because we risk not
// rendering the most up-to-date view of state otherwise.
this._is_registered = true;
if (this.state.register_session_id) {
// The user came in through an email validation link. To avoid overwriting
// their session, check to make sure the session isn't someone else.
const sessionOwner = Lifecycle.getStoredSessionOwner();
if (sessionOwner && sessionOwner !== credentials.userId) {
console.log(
`Found a session for ${sessionOwner} but ${credentials.userId} is trying to verify their ` +
`email address. Restoring the session for ${sessionOwner} with warning.`,
);
this._loadSession();

const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog");
// N.B. first param is passed to piwik and so doesn't want i18n
Modal.createTrackedDialog('Existing session on register', '',
QuestionDialog, {
title: _t('You are logged in to another account'),
description: _t(
"Thank you for verifying your email! The account you're logged into here " +
"(%(sessionUserId)s) appears to be different from the account you've verified an " +
"email for (%(verifiedUserId)s). If you would like to log in to %(verifiedUserId2)s, " +
"please log out first.", {
sessionUserId: sessionOwner,
verifiedUserId: credentials.userId,

// TODO: Fix translations to support reusing variables.
// https://github.com/vector-im/riot-web/issues/9086
verifiedUserId2: credentials.userId,
},
),
hasCancelButton: false,
});

return;
}
}
return Lifecycle.setLoggedIn(credentials);
},

Expand Down
2 changes: 2 additions & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -1352,6 +1352,8 @@
"Review terms and conditions": "Review terms and conditions",
"Old cryptography data detected": "Old cryptography data detected",
"Data from an older version of Riot has been detected. This will have caused end-to-end cryptography to malfunction in the older version. End-to-end encrypted messages exchanged recently whilst using the older version may not be decryptable in this version. This may also cause messages exchanged with this version to fail. If you experience problems, log out and back in again. To retain message history, export and re-import your keys.": "Data from an older version of Riot has been detected. This will have caused end-to-end cryptography to malfunction in the older version. End-to-end encrypted messages exchanged recently whilst using the older version may not be decryptable in this version. This may also cause messages exchanged with this version to fail. If you experience problems, log out and back in again. To retain message history, export and re-import your keys.",
"You are logged in to another account": "You are logged in to another account",
"Thank you for verifying your email! The account you're logged into here (%(sessionUserId)s) appears to be different from the account you've verified an email for (%(verifiedUserId)s). If you would like to log in to %(verifiedUserId2)s, please log out first.": "Thank you for verifying your email! The account you're logged into here (%(sessionUserId)s) appears to be different from the account you've verified an email for (%(verifiedUserId)s). If you would like to log in to %(verifiedUserId2)s, please log out first.",
"Unknown error discovering homeserver": "Unknown error discovering homeserver",
"Logout": "Logout",
"Your Communities": "Your Communities",
Expand Down