Passport handles expected errors as exceptions

[lindapaiste]

Increasing Access

We should follow best practices for error handling.

Feature enhancement details

Our sign in with Google and Github features are not handling errors in the proper way. We are treating login failures due to invalid credentials as if they were server exceptions. These error messages should be in the third argument of the done callback instead of the first argument.

This line is incorrect:

p5.js-web-editor/server/config/passport.js

Lines 126 to 128 in a9e518c

	done(
	new Error('GitHub account is already linked to another account.')
	);

This line is correct:

p5.js-web-editor/server/config/passport.js

Line 53 in a9e518c

done(null, false, { msg: 'Invalid email or password.' });

Explanation from passport documentation: (read the last paragraph)

A verify function yields under one of three conditions: success, failure, or an error.

If the verify function finds a user to which the credential belongs, and that credential is valid, it calls the callback with the authenticating user:
return cb(null, user);

If the credential does not belong to a known user, or is not valid, the verify function calls the callback with false to indicate an authentication failure:
return cb(null, false);

If an error occurs, such as the database not being available, the callback is called with an error, in idiomatic Node.js style:
return cb(err);

It is important to distinguish between the two failure cases that can occur. Authentication failures are expected conditions, in which the server is operating normally, even though invalid credentials are being received from the user (or a malicious adversary attempting to authenticate as the user). Only when the server is operating abnormally should err be set, to indicate an internal error.

[letscodedanish]

Hey @lindapaiste ,

I've addressed the issue regarding the handling of account suspension messages in the authentication strategies, specifically for GitHub and Google authentication. I've ensured that the authentication process fails gracefully with an appropriate account suspension message when a user is banned.

Let me know if there's anything else I can assist with or if you need further clarification on any aspect of the pull request.