Bug 11317 - Add a way to access files from the intranet
Summary: Add a way to access files from the intranet
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Blou
QA Contact: Josef Moravec
URL:
Keywords:
Depends on:
Blocks: 5771 28111 28151 21695
  Show dependency treegraph
 
Reported: 2013-11-27 22:03 UTC by Frédérick Capovilla
Modified: 2021-04-15 09:04 UTC (History)
19 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This feature allows to access files on the server from the staff interface. The directories where the files are stored need to be defined in the koha-conf.xml file. In order to be able to access the tool the staff patron requires either the superlibrarian or the new access_files permission.
Version(s) released in:


Attachments
Adds a page to access log files on the server from the intranet. (6.83 KB, patch)
2013-11-27 22:03 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
New version of the patch, now using datatables (7.20 KB, patch)
2014-02-18 17:01 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
Bug 11317: Follow up - Update ID to allow for permalinking (1.92 KB, patch)
2014-04-23 14:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11317: Follow up - Update ID to allow for permalinking (1.95 KB, patch)
2014-04-25 06:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11317: Follow up - Update ID to allow for permalinking (1.95 KB, patch)
2014-04-25 09:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11317: Adds a page to access log files on the server from the intranet. (8.04 KB, patch)
2015-02-16 15:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11317: Follow up - Update ID to allow for permalinking (2.01 KB, patch)
2015-02-16 15:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11317: Rename publiclogdir with publicdir (3.99 KB, patch)
2015-02-16 15:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11317: Correct critic, forbidden patterns issues when tested with koha-test-tools (4.21 KB, patch)
2015-03-12 19:09 UTC, Charles Farmer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11317: Adds a page to access log files on the server from the intranet. (8.10 KB, patch)
2015-06-02 18:08 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11317: Follow up - Update ID to allow for permalinking (2.08 KB, patch)
2015-06-02 18:08 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11317: Rename publiclogdir with publicdir (3.94 KB, patch)
2015-06-02 18:08 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11317: Correct critic, forbidden patterns issues when tested with koha-test-tools (4.34 KB, patch)
2015-06-02 18:08 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Test case : (4.97 KB, patch)
2015-11-13 21:18 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 11317: translate date with datetime format preference,replace in tools and rename preference in Koha-config.xml. Test case : (5.08 KB, patch)
2015-11-13 21:29 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 11317 - Rename config to 'accessdir' and fix qa (9.30 KB, patch)
2016-03-03 15:30 UTC, Blou
Details | Diff | Splinter Review
Bug 11317 - Allows for multiple directories to be accessible (4.07 KB, patch)
2016-03-03 16:38 UTC, Blou
Details | Diff | Splinter Review
Bug 11317: Adds a page to access log files on the server from the intranet. (8.14 KB, patch)
2016-04-06 01:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11317: Follow up - Update ID to allow for permalinking (2.12 KB, patch)
2016-04-06 01:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11317 - Rename config to 'accessdir' and fix qa (9.37 KB, patch)
2016-04-06 01:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11317: Adds a page to access log files on the server from the intranet. (8.14 KB, patch)
2016-04-06 01:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11317: Follow up - Update ID to allow for permalinking (2.12 KB, patch)
2016-04-06 01:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11317 - Rename config to 'accessdir' and fix qa (9.37 KB, patch)
2016-04-06 01:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11317 - Allows for multiple directories to be accessible (4.13 KB, patch)
2016-04-06 01:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11317 - Move file access under Tools and add specific permission (6.58 KB, patch)
2016-04-19 15:06 UTC, Blou
Details | Diff | Splinter Review
Bug 11317 - Move file access under Tools and add specific permission (10.24 KB, patch)
2016-04-19 15:18 UTC, Blou
Details | Diff | Splinter Review
Bug 11317 - Move file access under Tools and add specific permission (7.23 KB, patch)
2016-04-19 15:21 UTC, Blou
Details | Diff | Splinter Review
Bug 11317 - Allow sorting on date column in file access tool. (1.29 KB, patch)
2016-04-19 20:31 UTC, Blou
Details | Diff | Splinter Review
Bug 11317 - Remove datatables-strings.inc from .tt (959 bytes, patch)
2016-07-15 15:42 UTC, Blou
Details | Diff | Splinter Review
Bug 11317: Adds a page to access log files on the server from the intranet. (9.96 KB, patch)
2016-12-15 17:10 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 11317: Follow up - Update ID to allow for permalinking (2.12 KB, patch)
2016-12-15 17:10 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 11317 - Rename config to 'accessdir' and fix qa (8.80 KB, patch)
2016-12-15 17:10 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 11317 - Allows for multiple directories to be accessible (4.13 KB, patch)
2016-12-15 17:10 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 11317 - Move file access under Tools and add specific permission (7.25 KB, patch)
2016-12-15 17:11 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 11317 - Allow sorting on date column in file access tool. (1.29 KB, patch)
2016-12-15 17:11 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 11317 - Remove datatables-strings.inc from .tt (959 bytes, patch)
2016-12-15 17:11 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 11317: Adds a page to access log files on the server from the intranet. (9.98 KB, patch)
2017-05-02 15:06 UTC, Blou
Details | Diff | Splinter Review
Bug 11317: Follow up - Update ID to allow for permalinking (2.12 KB, patch)
2017-05-02 15:06 UTC, Blou
Details | Diff | Splinter Review
Bug 11317 - Rename config to 'accessdir' and fix qa (8.02 KB, patch)
2017-05-02 15:07 UTC, Blou
Details | Diff | Splinter Review
Bug 11317 - Allows for multiple directories to be accessible (4.13 KB, patch)
2017-05-02 15:07 UTC, Blou
Details | Diff | Splinter Review
Bug 11317 - Move file access under Tools and add specific permission (8.49 KB, patch)
2017-05-02 15:07 UTC, Blou
Details | Diff | Splinter Review
Bug 11317 - Update the link under reports (2.20 KB, patch)
2017-06-07 19:47 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Update the link under reports (3.02 KB, patch)
2017-06-07 19:56 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Follow-up - Fixing merge error and cosmetic changes (3.65 KB, patch)
2017-08-08 20:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11317 - Move file access under Tools and add specific permission (8.49 KB, patch)
2017-08-10 19:33 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Update the link under reports (1.69 KB, patch)
2017-08-10 20:01 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Follow-up - Fixing merge error and cosmetic changes (1.55 KB, patch)
2017-08-10 20:01 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Update the link under reports (1.69 KB, patch)
2017-08-10 20:29 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317: Adds a page to access log files on the server from the intranet. (9.87 KB, patch)
2017-08-11 15:39 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317: Follow up - Update ID to allow for permalinking (2.12 KB, patch)
2017-08-11 15:39 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Rename config to 'accessdir' and fix qa (8.02 KB, patch)
2017-08-11 15:39 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Allows for multiple directories to be accessible (4.13 KB, patch)
2017-08-11 15:39 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Move file access under Tools and add specific permission (8.49 KB, patch)
2017-08-11 15:40 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Update the link under reports (1.69 KB, patch)
2017-08-11 15:40 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Follow-up - Fixing merge error and cosmetic changes (1.55 KB, patch)
2017-08-11 15:40 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Follow-up - Fixing merge error and cosmetic changes (2.46 KB, patch)
2017-08-11 15:43 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Follow-up - Fixing merge error and cosmetic changes (2.46 KB, patch)
2017-08-16 14:08 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Follow-up - Fixing merge error and cosmetic changes (2.46 KB, patch)
2017-08-16 14:09 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 11317 - Follow-up - Fixing merge error and cosmetic changes (2.82 KB, patch)
2017-08-16 14:15 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11317: Adds a page to access log files on the server from the intranet. (9.94 KB, patch)
2017-08-16 14:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11317: Follow up - Update ID to allow for permalinking (2.19 KB, patch)
2017-08-16 14:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11317 - Rename config to 'accessdir' and fix qa (8.09 KB, patch)
2017-08-16 14:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11317 - Allows for multiple directories to be accessible (4.20 KB, patch)
2017-08-16 14:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11317 - Move file access under Tools and add specific permission (8.56 KB, patch)
2017-08-16 14:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11317 - Update the link under reports (1.76 KB, patch)
2017-08-16 14:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11317 - Follow-up - Fixing merge error and cosmetic changes (2.89 KB, patch)
2017-08-16 14:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11317 - File with same name in different directories now has distinct hash (851 bytes, patch)
2018-02-22 18:51 UTC, Roch D'Amour
Details | Diff | Splinter Review
Bug 11317 - File with same name in different directories now has distinct hash + QA followup (4.82 KB, patch)
2018-03-01 15:04 UTC, Roch D'Amour
Details | Diff | Splinter Review
Bug 11317: Adds a page to access log files on the server from the intranet. (9.98 KB, patch)
2018-03-06 07:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 11317: Update ID to allow for permalinking (2.22 KB, patch)
2018-03-06 07:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 11317: Rename config to 'accessdir' and fix qa (8.13 KB, patch)
2018-03-06 07:14 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 11317: Allows for multiple directories to be accessible (4.24 KB, patch)
2018-03-06 07:14 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 11317: Move file access under Tools and add specific permission (8.60 KB, patch)
2018-03-06 07:14 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 11317: Update the link under reports (1.80 KB, patch)
2018-03-06 07:14 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 11317: (Follow-up) Fixing merge error and cosmetic changes (2.93 KB, patch)
2018-03-06 07:14 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 11317: (Follow-up) File with same name in different directories now has distinct hash (917 bytes, patch)
2018-03-06 07:14 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 11317: (Follow-up) Fix tab chars and move javascript to the footer (4.29 KB, patch)
2018-03-06 07:14 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 11317: (QA follow-up) Fix datatable (1.85 KB, patch)
2018-03-06 07:14 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 11317: Make filename unicode-proof, renamed accessdir to access_dir and fix db update (7.07 KB, patch)
2018-04-12 19:42 UTC, Roch D'Amour
Details | Diff | Splinter Review
Bug 11317: Squash attachements to clean and fix merge errors: Add a way to access files from the intranet (16.92 KB, patch)
2018-04-17 18:45 UTC, Roch D'Amour
Details | Diff | Splinter Review
Bug 11317: (follow-up) Add two-level entry to koha-conf and remove unwanted characters (4.52 KB, patch)
2018-04-23 16:48 UTC, Roch D'Amour
Details | Diff | Splinter Review
Bug 11317: Squash attachements to clean and fix merge errors: Add a way to access files from the intranet (17.05 KB, patch)
2018-04-27 12:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11317: (follow-up) Add two-level entry to koha-conf and remove unwanted characters (4.62 KB, patch)
2018-04-27 12:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11317: Add a way to access files from the intranet (17.04 KB, patch)
2018-04-28 00:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11317: (follow-up) Add two-level entry to koha-conf and remove unwanted characters (4.66 KB, patch)
2018-04-28 00:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11317: (QA follow-up) Fix merge error and punctuation (2.23 KB, patch)
2018-04-28 00:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11317: (QA follow-up) Change tool name to be more general (3.58 KB, patch)
2018-04-28 00:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11317: Add id and class attributes to body (1.24 KB, patch)
2018-05-03 17:31 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Frédérick Capovilla 2013-11-27 22:03:06 UTC
Created attachment 23203 [details] [review]
Adds a page to access log files on the server from the intranet.

