From 16f3e1bb0351ca583454e6a6a2e816723190d4c5 Mon Sep 17 00:00:00 2001 From: Dimitrie Stefanescu Date: Mon, 31 Aug 2020 13:27:55 +0100 Subject: [PATCH] fix(subs): wrapps onConnection in a try catch and checks for auth headers some clients send the connection params with lowercase :/ --- app.js | 33 +++--- modules/auth/index.js | 2 - .../migrations/2020-05-29-thirdpartyapps.js | 1 - modules/auth/strategies/github.js | 1 - modules/auth/strategies/google.js | 2 - modules/auth/strategies/local.js | 1 - modules/core/index.js | 1 - modules/core/rest/download.js | 3 - modules/core/rest/upload.js | 2 - modules/core/services/objects.js | 104 +++++++++--------- modules/core/services/tokens.js | 2 - 11 files changed, 68 insertions(+), 84 deletions(-) diff --git a/app.js b/app.js index 2615bf4a8..e974e8e56 100644 --- a/app.js +++ b/app.js @@ -2,6 +2,8 @@ let http = require( 'http' ) +const url = require( 'url' ) +let WebSocket = require( 'ws' ) const express = require( 'express' ) const compression = require( 'compression' ) const appRoot = require( 'app-root-path' ) @@ -43,7 +45,6 @@ exports.init = async ( ) => { // Initialise default modules, including rest api handlers await init( app ) - let obj = graph( ) // Initialise graphql server graphqlServer = new ApolloServer( { @@ -51,23 +52,21 @@ exports.init = async ( ) => { context: contextApiTokenHelper, subscriptions: { onConnect: ( connectionParams, webSocket, context ) => { - // debug( `speckle:debug` )( 'ws on connect event' ) - // console.log( connectionParams ) - if ( connectionParams.Authorization || connectionParams.headers.Authorization ) { - let header = connectionParams.Authorization || connectionParams.headers.Authorization - let token = header.split( ' ' )[ 1 ] - return { token: token } + try { + if ( connectionParams.Authorization || connectionParams.authorization || connectionParams.headers.Authorization ) { + let header = connectionParams.Authorization || connectionParams.authorization || connectionParams.headers.Authorization + let token = header.split( ' ' )[ 1 ] + return { token: token } + } + } catch ( e ) { + throw new ForbiddenError( 'You need a token to subscribe' ) } - - throw new ForbiddenError( 'You need a token to subscribe' ) }, onDisconnect: ( webSocket, context ) => { - // console.log( context ) debug( `speckle:debug` )( 'ws on disconnect connect event' ) }, }, - tracing: process.env.NODE_ENV === 'test' || process.env.NODE_ENV === 'development', - // debug: true + tracing: process.env.NODE_ENV === 'development' } ) graphqlServer.applyMiddleware( { app: app } ) @@ -100,7 +99,7 @@ exports.startHttp = async ( app ) => { debug( 'speckle:http-startup' )( `👉 main application: http://localhost:${port}/` ) debug( 'speckle:http-startup' )( `👉 auth application: http://localhost:${port}/auth` ) debug( 'speckle:http-startup' )( `👉 setup application: http://localhost:${port}/setup` ) - debug( 'speckle:hint' )( `ℹī¸ Don't forget to run "npm run dev:frontend" in a different terminal to start the vue application.` ) + debug( 'speckle:hint' )( ` ℹī¸ Don't forget to run "npm run dev:frontend" in a different terminal to start the vue application.` ) } else { app.use( '/', express.static( `${appRoot}/frontend/dist` ) ) @@ -127,17 +126,19 @@ exports.startHttp = async ( app ) => { } catch ( error ) { res.json( { success: false, message: "Something went wrong" } ) } - } ); + } ) } let server = http.createServer( app ) + graphqlServer.installSubscriptionHandlers( server ) + graphqlServer.applyMiddleware( { app: app } ) + server.on( 'listening', ( ) => { - debug( `speckle:startup` )( `My name is Spockle Server, and I'm running at ${server.address().port}` ) + debug( `speckle:startup` )( ` 🚀 My name is Spockle Server, and I'm running at ${server.address().port}` ) } ) server.listen( port ) - return { server } } diff --git a/modules/auth/index.js b/modules/auth/index.js index 0195abecb..b287164b2 100644 --- a/modules/auth/index.js +++ b/modules/auth/index.js @@ -13,7 +13,6 @@ let authStrategies = [ ] exports.authStrategies = authStrategies exports.init = ( app, options ) => { - debug( 'speckle:modules' )( '🔑 \tInit app, authn and authz module' ) passport.serializeUser( ( user, done ) => done( null, user ) ) @@ -64,7 +63,6 @@ exports.init = ( app, options ) => { let authResponse = await createAppTokenFromAccessCode( { appId: req.body.appId, appSecret: req.body.appSecret, accessCode: req.body.accessCode, challenge: req.body.challenge } ) return res.send( authResponse ) - } catch ( err ) { debug( 'speckle:errors' )( err ) return res.status( 401 ).send( { err: err.message } ) diff --git a/modules/auth/migrations/2020-05-29-thirdpartyapps.js b/modules/auth/migrations/2020-05-29-thirdpartyapps.js index 0c843127e..cc3be1404 100644 --- a/modules/auth/migrations/2020-05-29-thirdpartyapps.js +++ b/modules/auth/migrations/2020-05-29-thirdpartyapps.js @@ -109,7 +109,6 @@ exports.up = async knex => { const mockAppScopes = [ { appId: 'mock', scopeName: 'streams:read' }, { appId: 'mock', scopeName: 'users:read' }, { appId: 'mock', scopeName: 'profile:email' } ] await knex( 'server_apps_scopes' ).insert( mockAppScopes ) - } exports.down = async knex => { diff --git a/modules/auth/strategies/github.js b/modules/auth/strategies/github.js index e23f6e1bf..1c404f676 100644 --- a/modules/auth/strategies/github.js +++ b/modules/auth/strategies/github.js @@ -9,7 +9,6 @@ const { findOrCreateUser } = require( `${appRoot}/modules/core/services/users` ) const { getApp, createAuthorizationCode, createAppTokenFromAccessCode } = require( '../services/apps' ) module.exports = ( app, session, sessionAppId, finalizeAuth ) => { - const strategy = { id: 'github', name: 'Github', diff --git a/modules/auth/strategies/google.js b/modules/auth/strategies/google.js index d1bce4472..792c21582 100644 --- a/modules/auth/strategies/google.js +++ b/modules/auth/strategies/google.js @@ -8,7 +8,6 @@ const { findOrCreateUser } = require( `${appRoot}/modules/core/services/users` ) const { getApp, createAuthorizationCode, createAppTokenFromAccessCode } = require( '../services/apps' ) module.exports = ( app, session, sessionAppId, finalizeAuth ) => { - const strategy = { id: 'google', name: 'Google', @@ -24,7 +23,6 @@ module.exports = ( app, session, sessionAppId, finalizeAuth ) => { callbackURL: strategy.callbackUrl, scope: [ 'profile', 'email' ], }, async ( accessToken, refreshToken, profile, done ) => { - let email = profile.emails[ 0 ].value let name = profile.displayName diff --git a/modules/auth/strategies/local.js b/modules/auth/strategies/local.js index 172f86dc2..f9cb34ff0 100644 --- a/modules/auth/strategies/local.js +++ b/modules/auth/strategies/local.js @@ -7,7 +7,6 @@ const { createUser, findOrCreateUser, validatePasssword, getUserByEmail } = requ const { getApp, createAuthorizationCode, createAppTokenFromAccessCode } = require( '../services/apps' ) module.exports = ( app, session, sessionAppId, finalizeAuth ) => { - const strategy = { id: 'local', name: 'Local', diff --git a/modules/core/index.js b/modules/core/index.js index 3cbb62460..e7a44d2a7 100644 --- a/modules/core/index.js +++ b/modules/core/index.js @@ -8,5 +8,4 @@ exports.init = async ( app, options ) => { // Initialises the two main bulk upload/download endpoints require( './rest/upload' )( app ) require( './rest/download' )( app ) - } \ No newline at end of file diff --git a/modules/core/rest/download.js b/modules/core/rest/download.js index 1c277d034..11c580ddd 100644 --- a/modules/core/rest/download.js +++ b/modules/core/rest/download.js @@ -8,9 +8,7 @@ const { contextMiddleware, validateScopes, authorizeResolver } = require( `${app const { getObject, getObjectChildrenStream } = require( '../services/objects' ) module.exports = ( app ) => { - app.get( '/objects/:streamId/:objectId', contextMiddleware, async ( req, res ) => { - if ( !req.context || !req.context.auth ) { return res.status( 401 ).end( ) } @@ -102,7 +100,6 @@ module.exports = ( app ) => { // TODO: is this needed/used? app.get( '/objects/:streamId/:objectId/single', async ( req, res ) => { - // TODO: authN & authZ checks let obj = await getObject( req.params.objectId ) diff --git a/modules/core/rest/upload.js b/modules/core/rest/upload.js index 38b255607..43511b52c 100644 --- a/modules/core/rest/upload.js +++ b/modules/core/rest/upload.js @@ -9,9 +9,7 @@ const { contextMiddleware, validateScopes, authorizeResolver } = require( `${app const { createObjects, createObjectsBatched } = require( '../services/objects' ) module.exports = ( app ) => { - app.post( '/objects/:streamId', contextMiddleware, async ( req, res ) => { - if ( !req.context || !req.context.auth ) { return res.status( 401 ).end( ) } diff --git a/modules/core/services/objects.js b/modules/core/services/objects.js index bf6262b49..c9657a880 100644 --- a/modules/core/services/objects.js +++ b/modules/core/services/objects.js @@ -20,7 +20,6 @@ const StreamCommits = ( ) => knex( 'stream_commits' ) module.exports = { async createObject( object ) { - let insertionObject = prepInsertionObject( object ) let closures = [ ] @@ -136,7 +135,6 @@ module.exports = { let t0 = performance.now( ) batch.forEach( obj => { - let insertionObject = prepInsertionObject( obj ) let totalChildrenCountByDepth = {} let totalChildrenCountGlobal = 0 @@ -279,60 +277,60 @@ module.exports = { let operatorsWhitelist = [ '=', '>', '>=', '<', '<=', '!=' ] let mainQuery = knex.with( 'objs', cteInnerQuery => { - // always select the id - cteInnerQuery.select( 'id' ).from( 'object_children_closure' ) - cteInnerQuery.select( 'createdAt' ) - cteInnerQuery.select( 'speckleType' ) - cteInnerQuery.select( 'totalChildrenCount' ) + // always select the id + cteInnerQuery.select( 'id' ).from( 'object_children_closure' ) + cteInnerQuery.select( 'createdAt' ) + cteInnerQuery.select( 'speckleType' ) + cteInnerQuery.select( 'totalChildrenCount' ) - // if there are any select fields, add them - if ( Array.isArray( select ) ) { - select.forEach( ( field, index ) => { - cteInnerQuery.select( knex.raw( 'jsonb_path_query(data, :path) as :name:', { path: "$." + field, name: '' + index } ) ) + // if there are any select fields, add them + if ( Array.isArray( select ) ) { + select.forEach( ( field, index ) => { + cteInnerQuery.select( knex.raw( 'jsonb_path_query(data, :path) as :name:', { path: "$." + field, name: '' + index } ) ) + } ) + // otherwise, get the whole object, as stored in the jsonb column + } else { + cteInnerQuery.select( 'data' ) + } + + // join on objects table + cteInnerQuery.join( 'objects', 'child', '=', 'objects.id' ) + .where( 'parent', objectId ) + .andWhere( 'minDepth', '<', depth ) + + // Add user provided filters/queries. + if ( Array.isArray( query ) && query.length > 0 ) { + cteInnerQuery.andWhere( nestedWhereQuery => { + query.forEach( ( statement, index ) => { + let castType = 'text' + if ( typeof statement.value === 'string' ) castType = 'text' + if ( typeof statement.value === 'boolean' ) castType = 'boolean' + if ( typeof statement.value === 'number' ) castType = 'numeric' + + if ( operatorsWhitelist.indexOf( statement.operator ) == -1 ) + throw new Error( 'Invalid operator for query' ) + + // Determine the correct where clause (where, and where, or where) + let whereClause + if ( index === 0 ) whereClause = 'where' + else if ( statement.verb && statement.verb.toLowerCase( ) === 'or' ) whereClause = 'orWhere' + else whereClause = 'andWhere' + + // Note: castType is generated from the statement's value and operators are matched against a whitelist. + // If comparing with strings, the jsonb_path_query(_first) func returns json encoded strings (ie, `bar` is actually `"bar"`), hence we need to add the qoutes manually to the raw provided comparison value. + nestedWhereQuery[ whereClause ]( knex.raw( `jsonb_path_query_first( data, ? )::${castType} ${statement.operator} ?? `, [ '$.' + statement.field, castType === 'text' ? `"${statement.value}"` : statement.value ] ) ) } ) - // otherwise, get the whole object, as stored in the jsonb column - } else { - cteInnerQuery.select( 'data' ) - } + } ) + } - // join on objects table - cteInnerQuery.join( 'objects', 'child', '=', 'objects.id' ) - .where( 'parent', objectId ) - .andWhere( 'minDepth', '<', depth ) - - // Add user provided filters/queries. - if ( Array.isArray( query ) && query.length > 0 ) { - cteInnerQuery.andWhere( nestedWhereQuery => { - query.forEach( ( statement, index ) => { - let castType = 'text' - if ( typeof statement.value === 'string' ) castType = 'text' - if ( typeof statement.value === 'boolean' ) castType = 'boolean' - if ( typeof statement.value === 'number' ) castType = 'numeric' - - if ( operatorsWhitelist.indexOf( statement.operator ) == -1 ) - throw new Error( 'Invalid operator for query' ) - - // Determine the correct where clause (where, and where, or where) - let whereClause - if ( index === 0 ) whereClause = 'where' - else if ( statement.verb && statement.verb.toLowerCase( ) === 'or' ) whereClause = 'orWhere' - else whereClause = 'andWhere' - - // Note: castType is generated from the statement's value and operators are matched against a whitelist. - // If comparing with strings, the jsonb_path_query(_first) func returns json encoded strings (ie, `bar` is actually `"bar"`), hence we need to add the qoutes manually to the raw provided comparison value. - nestedWhereQuery[ whereClause ]( knex.raw( `jsonb_path_query_first( data, ? )::${castType} ${statement.operator} ?? `, [ '$.' + statement.field, castType === 'text' ? `"${statement.value}"` : statement.value ] ) ) - } ) - } ) - } - - // Order by clause; validate direction! - let direction = orderBy.direction && orderBy.direction.toLowerCase( ) === 'desc' ? 'desc' : 'asc' - if ( orderBy.field === 'id' ) { - cteInnerQuery.orderBy( 'id', direction ) - } else { - cteInnerQuery.orderByRaw( knex.raw( `jsonb_path_query_first( data, ? ) ${direction}, id asc`, [ '$.' + orderBy.field ] ) ) - } - } ) + // Order by clause; validate direction! + let direction = orderBy.direction && orderBy.direction.toLowerCase( ) === 'desc' ? 'desc' : 'asc' + if ( orderBy.field === 'id' ) { + cteInnerQuery.orderBy( 'id', direction ) + } else { + cteInnerQuery.orderByRaw( knex.raw( `jsonb_path_query_first( data, ? ) ${direction}, id asc`, [ '$.' + orderBy.field ] ) ) + } + } ) .select( '*' ).from( 'objs' ) .joinRaw( 'RIGHT JOIN ( SELECT count(*) FROM "objs" ) c(total_count) ON TRUE' ) diff --git a/modules/core/services/tokens.js b/modules/core/services/tokens.js index d34d7c699..116ed3f00 100644 --- a/modules/core/services/tokens.js +++ b/modules/core/services/tokens.js @@ -33,7 +33,6 @@ module.exports = { }, async createToken( { userId, name, scopes, lifespan } ) { - let { tokenId, tokenString, tokenHash, lastChars } = await module.exports.createBareToken( ) if ( scopes.length === 0 ) throw new Error( 'No scopes provided' ) @@ -56,7 +55,6 @@ module.exports = { // Creates a personal access token for a user with a set of given scopes. async createPersonalAccessToken( userId, name, scopes, lifespan ) { - let { id, token } = await module.exports.createToken( { userId, name, scopes, lifespan } ) // Store the relationship