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 26 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
56 changes: 45 additions & 11 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,8 @@ export default class Provider extends RequestClient {
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)
}

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,14 +170,25 @@ export default class Provider extends RequestClient {
return
}

cleanup()
this.setAuthToken(data.token).then(() => resolve()).catch(reject)
mifi marked this conversation as resolved.
Show resolved Hide resolved
}

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)
})
}

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

refreshTokenUrl () {
return `${this.hostname}/${this.id}/refresh-token`
}
Expand All @@ -177,9 +210,10 @@ 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
if (!(err instanceof AuthError) || !authTokenAfter) throw err
mifi marked this conversation as resolved.
Show resolved Hide resolved

if (this.#refreshingTokenPromise == null) {
// Many provider requests may be starting at once, however refresh token should only be called once.
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 Down Expand Up @@ -227,7 +261,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
17 changes: 13 additions & 4 deletions packages/@uppy/companion-client/src/RequestClient.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'

import UserFacingApiError from '@uppy/utils/lib/UserFacingApiError'
// eslint-disable-next-line import/no-extraneous-dependencies
import pRetry, { AbortError } from 'p-retry'

Expand All @@ -20,7 +21,7 @@ function stripSlash (url) {
const retryCount = 10 // set to a low number, like 2 to test manual user retries
const socketActivityTimeoutMs = 5 * 60 * 1000 // set to a low number like 10000 to test this

const authErrorStatusCode = 401
export const authErrorStatusCode = 401

class HttpError extends Error {
statusCode
Expand All @@ -41,17 +42,25 @@ async function handleJSONResponse (res) {
}

let errMsg = `Failed request with status: ${res.status}. ${res.statusText}`
let errData
try {
const errData = await res.json()
errData = await res.json()

errMsg = errData.message ? `${errMsg} message: ${errData.message}` : errMsg
errMsg = errData.requestId
? `${errMsg} request-Id: ${errData.requestId}`
: errMsg
mifi marked this conversation as resolved.
Show resolved Hide resolved
} catch {
mifi marked this conversation as resolved.
Show resolved Hide resolved
/* if the response contains invalid JSON, let's ignore the error */
// 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 HttpError({ statusCode: res.status, message: errMsg })
}

Expand Down Expand Up @@ -211,7 +220,7 @@ export default class RequestClient {
return await handleJSONResponse(response)
} catch (err) {
// pass these through
if (err instanceof AuthError || err.name === 'AbortError') throw err
if (err instanceof AuthError || err instanceof UserFacingApiError || err.name === 'AbortError') throw err
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, we should move away from instanceof IMO


throw new ErrorWithCause(`Could not ${method} ${this.#getUrl(path)}`, {
cause: err,
Expand Down
6 changes: 4 additions & 2 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 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,27 +33,30 @@ 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))
}

const { access_token: accessToken, refresh_token: refreshToken } = grant.response

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

logger.debug(`Generating auth token for provider ${providerName}. refreshToken: ${refreshToken ? 'yes' : 'no'}`, 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
Loading