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 16 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
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 {
mifi marked this conversation as resolved.
Show resolved Hide resolved
// if the response contains invalid JSON, let's ignore the error data
throw new Error(errMsg)
mifi marked this conversation as resolved.
Show resolved Hide resolved
}

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
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
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
8 changes: 6 additions & 2 deletions packages/@uppy/companion/src/server/controllers/list.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
const { respondWithError } = require('../provider/error')

async function list ({ query, params, companion }, res, next) {
const { accessToken } = companion.providerTokens
const { providerUserSession } = companion
const { accessToken } = providerUserSession

try {
const data = await companion.provider.list({ companion, token: accessToken, directory: params.id, query })
// todo remove backward compat `token` param from all provider methods (because it can be found in providerUserSession)
const data = await companion.provider.list({
companion, token: accessToken, providerUserSession, directory: params.id, query,
})
res.json(data)
} catch (err) {
if (respondWithError(err, res)) return
Expand Down
11 changes: 5 additions & 6 deletions packages/@uppy/companion/src/server/controllers/logout.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,19 @@ async function logout (req, res, next) {
req.session.grant.dynamic = null
}
}
const { providerName } = req.params
const { companion } = req
const tokens = companion.allProvidersTokens ? companion.allProvidersTokens[providerName] : null
const { providerUserSession } = companion

if (!tokens) {
if (!providerUserSession) {
cleanSession()
res.json({ ok: true, revoked: false })
return
}

try {
const { accessToken } = tokens
const data = await companion.provider.logout({ token: accessToken, companion })
delete companion.allProvidersTokens[providerName]
const { accessToken } = providerUserSession
const data = await companion.provider.logout({ token: accessToken, providerUserSession, companion })
delete companion.providerUserSession
tokenService.removeFromCookies(res, companion.options, companion.provider.authProvider)
cleanSession()
res.json({ ok: true, ...data })
Expand Down
Loading
Loading