julianlam / nodebb-plugin-session-sharing

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

User logged out hook used twice in plugin.json #87

Closed cryptoethic closed 4 years ago

cryptoethic commented 4 years ago

Hi!

I have spotted that in plugin definition there is a connection to static:user.loggedOut hook twice:

{ "hook": "static:user.loggedOut", "method": "cleanup" },
...
{ "hook": "static:user.loggedOut", "method": "handleLogout" }

cleanup method looks a little bit more complex but as I understand it should give the same result. Maybe it is intended to have two hooks for user logging out registered but if so, then could you please explain me why? ;)

Best regards, cryptoethic

julianlam commented 4 years ago

No, there's no reason, they do the same thing :thinking:

I think I just implemented the same logic twice, separately. I'll have to review this tomorrow.

cryptoethic commented 4 years ago

If it might help:

Removing cookie this way doesn't work in every situation:

res.clearCookie(plugin.settings.cookieName);

The cleanup method's implementation works when above code fails.

julianlam commented 4 years ago

@cryptoethic Yes, I imagine that is the root cause... I must have implemented it back when my development environment was localhost:4567, and my environment is now localhost/forum (I swap back and forth every once in awhile), and when it didn't work I simply re-implemented it, not having realized I already implemented it once before :laughing:

julianlam commented 4 years ago

5c9fd1b2331ca9117154c425975e88261b6aef36

v4.6.7