hidden groups are invisible to non-members, #2758

src/controllers/groups.js

@@ -12,7 +12,7 @@ groupsController.list = function(req, res, next) {
 	groups.list({
 		truncateUserList: true,
 		expand: true,
-		uid: req.user ? req.user.uid : 0
+		uid: req.user ? parseInt(req.user.uid, 10) : 0
 	}, function(err, groups) {
 		if (err) {
 			return next(err);
@@ -27,17 +27,35 @@ groupsController.list = function(req, res, next) {
 groupsController.details = function(req, res, next) {
 	var uid = req.user ? parseInt(req.user.uid, 10) : 0;
 
-	groups.existsBySlug(req.params.slug, function(err, exists) {
-		if (exists) {
+	async.waterfall([
+		async.apply(groups.exists, res.locals.groupName),
+		function(exists, next) {
+			if (!exists) { return next(undefined, null); }
+
+			// Ensure the group isn't hidden either
+			groups.isHidden(res.locals.groupName, next);
+		},
+		function(hidden, next) {
+			if (hidden === null) { return next(undefined, false); }		// Group didn't exist, not ok
+
+			if (!hidden) {
+				next(null, true);
+			} else {
+				// If not, only members are granted access
+				groups.isMember(uid, res.locals.groupName, next);
+			}
+		}
+	], function(err, ok) {
+		if (ok) {
 			async.parallel({
 				group: function(next) {
-					groups.getByGroupslug(req.params.slug, {
+					groups.get(res.locals.groupName, {
 						expand: true,
 						uid: uid
 					}, next);
 				},
 				posts: function(next) {
-					groups.getLatestMemberPosts(req.params.slug, 10, uid, next);
+					groups.getLatestMemberPosts(res.locals.groupName, 10, uid, next);
 				}
 			}, function(err, results) {
 				if (err) {

src/groups.js

@@ -31,7 +31,7 @@ var async = require('async'),
 						if (!group) {
 							return false;
 						}
-						if (group.deleted || (group.hidden && !group.system) || (!options.showSystemGroups && group.system)) {
+						if (group.deleted || (group.hidden && !group.system && !group.isMember) || (!options.showSystemGroups && group.system)) {
 							return false;
 						} else if (options.removeEphemeralGroups && ephemeralGroups.indexOf(group.name) !== -1) {
 							return false;
@@ -61,12 +61,16 @@ var async = require('async'),
 				}
 
 				return groups;
-			}/*,
-			fixImageUrl: function(url) {
-				if (url) {
-					return url.indexOf('http') === -1 ? nconf.get('relative_path') + url : url;
-				}
-			}*/
+			},
+			applyWithName: function(method, args) {
+				// This method takes a slug and reapplies the passed-in call with the proper group name
+				Groups.getGroupNameByGroupSlug(args[0], function(err, groupName) {	// Assuming slug is the first argument
+					// As there is no good way of determining whether the last argument is the callback,
+					// if getGroupNameByGroupSlug fails, we continue assuming the group name is the same as the slug
+					if (!err) { Array.prototype.splice.call(args, 0, 1, groupName); }
+					method.apply(Groups, args);
+				});
+			}
 		};
 
 	Groups.list = function(options, callback) {
@@ -89,9 +93,6 @@ var async = require('async'),
 	};
 
 	Groups.get = function(groupName, options, callback) {
-		if (!arguments[0]) {
-			console.log(new Error.stack);
-		}
 		var	truncated = false,
 			numUsers;
 
@@ -275,6 +276,17 @@ var async = require('async'),
 		});
 	};
 
+	Groups.isHidden = function(groupName, callback) {
+		Groups.getGroupFields(groupName, ['hidden'], function(err, values) {
+			if (err) {
+				winston.warn('[groups.isHidden] Could not determine group hidden state (group: ' + groupName + ')');
+				return callback(null, true);	// Default true
+			}
+
+			callback(null, parseInt(values.hidden, 10));
+		});
+	};
+
 	Groups.getMembers = function(groupName, start, end, callback) {
 		db.getSortedSetRevRange('group:' + groupName + ':members', start, end, callback);
 	};
@@ -801,12 +813,9 @@ var async = require('async'),
 		});
 	};
 
-	Groups.getLatestMemberPosts = function(groupSlug, max, uid, callback) {
+	Groups.getLatestMemberPosts = function(groupName, max, uid, callback) {
 		async.waterfall([
-			async.apply(Groups.getGroupNameByGroupSlug, groupSlug),
-			function(groupName, next) {
-				Groups.getMembers(groupName, 0, -1, next);
-			},
+			async.apply(Groups.getMembers, groupName, 0, -1),
 			function(uids, next) {
 				if (!Array.isArray(uids) || !uids.length) {
 					return callback(null, []);

src/middleware/middleware.js

@@ -15,6 +15,7 @@ var app,
 	meta = require('./../meta'),
 	translator = require('./../../public/src/translator'),
 	user = require('./../user'),
+	groups = require('./../groups'),
 	db = require('./../database'),
 	categories = require('./../categories'),
 	topics = require('./../topics'),
@@ -518,6 +519,17 @@ middleware.publicTagListing = function(req, res, next) {
 	}
 };
 
+middleware.exposeGroupName = function(req, res, next) {
+	if (!req.params.hasOwnProperty('slug')) { return next(); }
+
+	groups.getGroupNameByGroupSlug(req.params.slug, function(err, groupName) {
+		if (err) { return next(err); }
+
+		res.locals.groupName = groupName;
+		next();
+	});
+};
+
 module.exports = function(webserver) {
 	app = webserver;
 	middleware.admin = require('./admin')(webserver);

src/routes/index.js

@@ -92,7 +92,7 @@ function userRoutes(app, middleware, controllers) {
  }
 
 function groupRoutes(app, middleware, controllers) {
-	var middlewares = [middleware.checkGlobalPrivacySettings];
+	var middlewares = [middleware.checkGlobalPrivacySettings, middleware.exposeGroupName];
 
 	setupPageRoute(app, '/groups', middleware, middlewares, controllers.groups.list);
 	setupPageRoute(app, '/groups/:slug', middleware, middlewares, controllers.groups.details);