-
Notifications
You must be signed in to change notification settings - Fork 438
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
Highlighting for author comments #321
Conversation
Thanks for working on this. Having author as part of the configuration means you can only have one author per isso instance, and adds an extra HTTP request. Wouldn't it make more sense to have a client setting for the author, so that it can change on a per-thread basis? |
I certainly agree on the idea, but couldn't quite figure out the best way to obfuscate user info. Here we can hash the authors email address / url and match against that so we don't have to worry about crawlers/spammers. If there's a good way to do this client side I'd be happy to change the implementation and extend the capability such that we can invoke per-thread authors. Do you have any suggestions to that end? |
The obfuscation would have to be done by whatever sets data-isso-author; i.e. we'd specify that it can be set to a hashed version of the authors' email address. |
During development I had a JavaScript function that produces the same results as the current hashes (with PBKDF2) generated from the server. The only downside is like multiple hundred milliseconds to generate this hash. |
(sorry for the late reply, I've been on vacation)
This is exactly what's happening already, and it's the server side that does this. Sticking to just one thread, one author for now, and using the current isso implementation; this is what I see a client side method being:
Now, with their obtained hash and an author highlight system enabled:
If you add in multiple authors, now each author needs to know their hash and each thread needs css descriptors (currently, the css in isso global). As someone who can work of development of a project like this, that's not too bad - and is exactly what I've done on my blog; but I think it's a tall order to ask this of regular users. One option would be to extend the cli portion of isso to spit out a hash given a set of credentials. This should be run during setup and then can be set manually in css. I don't really see the need to have client side hashing, as that means the implementation would give the client the author credentials to hash and make obfuscation a moot point. Multiple authors probably requires database extensions too now that I think about it, or a lot of manual intervention from the user side. So I think for the moment, I'd like to solve a single author problem first, with view of extending it to multiple authors later if possible. So with that in mind, I still agree with your points about minimising server requests, but still can't see a rational way of moving this client side AND keeping author credentials private. |
On Fri, Aug 04, 2017 at 09:46:21AM -0700, Tim DuBois wrote:
(sorry for the late reply, I've been on vacation)
> The obfuscation would have to be done by whatever sets data-isso-author; i.e. we'd specify that it can be set to a hashed version of the authors' email address.
This is exactly what's happening already, and it's the server side that does this.
Can you think of a better process for the user here?
Sticking to just one thread, one author for now, and using the current isso implementation; this is what I see a client side method being:
1. The author knows their email adress, but does not want it posted in plain text in each post.
2. Post a test comment.
3. View source and find their user hash.
4. Delete test comment.
Now, with their obtained hash and an author highlight system enabled:
5. Assign their hash to a css value in the isso class settings.
If you add in multiple authors, now each author needs to know their hash and each thread needs css descriptors (currently, the css in isso global).
As someone who can work of development of a project like this, that's not too bad - and is exactly what I've done on my blog; but I think it's a tall order to ask this of regular users.
One option would be to extend the cli portion of isso to spit out a hash given a set of credentials. This should be run during setup and then can be set manually in css. I don't really see the need to have client side hashing, as that means the implementation would give the client the author credentials to hash and make obfuscation a moot point.
Multiple authors probably requires database extensions too now that I think about it, or a lot of manual intervention from the user side. So I think for the moment, I'd like to solve a single author problem first, with view of extending it to multiple authors later if possible.
So with that in mind, I still agree with your points about minimising server requests, but still can't see a rational way of moving this client side AND keeping author credentials private.
There is certainly an argument to be made for both approaches. If the hash can
be provided client-side, then that allows for the author to be
different on a per-thread basis. But as you point out, that
makes the setup more complex for simpler use cases that could do with
just a setting in the configuration file.
If you're going to implement this server side then it would
at least be nice to avoid the extra complexity on the client
side and the extra HTTP request. You could just include a "highlight"
field in the JSON representation of comments that could be set by the
server and then be used for determining highlighting by the client code.
I also think the configuration variable shouldn't just say "author" in
that case, but perhaps take a list of email addresses from people to
highlight. That at least doesn't suggest that isso can't be used with
multi-author blogs.
(In the future, it would still be possible to *also* provide a list of hashes
to high-light on the client side)
Cheers,
Jelmer
|
Those are good ideas. I'll have a think about implementations and update this when I get the time. |
FI, I am using this bit of CSS: #isso-thread .avatar > svg[data-hash="0bb73c4f5196"] {
box-shadow: 0 0 12px #B80F28;
border-radius: 50%;
} I would have preferred to add a kind of badge, but the hash is on the SVG which doesn't have a relation with the header of the comment, so with just CSS, I don't think it's possible add a badge. And I didn't try better because it's good enough for me. |
@vincentbernat Nice bit of CSS, that was exactly what I was looking for. It's kind of a hack though and pretty limited, so I think having builtin support for owner styling is still useful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
per earlier comments
For the record, this is what works to generate the hash: import codecs
from hashlib import pbkdf2_hmac as pbkdf2
val = "admin@example.com".encode("utf-8")
# Default parameters, change if you have different config:
salt = b"Eech7co8Ohloopo9Ol6baimi"
iterations = 1000
dklen = 6
func = "sha1"
compute = pbkdf2(
hash_name=func,
password=val,
salt=salt,
iterations=iterations,
dklen=dklen
)
hash_ = compute
uhash = codecs.encode(hash_, "hex_codec").decode("utf-8")
print("uhash: %s" % uhash) Closing this since the OP has seemingly abandoned and not reacted to jelmer's review comments. Maybe one day a similar PR can find its way into the project. |
I've completed #82 and made it into a feature rather than a manual hack.
Under the [general] section in isso.conf there is now an
author
variable that users can set to an email which they use on their websites to differentiate themselves from other commenters.There were a few additions needed to get this done.
/author
requests (which returns the hash).conf
struct on the client side.isso-highlight
.I guess this implementation has no issue with using a users' IP too, although I haven't tested that. In fact, I haven't written any test in yet. Thought I'd get some feedback to see if this was something of interest first.