Receiver and Node Logo upload and default supports

fpietrosanti commented 11 years ago

In the Whistleblowing submission interface, it is shown a set of receivers that can receive submissions for a given context.

Those receivers also show an picture of the receivers.

Is not possible, from the admin interface, to configure the picture associated with the receiver.

hellais commented 11 years ago

The UI component for this has been implemented: Profile selection

It is still missing the logic for doing an upload with the backend, because we have not yet defined such interface.

Rethinking a bit about this issue I believe using jquery file uploader also for doing the uploads here is the best solution. @vecna what do you think?

vecna commented 11 years ago

I think is strictly related to #18 implementation, and with a dedicated file upload API for the admin, would be easy:

upload all the file admin likes
assign to a recipient the associated picture (eg: antani.jpg is the GLB logo, pippo.jpg is the face of Receiver1). This will need a selection window that show all the static files available (perhaps a little preview ? perhaps sorted by more recent uploaded ?) until admin select one file name.

the API will change in three section:

/admin/node (new not mandatory field: logo_file)
/admin/receiver (new not mandatory field: picture_file)
/admin/staticfiles (new CRUD)

Doubt:

need we to accept all the static file proposed by the admin, or we want enforce size/filetype (and perhaps, force them to be a generic static files, maybe a \ static thumbnails**)
(I willingfull removed a concern about kept a checksum inside of receiver/node table, in order to checks image consistencies, because at the moment security need to be guarantee in the node, but when we shift on encrypted database, this need to be included)

update after few minutes, @hellais

hellais commented 11 years ago

I have always found user interfaces where you have the list of all files that you have uploaded and you must choose amongst those the one that fits the case you are dealing with.

I find it more logical to have the user upload the file when they wish to change the image (after all a Receiver will not have more than 1 profile picture).

A practical reason for going for this solution is the fact that depending on the type of image being uploaded we will need to perform some server side transformations to it (scaling, for example). For this reason it is best to have knowledge of where the image will go at the time of upload.

I would suggest instead having an API like follows:

GET | POST /admin/node/upload-logo

GET | POST /admin/receiver/$receiver_id/profile-picture

In this case we should only be accepting file types that are images and perform scaling on the profile image to reduce it to 120x120px and on the logo image to 140x140.

vecna commented 11 years ago

if your concern are just about the UI: no problem, can be possible with easy upload a pic for a receiver, mark which receiver is addressed and scale in a thumbnail (anyway, is common, also in wordpress media gallery, having a list of files and select the preferred one for being included in the post, or set as post image)

but I'm working on an API that permit CRUD operation over static files (all was started here: https://github.com/globaleaks/GLBackend/issues/69 ). Its a more generic interface.

Client side would be simple, for the UI concern:

1) make a pic uploaded from the "Receiver" configuration page. 2) add a parameter in the POST, with "?usage=logo" or "?usage=UUID-OF-A-RECEIVER" and make the handler scale them.

being associated to a specific receiver, just require a POST parameter in the URL, but handler and operations remain the same. and this would handle all the static file an admin need to upload in the node (bootstrap.js theme, perhaps ?)

As default behavior, all the scaled thumbnail are stored as http://GLB/_static/UUID-OF-RECEIVER.png So can be easy also put a default pic for the receiver without an image.

vecna commented 11 years ago

Some new requirements would be present in GLBackend pip and system package.

PIL, Python Image Library, it's the library able to handle with easy image manipulation, and is used also inside django. PIL documentation for the Image module: http://www.pythonware.com/library/pil/handbook/image.htm

PIL require format dependent decoder. To import a JPEG, require the system package libjpeg-dev: http://stackoverflow.com/questions/8915296/decoder-jpeg-not-available-pil

and I've used "Pillow" fork of PIL, in requirements.txt, because supports zip/png natively.

fpietrosanti commented 11 years ago

Can't we avoid resizing picture to keep it simpler (code and dependencies) ?

If the size will be the same of gravatar or twitter, there will be no need to resize it as the end-user already have it.

vecna commented 11 years ago

I've already develop that code... anyway, I believe is not a big issue just insert a new line in requirements.txt and in apt-get, and this module (Image) would permit the generation of pseudo random picture for Receiver, in the case they without portrait.

commit in branch feature/thumbnails https://github.com/globaleaks/GLBackend/tree/feature/thumbnails

vecna commented 11 years ago

@fpietrosanti , I've thinked also if is possible avoid resize. It's difficult, to validate an input image you need to be able to acquire their size, and then you require a decoder for the JPEG/PNG format. then... decoder is needed also without resizing... and this permit us to implement our captcha, without use external services http://www.slideshare.net/AkramWaseem/random-and-dynamic-images-using-python-cgi

vecna commented 11 years ago

Static File upload specification

The branch feature/thumbnails extend the API with:

 POST /admin/staticfiles
 DELETE /admin/staticfiles/($filename)

POST in staticfiles works in the same way of /submission/$/files or /tip/$/files: expect the block of files upload by JqueryFileUploader format.