This patch adds a new report section (Report/log files) that lets you access log files in a specific directory on the server.

The directory to use can be specified by the "publiclogdir" config in koha-conf.xml :
<config>
 ...
 <publiclogdir>/var/log/koha-public</publiclogdir>
 ...
</config>

With this feature, you can configure cron scripts so they create files in the publiclogdir directory, and these files will then be accessible in the intranet.
This is useful when using the html export of overdue_notices.pl or the invalid_856u.sh script as a cron.

Sponsored-by: CCSR ( http://www.ccsr.qc.ca )
Comment 1 Jonathan Druart 2014-01-27 11:18:23 UTC
Frédérick,

Could you provide a test plan and replace tablesorter with datatables please?

Marked as Failed QA.
Comment 2 Frédérick Capovilla 2014-02-18 17:01:52 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2014-02-24 10:18:10 UTC
Removing the "log" as I think the use case is a bit wider - it could be an interesting feature to use with printed notices (overdues, holds, etc.)
Comment 4 Martin Renvoize 2014-04-23 13:34:53 UTC
I like this patch... but, I'de like to see it implemented such that the files in question have perma-links as opposed to the current dynamically created ones which could change.  I'de also love to see a followup that allowed files to require authentication or not.
Comment 5 Katrin Fischer 2014-04-23 13:40:35 UTC
I think I like the idea of this being 'required authentication' only and having another mostly separate feature for publicly available files.

We have been thinking about how the library could get to printed holds and overdue notices. Moving them to an SFTP server or similar would be an option, but having a secure way in Koha to access and download the files would be better.
Comment 6 Martin Renvoize 2014-04-23 14:53:41 UTC Comment hidden (obsolete)
Comment 7 Martin Renvoize 2014-04-23 14:56:02 UTC
Added a follow-up to create permalinks to files based upon filename only (We may want to link to a changing file in the case of logs).

I'de still like to see this changed to reflect different permissions more specifically, to enable a more general approach, as opposed to being a reports only feature.
Comment 8 Chris Cormack 2014-04-24 15:24:34 UTC
The second patch does not apply

Applying: Bug 11317: Follow up - Update ID to allow for permalinking
fatal: sha1 information is lacking or useless (reports/report_files.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Comment 9 Martin Renvoize 2014-04-25 06:37:25 UTC Comment hidden (obsolete)
Comment 10 Martin Renvoize 2014-04-25 06:39:47 UTC
Hmm, I found a minor bug in my code anyways, so I've uploaded a new patch.. However, I couldn't replicate your issue anyway Chris, are you still having issues?
Comment 11 Jonathan Druart 2014-04-25 09:56:34 UTC Comment hidden (obsolete)
Comment 12 Martin Renvoize 2014-04-25 10:02:10 UTC
Thanks Joubu.. no idea why it wasn't applying.. but your new one applies fine for me still and hopefully also for everyone else too..
Comment 13 Jonathan Druart 2015-02-16 15:03:27 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2015-02-16 15:03:58 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2015-02-16 15:04:03 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2015-02-16 15:04:43 UTC
I suggest to rename publiclogdir with publicdir. Feel free so obsolete the patch if you disagree.
Comment 17 Kyle M Hall 2015-02-20 14:30:53 UTC
 FAIL	reports/report_files.pl
   FAIL	  critic
		I/O layer ":utf8" used at line 78, column 9. Use ":encoding(UTF-8)" to get strict validation.
		Bareword file handle opened at line 78, column 9. See pages 202,204 of PBP.
   FAIL	  forbidden patterns
		forbidden pattern: trailing space char (line 61)
		forbidden pattern: trailing space char (line 8)
		forbidden pattern: tab char (line 45)
		forbidden pattern: trailing space char (line 60)
		forbidden pattern: tab char (line 52)
		forbidden pattern: tab char (line 41)
		forbidden pattern: tab char (line 42)
		forbidden pattern: Koha is now under the GPLv3 license (line 13)
		forbidden pattern: tab char (line 43)
		forbidden pattern: tab char (line 39)
		forbidden pattern: tab char (line 55)
		forbidden pattern: tab char (line 44)
		forbidden pattern: tab char (line 40)

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/reports/report_files.tt
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 10)
		forbidden pattern: To include datatables files, please use the include file (see bug 10868) (line 5)
		forbidden pattern: tab char (line 31)
		forbidden pattern: tab char (line 33)
		forbidden pattern: tab char (line 19)
		forbidden pattern: tab char (line 30)
		forbidden pattern: tab char (line 18)
		forbidden pattern: tab char (line 9)
Comment 18 Charles Farmer 2015-03-12 19:09:15 UTC Comment hidden (obsolete)
Comment 19 Mark Tompsett 2015-05-09 15:54:16 UTC
How does this relate to something like bug 6874? :)
Comment 20 Bernardo Gonzalez Kriegel 2015-06-02 18:08:32 UTC Comment hidden (obsolete)
Comment 21 Bernardo Gonzalez Kriegel 2015-06-02 18:08:40 UTC Comment hidden (obsolete)
Comment 22 Bernardo Gonzalez Kriegel 2015-06-02 18:08:47 UTC Comment hidden (obsolete)
Comment 23 Bernardo Gonzalez Kriegel 2015-06-02 18:08:54 UTC Comment hidden (obsolete)
Comment 24 Kyle M Hall 2015-06-26 16:29:35 UTC
Please provide a test plan.
Comment 25 Mark Tompsett 2015-06-26 16:38:44 UTC
(In reply to Kyle M Hall from comment #24)
> Please provide a test plan.

I'm assuming they followed comment #2, comment #13, and/or comment #20.
Comment 26 Maxime Beaulieu 2015-07-07 15:33:12 UTC
(In reply to Kyle M Hall from comment #24)
> Please provide a test plan.

Test case :

Preparation :
* Add the "publiclogdir" preference to your "koha-conf.xml" file. This
* preference should be in the "config" section. For this test cas, we
* will use the "/tmp/koha-public" directory :
<config>
 ...
  <publiclogdir>/tmp/koha-public</publiclogdir>
 ...
</config>
* Create the log directory on your server. Make sure this directory
* is readable by your web server :
$ mkdir /tmp/koha-public
* Add a test file in the log directory :
$ cd /tmp/koha-public
$ echo "Hello world!" > test.txt

Test :
* Go in the "Reports" section in the intranet and select "Report/log
* files"
* The "test.txt" should appear in the files list.
* Click on the "test.txt" file, a download dialog should appear.
* Download the file and verify its content. It should contain the
* text "Hello world!".
Comment 27 Martin Renvoize 2015-10-21 09:24:50 UTC
Could we have some documentation on what a sensible directory location would look like, and what permissions the files within it may require to be accessible by this route (and only this route)?
Comment 28 Katrin Fischer 2015-10-21 09:41:23 UTC
I am really keen on this feature, as Koha lacks a good way to access files like the ones generated for print notices.

I have a few concerns tho:
- The files should be stored in a directory on the server that is safe and cannot be accessed from outside as these files can and will most likely contain patron information in some way. How can we make this happen? I think the least we need to do is add a documentation saying that clearly.
- Related to the first thing... can we please rename publicdir again to indicate something that should not be public/open? internal_files? privatefiledir? Not really a good idea here, but someone else might have.
Comment 29 Martin Renvoize 2015-10-22 06:53:15 UTC
A further enhancement I'd love to see is multiple directories.  The was initially designed as a way to expose log files, and it shows.

It's fast turning into a more generic solution for exposing notices, logs, whatever... but putting notices under a tool called 'reports' and combining files with different purposes into one directory both make little sense..

I'm thinking along the lines of a config block of the form:

<staffdirs>
  <reports>
    <dir></dir>
    <permission></permission>
  </reports>
  <notices>
    <dir></dir>
    <permission></permission>
  </notices>
</staffdirs>

This would yield a much more comprehensive and viable feature
Comment 30 Katrin Fischer 2015-10-22 22:06:29 UTC
Some more notes:
- +use strict; +use warnings; > Please use Modern::Perl; instead
- The table should use a standard display format of Koha for the date and time information. The information form the patch doesn't translate and can't be configured with the dateformat/timeformat preferences: Thu Aug 2 16:41:30 2012
- I feel this would be better as a tool than as a link on the reports page - I don't really see a connection to reports so much, as it's a very general tool.
- I am still in favor of renaming the directory to something less 'public' and we should have some information on how to use this feature safely without exposing data accidentally via your web server that should be kept safe.

Possible enhancements:
- As the kind of files presented can be quite different, it would be nice if this had its own permission.
- Allow multiple directories - @Martin, maybe this could be added later?
Comment 31 Blou 2015-11-13 20:53:20 UTC
We have a hard time figuring a good name for publicdir.  Bouzid will do all the fixes you asked, but I expect someone will come with another patch just to change the config name, eventually.

Regarding the multidir suggestion, we like it.  But that's outside of the sponsored work, and I must say would be really easy to add once this is accepted.  Making this harder to accept from the start is probably not where we want to be.

Further hint as to what is looked for as for documentation would be a great help.  Not sure where to start.
Comment 32 Bouzid Fergani 2015-11-13 21:18:05 UTC Comment hidden (obsolete)
Comment 33 Bouzid Fergani 2015-11-13 21:29:51 UTC Comment hidden (obsolete)
Comment 34 Katrin Fischer 2015-11-22 21:11:46 UTC
Hi Bouzid,

thx for the new patches!

For documentation I have following ideas:
- Add a help page to the new page containing information on how to set this up.
- Show a message on the About page of Koha and/or on the page itself, if the directory is not set up correctly
- Show some kind of warning that people should set this up carefully because the files can include patron/sensitive information and what to look out for?

Could we set up a safe (not accessible by URL) default directory for people to use? (normal installation... packages?) I am not sure how this could best be done technically, but someone else might be able to help.
Comment 35 Blou 2016-03-03 15:30:02 UTC Comment hidden (obsolete)
Comment 36 Blou 2016-03-03 15:33:56 UTC
The last patch introduced 'filesaccess', but didn't replace all 'publicdir' reference.  I squashed the last three patches because I did not see the value of having a patch that changed to publicdir signed off followed by a patch to move it to accessdir then another patch to fix it...  This feature does not warrant that many patches in Koha's history.

An historian...
Comment 37 Blou 2016-03-03 16:38:59 UTC Comment hidden (obsolete)
Comment 38 Nick Clemens 2016-04-06 01:14:00 UTC Comment hidden (obsolete)
Comment 39 Nick Clemens 2016-04-06 01:14:06 UTC Comment hidden (obsolete)
Comment 40 Nick Clemens 2016-04-06 01:14:11 UTC Comment hidden (obsolete)
Comment 41 Nick Clemens 2016-04-06 01:15:05 UTC
Created attachment 49944 [details] [review]
Bug 11317: Adds a page to access log files on the server from the intranet.

The directory used is defined by the "publiclogdir" preference in
koha-conf.xml.

Test case :

Preparation :
* Add the "publiclogdir" preference to your "koha-conf.xml" file. This
* preference should be in the "config" section. For this test cas, we
* will use the "/tmp/koha-public" directory :
<config>
 ...
  <publiclogdir>/tmp/koha-public</publiclogdir>
 ...
</config>
* Create the log directory on your server. Make sure this directory
* is readable by your web server :
$ mkdir /tmp/koha-public
* Add a test file in the log directory :
$ cd /tmp/koha-public
$ echo "Hello world!" > test.txt

Test :
* Go in the "Reports" section in the intranet and select "Report/log
* files"
* The "test.txt" should appear in the files list.
* Click on the "test.txt" file, a download dialog should appear.
* Download the file and verify its content. It should contain the
* text "Hello world!".

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 42 Nick Clemens 2016-04-06 01:15:13 UTC
Created attachment 49945 [details] [review]
Bug 11317: Follow up - Update ID to allow for permalinking

This follow up modifies the id parameter to use a digest of the
filename to enable permalinking to files even if the array order
changes due to new files being created.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 43 Nick Clemens 2016-04-06 01:15:21 UTC
Created attachment 49946 [details] [review]
Bug 11317 - Rename config to 'accessdir' and fix qa

Also translate date with datetime format preference and move to Tools.

Test preparation :
* Add the "accessdir" preference to your "koha-conf.xml" file. This
* preference should be in the "config" section. For this test cas, we
* will use the "/tmp/files-access" directory :
<config>
 ...
  <accessdir>/tmp/files-access</accessdir>
 ...
</config>
* Create the log directory on your server. Make sure this directory
* is readable by your web server :
$ mkdir /tmp/files-access
* Add a test file in the log directory :
$ cd /tmp/files-access
$ echo "Hello world!" > test.txt

Test :
* Go in the "tools/Additional tools" section in the intranet and select "Report/log
* files"
* The "test.txt" should appear in the files list.
* Click on the "test.txt" file, a download dialog should appear.
* Download the file and verify its content. It should contain the
* text "Hello world!".

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 44 Nick Clemens 2016-04-06 01:15:30 UTC
Created attachment 49947 [details] [review]
Bug 11317 - Allows for multiple directories to be accessible

It is now possible to add as many <accessdir>SOMEDIR</accessdir> as needed to the config file.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 45 Nick Clemens 2016-04-06 01:17:04 UTC
Very minor conflict from EDI cleared

qa tool complains:
 FAIL	reports/report_files.pl
   OK	  critic
   OK	  forbidden patterns
   OK	  pod
   OK	  spelling
   FAIL	  valid
		Name "Tie::Hash::FIELDS" used only once: possible typo 
		Name "Cache::RemovalStrategy::LRU::FIELDS" used only once: possible typo 
		Name "Cache::RemovalStrategy::FIELDS" used only once: possible typo
Comment 46 Katrin Fischer 2016-04-06 06:12:47 UTC
Hi Nick, that qa test problem will be fixed by another patch soon. Is this otherwise ok?
Comment 47 Nick Clemens 2016-04-06 09:18:58 UTC
(In reply to Katrin Fischer from comment #46)
> Hi Nick, that qa test problem will be fixed by another patch soon. Is this
> otherwise ok?

Yes
Comment 48 Marcel de Rooy 2016-04-08 07:02:17 UTC
Applying: Bug 11317 - Rename config to 'accessdir' and fix qa
fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/reports/report_files.tt).
Repository lacks necessary blobs to fall back on 3-way merge.
Comment 49 Blou 2016-04-19 12:15:39 UTC
I can rebase the patches.  

But before that, I'd like to understand something: this was a report that became a tool, but it's still a report (reports/report_files.pl) and has report permissions.  While being always available in the tools list... as long as you have access to other tools (because with 'reports' right you can run the report if you have the URL, but you can't view it in tools-home.tt unless you have some other tool access, which you don't necessarily want to give).

My suggestion:
- Make it fully a report OR a tool.  Moving/renaming the files accordingly.
- Add a specific permission to it if it's a tool.

This is an original CCSR patch we've alreay put lot of hours on to make it acceptable, so I hate to pull the rug under so many Signed-off, and I fear the "In discussion", but the situation descrived above is live with a customer, so this is a real-world problem.  

If a QA would just tell me the standard acceptable solution, I'll code it quickly.
Comment 50 Katrin Fischer 2016-04-19 12:32:09 UTC
To me this feels more like a tool and I really like the idea of having a separate permission. Starting a little survey on #koha to get some more opinions.
Comment 51 Kyle M Hall 2016-04-19 12:39:48 UTC
(In reply to Blou from comment #49)

I think from a usability/permissions/technical standpoint this should probably be considered a report. This script does not alter data, but only makes it accessible. This makes it technically a "report" to me.

Furthermore, it seems likely that the librarians needing to access this data are the same ones who would run other reports, and not necessarily be the same ones running tools. This to me is another reason to move it back to the reports page.
Comment 52 Blou 2016-04-19 12:45:09 UTC
Although viewiing the action logs is a tool, not a report.  So I'm not sure this fits.  
Personnally, I view "reports" really as giving me a report.  And reports do not have sub-permissions, really.
Comment 53 Blou 2016-04-19 13:04:35 UTC
#koha voted for "tools".  I will add new patch moving files under /tools, and add new permission for it.
Comment 54 Blou 2016-04-19 15:06:22 UTC Comment hidden (obsolete)
Comment 55 Blou 2016-04-19 15:09:06 UTC
THe little patch that could.  Should cover all the points discussed EXCEPT the sha1 which I did not experience.  I would have done a squash, but this little patch is easier to review.

I *think* i covered all bases.  Template, code, db...  Hopefully.  I hope...
Comment 56 Blou 2016-04-19 15:18:26 UTC Comment hidden (obsolete)
Comment 57 Blou 2016-04-19 15:21:08 UTC Comment hidden (obsolete)
Comment 58 Blou 2016-04-19 20:31:39 UTC Comment hidden (obsolete)
Comment 59 Blou 2016-07-15 15:42:41 UTC Comment hidden (obsolete)
Comment 60 Nicolas Legrand 2016-10-12 12:21:43 UTC Comment hidden (obsolete)
Comment 61 Charles Farmer 2016-12-15 17:06:20 UTC
For easier testing, I'm resubmitting the whole shebang rebased on the current master.

Patches to follow
Comment 62 Charles Farmer 2016-12-15 17:10:31 UTC
Created attachment 58220 [details] [review]
Bug 11317: Adds a page to access log files on the server from the intranet.

The directory used is defined by the "publiclogdir" preference in
koha-conf.xml.

Test case :

Preparation :
* Add the "publiclogdir" preference to your "koha-conf.xml" file. This
* preference should be in the "config" section. For this test cas, we
* will use the "/tmp/koha-public" directory :
<config>
 ...
  <publiclogdir>/tmp/koha-public</publiclogdir>
 ...
</config>
* Create the log directory on your server. Make sure this directory
* is readable by your web server :
$ mkdir /tmp/koha-public
* Add a test file in the log directory :
$ cd /tmp/koha-public
$ echo "Hello world!" > test.txt

Test :
* Go in the "Reports" section in the intranet and select "Report/log
* files"
* The "test.txt" should appear in the files list.
* Click on the "test.txt" file, a download dialog should appear.
* Download the file and verify its content. It should contain the
* text "Hello world!".

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 63 Charles Farmer 2016-12-15 17:10:42 UTC
Created attachment 58221 [details] [review]
Bug 11317: Follow up - Update ID to allow for permalinking

This follow up modifies the id parameter to use a digest of the
filename to enable permalinking to files even if the array order
changes due to new files being created.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 64 Charles Farmer 2016-12-15 17:10:48 UTC
Created attachment 58222 [details] [review]
Bug 11317 - Rename config to 'accessdir' and fix qa

Also translate date with datetime format preference and move to Tools.

Test preparation :
* Add the "accessdir" preference to your "koha-conf.xml" file. This
* preference should be in the "config" section. For this test cas, we
* will use the "/tmp/files-access" directory :
<config>
 ...
  <accessdir>/tmp/files-access</accessdir>
 ...
</config>
* Create the log directory on your server. Make sure this directory
* is readable by your web server :
$ mkdir /tmp/files-access
* Add a test file in the log directory :
$ cd /tmp/files-access
$ echo "Hello world!" > test.txt

Test :
* Go in the "tools/Additional tools" section in the intranet and select "Report/log
* files"
* The "test.txt" should appear in the files list.
* Click on the "test.txt" file, a download dialog should appear.
* Download the file and verify its content. It should contain the
* text "Hello world!".

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 65 Charles Farmer 2016-12-15 17:10:55 UTC
Created attachment 58223 [details] [review]
Bug 11317 - Allows for multiple directories to be accessible

It is now possible to add as many <accessdir>SOMEDIR</accessdir> as needed to the config file.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 66 Charles Farmer 2016-12-15 17:11:00 UTC
Created attachment 58224 [details] [review]
Bug 11317 - Move file access under Tools and add specific permission

Moves the .pl and .tt under tools/ instead of reports/ and adds tools/access_files permission.

Testing
-1) Having done requirements of previous patches, like adding 'accessdir' to koha-conf...
0) apply patch, update database to add permission option.
1) Login as NON-superadmin.  Go under tools, see no Report/Log under the third column
2) Go to add tools/access_file permission to user
3) See new entry under tools third column.
4) validate link is ok.

