From 0c46b044da276aac0f727c44a681c90cdd2df3e3 Mon Sep 17 00:00:00 2001 From: Isaac Abadi Date: Sat, 6 May 2023 23:29:20 -0400 Subject: [PATCH] Improved tests for multi-user mode --- backend/app.js | 44 +++++++++++------ backend/authentication/auth.js | 86 ++++++++++++++-------------------- backend/test/tests.js | 26 ++++++---- 3 files changed, 81 insertions(+), 75 deletions(-) diff --git a/backend/app.js b/backend/app.js index 43ab6bf..2df03cf 100644 --- a/backend/app.js +++ b/backend/app.js @@ -1926,9 +1926,34 @@ app.post('/api/clearAllLogs', optionalJwt, async function(req, res) { // user authentication -app.post('/api/auth/register' - , optionalJwt - , auth_api.registerUser); +app.post('/api/auth/register', optionalJwt, async (req, res) => { + const userid = req.body.userid; + const username = req.body.username; + const plaintextPassword = req.body.password; + + if (userid !== 'admin' && !config_api.getConfigItem('ytdl_allow_registration') && !req.isAuthenticated() && (!req.user || !exports.userHasPermission(req.user.uid, 'settings'))) { + logger.error(`Registration failed for user ${userid}. Registration is disabled.`); + res.sendStatus(409); + return; + } + + if (plaintextPassword === "") { + logger.error(`Registration failed for user ${userid}. A password must be provided.`); + res.sendStatus(409); + return; + } + + const new_user = await auth_api.registerUser(userid, username, plaintextPassword); + + if (!new_user) { + res.sendStatus(409); + return; + } + + res.send({ + user: new_user + }); +}); app.post('/api/auth/login' , auth_api.passport.authenticate(['local', 'ldapauth'], {}) , auth_api.generateJWT @@ -1980,18 +2005,7 @@ app.post('/api/updateUser', optionalJwt, async (req, res) => { app.post('/api/deleteUser', optionalJwt, async (req, res) => { let uid = req.body.uid; try { - let success = false; - let usersFileFolder = config_api.getConfigItem('ytdl_users_base_path'); - const user_folder = path.join(__dirname, usersFileFolder, uid); - const user_db_obj = await db_api.getRecord('users', {uid: uid}); - if (user_db_obj) { - // user exists, let's delete - await fs.remove(user_folder); - await db_api.removeRecord('users', {uid: uid}); - success = true; - } else { - logger.error(`Could not find user with uid ${uid}`); - } + const success = await auth_api.deleteUser(uid); res.send({success: success}); } catch (err) { logger.error(err); diff --git a/backend/authentication/auth.js b/backend/authentication/auth.js index 30e1e02..69fa6c7 100644 --- a/backend/authentication/auth.js +++ b/backend/authentication/auth.js @@ -6,6 +6,8 @@ const db_api = require('../db'); const jwt = require('jsonwebtoken'); const { uuid } = require('uuidv4'); const bcrypt = require('bcryptjs'); +const fs = require('fs-extra'); +const path = require('path'); var LocalStrategy = require('passport-local').Strategy; var LdapStrategy = require('passport-ldapauth'); @@ -16,7 +18,7 @@ var JwtStrategy = require('passport-jwt').Strategy, let SERVER_SECRET = null; let JWT_EXPIRATION = null; let opts = null; -let saltRounds = null; +let saltRounds = 10; exports.initialize = function () { /************************* @@ -31,8 +33,6 @@ exports.initialize = function () { }); } - saltRounds = 10; - // Sometimes this value is not properly typed: https://github.com/Tzahi12345/YoutubeDL-Material/issues/813 JWT_EXPIRATION = config_api.getConfigItem('ytdl_jwt_expiration'); if (!(+JWT_EXPIRATION)) { @@ -113,55 +113,41 @@ exports.passport.deserializeUser(function(user, done) { /*************************************** * Register user with hashed password **************************************/ -exports.registerUser = async function(req, res) { - var userid = req.body.userid; - var username = req.body.username; - var plaintextPassword = req.body.password; - - if (userid !== 'admin' && !config_api.getConfigItem('ytdl_allow_registration') && !req.isAuthenticated() && (!req.user || !exports.userHasPermission(req.user.uid, 'settings'))) { - res.sendStatus(409); - logger.error(`Registration failed for user ${userid}. Registration is disabled.`); - return; - } - if (plaintextPassword === "") { - res.sendStatus(400); - logger.error(`Registration failed for user ${userid}. A password must be provided.`); - return; +exports.registerUser = async (userid, username, plaintextPassword) => { + const hash = await bcrypt.hash(plaintextPassword, saltRounds); + const new_user = generateUserObject(userid, username, hash); + // check if user exists + if (await db_api.getRecord('users', {uid: userid})) { + // user id is taken! + logger.error('Registration failed: UID is already taken!'); + return null; + } else if (await db_api.getRecord('users', {name: username})) { + // user name is taken! + logger.error('Registration failed: User name is already taken!'); + return null; + } else { + // add to db + await db_api.insertRecordIntoTable('users', new_user); + logger.verbose(`New user created: ${new_user.name}`); + return new_user; } +} - bcrypt.hash(plaintextPassword, saltRounds) - .then(async function(hash) { - let new_user = generateUserObject(userid, username, hash); - // check if user exists - if (await db_api.getRecord('users', {uid: userid})) { - // user id is taken! - logger.error('Registration failed: UID is already taken!'); - res.status(409).send('UID is already taken!'); - } else if (await db_api.getRecord('users', {name: username})) { - // user name is taken! - logger.error('Registration failed: User name is already taken!'); - res.status(409).send('User name is already taken!'); - } else { - // add to db - await db_api.insertRecordIntoTable('users', new_user); - logger.verbose(`New user created: ${new_user.name}`); - res.send({ - user: new_user - }); - } - }) - .then(function(result) { - - }) - .catch(function(err) { - logger.error(err); - if( err.code == 'ER_DUP_ENTRY' ) { - res.status(409).send('UserId already taken'); - } else { - res.sendStatus(409); - } - }); +exports.deleteUser = async (uid) => { + let success = false; + let usersFileFolder = config_api.getConfigItem('ytdl_users_base_path'); + const user_folder = path.join(__dirname, usersFileFolder, uid); + const user_db_obj = await db_api.getRecord('users', {uid: uid}); + if (user_db_obj) { + // user exists, let's delete + await fs.remove(user_folder); + await db_api.removeRecord('users', {uid: uid}); + success = true; + } else { + logger.error(`Could not find user with uid ${uid}`); + } + return success; } /*************************************** @@ -326,7 +312,7 @@ exports.getUserVideos = async function(user_uid, type) { } exports.getUserVideo = async function(user_uid, file_uid, requireSharing = false) { - let file = await db_api.getRecord('files', {file_uid: file_uid}); + let file = await db_api.getRecord('files', {uid: file_uid}); // prevent unauthorized users from accessing the file info if (file && !file['sharingEnabled'] && requireSharing) file = null; diff --git a/backend/test/tests.js b/backend/test/tests.js index 0d2057c..a43a93b 100644 --- a/backend/test/tests.js +++ b/backend/test/tests.js @@ -336,16 +336,22 @@ describe('Database', async function() { }); describe('Multi User', async function() { - let user = null; - const user_to_test = 'admin'; - const sub_to_test = 'dc834388-3454-41bf-a618-e11cb8c7de1c'; - const playlist_to_test = 'ysabVZz4x'; + const user_to_test = 'test_user'; + const user_password = 'test_pass'; + const sub_to_test = ''; + const playlist_to_test = ''; beforeEach(async function() { await db_api.connectToDB(); - user = await auth_api.login('admin', 'pass'); + await auth_api.deleteUser(user_to_test); }); - describe('Authentication', function() { - it('login', async function() { + describe('Basic', function() { + it('Register', async function() { + const user = await auth_api.registerUser(user_to_test, user_to_test, user_password); + assert(user); + }); + it('Login', async function() { + await auth_api.registerUser(user_to_test, user_to_test, user_password); + const user = await auth_api.login(user_to_test, user_password); assert(user); }); }); @@ -361,14 +367,14 @@ describe('Multi User', async function() { }); it('Video access - disallowed', async function() { - await db_api.setVideoProperty(video_to_test, {sharingEnabled: false}, user_to_test); - const video_obj = auth_api.getUserVideo('admin', video_to_test, true); + await db_api.setVideoProperty(video_to_test, {sharingEnabled: false}); + const video_obj = auth_api.getUserVideo(user_to_test, video_to_test, true); assert(!video_obj); }); it('Video access - allowed', async function() { await db_api.setVideoProperty(video_to_test, {sharingEnabled: true}, user_to_test); - const video_obj = auth_api.getUserVideo('admin', video_to_test, true); + const video_obj = auth_api.getUserVideo(user_to_test, video_to_test, true); assert(video_obj); }); });