Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provider user sessions #4619

Merged
merged 38 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
df98cca
remove useless line
mifi Aug 9, 2023
9ccf2ae
fix broken cookie removal logic
mifi Aug 9, 2023
04128f9
fix mime type of thumbnails
mifi Aug 9, 2023
9ceac7c
simplify/speedup token generation
mifi Aug 9, 2023
600b2d0
use instanceof instead of prop check
mifi Aug 9, 2023
da67ac9
Implement alternative provider auth
mifi Aug 9, 2023
7d59489
refactor
mifi Aug 13, 2023
ab06da6
use respondWithError
mifi Aug 13, 2023
16d4e3f
fix prepareStream
mifi Aug 13, 2023
2092387
don't throw when missing i18n key
mifi Aug 13, 2023
f575dbc
fix bugged try/catch
mifi Aug 13, 2023
754e2e0
allow aborting login too
mifi Aug 13, 2023
6441123
add json http error support
mifi Aug 13, 2023
f503d1f
don't tightly couple auth form with html form
mifi Aug 13, 2023
272e3d1
fix i18n
mifi Aug 14, 2023
f2e5aa4
make contentType parameterized
mifi Aug 14, 2023
8016a5d
merge main
mifi Sep 6, 2023
9f160f4
Merge branch 'main' into provider-user-sessions
mifi Sep 6, 2023
a28fcec
allow sending certain errors to the user
mifi Sep 6, 2023
3166db9
don't have default content-type
mifi Sep 6, 2023
5c20186
make a loginSimpleAuth api too
mifi Sep 7, 2023
b33f5b1
Merge branch 'main' into provider-user-sessions
mifi Oct 2, 2023
7549da4
make removeAuthToken protected
mifi Oct 2, 2023
8794f63
Merge branch 'main' into provider-user-sessions
mifi Nov 27, 2023
42f74b2
fix lint
mifi Nov 27, 2023
7a84c8d
run yarn format
mifi Nov 27, 2023
38eee72
Apply suggestions from code review
mifi Nov 29, 2023
70a7a48
fix broken merge conflict
mifi Nov 29, 2023
0d81a9b
improve inheritance
mifi Dec 1, 2023
96eb565
fix bug
mifi Dec 1, 2023
67d8595
fix bug with dynamic grant config
mifi Dec 2, 2023
5025a73
use duck typing for error checks
mifi Dec 5, 2023
761dcdc
Apply suggestions from code review
mifi Dec 5, 2023
95c8e38
Merge branch 'main' into provider-user-sessions
mifi Dec 5, 2023
64ce471
fix broken lint fix script
mifi Dec 5, 2023
654da1a
fix broken merge code
mifi Dec 5, 2023
a69a1fe
try to fix flakey tets
mifi Dec 5, 2023
6d766cb
fix lint
mifi Dec 5, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions e2e/cypress/integration/dashboard-tus.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import {
interceptCompanionUrlRequest,
interceptCompanionUnsplashRequest,
runRemoteUrlImageUploadTest,
runRemoteUnsplashUploadTest,
} from './reusable-tests'
Expand All @@ -15,8 +13,6 @@ describe('Dashboard with Tus', () => {
cy.intercept('/files/*').as('tus')
cy.intercept({ method: 'POST', pathname: '/files' }).as('post')
cy.intercept({ method: 'PATCH', pathname: '/files/*' }).as('patch')
interceptCompanionUrlRequest()
interceptCompanionUnsplashRequest()
})