Sponsored by : CCSR
Comment 67 Charles Farmer 2016-12-15 17:11:06 UTC
Created attachment 58225 [details] [review]
Bug 11317 - Allow sorting on date column in file access tool.

For some reason, the table was blocking sorting based on timestamp, even though most users would probably want that
since the feature is used primarily to access generated (daily) reports and finding the last ones is very important.
Comment 68 Charles Farmer 2016-12-15 17:11:12 UTC
Created attachment 58226 [details] [review]
Bug 11317 - Remove datatables-strings.inc from .tt
Comment 69 Katrin Fischer 2017-05-01 23:53:33 UTC
Sorry, this does no longer apply:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 11317: Adds a page to access log files on the server from the intranet.
Auto packing the repository in background for optimum performance.
See "git help gc" for manual housekeeping.
Applying: Bug 11317: Follow up - Update ID to allow for permalinking
Auto packing the repository in background for optimum performance.
See "git help gc" for manual housekeeping.
Applying: Bug 11317 - Rename config to 'accessdir' and fix qa
Applying: Bug 11317 - Allows for multiple directories to be accessible
Applying: Bug 11317 - Move file access under Tools and add specific permission
fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 11317 - Move file access under Tools and add specific permission
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-11317---Move-file-access-under-Tools-and-add-s-D7DJsU.patch
Comment 70 Blou 2017-05-02 15:06:53 UTC
Created attachment 62971 [details] [review]
Bug 11317: Adds a page to access log files on the server from the intranet.

