search

julianlam / nodebb-plugin-session-sharing

Allows login sessions from your app to persist in NodeBB
MIT License
86 stars 66 forks source link

Verify User throws error for Banned users #104

Closed uplift closed 2 years ago

uplift commented 2 years ago

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?

https://github.com/julianlam/nodebb-plugin-session-sharing/blob/bf94e60d23c41f48c77980ced441d195e0b43824/library.js#L208-L222

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 :smile:

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.