Is intended for the Node Admin, who need to upload files available as static files from the node. Every file uploaded thru this API, keep the same filename, and is downloadable without GLBackend interaction from $NODEURL/static/ path.

Portrait and Logo logic

Node Logo can be get by a static URL: $NODEURL/static/globaleaks_logo.png (the reserved name "globaleaks_logo" is specify in GLSetting

Receiver portrait can be get by static URL: $NODEURL/static/receiver-UUID_120.png (the 120x120 px) or $NODEURL/static/receiver-UUID_40.png

Upload of a new GlobaLeaks logo

To select an image as new logo, the admin has to:

upload just one file (one image)
specify in the URI, the option ?globaleaks_logo (POST $NODEURL/admin/staticfiles?globaleaks_logo)

This cause that the image is resized to 140x140, is renamed in globaleaks_logo.png overwriting the previous image. the original filename is ignored.

Upload a new Receiver Portrait

Same behavior of GlobaLeaks node, but in the URI query ?receiver-UUID need to be specified. the UUID is validated among the existent receiver, and the portrait files are saved in:

$NODEURL/static/$UUID_120.png
$NODEURL/static/$UUID_40.png

Next steps

implement tests
implement a default pic for Node logo
implement a default portrait for Receivers
update the GLClient with this new API

cc @evilaliv3 @hellais

vecna commented 11 years ago

Renamed ticket from: A receiver picture can't be configured from admin interface to Receiver and Node Logo upload and default supports

vecna commented 11 years ago

proposal of default pic for new nodes: globaleaks_logo

For the receiver ?

vecna commented 11 years ago

Note, in order to avoid mistakes, for some days shall we keep this alert:

 (glenv)vecna@X-4 ~/progetti/GLBackend $ bin/startglobaleaks  
 Since 20/03 a new library is required, and is missing in your environment
 Please execute:
 sudo apt-get install libjpeg-dev
 and:
 pip install -r requirements.txt

This would catch adopters/tester that have not update their system, it's a check implemented in startglobaleaks and is catch when tried to "import PIL" (documentation updated: https://github.com/globaleaks/GlobaLeaks/wiki/Linux )

hellais commented 11 years ago

For me this is ok. I would have gone with an API like the one I described above, but this is also fine and since it's implemented I would stick with this one.

Regarding the logo I find it a bit confusing as the font is not the default globaleaks font and the text "Your globaleaks" does not appear anywhere else.

I remember we had somewhere a globaleaks logo that was square and did not include any text, but only the two heads. I can produce one if you can attach the vector version of the GL logo (.pdf one).

vecna commented 11 years ago

it's not a problem produce the two head logo, I've added the "Your" globaleaks, to be sure that wannabe-transparency-initiative have clear the fact that need to change the logo, but, well, probably they will do anyway.

what about the default receiver pic ? a static one can fit well, at the moment I'm using a random gradient like the simplest from: http://lost-theory.org/python/dynamicimg.html

globaleaks_logo

vecna commented 11 years ago

The GLBackend side is completed (but missing the unitTest), I've put in GLClient a branch, where thumbnails are looked by default in the /static/ directory, for GL-logo and Receiver portrait. https://github.com/globaleaks/GLClient/tree/thumbnails-by-default https://github.com/globaleaks/GLBackend/pull/83

hellais commented 11 years ago

See the review in https://github.com/globaleaks/GLBackend/pull/83

hellais commented 11 years ago

This is implemented in the client, testing it against the branch in globaleaks/GLBackend#83.

vecna commented 11 years ago

@hellais there are an issue, apparently related to the client:

2013-03-28 16:49:54+0100 [-] 200 GET /admin/receiver (127.0.0.1) 182.68ms 2013-03-28 16:49:54+0100 [-] 200 GET /admin/node (127.0.0.1) 182.71ms 2013-03-28 16:49:54+0100 [-] 200 GET /admin/notification (127.0.0.1) 184.33ms 2013-03-28 16:50:06+0100 [HTTPConnection,0,127.0.0.1] 412 POST /admin/staticfiles?globaleaks_logo (127.0.0.1) 1.04ms

also if the user is authenticated, the error message receiver is 412, like is missing the X-Session or the cookie (well, dumping the session header, at the moment, the cookie is present but not the X-Session, I believe need to be the opposite, right ?)

hellais commented 11 years ago

@vecna, yes you are right. This is happening, because jQuery File uploader does not handle the HTTP Heading injection business that we do with all the other HTTP requests.

Can you also check out if this bug also happens in the /tip/ page of the WB. If it doesn't work here, it must not also work there, and if it doesn't then it's probably a security bug.

vecna commented 11 years ago

@ħellais gotcha! there are a security bug (missing check).

vecna commented 11 years ago

at the moment integrated in master (GLClient and GLBackend). still open security bug related to JQFU and X-Session

vecna commented 11 years ago

switched the flags from: Admin GLB GLC MissinFeature, to, GLC Security bug

hellais commented 11 years ago

This has been fixed and implemented in: https://github.com/globaleaks/GLClient/commit/9eb120dbe26a0b6f5a18379930b989c51b72b701

globaleaks / globaleaks-whistleblowing-software