The directory used is defined by the "publiclogdir" preference in
koha-conf.xml.

Test case :

Preparation :
* Add the "publiclogdir" preference to your "koha-conf.xml" file. This
* preference should be in the "config" section. For this test cas, we
* will use the "/tmp/koha-public" directory :
<config>
 ...
  <publiclogdir>/tmp/koha-public</publiclogdir>
 ...
</config>
* Create the log directory on your server. Make sure this directory
* is readable by your web server :
$ mkdir /tmp/koha-public
* Add a test file in the log directory :
$ cd /tmp/koha-public
$ echo "Hello world!" > test.txt

Test :
* Go in the "Reports" section in the intranet and select "Report/log
* files"
* The "test.txt" should appear in the files list.
* Click on the "test.txt" file, a download dialog should appear.
* Download the file and verify its content. It should contain the
* text "Hello world!".

Sponsored by : CCSR

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 71 Blou 2017-05-02 15:06:58 UTC
Created attachment 62972 [details] [review]
Bug 11317: Follow up - Update ID to allow for permalinking

This follow up modifies the id parameter to use a digest of the
filename to enable permalinking to files even if the array order
changes due to new files being created.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 72 Blou 2017-05-02 15:07:03 UTC
Created attachment 62973 [details] [review]
Bug 11317 - Rename config to 'accessdir' and fix qa

