From 13b73acee6bdd31582759c9812bf8b5329c1897d Mon Sep 17 00:00:00 2001
From: Julien Fontanet <julien.fontanet@isonoe.net>
Date: Mon, 30 Dec 2013 17:19:36 +0100
Subject: [PATCH] The `user` namespace of the API resides in its own module.

It has been rewritten in CoffeeScript, a lot of bugs have been fixed
and it is much more commented.
---
 src/api.js          | 167 ++++++++------------------------------------
 src/api/user.coffee | 113 ++++++++++++++++++++++++++++++
 src/xo.coffee       |   2 +-
 3 files changed, 142 insertions(+), 140 deletions(-)
 create mode 100644 src/api/user.coffee

diff --git a/src/api.js b/src/api.js
index 59a4f5ed8..10d6f453d 100644
--- a/src/api.js
+++ b/src/api.js
@@ -158,26 +158,51 @@ Api.prototype.getUserPublicProperties = function (user) {
 	return _.pick(properties, 'id', 'email', 'permission');
 };
 
+Api.prototype.throw = function (errorId) {
+	throw Api.err[errorId];
+};
+
 //////////////////////////////////////////////////////////////////////
 
 Api.fn  = {};
 
 var $register = function (path, fn) {
+	var component, current;
+
 	if (!_.isArray(path))
 	{
 		path = path.split('.');
 	}
 
-	var current = Api.fn;
+	current = Api.fn;
 	for (var i = 0, n = path.length - 1; i < n; ++i)
 	{
-		var component = path[i];
+		component = path[i];
 		current = (current[component] || (current[component] = {}));
 	}
 
-	current[path[n]] = fn;
+	if (_.isFunction(fn))
+	{
+		current[path[n]] = fn;
+	}
+	else
+	{
+		// If it is not an function but an object, copies its
+		// properties.
+
+		component = path[n];
+		current = (current[component] || (current[component] = {}));
+
+		for (var prop in fn)
+		{
+			current[prop] = fn[prop];
+		}
+	}
 };
 
