autohost - npm Package Compare versions

Comparing version 0.4.7 to 0.5.0

CHANGELOG.md

@@ -1,3 +0,11 @@
-## 0.4.x
+## 0.5.x
 
+### 0.5.0
+ * A query string value can no longer override a URL parameter (Matches behavior of the `data` object)
+ * Correct issue where insufficient parameters were passed to authorize predicates in hyped
+ * Default authorization (permission) checks to the end of all middleware
+ * Allow a placeholder to control when authorization checks occur in middleware stacks
+ * Return JSON `{ message: ... }` for all error responses
+ * Fix issue where express middleware that responds with an error code did not end HTTP stack evaluation
+
 ### 0.4.7
@@ -4,0 +12,0 @@ * Add authorize predicate to resource actions to support alternate authorization approach

package.json

 {
   "name": "autohost",
-  "version": "0.4.7",
+  "version": "0.5.0",
   "description": "Resource driven, transport agnostic host",
@@ -5,0 +5,0 @@ "main": "src/index.js",

README.md

@@ -304,3 +304,3 @@ # autohost
 	function( envelope, next ) {
-		// invokes the next middelware or handle call
+		// invokes the next middleware or handle call
 		return next();
@@ -378,4 +378,6 @@ },
 ### authorize
-An optional predicate for checking the users permission. The envelope is provided to the function and should return a boolean or a promise that resolves to a boolean indicating whether or not the user can perform the requested action.
+The `authorize` predicate was added to actions to allow for much more fine grained, explicit control of user authorization. By default, authorization checks are performed after _all_ middleware has run. This allows middleware to provide any necessary data on the envelope's context so that the authorization strategy can use this in determining the user's access level.
 
+To change this, provide the string "authorize" in place of a middleware call in the resource or action middleware property and the check will be performed where the string appeared in the stack. This allows an application to short-circuit the stack and avoid running middleware that performs expensive and unnecessary i/o if the user does not have permission to perform the action.
+
 > Note: when present, this _overrides_ rather than augments any authorization check that would have been performed by a configured autohost auth library.
@@ -589,3 +591,3 @@
 
-## Auth
+## Auth - via Auth Provider
 Authentication and authorization are supplied by an auth provider library that conforms to autohost's auth specifications. You can read more about that at [here](/blob/master/docs/authprovider.md).
@@ -592,0 +594,0 @@

src/http/adapter.js

@@ -52,8 +52,9 @@ var path = require( 'path' );
 
-function checkPermissionFor( state, permissionCheck, user, action ) {
+function checkPermissionFor( state, permissionCheck, meta, action, envelope, next ) {
 	log.debug( 'Checking %s\'s permissions for %s',
-		state.config.getUserString( user ), action
+		state.config.getUserString( envelope.user ), meta.alias
 	);
-	state.metrics.authorizationAttempts.record( 1, { name: 'HTTP_AUTHORIZATION_ATTEMPTS' } );
+	meta.authAttempted();
 	var timer = state.metrics.authorizationTimer();
+
 	function onError( err ) {
@@ -63,8 +64,29 @@ log.error( 'Error during check permissions: %s', err.stack );
 		timer.record( { name: 'HTTP_AUTHORIZATION_DURATION' } );
-		return false;
+		return {
+			status: 500,
+			data: { message: 'Server error at ' + envelope.path }
+		};
 	}
+
 	function onPermission( granted ) {
 		timer.record( { name: 'HTTP_AUTHORIZATION_DURATION' } );
-		return granted;
+		if( granted ) {
+			meta.authGranted();
+			log.debug( 'HTTP activation of action %s (%s %s) for %j granted',
+				meta.alias, action.method, meta.url, state.config.getUserString( envelope.user )
+			);
+			return next();
+		} else {
+			meta.authRejected();
+			log.debug( 'User %s was denied HTTP activation of action %s (%s %s)',
+				state.config.getUserString( envelope.user ), meta.alias, action.method, meta.url
+			);
+			return {
+				status: 403,
+				//headers: { 'Content-Type': 'application/json' },
+				data: { message: 'User lacks sufficient permissions' }
+			};
+		}
 	}
+
 	return permissionCheck()
@@ -108,18 +130,2 @@ .then( onPermission, onError );
 		},
-		getPermissionCheck: function( req ) {
-			var check;
-			if( action.authorize ) {
-				var envelope = this.envelope;
-				check = function() {
-					var can = action.authorize.bind( resource )( envelope );
-					return can && can.then ? can : when.resolve( can );
-				};
-				return checkPermissionFor.bind( undefined, state, check, req.user, alias );
-			} else if( state.auth && state.auth.checkPermission ) {
-				check = state.auth.checkPermission.bind( state.auth, req.user, alias, req.context );
-				return checkPermissionFor.bind( undefined, state, check, req.user, alias );
-			} else {
-				return undefined;
-			}
-		},
 		getTimer: function() {
@@ -135,2 +141,18 @@ return state.metrics.timer( resourceKey.concat( 'duration' ) );
 
+function getPermissionCheck( state, meta, resource, action, envelope ) {
+		var check;
+		if( action.authorize ) {
+			check = function() {
+				var can = action.authorize.bind( resource )( envelope, envelope.context );
+				return can && can.then ? can : when.resolve( can );
+			};
+			return checkPermissionFor.bind( undefined, state, check, meta, action );
+		} else if( state.auth && state.auth.checkPermission ) {
+			check = state.auth.checkPermission.bind( state.auth, envelope.user, meta.alias, envelope.context );
+			return checkPermissionFor.bind( undefined, state, check, meta, action );
+		} else {
+			return function( envelope, next ) { return next(); };
+		}
+	}
+
 function hasPrefix( state, url ) {
@@ -148,2 +170,4 @@ var prefix = state.http.buildUrl(
 	var stack = [];
+	var checkedPermissions = false;
+	var authCheck = getPermissionCheck( state, meta, resource, action, envelope );
 	if( resource.middleware ) {
@@ -155,3 +179,20 @@ stack = stack.concat( resource.middleware );
 	}
+
+	// check to see if a placeholder for permission check
+	// is in the stack to short-circuit expensive middleware calls
+	// that would be unnecessary if the rquesting user lacks
+	// requisite permissions
+	stack = _.map( stack, function( f ) {
+		if( f === 'authorize' ) {
+			checkedPermissions = true;
+			return authCheck;
+		} else {
+			return f;
+		}
+	} );
+	if( !checkedPermissions ) {
+		stack.push( authCheck );
+	}
 	stack.push( action.handle );
+
 	if ( meta.handleErrors ) {
@@ -171,3 +212,8 @@ try {
 			var onResult = function onResult( x ) {
-				envelope.handleReturn( state.config, resource, action, x );
+				if( x ) {
+					envelope.handleReturn( state.config, resource, action, x );
+				} else {
+					log.warn( 'Handle at route: %s %s returned undefined',
+						action.method.toUpperCase(), action.url );
+				}
 			};
@@ -218,7 +264,6 @@ result.then( onResult, onResult );
 		meta.getEnvelope( req, res );
-		var authCheck = meta.getPermissionCheck( req );
+
 		req._metricKey = meta.metricKey;
 		req._resource = resource.name;
 		req._action = actionName;
-		req._checkPermission = authCheck;
 		var timer = meta.getTimer();
@@ -229,23 +274,3 @@ res.once( 'finish', function() {
 
-		if ( authCheck ) {
-			meta.authAttempted();
-			authCheck()
-				.then( function onPermission( pass ) {
-					if ( pass ) {
-						meta.authGranted();
-						log.debug( 'HTTP activation of action %s (%s %s) for %j granted',
-							meta.alias, action.method, meta.url, state.config.getUserString( req.user ) );
-						respond( state, meta, req, res, resource, action );
-					} else {
-						meta.authRejected();
-						log.debug( 'User %s was denied HTTP activation of action %s (%s %s)',
-							state.config.getUserString( req.user ), meta.alias, action.method, meta.url );
-						if ( !res._headerSent ) {
-							res.status( 403 ).send( 'User lacks sufficient permissions' );
-						}
-					}
-				} );
-		} else {
-			respond( state, meta, req, res, resource, action );
-		}
+		respond( state, meta, req, res, resource, action );
 	} );
@@ -252,0 +277,0 @@ }

src/http/http.js

@@ -212,3 +212,3 @@ var path = require( 'path' );
 					log.debug( 'ERROR! route: %s %s failed with %s', method.toUpperCase(), url, err.stack );
-					res.status( 500 ).send( 'Server error at ' + method.toUpperCase() + ' ' + url );
+					res.status( 500 ).send( { message: 'Server error at ' + method.toUpperCase() + ' ' + url } );
 				}
@@ -215,0 +215,0 @@ } else {

src/http/httpEnvelope.js

@@ -33,6 +33,8 @@ var request;
 			var val = source[ key ];
-			if ( this.data[ key ] === undefined || this.data[ key ] === null ) {
+			if ( !this.data.hasOwnProperty( key ) ) {
 				this.data[ key ] = val;
 			}
-			this.params[ key ] = val;
+			if ( !this.params.hasOwnProperty( key ) ) {
+				this.params[ key ] = val;
+			}
 		}.bind( this ) );
@@ -39,0 +41,0 @@ }.bind( this ) );

src/http/passport.js

@@ -48,8 +48,13 @@ var _ = require( 'lodash' );
 		// during a socket connection, express is not fully initialized and this call fails ... hard
-		try {
-			res.status( 500 ).send( 'Could not determine user permissions' );
-		} catch ( err ) {
-			log.warn( 'Could not reply with user permission error to request before express is fully initialized.' );
+		if( state.config.socketio && /socket.io/.test( req.url ) ) {
+			next();
+		} else if ( state.config.websocket && /websocket/.test( req.url ) ) {
+			next();
+		} else {
+			try {
+				res.status( 500 ).send( { message: 'Could not determine user permissions' } );
+			} catch ( err ) {
+				log.warn( 'Could not reply with user permission error to request before express is fully initialized.' );
+			}
 		}
-		next();
 	}
@@ -56,0 +61,0 @@

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

187820

increased by27.35%

Number of package files

31

increased by6.9%

Lines of code

2519

increased by1%

Number of lines in readme file

773

increased by0.26%

No dependency changes