Also translate date with datetime format preference and move to Tools.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 73 Blou 2017-05-02 15:07:08 UTC
Created attachment 62974 [details] [review]
Bug 11317 - Allows for multiple directories to be accessible

It is now possible to add as many <accessdir>SOMEDIR</accessdir> as needed to the config file.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 74 Blou 2017-05-02 15:07:12 UTC
Created attachment 62975 [details] [review]
Bug 11317 - Move file access under Tools and add specific permission

- Moves the .pl and .tt under tools/ instead of reports/ and adds tools/access_files permission.
- Remove datatables-strings.inc from .tt
- Allow sorting on date column in file access tool.  For some reason, the table was blocking sorting based on timestamp,
  even though most users would probably want that since the feature is used primarily to access generated (daily) reports i
  and finding the last ones is very important.

Testing
-1) Having done requirements of previous patches, like adding 'accessdir' to koha-conf...
0) apply patch, update database to add permission option.
1) Login as NON-superadmin.  Go under tools, see no Report/Log under the third column
2) Go to add tools/access_file permission to user
3) See new entry under tools third column.
4) validate link is ok.
Comment 75 Blou 2017-05-02 15:13:49 UTC
Rebased the (evil Sha1!!) patch .  Squashed the unsigned patches (last three).  Ready for take off.
Comment 76 Katrin Fischer 2017-05-25 19:21:50 UTC
Ok, trying to get this tested :)

First: QA tools:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/reports/report_files.tt
   FAIL	  git manipulation
		The file has been added and deleted in the same patchset

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 55)
		forbidden pattern: tab char (line 68)
		forbidden pattern: tab char (line 63)
		forbidden pattern: tab char (line 61)
		forbidden pattern: tab char (line 64)
		forbidden pattern: tab char (line 58)
		forbidden pattern: tab char (line 65)
		forbidden pattern: tab char (line 73)
		forbidden pattern: tab char (line 59)
		forbidden pattern: tab char (line 57)
		forbidden pattern: tab char (line 60)

 FAIL	reports/report_files.pl
   FAIL	  git manipulation
		The file has been added and deleted in the same patchset

Easy to fix - as we got no sign-off yet, we'd just need to fix the tabs and squash the patches.

Second: Some code review

1) I think as the permission is under Tools and the files live there too, it should also be shown as a tool on the tools page instead of under reports. I'd put it under "Additional tools", maybe under the log viewer.
<li><a href="/cgi-bin/koha/reports/report_files.pl">Report/log files</a></li>

2) When I click on the link under Reports, I get a 'not found' error. What am I missing?

3) Looking at the history of this bug, are we missing 2 patches?
Bug 11317 - Allow sorting on date column in file access tool. (1.29 KB, patch)
Bug 11317 - Remove datatables-strings.inc from .tt (959 bytes, patch) 

4) 
+use strict;
+use warnings;
Shoudl be: use Modern::Perl;

Sorry, can't test further because of 2)!
Comment 77 Philippe Audet-Fortin 2017-06-07 19:47:15 UTC
Created attachment 64088 [details] [review]
Bug 11317 - Update the link under reports
Comment 78 Philippe Audet-Fortin 2017-06-07 19:56:53 UTC
Created attachment 64089 [details] [review]
Bug 11317 - Update the link under reports

Signed-off-by: Paudet <philippe.audet-fortin@inlibro.com>
Comment 79 Blou 2017-07-03 16:52:42 UTC
Should have been in Need Signoff for a month, now.
Comment 80 Katrin Fischer 2017-08-08 20:49:28 UTC
I am testing with a kohadevbox, but have problems to access the file, even though it is listed on the page. This is what I did:

- Added the entry to koha-conf.xml, copied from the plugins/uploads default
  <accessdir>/var/lib/koha/kohadev/files</accessdir>
- Created a file with runreport.pl and moved it to the files directory
- The file is listed on the tool page
- When I try to access the file, I get "internal server error"
- The Plack error log shows:
  /home/vagrant/kohaclone/tools/access_files.pl line 119.
  I thought it might be permission related, but even with 
  777 the problem remains:
  -rwxrwxrwx 1 kohadev-koha kohadev-koha 592 Aug  8  2017 report1.csv

While testing I've made a few little changes:
- Fixed conflicts on reports-home.tt from adding/removing the link before moving it to tools
- Slightly reworded second line on the tools page to be a bit more verbose.
- Added entry to the tools-menu.inc.

Attaching this as a follow-up for sign-off if you agree.
Comment 81 Katrin Fischer 2017-08-08 20:51:48 UTC
Created attachment 65691 [details] [review]
Bug 11317 - Follow-up - Fixing merge error and cosmetic changes

- Fixed conflicts on reports-home.tt from adding/removing
  the link before moving it to tools
- Slightly reworded second line on the tools page to be
  a bit more verbose.
- Added entry to the tools-menu.inc.
Comment 82 Philippe Audet-Fortin 2017-08-10 19:33:02 UTC
Created attachment 65851 [details] [review]
Bug 11317 - Move file access under Tools and add specific permission

- Moves the .pl and .tt under tools/ instead of reports/ and adds tools/access_files permission.
- Remove datatables-strings.inc from .tt
- Allow sorting on date column in file access tool.  For some reason, the table was blocking sorting based on timestamp,
  even though most users would probably want that since the feature is used primarily to access generated (daily) reports i
  and finding the last ones is very important.

Testing
-1) Having done requirements of previous patches, like adding 'accessdir' to koha-conf...
0) apply patch, update database to add permission option.
1) Login as NON-superadmin.  Go under tools, see no Report/Log under the third column
2) Go to add tools/access_file permission to user
3) See new entry under tools third column.
4) validate link is ok.
Comment 83 Katrin Fischer 2017-08-10 19:51:10 UTC
Philippe, can this be tested again?
Comment 84 Philippe Audet-Fortin 2017-08-10 20:01:14 UTC Comment hidden (obsolete)
Comment 85 Philippe Audet-Fortin 2017-08-10 20:01:44 UTC
Created attachment 65856 [details] [review]
Bug 11317 - Follow-up - Fixing merge error and cosmetic changes
Comment 86 Philippe Audet-Fortin 2017-08-10 20:29:45 UTC
Created attachment 65857 [details] [review]
Bug 11317 - Update the link under reports
Comment 87 Philippe Audet-Fortin 2017-08-10 20:37:30 UTC
The new attachment are there because I had sha1 error. I change nothing. 
However I don't see why you can't access files. Everything seem to work. I tried with different type of files and permissions and I can't replicate the error.
Comment 88 Katrin Fischer 2017-08-10 20:41:54 UTC
Philippe, you also killed my follow-up :(
Comment 89 Katrin Fischer 2017-08-10 21:31:49 UTC
Please:
- Remove link from the tools page
- Include link in the tools menu koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc
Comment 90 Philippe Audet-Fortin 2017-08-11 13:21:55 UTC
When you say remove the link from the tools page, do you mean report page?
Comment 91 Katrin Fischer 2017-08-11 14:37:57 UTC
Sorry, yes, that's what I meant!
Comment 92 Philippe Audet-Fortin 2017-08-11 15:39:33 UTC
Created attachment 65909 [details] [review]
Bug 11317: Adds a page to access log files on the server from the intranet.

The directory used is defined by the "publiclogdir" preference in
koha-conf.xml.

Test case :

Preparation :
* Add the "publiclogdir" preference to your "koha-conf.xml" file. This
* preference should be in the "config" section. For this test cas, we
* will use the "/tmp/koha-public" directory :
<config>
 ...
  <publiclogdir>/tmp/koha-public</publiclogdir>
 ...
</config>
* Create the log directory on your server. Make sure this directory
* is readable by your web server :
$ mkdir /tmp/koha-public
* Add a test file in the log directory :
$ cd /tmp/koha-public
$ echo "Hello world!" > test.txt

Test :
* Go in the "Reports" section in the intranet and select "Report/log
* files"
* The "test.txt" should appear in the files list.
* Click on the "test.txt" file, a download dialog should appear.
* Download the file and verify its content. It should contain the
* text "Hello world!".

Sponsored by : CCSR

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 93 Philippe Audet-Fortin 2017-08-11 15:39:41 UTC
Created attachment 65910 [details] [review]
Bug 11317: Follow up - Update ID to allow for permalinking

This follow up modifies the id parameter to use a digest of the
filename to enable permalinking to files even if the array order
changes due to new files being created.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 94 Philippe Audet-Fortin 2017-08-11 15:39:47 UTC
Created attachment 65911 [details] [review]
Bug 11317 - Rename config to 'accessdir' and fix qa

Also translate date with datetime format preference and move to Tools.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 95 Philippe Audet-Fortin 2017-08-11 15:39:55 UTC
Created attachment 65912 [details] [review]
Bug 11317 - Allows for multiple directories to be accessible

It is now possible to add as many <accessdir>SOMEDIR</accessdir> as needed to the config file.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 96 Philippe Audet-Fortin 2017-08-11 15:40:02 UTC
Created attachment 65913 [details] [review]
Bug 11317 - Move file access under Tools and add specific permission

- Moves the .pl and .tt under tools/ instead of reports/ and adds tools/access_files permission.
- Remove datatables-strings.inc from .tt
- Allow sorting on date column in file access tool.  For some reason, the table was blocking sorting based on timestamp,
  even though most users would probably want that since the feature is used primarily to access generated (daily) reports i
  and finding the last ones is very important.

Testing
-1) Having done requirements of previous patches, like adding 'accessdir' to koha-conf...
0) apply patch, update database to add permission option.
1) Login as NON-superadmin.  Go under tools, see no Report/Log under the third column
2) Go to add tools/access_file permission to user
3) See new entry under tools third column.
4) validate link is ok.
Comment 97 Philippe Audet-Fortin 2017-08-11 15:40:10 UTC
Created attachment 65914 [details] [review]
Bug 11317 - Update the link under reports
Comment 98 Philippe Audet-Fortin 2017-08-11 15:40:16 UTC
Created attachment 65915 [details] [review]
Bug 11317 - Follow-up - Fixing merge error and cosmetic changes
Comment 99 Philippe Audet-Fortin 2017-08-11 15:43:14 UTC
Created attachment 65918 [details] [review]
Bug 11317 - Follow-up - Fixing merge error and cosmetic changes
Comment 100 Philippe Audet-Fortin 2017-08-11 15:47:18 UTC
Sorry for the mess in the comment. 
I made the changes and now the patch apply without conflict.
Comment 101 Katrin Fischer 2017-08-12 10:07:28 UTC
Hi Phillipe, follow-up is looking good, but I still can't open or download the file. This is the error from the plack-error-log:
exited nonzero: 1 at /home/vagrant/kohaclone/tools/access_files.pl line 120.

