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

Draft: Webdav provider poc #4621

Draft
wants to merge 55 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
55 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
f0ba00f
implement webdav poc
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
b10edf9
Merge branch 'provider-user-sessions' into webdav-provider-poc
mifi Sep 7, 2023
367a0de
implement `size` for webdav
mifi Sep 7, 2023
9600104
use new loginSimpleAuth method
mifi Sep 7, 2023
55a9351
todo-fixme
mifi Sep 7, 2023
13642fb
hopefully fix url replacer
mifi Sep 7, 2023
59262c1
upgrade webdav
mifi Sep 7, 2023
b33f5b1
Merge branch 'main' into provider-user-sessions
mifi Oct 2, 2023
47c4025
Merge branch 'provider-user-sessions' into webdav-provider-poc
mifi Oct 2, 2023
4be2b6f
make removeAuthToken protected
mifi Oct 2, 2023
ff13823
support both nextcloud and normal webdavs
mifi Oct 2, 2023
63d0b0c
trim inpiut
mifi Oct 2, 2023
7549da4
make removeAuthToken protected
mifi Oct 2, 2023
a795d05
Merge branch 'provider-user-sessions' into webdav-provider-poc
mifi Oct 2, 2023
0542371
rename webdav to webdavOauth, pass providerOptions
arturi Oct 19, 2023
1302574
Fix incorrect options nesting when creating the webdav client
arturi Oct 20, 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
2af7b24
Merge branch 'main' into provider-user-sessions
mifi Dec 7, 2023
c6cafdc
Merge branch 'provider-user-sessions' into webdav-provider-poc
mifi Dec 7, 2023
828c0ae
Merge branch 'main' into webdav-provider-poc
mifi Dec 7, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ module.exports = {
'packages/@uppy/utils/src/**/*.js',
'packages/@uppy/vue/src/**/*.js',
'packages/@uppy/webcam/src/**/*.js',
'packages/@uppy/webdav/src/**/*.js',
'packages/@uppy/xhr-upload/src/**/*.js',
'packages/@uppy/zoom/src/**/*.js',
],
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
2 changes: 1 addition & 1 deletion packages/@uppy/companion-client/src/AuthError.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class AuthError extends Error {
constructor () {
super('Authorization required')
this.name = 'AuthError'
this.isAuthError = true
this.isAuthError = true // todo remove in next major and pull AuthError into a shared package
}
}

Expand Down
46 changes: 34 additions & 12 deletions packages/@uppy/companion-client/src/Provider.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
'use strict'

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

