Verify User throws error for Banned users

[uplift]

Now that banned users can still login and are just in a banned group (with reduced privileges) does the banned error need to be thrown?

nodebb-plugin-session-sharing/library.js

Lines 208 to 222 in bf94e60

	plugin.verifyUser = function (token, uid, isNewUser, callback) {
	plugins.hooks.fire('static:sessionSharing.verifyUser', {
	uid: uid,
	isNewUser: isNewUser,
	token: token,
	}, function (err) {
	if (err) {
	return callback(err);
	}
	// Check ban state of user, reject if banned
	user.bans.isBanned(uid, function (err, banned) {
	callback(err || banned ? new Error('banned') : null, uid);
	});
	});
	};

Should the isBanned check be called still (to update banned status?) or just remove it all? Ive got a upcoming PR with this fix but not sure if isBanned should stay before submitting.

[julianlam]

Good call! The fix is a little more complicated however... The default would be to disallow banned users (so as to maintain backwards compatibility), but a new option should be added to allow banned users to pass through. At the groups handler, they should be added to the banned users group, I suppose.

[uplift]

Wouldn't the ban process take care of of adding/leaving the necessary groups? I can add the option to allow backwards compatibility to my PR.

[julianlam]

Wouldn't the ban process take care of of adding/leaving the necessary groups?

@uplift Probably, yes, I added that in because I'm never 100% sure, but this is something I discover as I implement. I'm sure you'll double-check as you go 😄

I can add the option to allow backwards compatibility to my PR.

Please, if you don't mind. Really appreciate you helping us out here.