Which is odd, as the file does end at line 107... :(
Comment 102 Blou 2017-08-14 12:11:07 UTC
plack-error.log?  Curious, do you have the issue when not using plack?  Must be some kind of permission issue... :(

Here it works like a charm.  But Philippe will confirm himself.
Comment 103 Katrin Fischer 2017-08-15 14:13:08 UTC
I have tested with Plack deactivated and it does WORK.

Using Plack is strongly recommende/standard setup now, so we really need it to work under Plack too. I am leaving this as Failed QA and hope that the new information will help you to fix it. We are close, please don't give up now.

The QA tools have some complaints, maybe it would help to squash at least the patches by the same authors:

 OK	etc/koha-conf.xml
   OK	  git manipulation
   OK	  xml_valid

 OK	koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 OK	koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/reports/report_files.tt
   OK	  forbidden patterns
   FAIL	  git manipulation
		The file has been added and deleted in the same patchset
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 55)
		forbidden pattern: tab char (line 59)
		forbidden pattern: tab char (line 65)
		forbidden pattern: tab char (line 63)
		forbidden pattern: tab char (line 61)
		forbidden pattern: tab char (line 64)
		forbidden pattern: tab char (line 57)
		forbidden pattern: tab char (line 60)
		forbidden pattern: tab char (line 58)
		forbidden pattern: tab char (line 68)
		forbidden pattern: tab char (line 72)
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 OK	koha-tmpl/intranet-tmpl/prog/en/modules/tools/access_files.tt
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 OK	koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	reports/report_files.pl
   OK	  critic
   OK	  forbidden patterns
   FAIL	  git manipulation
		The file has been added and deleted in the same patchset
   OK	  pod
   OK	  spelling
   OK	  valid

 OK	tools/access_files.pl
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   OK	  spelling
   OK	  valid
Comment 104 Katrin Fischer 2017-08-15 15:21:17 UTC
Global var in the BEGIN block might be the cause.
Comment 105 Philippe Audet-Fortin 2017-08-16 14:08:36 UTC Comment hidden (obsolete)
Comment 106 Philippe Audet-Fortin 2017-08-16 14:09:50 UTC Comment hidden (obsolete)
Comment 107 Philippe Audet-Fortin 2017-08-16 14:15:54 UTC
Created attachment 66093 [details] [review]
Bug 11317 - Follow-up - Fixing merge error and cosmetic changes
Comment 108 Philippe Audet-Fortin 2017-08-16 14:22:55 UTC
Hi Katrine,
I found the problem. It didn't work with plack because when a file was download, access_files.pl exited with error flag even there had no error. It seem like plack doesn't love that.
Comment 109 Katrin Fischer 2017-08-16 14:42:46 UTC
Created attachment 66094 [details] [review]
[SIGNED OFF] Bug 11317: Adds a page to access log files on the server from the intranet.

The directory used is defined by the "publiclogdir" preference in
koha-conf.xml.

Test case :

Preparation :
* Add the "publiclogdir" preference to your "koha-conf.xml" file. This
* preference should be in the "config" section. For this test cas, we
* will use the "/tmp/koha-public" directory :
<config>
 ...
  <publiclogdir>/tmp/koha-public</publiclogdir>
 ...
</config>
* Create the log directory on your server. Make sure this directory
* is readable by your web server :
$ mkdir /tmp/koha-public
* Add a test file in the log directory :
$ cd /tmp/koha-public
$ echo "Hello world!" > test.txt

Test :
* Go in the "Reports" section in the intranet and select "Report/log
* files"
* The "test.txt" should appear in the files list.
* Click on the "test.txt" file, a download dialog should appear.
* Download the file and verify its content. It should contain the
* text "Hello world!".

Sponsored by : CCSR

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 110 Katrin Fischer 2017-08-16 14:42:52 UTC
Created attachment 66095 [details] [review]
[SIGNED OFF] Bug 11317: Follow up - Update ID to allow for permalinking

This follow up modifies the id parameter to use a digest of the
filename to enable permalinking to files even if the array order
changes due to new files being created.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 111 Katrin Fischer 2017-08-16 14:42:58 UTC
Created attachment 66096 [details] [review]
[SIGNED OFF] Bug 11317 - Rename config to 'accessdir' and fix qa

Also translate date with datetime format preference and move to Tools.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 112 Katrin Fischer 2017-08-16 14:43:04 UTC
Created attachment 66097 [details] [review]
[SIGNED OFF] Bug 11317 - Allows for multiple directories to be accessible

It is now possible to add as many <accessdir>SOMEDIR</accessdir> as needed to the config file.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 113 Katrin Fischer 2017-08-16 14:43:10 UTC
Created attachment 66098 [details] [review]
[SIGNED OFF] Bug 11317 - Move file access under Tools and add specific permission

- Moves the .pl and .tt under tools/ instead of reports/ and adds tools/access_files permission.
- Remove datatables-strings.inc from .tt
- Allow sorting on date column in file access tool.  For some reason, the table was blocking sorting based on timestamp,
  even though most users would probably want that since the feature is used primarily to access generated (daily) reports i
  and finding the last ones is very important.

Testing
-1) Having done requirements of previous patches, like adding 'accessdir' to koha-conf...
0) apply patch, update database to add permission option.
1) Login as NON-superadmin.  Go under tools, see no Report/Log under the third column
2) Go to add tools/access_file permission to user
3) See new entry under tools third column.
4) validate link is ok.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 114 Katrin Fischer 2017-08-16 14:43:15 UTC
Created attachment 66099 [details] [review]
[SIGNED OFF] Bug 11317 - Update the link under reports

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 115 Katrin Fischer 2017-08-16 14:43:21 UTC
Created attachment 66100 [details] [review]
[SIGNED OFF] Bug 11317 - Follow-up - Fixing merge error and cosmetic changes

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 116 Josef Moravec 2017-12-22 15:22:19 UTC
Sorry, I see it is long process to get this to Koha, but I have one more concern:

If there are files with the same name in multiple directories, they have the same hash! Than only one of them is always downloaded, although both (or more) are listed.

