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

WIP: Users #261

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

WIP: Users #261

wants to merge 5 commits into from

Conversation

panta82
Copy link
Contributor

@panta82 panta82 commented Jul 17, 2016

Add a limited support for user accounts. A way for the site owner to:

  • stylize their own comments
  • do the same for moderators/contributors/friends
  • prevent anyone else from posting in their name.

For majority of posters, everything works the same as before.

See docs and configuration comments for more details.

TODO-s and remarks:

  • Password is transmitted with each POST. Not a big deal for now, but users should probably be warned they should switch to HTTPS and/or not use their shared password.
  • I added some CSS and js for better validation + bugfixes.
  • Translated the word Password using google translate. If there are translators around, they should give it a look.
  • Server doesn't know how to return a proper JSON formatted error. Look into that.
  • Added the most basic server-side validation for email. This is needed so that anon users can't impersonate a named user.
  • Tests not updated or written. TODO if this pull request is of interest to upstream.

@posativ
Copy link
Collaborator

posativ commented Sep 20, 2016

This is a pretty neat solution, I'll need to review it more thoroughly. I don't like the user management in Isso, it should be solved by $framework a level above (which may provide pluggable user management, whatever. Not a realm of Isso IMHO).

@panta82
Copy link
Contributor Author

panta82 commented Sep 21, 2016

@posativ

Yeah, the real deal would be adding an extensible user support, where the current anon system is just the "default provider" or something. I can also see how any user system would detract from the KISS style of this package.

However, the inability to stylize and protect my own comments (which is expected behavior in any commenting system) was a deal breaker for me. So I added this simple thing for my own purposes.

@StructByLightning
Copy link

Any update on this? Styling mod comments and preventing impersonation is a really useful feature.

*/
this.on = function(type, listener, prevent) {
if (Array.isArray(type)) {
var that = this;
type.forEach(function (type) {
Copy link
Contributor

@brad brad Dec 25, 2018

Choose a reason for hiding this comment

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

Shouldn't a different var name be used for the internally-scoped type? Even if just for clarity...

@brad
Copy link
Contributor

brad commented Dec 25, 2018

This is really nice, love the additional error highlighting, that it something I thought was missing. And the user system is so simple, I don't think there is a need for a full fledged framework user system. These are must have changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants