From 8b1a05d9e09dca317ca38422d75f459a7e1784aa Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Mon, 19 Nov 2018 14:36:31 +0000 Subject: [PATCH 1/2] [FIX] web_unsplash: don't expose client_id in network Unsplash CEO asked us to stop exposing our key client side (even if it was only in the browser network and previously allowed by their API team leader). After multiple exchange, it has been negociated that: 1. Our non saas user won't be able to apply for a production key and should prefix they Unsplash application name by 'Odoo:'. Their key will remain in test mode. Indeed, Unsplash won't be able to review every Odoo application and ensure it respects the API terms and it is a real application. 2. Our saas user will query Unsplash with a dedicated enterprise key 3. The documentation should explain it all Task-1911345 --- addons/web_unsplash/__manifest__.py | 2 +- addons/web_unsplash/controllers/main.py | 52 ++++++++++++++++--- .../static/src/js/unsplash_image_widget.js | 6 +-- .../web_unsplash/static/src/js/unsplashapi.js | 43 ++++----------- .../static/src/xml/unsplash_image_widget.xml | 2 +- 5 files changed, 57 insertions(+), 48 deletions(-) diff --git a/addons/web_unsplash/__manifest__.py b/addons/web_unsplash/__manifest__.py index 3ad4b1334b3..a139e64743b 100644 --- a/addons/web_unsplash/__manifest__.py +++ b/addons/web_unsplash/__manifest__.py @@ -4,7 +4,7 @@ 'name': 'Unsplash Image Library', 'category': 'Web', 'summary': 'Find free high-resolution images from Unsplash', - 'version': '1.0', + 'version': '1.1', 'description': """Explore the free high-resolution image library of Unsplash.com and find images to use in Odoo. An Unsplash search bar is added to the image library modal.""", 'depends': ['base_setup', 'web_editor'], 'data': [ diff --git a/addons/web_unsplash/controllers/main.py b/addons/web_unsplash/controllers/main.py index c53c03ba5bf..7a53cd8e115 100644 --- a/addons/web_unsplash/controllers/main.py +++ b/addons/web_unsplash/controllers/main.py @@ -9,12 +9,34 @@ import werkzeug.utils from PIL import Image from odoo import http, tools, _ from odoo.http import request +from werkzeug.urls import url_encode logger = logging.getLogger(__name__) class Web_Unsplash(http.Controller): + def _get_access_key(self): + if request.env.user._has_unsplash_key_rights(): + return request.env['ir.config_parameter'].sudo().get_param('unsplash.access_key') + raise werkzeug.exceptions.NotFound() + + def _notify_download(self, url): + ''' Notifies Unsplash from an image download. (API requirement) + :param url: the download_url of the image to be notified + + This method won't return anything. This endpoint should just be + pinged with a simple GET request for Unsplash to increment the image + view counter. + ''' + try: + if not url.startswith('https://api.unsplash.com/photos/'): + raise Exception(_("ERROR: Unknown Unsplash notify URL!")) + access_key = self._get_access_key() + requests.get(url, params=url_encode({'client_id': access_key})) + except Exception as e: + logger.exception("Unsplash download notification failed: " + str(e)) + # ------------------------------------------------------ # add unsplash image url # ------------------------------------------------------ @@ -22,8 +44,14 @@ class Web_Unsplash(http.Controller): def save_unsplash_url(self, unsplashurls=None, **kwargs): """ unsplashurls = { - image_id1: image_url1, - image_id2: image_url2, + image_id1: { + url: image_url, + download_url: download_url, + }, + image_id2: { + url: image_url, + download_url: download_url, + }, ..... } """ @@ -82,13 +110,23 @@ class Web_Unsplash(http.Controller): attachment.generate_access_token() uploads.extend(attachment.read(['name', 'mimetype', 'checksum', 'res_id', 'res_model', 'access_token', 'url'])) + # Notifies Unsplash from an image download. (API requirement) + self._notify_download(value.get('download_url')) + return uploads - @http.route("/web_unsplash/get_client_id", type='json', auth="user") - def get_unsplash_client_id(self, **post): - if request.env.user._has_unsplash_key_rights(): - return request.env['ir.config_parameter'].sudo().get_param('unsplash.access_key') - raise werkzeug.exceptions.NotFound() + @http.route("/web_unsplash/fetch_images", type='json', auth="user") + def fetch_unsplash_images(self, **post): + access_key = self._get_access_key() + app_id = self.get_unsplash_app_id() + if not access_key or not app_id: + return {'error': 'key_not_found'} + post['client_id'] = access_key + response = requests.get('https://api.unsplash.com/search/photos/', params=url_encode(post)) + if response.status_code == requests.codes.ok: + return response.json() + else: + return {'error': response.status_code} @http.route("/web_unsplash/get_app_id", type='json', auth="public") def get_unsplash_app_id(self, **post): diff --git a/addons/web_unsplash/static/src/js/unsplash_image_widget.js b/addons/web_unsplash/static/src/js/unsplash_image_widget.js index d353694ddea..ab900d6a03c 100644 --- a/addons/web_unsplash/static/src/js/unsplash_image_widget.js +++ b/addons/web_unsplash/static/src/js/unsplash_image_widget.js @@ -88,10 +88,6 @@ ImageWidget.include({ res_id: self.options.res_id, } }).then(function (images) { - for (var img in self._unsplash.selectedImages) { - self.unsplashAPI.notifyDownload(self._unsplash.selectedImages[img].download_url); - } - _.each(images, function (image) { image.src = image.url; image.isDocument = !(/gif|jpe|jpg|png/.test(image.mimetype)); @@ -140,7 +136,7 @@ ImageWidget.include({ self.$('.unsplash_img_container').html(QWeb.render('web_unsplash.dialog.image.content', { rows: rows })); self._highlightSelectedImages(); }).fail(function (err) { - self.$('.unsplash_img_container').html(QWeb.render('web_unsplash.dialog.error.content', err)); + self.$('.unsplash_img_container').html(QWeb.render('web_unsplash.dialog.error.content', { status: err })); }).always(function () { self._toggleAttachmentContaines(false); }); diff --git a/addons/web_unsplash/static/src/js/unsplashapi.js b/addons/web_unsplash/static/src/js/unsplashapi.js index 0ceb2b8f8a7..a6ec5b75324 100644 --- a/addons/web_unsplash/static/src/js/unsplashapi.js +++ b/addons/web_unsplash/static/src/js/unsplashapi.js @@ -38,45 +38,15 @@ var UnsplashCore = Class.extend(Mixins.EventDispatcherMixin, ServicesMixin, { if (cachedData && (cachedData.images.length >= to || (cachedData.totalImages !== 0 && cachedData.totalImages < to))) { return $.when({ images: cachedData.images.slice(from, to), isMaxed: to > cachedData.totalImages }); } - return this._getAPIKey().then(function (clientID) { - if (!clientID) { - return $.Deferred().reject({ key_not_found: true }); - } - return self._fetchImages(query).then(function (cachedData) { - return { images: cachedData.images.slice(from, to), isMaxed: to > cachedData.totalImages }; - }); + return self._fetchImages(query).then(function (cachedData) { + return { images: cachedData.images.slice(from, to), isMaxed: to > cachedData.totalImages }; }); }, - /** - * Notifies Unsplash from an image download. (API requirement) - * - * @param {String} url url of the image to notify - */ - notifyDownload: function (url) { - $.get(url, { client_id: this.clientId }); - }, //-------------------------------------------------------------------------- // Private //-------------------------------------------------------------------------- - /** - * Checks and retrieves the unsplash API key - * - * @private - */ - _getAPIKey: function () { - var self = this; - if (this.clientId) { - return $.Deferred().resolve(self.clientId); - } - return this._rpc({ - route: '/web_unsplash/get_client_id', - }).then(function (res) { - self.clientId = res; - return res; - }); - }, /** * Fetches images from unsplash and stores it in cache * @@ -96,10 +66,15 @@ var UnsplashCore = Class.extend(Mixins.EventDispatcherMixin, ServicesMixin, { var payload = { query: query, page: cachedData.pageCached + 1, - client_id: this.clientId, per_page: 30, // max size from unsplash API }; - return $.get('https://api.unsplash.com/search/photos/', payload).then(function (result) { + return this._rpc({ + route: '/web_unsplash/fetch_images', + params: payload, + }).then(function (result) { + if (result.error) { + return $.Deferred().reject(result.error); + } cachedData.pageCached++; cachedData.images.push.apply(cachedData.images, result.results); cachedData.maxPages = result.total_pages; diff --git a/addons/web_unsplash/static/src/xml/unsplash_image_widget.xml b/addons/web_unsplash/static/src/xml/unsplash_image_widget.xml index 409c435884b..05202e073b2 100644 --- a/addons/web_unsplash/static/src/xml/unsplash_image_widget.xml +++ b/addons/web_unsplash/static/src/xml/unsplash_image_widget.xml @@ -50,7 +50,7 @@
- + Unsplash requires an access key and an application ID From 55073e3baa97fa1285695233f30044bd7c698858 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Mon, 19 Nov 2018 17:09:40 +0000 Subject: [PATCH 2/2] [FIX] web_unsplash: add unsplash searched terms in attachement name/url Before this commit: An unsplash image would be saved as an attachement with: - A name like 'x54ze78z.JPEG' - An url like '/unsplash/x54ze78z.JPEG' - A filename like 'x54ze78z.JPGED' We would not store the searched terms in the name/url. Then, we would not be able to search on local attachement in the media dialog to retrieve the images based on what we searched. Now: We store the searched terms in the name/url so it can be used later in the media dialog search: - Name like 'big_lion.jpg' (duplicates may be possible) - Url like '/unsplash/x54ze78z/big_lion.jpg' - Filename like 'unsplash_x54ze78z_big_lion.jpg' Task-1911345 --- addons/web_unsplash/controllers/main.py | 24 ++++++++++++++----- .../static/src/js/unsplash_beacon.js | 4 ++-- .../static/src/js/unsplash_image_widget.js | 1 + 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/addons/web_unsplash/controllers/main.py b/addons/web_unsplash/controllers/main.py index 7a53cd8e115..219342d1c39 100644 --- a/addons/web_unsplash/controllers/main.py +++ b/addons/web_unsplash/controllers/main.py @@ -55,12 +55,23 @@ class Web_Unsplash(http.Controller): ..... } """ + def slugify(s): + ''' Keeps only alphanumeric characters, hyphens and spaces from a string. + The string will also be truncated to 1024 characters max. + :param s: the string to be filtered + :return: the sanitized string + ''' + return "".join([c for c in s if c.isalnum() or c in list("- ")])[:1024] + if not unsplashurls: return [] uploads = [] Attachments = request.env['ir.attachment'] + query = kwargs.get('query', '') + query = slugify(query) + res_model = kwargs.get('res_model', 'ir.ui.view') if res_model != 'ir.ui.view' and kwargs.get('res_id'): res_id = int(kwargs['res_id']) @@ -87,20 +98,21 @@ class Web_Unsplash(http.Controller): logger.exception("Timeout: " + str(e)) continue - name = key - # optimized image before save if mimetype in ('image/jpeg', 'image/png'): image = Image.open(io.BytesIO(datas)) if image.format in ('PNG', 'JPEG'): datas = tools.image_save_for_web(image) # append image extension in name - name += '.' + image.format + query += '.' + str.lower(image.format) + + # /unsplash/5gR788gfd/lion + url_frags = ['unsplash', key, query] attachment = Attachments.create({ - 'name': name, - 'url': '/unsplash/' + name, - 'datas_fname': name, + 'name': query, + 'url': '/' + '/'.join(url_frags), + 'datas_fname': '_'.join(url_frags), 'mimetype': mimetype, 'datas': base64.b64encode(datas), 'public': res_model == 'ir.ui.view', diff --git a/addons/web_unsplash/static/src/js/unsplash_beacon.js b/addons/web_unsplash/static/src/js/unsplash_beacon.js index f8e74a3b759..179d566cee3 100644 --- a/addons/web_unsplash/static/src/js/unsplash_beacon.js +++ b/addons/web_unsplash/static/src/js/unsplash_beacon.js @@ -7,8 +7,8 @@ var ajax = require('web.ajax'); base.ready().then(function () { var unsplash_images = []; _.each($('img[src*="/unsplash/"]'), function (img, index) { - // get image id from URL (`/unsplash/xYdf5feoI.jpg` -> `xYdf5feoI`) - unsplash_images.push(img.src.split('/unsplash/')[1].split('.')[0]); + // get image id from URL (`http://www.domain.com:1234/unsplash/xYdf5feoI/lion.jpg` -> `xYdf5feoI`) + unsplash_images.push(img.src.split('/unsplash/')[1].split('/')[0]); }); if (unsplash_images.length) { ajax.jsonRpc('/web_unsplash/get_app_id', 'call').then(function (appID) { diff --git a/addons/web_unsplash/static/src/js/unsplash_image_widget.js b/addons/web_unsplash/static/src/js/unsplash_image_widget.js index ab900d6a03c..6eb393d3798 100644 --- a/addons/web_unsplash/static/src/js/unsplash_image_widget.js +++ b/addons/web_unsplash/static/src/js/unsplash_image_widget.js @@ -86,6 +86,7 @@ ImageWidget.include({ unsplashurls: self._unsplash.selectedImages, res_model : self.options.res_model, res_id: self.options.res_id, + query: self._unsplash.query, } }).then(function (images) { _.each(images, function (image) {