The path of each file should be presented in the table too.
Comment 117 Roch D'Amour 2018-02-22 18:51:26 UTC
Created attachment 72097 [details] [review]
Bug 11317 - File with same name in different directories now has distinct hash
Comment 118 Roch D'Amour 2018-02-22 19:12:29 UTC
Forgot to add test plan... Here we go :

- Generate file hash based on fullpath + filename. This make sure all files are accessible, even if they have the same filename.

Testing:
0. Make sure the requirements from the last patches are met
1. Add 2 accessdir in your koha-conf, i.e. "/tmp/test" and "/tmp/moretest".
2. Add a file in both directories using the same name for the file:
-    echo "small file" > /tmp/test/log
-    echo "the size of this file is quite huge" > /tmp/moretest/log
3. Go to Home › Tools › Report/log files and you should see both files with different size.
4. Download (or open) both files. They should be different.

Thanks for testing
Comment 119 Josef Moravec 2018-02-25 08:25:46 UTC
Sorry, still some issues found with this:

1) Please undo the changes of spaces to tabs in reports-home.tt - there is no reason to touch these lines and tab char is forbidden

2) Move javascript to the end of page - see JS12 rule: https://wiki.koha-community.org/wiki/Coding_Guidelines#JS12:_Include_javascript_at_the_end_of_template

3) Please add _[% KOHA_VERSION %] to css path, see bug 12904
Comment 120 Roch D'Amour 2018-03-01 15:04:52 UTC
Created attachment 72309 [details] [review]
Bug 11317 - File with same name in different directories now has distinct hash + QA followup

Testing:
0. Make sure the requirements from the last patches are met
1. Add 2 accessdir in your koha-conf, i.e. "/tmp/test" and "/tmp/moretest".
2. Add a file in both directories using the same name for the file:
-    echo "small file" > /tmp/test/log
-    echo "the size of this file is quite huge" > /tmp/moretest/log
3. Go to Home › Tools › Report/log files and you should see both files with different size.
4. Download (or open) both files. They should be different.

Thanks for testing
Comment 121 Josef Moravec 2018-03-06 07:13:51 UTC
Created attachment 72456 [details] [review]
Bug 11317: Adds a page to access log files on the server from the intranet.

The directory used is defined by the "publiclogdir" preference in
koha-conf.xml.

Test case :

Preparation :
* Add the "publiclogdir" preference to your "koha-conf.xml" file. This
* preference should be in the "config" section. For this test cas, we
* will use the "/tmp/koha-public" directory :
<config>
 ...
  <publiclogdir>/tmp/koha-public</publiclogdir>
 ...
</config>
* Create the log directory on your server. Make sure this directory
* is readable by your web server :
$ mkdir /tmp/koha-public
* Add a test file in the log directory :
$ cd /tmp/koha-public
$ echo "Hello world!" > test.txt

Test :
* Go in the "Reports" section in the intranet and select "Report/log
* files"
* The "test.txt" should appear in the files list.
* Click on the "test.txt" file, a download dialog should appear.
* Download the file and verify its content. It should contain the
* text "Hello world!".

Sponsored by : CCSR

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 122 Josef Moravec 2018-03-06 07:13:57 UTC
Created attachment 72457 [details] [review]
Bug 11317: Update ID to allow for permalinking

This follow up modifies the id parameter to use a digest of the
filename to enable permalinking to files even if the array order
changes due to new files being created.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 123 Josef Moravec 2018-03-06 07:14:03 UTC
Created attachment 72458 [details] [review]
Bug 11317: Rename config to 'accessdir' and fix qa

Also translate date with datetime format preference and move to Tools.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 124 Josef Moravec 2018-03-06 07:14:09 UTC
Created attachment 72459 [details] [review]
Bug 11317: Allows for multiple directories to be accessible

It is now possible to add as many <accessdir>SOMEDIR</accessdir> as needed to the config file.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 125 Josef Moravec 2018-03-06 07:14:15 UTC
Created attachment 72460 [details] [review]
Bug 11317: Move file access under Tools and add specific permission

- Moves the .pl and .tt under tools/ instead of reports/ and adds tools/access_files permission.
- Remove datatables-strings.inc from .tt
- Allow sorting on date column in file access tool.  For some reason, the table was blocking sorting based on timestamp,
  even though most users would probably want that since the feature is used primarily to access generated (daily) reports i
  and finding the last ones is very important.

Testing
-1) Having done requirements of previous patches, like adding 'accessdir' to koha-conf...
0) apply patch, update database to add permission option.
1) Login as NON-superadmin.  Go under tools, see no Report/Log under the third column
2) Go to add tools/access_file permission to user
3) See new entry under tools third column.
4) validate link is ok.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 126 Josef Moravec 2018-03-06 07:14:20 UTC
Created attachment 72461 [details] [review]
Bug 11317: Update the link under reports

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 127 Josef Moravec 2018-03-06 07:14:25 UTC
Created attachment 72462 [details] [review]
Bug 11317: (Follow-up) Fixing merge error and cosmetic changes

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 128 Josef Moravec 2018-03-06 07:14:31 UTC
Created attachment 72463 [details] [review]
Bug 11317: (Follow-up) File with same name in different directories now has distinct hash

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 129 Josef Moravec 2018-03-06 07:14:36 UTC
Created attachment 72464 [details] [review]
Bug 11317: (Follow-up) Fix tab chars and move javascript to the footer

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 130 Josef Moravec 2018-03-06 07:14:42 UTC
Created attachment 72465 [details] [review]
Bug 11317: (QA follow-up) Fix datatable

Fixes the link to datatables.css
Fixes the footer javascript

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 131 Jonathan Druart 2018-03-28 21:07:43 UTC
1. Not unicode proof, create a file named "❤" in your accessdir and boom:
Can't call method "mtime" on an undefined value at /home/vagrant/kohaclone/tools/access_files.pl line 61.
2. Please rename accessdir with access_dir
3. Can we get an up-to-date and complete test plan?
4. Why do not we have a default value for accessdir?
5. What is the main purpose of this development? How do we want to advert it? Still log files?
6. Did someone test with very big files (see bug 8437)? Should we put a limit?
Comment 132 Katrin Fischer 2018-03-28 21:42:38 UTC
The use case for us would be to allow access to files generated on the server - not logs, but print notices. They contain patron information, so you need to secure them behind login.
Comment 133 Katrin Fischer 2018-04-12 13:41:12 UTC
This is about to miss another release again :( Can we get some discussion going based on RM feedback?
Comment 134 Roch D'Amour 2018-04-12 15:08:44 UTC
Working on this and wondering what could be a good default value for accessdir?

Also, renaming accessdir to access_dir make sense to me, I will include this change in my fix to make this feature unicode proof.
Comment 135 Roch D'Amour 2018-04-12 19:42:39 UTC
Created attachment 74119 [details] [review]
Bug 11317: Make filename unicode-proof, renamed accessdir to access_dir and fix db update

This patch does many small things :
- Added "INSERT IGNORE" in the atomic update
- Renamed accessdir to access_dir
- Now decode the filename to utf8 to make sure the stat() method point at it.

Test plan:
1. Define <access_dir>/tmp/koha-public</access_dir> in your koha_conf
2. change dir to /tmp/koha-public
3. create a file with a normal name, i.e. echo "bonjour koha" > testfile.txt
4. create a file with a name containing a unicode character, i.e. touch hi❤.txt
5. Go in the "Reports" section in the intranet and select "Report/log files"
6. Verify that the filename "hi❤.txt" looks fine
7. Click on the "testfile.txt" file, a download dialog should appear.
8. Download the file and verify its content. It should contain the text "bonjour koha".

Sponsored by : CCSR
Comment 136 Katrin Fischer 2018-04-15 09:17:33 UTC
I am very sorry, this doesn't apply for me. I tried to fix it, but I got stuck with the last 3 patches:

Applying: Bug 11317: (Follow-up) Fix tab chars and move javascript to the footer
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt).
error: could not build fake ancestor
Patch failed at 0001 Bug 11317: (Follow-up) Fix tab chars and move javascript to the footer
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am -i --continue".
If you prefer to skip this patch, run "git am -i --skip" instead.
To restore the original branch and stop patching, run "git am -i --abort".

Please also note that bug 20538 will require some changes, but they should be fast!

For the path, I am no expert. Maybe lean on the sample paths for uploaded files or plugins?
Comment 137 Roch D'Amour 2018-04-17 18:45:30 UTC
Created attachment 74379 [details] [review]
Bug 11317: Squash attachements to clean and fix merge errors: Add a way to access files from the intranet

