From 1e43c16654d3bb69178336c9b7d6c7df129028d0 Mon Sep 17 00:00:00 2001 From: Ifedapo Olarewaju Date: Sun, 13 May 2018 19:40:57 +0100 Subject: [PATCH 1/2] refactor: use local storage to store provider token --- src/plugins/Instagram/index.js | 5 +++ src/plugins/Tus.js | 27 +++++----------- src/plugins/Url/index.js | 1 - src/plugins/XHRUpload.js | 57 +++++++++++++-------------------- src/server/Provider.js | 15 +++++++++ src/server/RequestClient.js | 38 ++++++++++++++-------- src/views/ProviderView/index.js | 36 +++++++-------------- 7 files changed, 86 insertions(+), 93 deletions(-) diff --git a/src/plugins/Instagram/index.js b/src/plugins/Instagram/index.js index f6bafaa7c..71dc23f94 100644 --- a/src/plugins/Instagram/index.js +++ b/src/plugins/Instagram/index.js @@ -84,6 +84,11 @@ module.exports = class Instagram extends Plugin { } getItemIcon (item) { + if (!item.images) { + return + + + } return } diff --git a/src/plugins/Tus.js b/src/plugins/Tus.js index e0c3c3d42..4fdcf1f82 100644 --- a/src/plugins/Tus.js +++ b/src/plugins/Tus.js @@ -238,31 +238,20 @@ module.exports = class Tus extends Plugin { .catch(reject) } - fetch(file.remote.url, { - method: 'post', - credentials: 'include', - headers: { - 'Accept': 'application/json', - 'Content-Type': 'application/json' - }, - body: JSON.stringify(Object.assign({}, file.remote.body, { + this.uppy.emit('upload-started', file) + file.remote.provider.post( + file.remote.url, + Object.assign({}, file.remote.body, { endpoint: opts.endpoint, uploadUrl: opts.uploadUrl, protocol: 'tus', size: file.data.size, metadata: file.meta - })) - }) - .then((res) => { - if (res.status < 200 || res.status > 300) { - return reject(res.statusText) - } - - return res.json().then((data) => { - this.uppy.setFileState(file.id, { serverToken: data.token }) - file = this.uppy.getFile(file.id) - return file }) + ).then((res) => { + this.uppy.setFileState(file.id, { serverToken: res.token }) + file = this.getFile(file.id) + return file }) .then((file) => { return this.connectToServerSocket(file) diff --git a/src/plugins/Url/index.js b/src/plugins/Url/index.js index 125be5867..38716cdfc 100644 --- a/src/plugins/Url/index.js +++ b/src/plugins/Url/index.js @@ -4,7 +4,6 @@ const { h } = require('preact') const { RequestClient } = require('../../server') const UrlUI = require('./UrlUI.js') const { toArray } = require('../../core/Utils') -require('whatwg-fetch') /** * Url diff --git a/src/plugins/XHRUpload.js b/src/plugins/XHRUpload.js index ba1a65a53..09315ddb4 100644 --- a/src/plugins/XHRUpload.js +++ b/src/plugins/XHRUpload.js @@ -310,49 +310,38 @@ module.exports = class XHRUpload extends Plugin { fields[name] = file.meta[name] }) - fetch(file.remote.url, { - method: 'post', - credentials: 'include', - headers: { - 'Accept': 'application/json', - 'Content-Type': 'application/json' - }, - body: JSON.stringify(Object.assign({}, file.remote.body, { + file.remote.provider.post( + file.remote.url, + Object.assign({}, file.remote.body, { endpoint: opts.endpoint, size: file.data.size, fieldname: opts.fieldName, metadata: fields, headers: opts.headers - })) - }) + }) + ) .then((res) => { - if (res.status < 200 && res.status > 300) { - return reject(res.statusText) - } + const token = res.token + const host = getSocketHost(file.remote.host) + const socket = new UppySocket({ target: `${host}/api/${token}` }) - res.json().then((data) => { - const token = data.token - const host = getSocketHost(file.remote.host) - const socket = new UppySocket({ target: `${host}/api/${token}` }) + socket.on('progress', (progressData) => emitSocketProgress(this, progressData, file)) - socket.on('progress', (progressData) => emitSocketProgress(this, progressData, file)) + socket.on('success', (data) => { + const resp = opts.getResponseData(data.response.responseText, data.response) + const uploadURL = resp[opts.responseUrlFieldName] + this.uppy.emit('upload-success', file, resp, uploadURL) + socket.close() + return resolve() + }) - socket.on('success', (data) => { - const resp = opts.getResponseData(data.response.responseText, data.response) - const uploadURL = resp[opts.responseUrlFieldName] - this.uppy.emit('upload-success', file, resp, uploadURL) - socket.close() - return resolve() - }) - - socket.on('error', (errData) => { - const resp = errData.response - const error = resp - ? opts.getResponseError(resp.responseText, resp) - : Object.assign(new Error(errData.error.message), { cause: errData.error }) - this.uppy.emit('upload-error', file, error) - reject(error) - }) + socket.on('error', (errData) => { + const resp = errData.response + const error = resp + ? opts.getResponseError(resp.responseText, resp) + : Object.assign(new Error(errData.error.message), { cause: errData.error }) + this.uppy.emit('upload-error', file, error) + reject(error) }) }) }) diff --git a/src/server/Provider.js b/src/server/Provider.js index b829bbe87..cbba152a1 100644 --- a/src/server/Provider.js +++ b/src/server/Provider.js @@ -14,6 +14,17 @@ module.exports = class Provider extends RequestClient { this.id = this.provider this.authProvider = opts.authProvider || this.provider this.name = this.opts.name || _getName(this.id) + this.tokenKey = `uppy-server-${this.id}-auth-token` + } + + get defaultHeaders () { + return Object.assign({}, super.defaultHeaders, {'uppy-auth-token': localStorage.getItem(this.tokenKey)}) + } + + // @todo(i.olarewaju) consider whether or not this method should be exposed + setAuthToken (token) { + // @todo(i.olarewaju) add fallback for OOM storage + localStorage.setItem(this.tokenKey, token) } checkAuth () { @@ -37,5 +48,9 @@ module.exports = class Provider extends RequestClient { logout (redirect = location.href) { return this.get(`${this.id}/logout?redirect=${redirect}`) + .then((res) => { + localStorage.removeItem(this.tokenKey) + return res + }) } } diff --git a/src/server/RequestClient.js b/src/server/RequestClient.js index 80ef057ba..d7bdcb65f 100644 --- a/src/server/RequestClient.js +++ b/src/server/RequestClient.js @@ -15,6 +15,13 @@ module.exports = class RequestClient { return uppyServer && uppyServer[host] ? uppyServer[host] : host } + get defaultHeaders () { + return { + 'Accept': 'application/json', + 'Content-Type': 'application/json' + } + } + onReceiveResponse (response) { const state = this.uppy.getState() const uppyServer = state.uppyServer || {} @@ -32,13 +39,9 @@ module.exports = class RequestClient { } get (path) { - return fetch(`${this.hostname}/${path}`, { + return fetch(this._getUrl(path), { method: 'get', - credentials: 'include', - headers: { - 'Accept': 'application/json', - 'Content-Type': 'application/json' - } + headers: this.defaultHeaders }) // @todo validate response status before calling json .then(this.onReceiveResponse) @@ -46,18 +49,25 @@ module.exports = class RequestClient { } post (path, data) { - return fetch(`${this.hostname}/${path}`, { + return fetch(this._getUrl(path), { method: 'post', - credentials: 'include', - headers: { - 'Accept': 'application/json', - 'Content-Type': 'application/json' - }, + headers: this.defaultHeaders, body: JSON.stringify(data) }) .then(this.onReceiveResponse) - // @todo validate response status before calling json - .then((res) => res.json()) + .then((res) => { + if (res.status < 200 || res.status > 300) { + throw new Error(res.statusText) + } + return res.json() + }) + } + + _getUrl (url) { + if (url.startsWith('http:') || url.startsWith('https:')) { + return url + } + return `${this.hostname}/${url}` } delete (path, data) { diff --git a/src/views/ProviderView/index.js b/src/views/ProviderView/index.js index a5b9858c9..ff6c1fef0 100644 --- a/src/views/ProviderView/index.js +++ b/src/views/ProviderView/index.js @@ -186,7 +186,8 @@ module.exports = class ProviderView { url: `${this.Provider.fileUrl(this.plugin.getItemRequestPath(file))}`, body: { fileId: this.plugin.getItemId(file) - } + }, + provider: this.Provider } } @@ -446,35 +447,20 @@ module.exports = class ProviderView { } handleAuth () { - const urlId = Math.floor(Math.random() * 999999) + 1 - const redirect = `${location.href}${location.search ? '&' : '?'}id=${urlId}` - - const authState = btoa(JSON.stringify({ redirect })) + const authState = btoa(JSON.stringify({ origin: location.origin })) const link = `${this.Provider.authUrl()}?state=${authState}` const authWindow = window.open(link, '_blank') - authWindow.opener = null - const checkAuth = () => { - let authWindowUrl - - try { - authWindowUrl = authWindow.location.href - } catch (e) { - if (e instanceof DOMException || e instanceof TypeError) { - return setTimeout(checkAuth, 100) - } else throw e - } - - // split url because chrome adds '#' to redirects - if (authWindowUrl && authWindowUrl.split('#')[0] === redirect) { - authWindow.close() - this._loaderWrapper(this.Provider.checkAuth(), this.plugin.onAuth, this.handleError) - } else { - setTimeout(checkAuth, 100) + const handleToken = (e) => { + if (e.origin !== this.plugin.opts.host || e.source !== authWindow) { + return } + authWindow.close() + window.removeEventListener('message', handleToken) + this.Provider.setAuthToken(e.data.token) + this._loaderWrapper(this.Provider.checkAuth(), this.plugin.onAuth, this.handleError) } - - checkAuth() + window.addEventListener('message', handleToken) } handleError (error) { From 34d45576497fe08a62eacd312b0bfb741d6bf8af Mon Sep 17 00:00:00 2001 From: Ifedapo Olarewaju Date: Wed, 23 May 2018 10:23:58 +0100 Subject: [PATCH 2/2] review: store providers as options --- src/plugins/Tus.js | 6 ++++-- src/plugins/XHRUpload.js | 4 +++- src/server/RequestClient.js | 2 +- src/views/ProviderView/index.js | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/plugins/Tus.js b/src/plugins/Tus.js index 4fdcf1f82..d47d7ace2 100644 --- a/src/plugins/Tus.js +++ b/src/plugins/Tus.js @@ -1,6 +1,7 @@ const Plugin = require('../core/Plugin') const tus = require('tus-js-client') const UppySocket = require('../core/UppySocket') +const Provider = require('../server/Provider') const { emitSocketProgress, getSocketHost, @@ -239,7 +240,8 @@ module.exports = class Tus extends Plugin { } this.uppy.emit('upload-started', file) - file.remote.provider.post( + const provider = new Provider(this.uppy, file.remote.providerOptions) + provider.post( file.remote.url, Object.assign({}, file.remote.body, { endpoint: opts.endpoint, @@ -250,7 +252,7 @@ module.exports = class Tus extends Plugin { }) ).then((res) => { this.uppy.setFileState(file.id, { serverToken: res.token }) - file = this.getFile(file.id) + file = this.uppy.getFile(file.id) return file }) .then((file) => { diff --git a/src/plugins/XHRUpload.js b/src/plugins/XHRUpload.js index 09315ddb4..1923d0e6b 100644 --- a/src/plugins/XHRUpload.js +++ b/src/plugins/XHRUpload.js @@ -2,6 +2,7 @@ const Plugin = require('../core/Plugin') const cuid = require('cuid') const Translator = require('../core/Translator') const UppySocket = require('../core/UppySocket') +const Provider = require('../server/Provider') const { emitSocketProgress, getSocketHost, @@ -310,7 +311,8 @@ module.exports = class XHRUpload extends Plugin { fields[name] = file.meta[name] }) - file.remote.provider.post( + const provider = new Provider(this.uppy, file.remote.providerOptions) + provider.post( file.remote.url, Object.assign({}, file.remote.body, { endpoint: opts.endpoint, diff --git a/src/server/RequestClient.js b/src/server/RequestClient.js index d7bdcb65f..01c9edc7f 100644 --- a/src/server/RequestClient.js +++ b/src/server/RequestClient.js @@ -64,7 +64,7 @@ module.exports = class RequestClient { } _getUrl (url) { - if (url.startsWith('http:') || url.startsWith('https:')) { + if (/^https?:/.test(url)) { return url } return `${this.hostname}/${url}` diff --git a/src/views/ProviderView/index.js b/src/views/ProviderView/index.js index ff6c1fef0..fd87bbe7e 100644 --- a/src/views/ProviderView/index.js +++ b/src/views/ProviderView/index.js @@ -187,7 +187,7 @@ module.exports = class ProviderView { body: { fileId: this.plugin.getItemId(file) }, - provider: this.Provider + providerOptions: this.Provider.opts } }