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

Commit

Permalink
Merge pull request #2768 from matrix-org/travis/verify-email-state
Browse files Browse the repository at this point in the history
Don't trample over existing sessions when verifying email addresses
  • Loading branch information
turt2live authored Mar 8, 2019
2 parents 47b6356 + 1d71c05 commit 14b3d55
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 24 deletions.
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() {
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

0 comments on commit 14b3d55

Please sign in to comment.