This squash contains all of these commits:
- Adds a page to access log files on the server from the intranet
- Update ID to allow for permalinking
- Rename config to "'accessdir' and fix qa
- Allows for multiple directories to be accessible
- Update the link under reports
- (Follow-up) Fixing merge error and cosmetic changes
- (Follow-up) Fix tab chars and move javascript to the footer
- (QA Follow-up) Fix datatable
- Make filename unicode-proof, renamed accessdir to access_dir and fix update

Test plans:
- Apply patch, update database
- Add to koha-conf:
<access_dir>/tmp/koha-public/one</access_dir>
<access_dir>/tmp/koha-public/two</access_dir>
<access_dir>/tmp/koha-public</access_dir>
- Create these directories ( mkdir /tmp/koha-public , etc...)
- Create these files:
echo "hello world!" > /tmp/koha-public/❤
echo "test" > /tmp/koha-public/one/samename.txt
echo "this is not the same" > /tmp/koha-public/two/samename.txt
- Login as Superadmin, go to tools > reports files
    - Click on ❤, make sure it's downloadable and readable
    - Click on both samename.txt, look inside and make sure the file is different
- Login as NON-superadmin. Go under tools, see no Report/Log under the third column
    - Go to add tools/access_file permission to user
    - See new entry under tools third column.
    - validate link is ok.
Comment 138 Tomás Cohen Arazi 2018-04-19 14:37:44 UTC
(In reply to Roch D'Amour from comment #134)
> Working on this and wondering what could be a good default value for
> accessdir?
> 
> Also, renaming accessdir to access_dir make sense to me, I will include this
> change in my fix to make this feature unicode proof.

This configuration entry should default to empty.

The patch is missing to add the entry in debian/templates/koha-conf-site.xml.in
Comment 139 Katrin Fischer 2018-04-22 21:58:26 UTC
Roch, just something I noticed on another Inlibro bug earlier already: Please provide fixes that QA or RM ask for as separate follow-up patches. This way you can keep the sign-off line on the initial patch. If not told otherwise it's also ok to switch back to Signed-off.
Comment 140 Katrin Fischer 2018-04-22 22:26:36 UTC
Please fix the errors from the QA script and add the changes to debian/templates/koha-conf-site.xml.in

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt
   FAIL	  forbidden patterns
		forbidden pattern: merge marker (<<<<<<<) (line 66)
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/tools/access_files.tt
   FAIL	  forbidden patterns
		forbidden pattern: Do *not* include full path to css files, use the Asset TT plugin (bug 20538) (line 5)
		forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 55)
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template
Comment 141 Roch D'Amour 2018-04-23 14:05:17 UTC
(In reply to Tomás Cohen Arazi from comment #138)

> This configuration entry should default to empty.
> 
> The patch is missing to add the entry in
> debian/templates/koha-conf-site.xml.in

I don't think I understood clearly. When I look in koha-conf-site.xml.in, I have:

 <!-- Accessible directory from the staff client, uncomment the following line and define a valid path to let the intranet user access it-->
 <!-- <access_dir></access_dir> -->

Should I uncomment this line?
Comment 142 Roch D'Amour 2018-04-23 16:48:22 UTC
Created attachment 74756 [details] [review]
Bug 11317: (follow-up) Add two-level entry to koha-conf and remove unwanted characters

- Removed merge marker
- Changed include path in favor of using the Asset tt plugin (bug 20538)
- Changed access_dir to a two-level entry for clarity

Test plans stay the same, just make sure that the two-level configuration entry
work properly and everything pass QA.
Comment 143 Kyle M Hall 2018-04-27 12:44:57 UTC
Created attachment 74911 [details] [review]
Bug 11317: Squash attachements to clean and fix merge errors: Add a way to access files from the intranet

This squash contains all of these commits:
- Adds a page to access log files on the server from the intranet
- Update ID to allow for permalinking
- Rename config to "'accessdir' and fix qa
- Allows for multiple directories to be accessible
- Update the link under reports
- (Follow-up) Fixing merge error and cosmetic changes
- (Follow-up) Fix tab chars and move javascript to the footer
- (QA Follow-up) Fix datatable
- Make filename unicode-proof, renamed accessdir to access_dir and fix update

Test plans:
- Apply patch, update database
- Add to koha-conf:
<access_dir>/tmp/koha-public/one</access_dir>
<access_dir>/tmp/koha-public/two</access_dir>
<access_dir>/tmp/koha-public</access_dir>
- Create these directories ( mkdir /tmp/koha-public , etc...)
- Create these files:
echo "hello world!" > /tmp/koha-public/❤
echo "test" > /tmp/koha-public/one/samename.txt
echo "this is not the same" > /tmp/koha-public/two/samename.txt
- Login as Superadmin, go to tools > reports files
    - Click on ❤, make sure it's downloadable and readable
    - Click on both samename.txt, look inside and make sure the file is different
- Login as NON-superadmin. Go under tools, see no Report/Log under the third column
    - Go to add tools/access_file permission to user
    - See new entry under tools third column.
    - validate link is ok.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 144 Kyle M Hall 2018-04-27 12:45:08 UTC
Created attachment 74912 [details] [review]
Bug 11317: (follow-up) Add two-level entry to koha-conf and remove unwanted characters

- Removed merge marker
- Changed include path in favor of using the Asset tt plugin (bug 20538)
- Changed access_dir to a two-level entry for clarity

Test plans stay the same, just make sure that the two-level configuration entry
work properly and everything pass QA.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 145 Katrin Fischer 2018-04-28 00:34:53 UTC
Created attachment 74916 [details] [review]
Bug 11317: Add a way to access files from the intranet

This squash contains all of these commits:
- Adds a page to access log files on the server from the intranet
- Update ID to allow for permalinking
- Rename config to "'accessdir' and fix qa
- Allows for multiple directories to be accessible
- Update the link under reports
- (Follow-up) Fixing merge error and cosmetic changes
- (Follow-up) Fix tab chars and move javascript to the footer
- (QA Follow-up) Fix datatable
- Make filename unicode-proof, renamed accessdir to access_dir and fix update

Test plans:
- Apply patch, update database
- Add to koha-conf:
<access_dir>/tmp/koha-public/one</access_dir>
<access_dir>/tmp/koha-public/two</access_dir>
<access_dir>/tmp/koha-public</access_dir>
- Create these directories ( mkdir /tmp/koha-public , etc...)
- Create these files:
echo "hello world!" > /tmp/koha-public/❤
echo "test" > /tmp/koha-public/one/samename.txt
echo "this is not the same" > /tmp/koha-public/two/samename.txt
- Login as Superadmin, go to tools > reports files
    - Click on ❤, make sure it's downloadable and readable
    - Click on both samename.txt, look inside and make sure the file is different
- Login as NON-superadmin. Go under tools, see no Report/Log under the third column
    - Go to add tools/access_file permission to user
    - See new entry under tools third column.
    - validate link is ok.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 146 Katrin Fischer 2018-04-28 00:35:02 UTC
Created attachment 74917 [details] [review]
Bug 11317: (follow-up) Add two-level entry to koha-conf and remove unwanted characters

- Removed merge marker
- Changed include path in favor of using the Asset tt plugin (bug 20538)
- Changed access_dir to a two-level entry for clarity

Test plans stay the same, just make sure that the two-level configuration entry
work properly and everything pass QA.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 147 Katrin Fischer 2018-04-28 00:35:10 UTC
Created attachment 74918 [details] [review]
Bug 11317: (QA follow-up) Fix merge error and punctuation

2 tiny fixes:
- Remove space before : as this is not correct in English
- Remove an errorenous commented line that probably snuck back
  in from a merge
Comment 148 Katrin Fischer 2018-04-28 00:35:18 UTC
Created attachment 74919 [details] [review]
Bug 11317: (QA follow-up) Change tool name to be more general

I think this tool could be used in multiple ways, for all kinds
of files that you want to protect by login in Koha. So a more
general name might work better.

This is a suggestion, not mandatory.
Comment 149 Jonathan Druart 2018-05-03 16:40:34 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 150 Jonathan Druart 2018-05-03 17:31:20 UTC
Created attachment 75019 [details] [review]
Bug 11317: Add id and class attributes to body

This fixes xt/tt_valid.t
 #   Failed test '<body> tag with id and class attributes'
 #   at xt/tt_valid.t line 91
 # Files list:
 # intranet-tmpl/prog/en/modules/tools/access_files.tt: 8
 # Looks like you failed 1 test of 3.
 [16:44:56] xt/tt_valid.t
 Dubious, test returned 1 (wstat 256, 0x100)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>