const getName = (id) => {
return id.split('-').map((s) => s.charAt(0).toUpperCase() + s.slice(1)).join(' ')
Expand Down Expand Up @@ -39,6 +40,7 @@ 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 () {
Expand All @@ -60,7 +62,7 @@ export default class Provider extends RequestClient {
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
}
Expand All @@ -73,7 +75,7 @@ export default class Provider extends RequestClient {
return this.uppy.getPlugin(this.pluginId).storage.getItem(this.tokenKey)
}

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

Expand All @@ -91,24 +93,36 @@ 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) {
async login ({ 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) {
this.uppy.log.warn('ignoring event from unknown source', e)
Expand Down Expand Up @@ -138,11 +152,18 @@ export default class Provider extends RequestClient {
return
}

authWindow.close()
window.removeEventListener('message', handleToken)
cleanup()
this.setAuthToken(data.token)
resolve()
}

cleanup = () => {
authWindow.close()
window.removeEventListener('message', handleToken)
signal.removeEventListener('abort', cleanup)
}

signal.addEventListener('abort', cleanup)
window.addEventListener('message', handleToken)
})
}
Expand All @@ -160,10 +181,11 @@ export default class Provider extends RequestClient {
await this.#refreshingTokenPromise

try {
// throw Object.assign(new Error(), { isAuthError: true }) // testing simulate access token expired (to refresh token)
// throw new AuthError() // testing simulate access token expired (to refresh token)
return await super.request(...args)
} catch (err) {
if (!err.isAuthError) throw err // only handle auth errors (401 from provider)
if (!this.supportsRefreshToken) throw err
if (!(err instanceof AuthError)) throw err // only handle auth errors (401 from provider)

await this.#refreshingTokenPromise

Expand Down Expand Up @@ -204,7 +226,7 @@ export default class Provider extends RequestClient {

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

Expand Down
27 changes: 20 additions & 7 deletions packages/@uppy/companion-client/src/RequestClient.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict'

import UserFacingApiError from '@uppy/utils/lib/UserFacingApiError'

import fetchWithNetworkError from '@uppy/utils/lib/fetchWithNetworkError'
import ErrorWithCause from '@uppy/utils/lib/ErrorWithCause'
import AuthError from './AuthError.js'
Expand All @@ -11,8 +13,10 @@ function stripSlash (url) {
return url.replace(/\/$/, '')
}

export const authErrorStatusCode = 401

async function handleJSONResponse (res) {
if (res.status === 401) {
if (res.status === authErrorStatusCode) {
throw new AuthError()
}

Expand All @@ -22,11 +26,20 @@ async function handleJSONResponse (res) {
}

let errMsg = `Failed request with status: ${res.status}. ${res.statusText}`
let errData
try {
const errData = await jsonPromise
errMsg = errData.message ? `${errMsg} message: ${errData.message}` : errMsg
errMsg = errData.requestId ? `${errMsg} request-Id: ${errData.requestId}` : errMsg
} catch { /* if the response contains invalid JSON, let's ignore the error */ }
errData = await jsonPromise
} catch {
// if the response contains invalid JSON, let's ignore the error data
throw new Error(errMsg)
}

if (res.status >= 400 && res.status <= 499 && errData.message) {
throw new UserFacingApiError(errData.message)
}

errMsg = errData.message ? `${errMsg} message: ${errData.message}` : errMsg
errMsg = errData.requestId ? `${errMsg} request-Id: ${errData.requestId}` : errMsg
throw new Error(errMsg)
}

Expand Down Expand Up @@ -162,9 +175,9 @@ export default class RequestClient {
body: data ? JSON.stringify(data) : null,
})
if (!skipPostResponse) this.onReceiveResponse(response)
return handleJSONResponse(response)
return await handleJSONResponse(response)
} catch (err) {
if (err?.isAuthError) throw err
if (err instanceof AuthError || err instanceof UserFacingApiError) throw err // some errors must be passed through
throw new ErrorWithCause(`Could not ${method} ${this.#getUrl(path)}`, { cause: err })
}
}
Expand Down
2 changes: 2 additions & 0 deletions packages/@uppy/companion/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"express-prom-bundle": "6.5.0",
"express-request-id": "1.4.1",
"express-session": "1.17.3",
"fast-xml-parser": "^4.2.7",
"form-data": "^3.0.0",
"got": "11",
"grant": "5.4.21",
Expand All @@ -68,6 +69,7 @@
"serialize-javascript": "^6.0.0",
"tus-js-client": "^3.0.0",
"validator": "^13.0.0",
"webdav": "^5.2.3",
"ws": "8.8.1"
},
"devDependencies": {
Expand Down
8 changes: 5 additions & 3 deletions packages/@uppy/companion/src/companion.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const jobs = require('./server/jobs')
const logger = require('./server/logger')
const middlewares = require('./server/middlewares')
const { getMaskableSecrets, defaultOptions, validateConfig } = require('./config/companion')
const { ProviderApiError, ProviderAuthError } = require('./server/provider/error')
const { ProviderApiError, ProviderUserError, ProviderAuthError } = require('./server/provider/error')
const { getCredentialsOverrideMiddleware } = require('./server/provider/credentials')
// @ts-ignore
const { version } = require('../package.json')
Expand Down Expand Up @@ -52,7 +52,7 @@ const interceptGrantErrorResponse = interceptor((req, res) => {
})

// make the errors available publicly for custom providers
module.exports.errors = { ProviderApiError, ProviderAuthError }
module.exports.errors = { ProviderApiError, ProviderUserError, ProviderAuthError }
module.exports.socket = require('./server/socket')

module.exports.setLoggerProcessName = setLoggerProcessName
Expand Down Expand Up @@ -126,6 +126,8 @@ module.exports.app = (optionsArg = {}) => {
app.get('/:providerName/logout', middlewares.hasSessionAndProvider, middlewares.hasOAuthProvider, middlewares.gentleVerifyToken, controllers.logout)
app.get('/:providerName/send-token', middlewares.hasSessionAndProvider, middlewares.hasOAuthProvider, middlewares.verifyToken, controllers.sendToken)

app.post('/:providerName/simple-auth', express.json(), middlewares.hasSessionAndProvider, middlewares.hasBody, middlewares.hasSimpleAuthProvider, controllers.simpleAuth)

app.get('/:providerName/list/:id?', middlewares.hasSessionAndProvider, middlewares.verifyToken, controllers.list)
// backwards compat:
app.get('/search/:providerName/list', middlewares.hasSessionAndProvider, middlewares.verifyToken, controllers.list)
Expand All @@ -136,7 +138,7 @@ module.exports.app = (optionsArg = {}) => {

app.get('/:providerName/thumbnail/:id', middlewares.hasSessionAndProvider, middlewares.hasOAuthProvider, middlewares.cookieAuthToken, middlewares.verifyToken, controllers.thumbnail)

app.param('providerName', providerManager.getProviderMiddleware(providers))
app.param('providerName', providerManager.getProviderMiddleware(providers, grantConfig))

if (app.get('env') !== 'test') {
jobs.startCleanUpJob(options.filePath)
Expand Down
11 changes: 11 additions & 0 deletions packages/@uppy/companion/src/config/grant.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,16 @@ module.exports = () => {
access_url: 'https://zoom.us/oauth/token',
callback: '/zoom/callback',
},
webdav: {
transport: 'session',
// use 'subdomain' for full domain (hostname) similar to mastodon:
// https://github.com/simov/grant/blob/6e0692dfdd83edbc4ee82629ba0fe8f986d5879d/config/oauth.json#L631
dynamic: ['subdomain'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we should really set this as default/encourage it, it might be possible to craft a malicious redirect url with that and use it for phishing... haven't come up with anything clever for that so far (one of the reasons we did not enable this in production so far)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good point, but I wonder why grant does it for Mastodon if it's possible to abuse it for malicious purposes... Do you have any example of how one could abuse it?

authorize_url: 'https://[subdomain]/apps/oauth2/authorize',
access_url: 'https://[subdomain]/apps/oauth2/api/v1/token',
scope: ['profile'],
oauth: 2,
callback: '/webdav/callback',
},
}
}
15 changes: 9 additions & 6 deletions packages/@uppy/companion/src/server/controllers/callback.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,28 @@ module.exports = function callback (req, res, next) { // eslint-disable-line no-

const grant = req.session.grant || {}

const grantDynamic = oAuthState.getGrantDynamicFromRequest(req)
const origin = grantDynamic.state && oAuthState.getFromState(grantDynamic.state, 'origin', req.companion.options.secret)

if (!grant.response?.access_token) {
logger.debug(`Did not receive access token for provider ${providerName}`, null, req.id)
logger.debug(grant.response, 'callback.oauth.resp', req.id)
const state = oAuthState.getDynamicStateFromRequest(req)
const origin = state && oAuthState.getFromState(state, 'origin', req.companion.options.secret)
return res.status(400).send(closePageHtml(origin))
}

if (!req.companion.allProvidersTokens) req.companion.allProvidersTokens = {}
req.companion.allProvidersTokens[providerName] = {
req.companion.providerUserSession = {
accessToken: grant.response.access_token,
refreshToken: grant.response.refresh_token, // might be undefined for some providers
...req.companion.providerClass.grantDynamicToUserSession({ grantDynamic }),
}

logger.debug(`Generating auth token for provider ${providerName}`, null, req.id)
const uppyAuthToken = tokenService.generateEncryptedAuthToken(
req.companion.allProvidersTokens, req.companion.options.secret,
{ [providerName]: req.companion.providerUserSession },
req.companion.options.secret, req.companion.providerClass.authStateExpiry,
)

tokenService.addToCookiesIfNeeded(req, res, uppyAuthToken)
tokenService.addToCookiesIfNeeded(req, res, uppyAuthToken, req.companion.providerClass.authStateExpiry)

return res.redirect(req.companion.buildURL(`/${providerName}/send-token?uppyAuthToken=${uppyAuthToken}`, true))
}
33 changes: 26 additions & 7 deletions packages/@uppy/companion/src/server/controllers/connect.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
const atob = require('atob')
const oAuthState = require('../helpers/oauth-state')

const queryString = (params, prefix = '?') => {
const str = new URLSearchParams(params).toString()
return str ? `${prefix}${str}` : ''
}

/**
* initializes the oAuth flow for a provider.
*
Expand All @@ -9,23 +14,37 @@ const oAuthState = require('../helpers/oauth-state')
*/
module.exports = function connect (req, res) {
const { secret } = req.companion.options
let state = oAuthState.generateState(secret)
const stateObj = oAuthState.generateState()

if (req.query.state) {
const origin = JSON.parse(atob(req.query.state))
state = oAuthState.addToState(state, origin, secret)
const { origin } = JSON.parse(atob(req.query.state))
stateObj.origin = origin
}

if (req.companion.options.server.oauthDomain) {
state = oAuthState.addToState(state, { companionInstance: req.companion.buildURL('', true) }, secret)
stateObj.companionInstance = req.companion.buildURL('', true)
}

if (req.companion.clientVersion) {
state = oAuthState.addToState(state, { clientVersion: req.companion.clientVersion }, secret)
stateObj.clientVersion = req.companion.clientVersion
}

if (req.query.uppyPreAuthToken) {
state = oAuthState.addToState(state, { preAuthToken: req.query.uppyPreAuthToken }, secret)
stateObj.preAuthToken = req.query.uppyPreAuthToken
}

res.redirect(req.companion.buildURL(`/connect/${req.companion.provider.authProvider}?state=${state}`, true))
const state = oAuthState.encodeState(stateObj, secret)
const { provider, providerGrantConfig } = req.companion

// pass along grant's dynamic config (if specified for the provider in its grant config `dynamic` section)
const grantDynamicConfig = Object.fromEntries(providerGrantConfig.dynamic?.map(p => [p, req.query[p]]) || [])

const providerName = provider.authProvider
const qs = queryString({
...grantDynamicConfig,
state,
})

// Now we redirect to grant's /connect endpoint, see `app.use(Grant(grantConfig))`
res.redirect(req.companion.buildURL(`/connect/${providerName}${qs}`, true))
}
5 changes: 3 additions & 2 deletions packages/@uppy/companion/src/server/controllers/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ const { startDownUpload } = require('../helpers/upload')

async function get (req, res) {
const { id } = req.params
const { accessToken } = req.companion.providerTokens
const { providerUserSession } = req.companion
const { accessToken } = providerUserSession
const { provider } = req.companion

async function getSize () {
return provider.size({ id, token: accessToken, query: req.query })
}

async function download () {
const { stream } = await provider.download({ id, token: accessToken, query: req.query })
const { stream } = await provider.download({ id, token: accessToken, providerUserSession, query: req.query })
return stream
}

Expand Down
1 change: 1 addition & 0 deletions packages/@uppy/companion/src/server/controllers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module.exports = {
get: require('./get'),
thumbnail: require('./thumbnail'),
list: require('./list'),
simpleAuth: require('./simple-auth'),
logout: require('./logout'),
connect: require('./connect'),
preauth: require('./preauth'),
Expand Down