+// User management.
+$register('user', require('./api/user'));
+
 //--------------------------------------------------------------------
 
 Api.fn.api = {
@@ -262,142 +287,6 @@ Api.fn.session = {
 	'destroyToken': 'token.delete',
 };
 
-// User management.
-Api.fn.user = {
-	'create': function (session, req) {
-		var p_email = req.params.email;
-		var p_pass = req.params.password;
-		var p_perm = req.params.permission;
-
-		if (!p_email || !p_pass)
-		{
-			throw Api.err.INVALID_PARAMS;
-		}
-
-		this.checkPermission(session, 'admin');
-
-		var user = $waitPromise(
-			this.xo.users.create(p_email, p_pass, p_perm)
-		);
-
-		return (''+ user.id);
-	},
-
-	'delete': function (session, req) {
-		var p_id = req.params.id;
-		if (undefined === p_id)
-		{
-			throw Api.err.INVALID_PARAMS;
-		}
-
-		this.checkPermission(session, 'admin');
-
-		if (!this.xo.users.remove(p_id))
-		{
-			throw Api.err.NO_SUCH_OBJECT;
-		}
-
-		return true;
-	},
-
-	'changePassword': function (session, req) {
-		var p_old = req.params.old;
-		var p_new = req.params['new'];
-		if ((undefined === p_old) || (undefined === p_new))
-		{
-			throw Api.err.INVALID_PARAMS;
-		}
-
-		var user_id = session.get('user_id');
-		if (undefined === user_id)
-		{
-			throw Api.err.UNAUTHORIZED;
-		}
-
-		var user = this.xo.users.first(user_id);
-		if (!user.checkPassword(p_old))
-		{
-			throw Api.err.INVALID_CREDENTIAL;
-		}
-
-		user.setPassword(p_new);
-		$waitPromise(this.xo.users.update(user));
-
-		return true;
-	},
-
-	'get': function (session, req) {
-		var p_id = req.params.id;
-		if (undefined === p_id)
-		{
-			throw Api.err.INVALID_PARAMS;
-		}
-
-		// Only an administrator can see another user.
-		if (session.get('user_id') !== p_id)
-		{
-			this.checkPermission(session, 'admin');
-		}
-
-		var user = $waitPromise(this.xo.users.first(p_id));
-		if (!user)
-		{
-			throw Api.err.NO_SUCH_OBJECT;
-		}
-
-		return _.pick(user.properties, 'id', 'email', 'permission');
-	},
-
-	'getAll': function (session) {
-		this.checkPermission(session, 'admin');
-
-		var users = $waitPromise(this.xo.users.get());
-		for (var i = 0, n = users.length; i < n; ++i)
-		{
-			users[i] = this.getUserPublicProperties(users[i]);
-		}
-		return users;
-	},
-
-	'set': function (session, request) {
-		var p_id = request.params.id;
-		var p_email = request.params.email;
-		var p_password = request.params.password;
-		var p_permission = request.params.permission;
-
-		if ((undefined === p_id)
-			|| ((undefined === p_email)
-				&& (undefined === p_password)
-				&& (undefined === p_permission)))
-		{
-			throw Api.err.INVALID_PARAMS;
-		}
-
-		this.checkPermission(session, 'admin');
-
-		// TODO: Check there are no invalid parameter.
-		var user = $waitPromise(this.xo.users.first(p_id));
-		// TODO: Check user exists.
-
-		// Gets the user to update.
-
-		// TODO: Check undefined value are ignored.
-		user.set({
-			'email': p_email,
-			'permission': p_permission,
-		});
-
-		if (p_password)
-		{
-			user.setPassword(p_password);
-		}
-
-		$waitPromise(this.xo.users.update(user));
-
-		return true;
-	},
-};
-
 // Token management.
 Api.fn.token = {
 	'create': function (session) {
diff --git a/src/api/user.coffee b/src/api/user.coffee
new file mode 100644
index 000000000..cf5038896
--- /dev/null
+++ b/src/api/user.coffee
@@ -0,0 +1,113 @@
+{$waitPromise} = require '../fibers-utils'
+
+#=====================================================================
+
+# Creates a new user.
+exports.create = (session, request) ->
+  {email, password, permission} = request.params
+  @throw 'INVALID_PARAMS' unless email? and password?
+
+  # Current user must be administrator.
+  @checkPermission session, 'admin'
+
+  # Creates the user.
+  user = $waitPromise @xo.users.create email, password, permission
+
+  # Returns the identifier of the new user.
+  user.id
+
+# Deletes an existing user.
+#
+# FIXME: a user should not be able to delete itself.
+exports.delete = (session, request) ->
+  {id} = request.params
+  @throw 'INVALID_PARAMS' unless id?
+
+  # Current user must be administrator.
+  @checkPermission session, 'admin'
+
+  # Throws an error if the user did not exist.
+  @throw 'NO_SUCH_OBJECT' unless $waitPromise @xo.users.remove id
+
+  # Returns true.
+  true
+
+# Changes the password of the current user.
+exports.changePassword = (session, request) ->
+  {old, new: newP} = request.params
+  @throw 'INVALID_PARAMS' unless p_old? and newP?
+
+  # Current user must be signed in.
+  @checkPermission session
+
+  # Gets the current user (which MUST exist).
+  user = $waitPromise @xo.users.first session.get 'user_id'
+
+  # Checks its old password.
+  @throw 'INVALID_CREDENTIAL' unless user.checkPassword old
+
+  # Sets the new password.
+  user.setPassword newP
+
+  # Updates the user.
+  $waitPromise @xo.users.update user
+
+  # Returns true.
+  true
+
+# Returns the user with a given identifier.
+exports.get = (session, request) ->
+  {id} = request.params
+  @throw 'INVALID_PARAMS' unless id?
+
+  # Only an administrator can see another user.
+  @checkPermission session, 'admin' unless session.get 'user_id' is p_id
+
+  # Retrieves the user.
+  user = $waitPromise @xo.users.first p_id
+
+  # Throws an error if it did not exist.
+  @throw 'NO_SUCH_OBJECT' unless user
+
+  # Returns public properties.
+  @getUserPublicProperties user
+
+# Returns all users.
+exports.getAll = (session) ->
+  # Only an administrator can see all users.
+  @checkPermission session, 'admin'
+
+  # Retrieves the users.
+  users = $waitPromise @xo.users.get()
+
+  # Filters out private properties.
+  for user, i in users
+    users[i] = @getUserPublicProperties user
+
+  # Returns the users.
+  users
+
+# Changes the properties of an existing user.
+exports.set = (session, request) ->
+  {id, email, password, permission} = request.params
+  @throw 'INVALID_PARAMS' unless id? and (email? or password? or permission?)
+
+  # Only an administrator can modify an user.
+  @checkPermission session, 'admin'
+
+  # Retrieves the user.
+  user = $waitPromise @xo.users.first id
+
+  # Throws an error if it did not exist.
+  @throw 'NO_SUCH_OBJECT' unless user
+
+  # Updates the provided properties.
+  user.set {email} if email?
+  user.set {permission} if permission?
+  user.setPassword password if password?
+
+  # Updates the user.
+  $waitPromise @xo.users.update user
+
+  # Returns true.
+  true
diff --git a/src/xo.coffee b/src/xo.coffee
index e5745147c..316bf443d 100644
--- a/src/xo.coffee
+++ b/src/xo.coffee
@@ -147,7 +147,7 @@ class $XO extends $EventEmitter
     @users.on 'remove', (ids) =>
       @emit "user.revoked:#{id}" for id in ids
       tokens = @tokens.get {user_id: id}
-      @token.remove (token.id for token in tokens)
+      @tokens.remove (token.id for token in tokens)
 
     # Collections of XAPI objects mapped to XO API.
     refsToUUIDs = { # Needed for the mapping.