Authentication

[aristippe]

In trying to add required authentication, the current way is through a decorator. Using that or the newer method of the LoginRequiredMixin mixin for class-based views works well, though not everyone wants required authentication. I'm not sure yet what is the easiest and best way to toggle a setting depending on what one wants.

An additional nice feature would be when authentication is enabled, to use HTTP basic auth for the OPDS feed. After looking a bit at various pre-written decorators, here's one:

https://github.com/m7v8/django-basic-authentication-decorator

[sinergatis]

An authentication cleanup would indeed be needed, as currently some views might not have the right checks and there are some Pathagar setting (ALLOW_PUBLIC_ADD_BOOKS) that are not really honored. Perhaps we should try to agree on "what" should be done before "how" it should be done (basically a matter of checking the parent repository and playing with the code a bit), as it is not really 100% clear to me yet? :confounded:

My still not too deeply researched impression is that the main use case on the original repo was having Pathagar deployed on a "public" server of sorts (school, library, etc), and thus the books lists and feeds be available to non-logged in users, and make "add book" page available to non-logged in users via the ALLOW_PUBLIC_ADD_BOOKS. Would that make sense?

If that is indeed the case, for supporting both the "public" and the "fully private" server scenario, we might want to add other variable for it, along with probably a tiny decorator or something similar. I'm undecided on the "make the site private, but the feeds public" scenario - would it have its uses?

[aristippe]

Indeed, Pathagar originally had no authentication requirements and I believe ALLOW_PUBLIC_ADD_BOOKS might have been the only check. The login_required decorator was my later addition. I was thinking eventually an elegant way to toggle authentication would be needed. Looking at what others have done to require authentication, it's been using a decorator or mixin. For a toggle, some setting either in settings or perhaps a later server model (?) that would be used to keep track of other default settings, some of which can be customized in a future user profile model seems like a good idea. If there was some way to wrap the toggle instead of adding a check in each view, that might be better.

When authentication would be required, it would be for all views. The HTTP basic auth doesn't make one view, like OPDS, work without authentication, but provides that view with the form of authentication that OPDS supports.

[sinergatis]

I took a small detour to come back to this issue, in the hopes of simplifying a bit the use of the settings variables and allow for the original "Pathagar deployed in public server" functionality again. A couple of questions that I encountered on the process:

• could you let me know if this urls.py entry does have a purpose currently? Seems that views.author does not really accept a tag and seems to be a leftover of sorts.
• currently all users seem to be able to add, edit and delete books. I'm wondering if something should be done about this, by either only allowing admins to perform those tasks by default (except for add, maybe), or providing a setting for supporting the "we trust all registered users" vs "we don't really trust all registered users" scenarios.

Off the top of my head, I also recall talking about allowing the uploader to edit as well (although the models currently do not support that) and more fine grained changes (tags?), so feel free to comment on those as well - I'm mainly hoping to at least get to a point where the basic scenarios are properly supported, but can throw in more detailed features if needed!

[aristippe]

I was away for a few days and now see these changes. Awesome! To answer your questions:

… Seems that views.author does not really accept a tag and seems to be a leftover of sorts.

That was a new addition for an author detail page. Maybe you missed it? Clicking the name of an author on any page should go to it.

currently all users seem to be able to add, edit and delete books.

So far, maybe others might want finer control but I'm ok with upload if either allow public, or if registration is required, allow all users. For edit, I'm hesistant to allow all users. Uploader certainly (once that's added), and maybe some special moderator class. I'm not sure if many are going to want to allow all users to edit, but for a group of users, would that be some django group or is there another method? I'll have to look into it. Delete is probably best reserved for admins, though some future reporting function to some admin view might be a good idea.

[sinergatis]

Cool!

That was a new addition for an author detail page. Maybe you missed it? Clicking the name of an author on any page should go to it.

Hmmm, I'm referring to the one near the top of the url patterns (line 25-ish):

    url(r'^authors/(?P<tag>.+)/$', views.authors,
        {}, 'authors'),

could you re-check and let me know? If I'm not mistaken, the author details url seems to be author_detail, and view.authors most definitely seems not to accept a tag parameter.

So far, maybe others might want finer control but I'm ok with upload if either allow public, or if registration is required, allow all users. For edit, I'm hesistant to allow all users. Uploader certainly (once that's added), and maybe some special moderator class. I'm not sure if many are going to want to allow all users to edit, but for a group of users, would that be some django group or is there another method? I'll have to look into it. Delete is probably best reserved for admins, though some future reporting function to some admin view might be a good idea.

Yeah, I think we are pretty much aligned - the main problem was that, at this stage (without the pull request changes), any user can edit/delete/etc, which I also feel it won't really be the usual scenario! I actually went ahead and implemented a new variable for controlling that (and defaulting to "users can't edit") at 342bd3c.

As the use cases are starting to get a bit fine-grained, switching to something that uses Django groups and permissions makes sense - probably some code that, at startup, reads the settings and setups the permissions for a predefined set of groups, and switch to using permission_required decorators, etc. The good part is that we now have a "map" of permissions of sorts on the tests, so any further changes will hopefully be rather easy!

[aristippe]

could you re-check and let me know? If I'm not mistaken, the author details url seems to be author_detail, and view.authors most definitely seems not to accept a tag parameter.

Ah yes you're correct. :) The authors view I added in an earlier commit. I forget why at the moment. Maybe it was for an earlier iteration of the authors list view that I was planning before, or perhaps for an anticipated author list to be added to OPDS.

For everything else, thanks for the recent work on the pull requests! The variable for all users edit sounds like a good addition, though at the moment, I'm not sure in what cases one might want to use that. For groups and permissions, indeed it sounds good though I haven't looked into how it works.

For the pull requests, I haven't had a chance to look at them. Will try later this week. If they look good to you, go ahead an merge them. :)

[sinergatis]

Ah yes you're correct. :) The authors view I added in an earlier commit. I forget why at the moment. Maybe it was for an earlier iteration of the authors list view that I was planning before, or perhaps for an anticipated author list to be added to OPDS.

No worries, it certainly looked like a very understandable leftover but wanted to be 100% sure, thanks for looking into it!

The pull requests are quite close to being completed - maybe you can take a quick look at the last two questions at https://github.com/aristippe/pathagar/pull/41#issue-138549935 and let me know your thoughts? They are basically the main unanswered issues left open (and can be left out of the pull request anyway), and I'd indeed love to be able to merge them and proceed with other stuff!

[aristippe]

I had looked briefly at the pull request and had started to think about a comment, and then switched away to a book that might help me figure out how to make a desktop epub app. Questions answered. Indeed merging and figuring out what's next would be great. :) That reminds me there's one thing I want to do: change the templates to display site name instead of 'Pathagar'. That I will definitely try to do soon.

[sinergatis]

change the templates to display site name instead of 'Pathagar'. That I will definitely try to do soon.

Cool! Probably related to #43, perhaps by including it on base.html (right before the </title> end tag and after the {% block title %}...{% endblock %}), and updating the rest of the templates accordingly. We currently use the Django sites framework, so probably the site name should be retrieved from there somehow.

[sinergatis]

Closing this issue after #41 and #45! We probably want to switch to using Django groups and permissions or refine it a bit, but we can do it on a new issue.

[aristippe]

Cool, thanks! groups sounds like a good idea though like with other things, nice to have and whenever. btw, HTTP auth would be required for the RSS/OPDS feed though I'm perhaps a few weeks or months away from trying to regularly use it so that can wait too; and I'll try to take a look. Since HTTP auth is plain text, whenever we get around to that, maybe some optional config to enforce HTTPS would be nice.