it('should upload cat image successfully', () => {
Expand Down
14 changes: 7 additions & 7 deletions e2e/cypress/integration/dashboard-xhr.spec.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import {
interceptCompanionUrlRequest,
interceptCompanionUnsplashRequest,
interceptCompanionUrlMetaRequest,
runRemoteUrlImageUploadTest,
runRemoteUnsplashUploadTest,
} from './reusable-tests'

describe('Dashboard with XHR', () => {
beforeEach(() => {
cy.visit('/dashboard-xhr')
interceptCompanionUrlRequest()
interceptCompanionUnsplashRequest()
})

it('should upload remote image with URL plugin', () => {
Expand All @@ -22,8 +19,9 @@ describe('Dashboard with XHR', () => {
cy.get('.uppy-Url-input').type(
'http://localhost:4678/file-with-content-disposition',
)
interceptCompanionUrlMetaRequest()
cy.get('.uppy-Url-importButton').click()
cy.wait('@url').then(() => {
cy.wait('@url-meta').then(() => {
cy.get('.uppy-Dashboard-Item-name').should('contain', fileName)
cy.get('.uppy-Dashboard-Item-status').should('contain', '84 KB')
})
Expand All @@ -32,8 +30,9 @@ describe('Dashboard with XHR', () => {
it('should return correct file name with URL plugin from remote image without Content-Disposition', () => {
cy.get('[data-cy="Url"]').click()
cy.get('.uppy-Url-input').type('http://localhost:4678/file-no-headers')
interceptCompanionUrlMetaRequest()
cy.get('.uppy-Url-importButton').click()
cy.wait('@url').then(() => {
cy.wait('@url-meta').then(() => {
cy.get('.uppy-Dashboard-Item-name').should('contain', 'file-no')
cy.get('.uppy-Dashboard-Item-status').should('contain', '0')
})
Expand All @@ -50,8 +49,9 @@ describe('Dashboard with XHR', () => {
cy.get('.uppy-Url-input').type(
'http://localhost:4678/file-with-content-disposition',
)
interceptCompanionUrlMetaRequest()
cy.get('.uppy-Url-importButton').click()
cy.wait('@url').then(() => {
cy.wait('@url-meta').then(() => {
cy.get('.uppy-Dashboard-Item-name').should('contain', 'file-with')
cy.get('.uppy-Dashboard-Item-status').should('contain', '123 B')
})
Expand Down
25 changes: 19 additions & 6 deletions e2e/cypress/integration/reusable-tests.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
/* global cy */

export const interceptCompanionUrlRequest = () =>
cy.intercept('http://localhost:3020/url/*').as('url')
export const interceptCompanionUnsplashRequest = () =>
cy.intercept('http://localhost:3020/search/unsplash/*').as('unsplash')
const interceptCompanionUrlRequest = () =>
cy
.intercept({ method: 'POST', url: 'http://localhost:3020/url/get' })
.as('url')
export const interceptCompanionUrlMetaRequest = () =>
cy
.intercept({ method: 'POST', url: 'http://localhost:3020/url/meta' })
.as('url-meta')

export function runRemoteUrlImageUploadTest() {
cy.get('[data-cy="Url"]').click()
cy.get('.uppy-Url-input').type(
'https://raw.githubusercontent.com/transloadit/uppy/main/e2e/cypress/fixtures/images/cat.jpg',
)
cy.get('.uppy-Url-importButton').click()
interceptCompanionUrlRequest()
cy.get('.uppy-StatusBar-actionBtn--upload').click()
cy.wait('@url').then(() => {
cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete')
Expand All @@ -20,8 +25,12 @@ export function runRemoteUrlImageUploadTest() {
export function runRemoteUnsplashUploadTest() {
cy.get('[data-cy="Unsplash"]').click()
cy.get('.uppy-SearchProvider-input').type('book')
cy.intercept({
method: 'GET',
url: 'http://localhost:3020/search/unsplash/list?q=book',
}).as('unsplash-list')
cy.get('.uppy-SearchProvider-searchButton').click()
cy.wait('@unsplash')
cy.wait('@unsplash-list')
// Test that the author link is visible
cy.get('.uppy-ProviderBrowserItem')
.first()
Expand All @@ -34,8 +43,12 @@ export function runRemoteUnsplashUploadTest() {
cy.get('a').should('have.css', 'display', 'block')
})
cy.get('.uppy-c-btn-primary').click()
cy.intercept({
method: 'POST',
url: 'http://localhost:3020/search/unsplash/get/*',
}).as('unsplash-get')
cy.get('.uppy-StatusBar-actionBtn--upload').click()
cy.wait('@unsplash').then(() => {
cy.wait('@unsplash-get').then(() => {
cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete')
})
}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
"contributors:save": "yarn node ./bin/update-contributors.mjs",
"dev:with-companion": "npm-run-all --parallel start:companion dev",
"dev": "yarn workspace @uppy-dev/dev dev",
"lint:fix": "yarn run lint -- --fix",
"lint:fix": "yarn lint --fix",
"lint:markdown": "remark -f -q -i .remarkignore . .github/CONTRIBUTING.md",
"lint:staged": "lint-staged",
"lint:css": "stylelint ./packages/**/*.scss",
Expand Down
1 change: 1 addition & 0 deletions packages/@uppy/box/src/Box.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export default class Box extends UIPlugin {
companionCookiesRule: this.opts.companionCookiesRule,
provider: 'box',
pluginId: this.id,
supportsRefreshToken: false,
})

this.defaultLocale = locale
Expand Down
5 changes: 4 additions & 1 deletion packages/@uppy/companion-client/src/AuthError.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
'use strict'

class AuthError extends Error {
constructor () {
constructor() {
super('Authorization required')
this.name = 'AuthError'

// we use a property because of instanceof is unsafe:
// https://github.com/transloadit/uppy/pull/4619#discussion_r1406225982
this.isAuthError = true
}
}
Expand Down
86 changes: 60 additions & 26 deletions packages/@uppy/companion-client/src/Provider.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
'use strict'

import RequestClient from './RequestClient.js'
import RequestClient, { authErrorStatusCode } from './RequestClient.js'
import * as tokenStorage from './tokenStorage.js'


const getName = (id) => {
return id.split('-').map((s) => s.charAt(0).toUpperCase() + s.slice(1)).join(' ')
}

function getOrigin () {
function getOrigin() {
// eslint-disable-next-line no-restricted-globals
return location.origin
}

function getRegex (value) {
function getRegex(value) {
if (typeof value === 'string') {
return new RegExp(`^${value}$`)
} if (value instanceof RegExp) {
Expand All @@ -21,7 +22,7 @@ function getRegex (value) {
return undefined
}

function isOriginAllowed (origin, allowedOrigin) {
function isOriginAllowed(origin, allowedOrigin) {
const patterns = Array.isArray(allowedOrigin) ? allowedOrigin.map(getRegex) : [getRegex(allowedOrigin)]
return patterns
.some((pattern) => pattern?.test(origin) || pattern?.test(`${origin}/`)) // allowing for trailing '/'
Expand All @@ -30,7 +31,7 @@ function isOriginAllowed (origin, allowedOrigin) {
export default class Provider extends RequestClient {
#refreshingTokenPromise

constructor (uppy, opts) {
constructor(uppy, opts) {
super(uppy, opts)
this.provider = opts.provider
this.id = this.provider
Expand All @@ -39,9 +40,10 @@ export default class Provider extends RequestClient {
this.tokenKey = `companion-${this.pluginId}-auth-token`
this.companionKeysParams = this.opts.companionKeysParams
this.preAuthToken = null
this.supportsRefreshToken = opts.supportsRefreshToken ?? true // todo false in next major
}

async headers () {
async headers() {
const [headers, token] = await Promise.all([super.headers(), this.#getAuthToken()])
const authHeaders = {}
if (token) {
Expand All @@ -56,32 +58,33 @@ export default class Provider extends RequestClient {
return { ...headers, ...authHeaders }
}

onReceiveResponse (response) {
onReceiveResponse(response) {
super.onReceiveResponse(response)
const plugin = this.uppy.getPlugin(this.pluginId)
const oldAuthenticated = plugin.getPluginState().authenticated
const authenticated = oldAuthenticated ? response.status !== 401 : response.status < 400
const authenticated = oldAuthenticated ? response.status !== authErrorStatusCode : response.status < 400
plugin.setPluginState({ authenticated })
return response
}

async setAuthToken (token) {
async setAuthToken(token) {
return this.uppy.getPlugin(this.pluginId).storage.setItem(this.tokenKey, token)
}

async #getAuthToken () {
async #getAuthToken() {
return this.uppy.getPlugin(this.pluginId).storage.getItem(this.tokenKey)
}

async #removeAuthToken () {
/** @protected */
async removeAuthToken() {
return this.uppy.getPlugin(this.pluginId).storage.removeItem(this.tokenKey)
}

/**
* Ensure we have a preauth token if necessary. Attempts to fetch one if we don't,
* or rejects if loading one fails.
*/
async ensurePreAuth () {
async ensurePreAuth() {
if (this.companionKeysParams && !this.preAuthToken) {
await this.fetchPreAuthToken()

Expand All @@ -91,24 +94,43 @@ export default class Provider extends RequestClient {
}
}

authUrl (queries = {}) {
// eslint-disable-next-line class-methods-use-this
authQuery() {
return {}
}

authUrl({ authFormData, query } = {}) {
const params = new URLSearchParams({
...query,
state: btoa(JSON.stringify({ origin: getOrigin() })),
...queries,
...this.authQuery({ authFormData }),
})

if (this.preAuthToken) {
params.set('uppyPreAuthToken', this.preAuthToken)
}

return `${this.hostname}/${this.id}/connect?${params}`
}

async login (queries) {
/** @protected */
async loginSimpleAuth({ uppyVersions, authFormData, signal }) {
const response = await this.post(`${this.id}/simple-auth`, { form: authFormData }, { qs: { uppyVersions }, signal })
this.setAuthToken(response.uppyAuthToken)
}

/** @protected */
async loginOAuth({ uppyVersions, authFormData, signal }) {
await this.ensurePreAuth()

signal.throwIfAborted()

return new Promise((resolve, reject) => {
const link = this.authUrl(queries)
const link = this.authUrl({ query: { uppyVersions }, authFormData })
const authWindow = window.open(link, '_blank')

let cleanup

const handleToken = (e) => {
if (e.source !== authWindow) {
let jsonData = ''
Expand Down Expand Up @@ -148,24 +170,35 @@ export default class Provider extends RequestClient {
return
}

cleanup()
resolve(this.setAuthToken(data.token))
}

cleanup = () => {
authWindow.close()
window.removeEventListener('message', handleToken)
this.setAuthToken(data.token).then(() => resolve()).catch(reject)
signal.removeEventListener('abort', cleanup)
}

signal.addEventListener('abort', cleanup)
window.addEventListener('message', handleToken)
})
}

refreshTokenUrl () {
async login({ uppyVersions, authFormData, signal }) {
return this.loginOAuth({ uppyVersions, authFormData, signal })
}

refreshTokenUrl() {
return `${this.hostname}/${this.id}/refresh-token`
}

fileUrl (id) {
fileUrl(id) {
return `${this.hostname}/${this.id}/get/${id}`
}

/** @protected */
async request (...args) {
async request(...args) {
await this.#refreshingTokenPromise

try {
Expand All @@ -177,6 +210,7 @@ export default class Provider extends RequestClient {

return await super.request(...args)
} catch (err) {
if (!this.supportsRefreshToken) throw err
// only handle auth errors (401 from provider), and only handle them if we have a (refresh) token
const authTokenAfter = await this.#getAuthToken()
if (!err.isAuthError || !authTokenAfter) throw err
Expand All @@ -192,7 +226,7 @@ export default class Provider extends RequestClient {
} catch (refreshTokenErr) {
if (refreshTokenErr.isAuthError) {
// if refresh-token has failed with auth error, delete token, so we don't keep trying to refresh in future
await this.#removeAuthToken()
await this.removeAuthToken()
}
throw err
} finally {
Expand All @@ -208,7 +242,7 @@ export default class Provider extends RequestClient {
}
}

async fetchPreAuthToken () {
async fetchPreAuthToken() {
if (!this.companionKeysParams) {
return
}
Expand All @@ -221,17 +255,17 @@ export default class Provider extends RequestClient {
}
}

list (directory, options) {
list(directory, options) {
return this.get(`${this.id}/list/${directory || ''}`, options)
}

async logout (options) {
async logout(options) {
const response = await this.get(`${this.id}/logout`, options)
await this.#removeAuthToken()
await this.removeAuthToken()
return response
}

static initPlugin (plugin, opts, defaultOpts) {
static initPlugin(plugin, opts, defaultOpts) {
/* eslint-disable no-param-reassign */
plugin.type = 'acquirer'
plugin.files = []
Expand Down
Loading