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

Reject login when external oauth login matches a different account #34

Open
wants to merge 2 commits into
base: master
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
22 changes: 15 additions & 7 deletions src/auth/strategies/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,37 +25,45 @@ export async function ExternalServiceCallback(
// If `user` exists, the user has already logged in with this service and is good-to-go
let user = await User.findOne({ [`services.${serviceName}.id`]: id });

if (session && session.email && session.firstName && session.lastName) {
let signupEmail = session.email.trim().toLowerCase();
let enteredEmail = session.email?.trim().toLowerCase();

// Ensure that the oauth method used is linked to the user with the same email address that is entered
if (enteredEmail && user && enteredEmail !== user.email) {
done(null, false, { message: "The 3rd party account you chose does not match the email address that you are trying to log in with. Please make sure your email address and the selected account match." });
Copy link
Member

Choose a reason for hiding this comment

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

Can you clarify the exact expectation in the error message by adding what aspect of the third-party account doesn't match (you mention the email address the user entered but the rest is vague)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Basically, I want them to login with a different 3rd party account. So Ill just tell them that. Let me know if this is good instead.

The {serviceName} account you selected is linked to a different email address than typed below. Please try again with a different 3rd party account.

Where service name is google, github, etc.

return;
}

if (session && enteredEmail && session.firstName && session.lastName) {
// Only create / modify user account if email and name exist on the session (set by login page)
let existingUser = await User.findOne({ email: signupEmail });
let existingUser = await User.findOne({ email: enteredEmail });

if (!user && serviceEmail && existingUser && existingUser.verifiedEmail && existingUser.email === serviceEmail) {
user = existingUser;
// Add new service
if (!user.services) {
user.services = {};
}

if (!user.services[serviceName]) {
user.services[serviceName] = {
id,
email: serviceEmail,
username
};
}

try {
user.markModified("services");
await user.save();
}
catch (err) {
} catch (err) {
done(err);
return;
}
} else if (!user && !existingUser) {
// Create an account
user = createNew<IUser>(User, {
...OAuthStrategy.defaultUserProperties,
email: signupEmail,
email: enteredEmail,
name: {
first: session.firstName,
preferred: session.preferredName,
Expand All @@ -81,7 +89,7 @@ export async function ExternalServiceCallback(
}

if (!user) {
done(null, false, { "message": "Could not match login to existing account" });
done(null, false, { message: "Could not match login to an existing account. Please try again with a different account or login method." });
return;
}

Expand Down
2 changes: 1 addition & 1 deletion src/routes/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ apiRoutes.get("/login-type", async (request, response) => {
});
});

apiRoutes.post("/signup-data", postParser, (request, response) => {
apiRoutes.post("/attach-session-data", postParser, (request, response) => {
function attachToSession(bodyProperty: keyof UserSessionData) {
if (!request.session) return;

Expand Down
19 changes: 12 additions & 7 deletions src/static/js/login.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,19 @@ function setUpStep(step) {
}

if (!passwordLogin.value) {
await fetch(`/api/attach-session-data`, {
...commonFetchSettings,
body: serializeQueryString({
email: email.value.trim()
})
});

let { type } = await fetch(`/api/login-type?email=${encodeURIComponent(email.value.trim())}`).then(response => response.json());
if (["gatech", "google", "github", "facebook"].includes(type)) {
window.location.href = `/auth/${type}`;
return;
}

if (type === "local") {
let passwordContainer = document.getElementById("hidden-password");
passwordContainer.classList.remove("hidden");
Expand All @@ -77,12 +85,8 @@ function setUpStep(step) {
passwordLogin.focus();
return;
}
await fetch(`/api/signup-data`, {
...commonFetchSettings,
body: serializeQueryString({ email: email.value.trim() })
});
}
else {
} else {
// User has entered a value in the password field, so try login via local strategy
await fetch(`/auth/login`, {
...commonFetchSettings,
body: serializeQueryString({
Expand All @@ -102,7 +106,8 @@ function setUpStep(step) {
errorBlock.textContent = "Please enter your first and last name. We use it to identify you online and at events!"
return;
}
await fetch(`/api/signup-data`, {

await fetch(`/api/attach-session-data`, {
...commonFetchSettings,
body: serializeQueryString({
firstName: firstNameValue,
Expand Down