Merge pull request 'refs #6434 feat: improve signIn method' (!1867) from 6434-signInLog_improve_test into test
gitea/salix/pipeline/head This commit looks good Details

Reviewed-on: #1867
Reviewed-by: Alex Moreno <alexm@verdnatura.es>
Reviewed-by: Javi Gallego <jgallego@verdnatura.es>
This commit is contained in:
Javier Segarra 2023-11-30 11:20:57 +00:00
commit b241efc961
5 changed files with 56 additions and 28 deletions

View File

@ -49,13 +49,7 @@ module.exports = Self => {
if (vnUser.twoFactor) if (vnUser.twoFactor)
throw new ForbiddenError(null, 'REQUIRES_2FA'); throw new ForbiddenError(null, 'REQUIRES_2FA');
} }
const validateLogin = await Self.validateLogin(user, password); return Self.validateLogin(user, password, ctx);
await Self.app.models.SignInLog.create({
token: validateLogin.token,
userFk: vnUser.id,
ip: ctx.req.ip
});
return validateLogin;
}; };
Self.passExpired = async vnUser => { Self.passExpired = async vnUser => {

View File

@ -2,7 +2,7 @@ const {models} = require('vn-loopback/server/server');
describe('VnUser Sign-in()', () => { describe('VnUser Sign-in()', () => {
const employeeId = 1; const employeeId = 1;
const unauthCtx = { const unAuthCtx = {
req: { req: {
headers: {}, headers: {},
connection: { connection: {
@ -15,20 +15,21 @@ describe('VnUser Sign-in()', () => {
const {VnUser, AccessToken, SignInLog} = models; const {VnUser, AccessToken, SignInLog} = models;
describe('when credentials are correct', () => { describe('when credentials are correct', () => {
it('should return the token if user uses email', async() => { it('should return the token if user uses email', async() => {
let login = await VnUser.signIn(unauthCtx, 'salesAssistant@mydomain.com', 'nightmare'); let login = await VnUser.signIn(unAuthCtx, 'salesAssistant@mydomain.com', 'nightmare');
let accessToken = await AccessToken.findById(login.token); let accessToken = await AccessToken.findById(login.token);
let ctx = {req: {accessToken: accessToken}}; let ctx = {req: {accessToken: accessToken}};
let signInLog = await SignInLog.find({where: {token: accessToken.id}}); let signInLog = await SignInLog.find({where: {token: accessToken.id}});
expect(signInLog.length).toEqual(1); expect(signInLog.length).toEqual(1);
expect(signInLog[0].userFk).toEqual(accessToken.userId); expect(signInLog[0].userFk).toEqual(accessToken.userId);
expect(signInLog[0].owner).toEqual(true);
expect(login.token).toBeDefined(); expect(login.token).toBeDefined();
await VnUser.logout(ctx.req.accessToken.id); await VnUser.logout(ctx.req.accessToken.id);
}); });
it('should return the token', async() => { it('should return the token', async() => {
let login = await VnUser.signIn(unauthCtx, 'salesAssistant', 'nightmare'); let login = await VnUser.signIn(unAuthCtx, 'salesAssistant', 'nightmare');
let accessToken = await AccessToken.findById(login.token); let accessToken = await AccessToken.findById(login.token);
let ctx = {req: {accessToken: accessToken}}; let ctx = {req: {accessToken: accessToken}};
@ -38,7 +39,7 @@ describe('VnUser Sign-in()', () => {
}); });
it('should return the token if the user doesnt exist but the client does', async() => { it('should return the token if the user doesnt exist but the client does', async() => {
let login = await VnUser.signIn(unauthCtx, 'PetterParker', 'nightmare'); let login = await VnUser.signIn(unAuthCtx, 'PetterParker', 'nightmare');
let accessToken = await AccessToken.findById(login.token); let accessToken = await AccessToken.findById(login.token);
let ctx = {req: {accessToken: accessToken}}; let ctx = {req: {accessToken: accessToken}};
@ -53,7 +54,7 @@ describe('VnUser Sign-in()', () => {
let error; let error;
try { try {
await VnUser.signIn(unauthCtx, 'IDontExist', 'TotallyWrongPassword'); await VnUser.signIn(unAuthCtx, 'IDontExist', 'TotallyWrongPassword');
} catch (e) { } catch (e) {
error = e; error = e;
} }
@ -74,7 +75,7 @@ describe('VnUser Sign-in()', () => {
const options = {transaction: tx}; const options = {transaction: tx};
await employee.updateAttribute('twoFactor', 'email', options); await employee.updateAttribute('twoFactor', 'email', options);
await VnUser.signIn(unauthCtx, 'employee', 'nightmare', options); await VnUser.signIn(unAuthCtx, 'employee', 'nightmare', options);
await tx.rollback(); await tx.rollback();
} catch (e) { } catch (e) {
await tx.rollback(); await tx.rollback();
@ -99,7 +100,7 @@ describe('VnUser Sign-in()', () => {
const options = {transaction: tx}; const options = {transaction: tx};
await employee.updateAttribute('passExpired', yesterday, options); await employee.updateAttribute('passExpired', yesterday, options);
await VnUser.signIn(unauthCtx, 'employee', 'nightmare', options); await VnUser.signIn(unAuthCtx, 'employee', 'nightmare', options);
await tx.rollback(); await tx.rollback();
} catch (e) { } catch (e) {
await tx.rollback(); await tx.rollback();

View File

@ -124,20 +124,42 @@ module.exports = function(Self) {
return email.send(); return email.send();
}); });
Self.signInValidate = (user, userToken) => {
/**
* Sign-in validate
* @param {String} user The user
* @param {Object} userToken Options
* @param {Object} token accessToken
* @param {Object} ctx context
*/
Self.signInValidate = async(user, userToken, token, ctx) => {
const [[key, value]] = Object.entries(Self.userUses(user)); const [[key, value]] = Object.entries(Self.userUses(user));
if (userToken[key].toLowerCase().trim() !== value.toLowerCase().trim()) { const isOwner = Self.rawSql(`SELECT ? = ? `, [userToken[key], value]);
console.error('ERROR!!! - Signin with other user', userToken, user); await Self.app.models.SignInLog.create({
userName: user,
token: token.id,
userFk: userToken.id,
ip: ctx.req.ip,
owner: isOwner
});
if (!isOwner)
throw new UserError('Try again'); throw new UserError('Try again');
}
}; };
Self.validateLogin = async function(user, password) { /**
* Validate login params
* @param {String} user The user
* @param {String} password
* @param {Object} ctx context
*/
Self.validateLogin = async function(user, password, ctx) {
const loginInfo = Object.assign({password}, Self.userUses(user)); const loginInfo = Object.assign({password}, Self.userUses(user));
const token = await Self.login(loginInfo, 'user'); const token = await Self.login(loginInfo, 'user');
const userToken = await token.user.get(); const userToken = await token.user.get();
Self.signInValidate(user, userToken);
if (ctx)
await Self.signInValidate(user, userToken, token, ctx);
try { try {
await Self.app.models.Account.sync(userToken.name, password); await Self.app.models.Account.sync(userToken.name, password);
@ -226,10 +248,12 @@ module.exports = function(Self) {
const env = process.env.NODE_ENV; const env = process.env.NODE_ENV;
const liliumUrl = await Self.app.models.Url.findOne({ const liliumUrl = await Self.app.models.Url.findOne({
where: {and: [ where: {
and: [
{appName: 'lilium'}, {appName: 'lilium'},
{environment: env} {environment: env}
]} ]
}
}); });
class Mailer { class Mailer {

View File

@ -1,5 +1,4 @@
-- --
-- Table structure for table `signInLog` -- Table structure for table `signInLog`
-- Description: log to debug cross-login error -- Description: log to debug cross-login error
@ -13,7 +12,9 @@ CREATE TABLE `account`.`signInLog` (
`token` varchar(255) NOT NULL , `token` varchar(255) NOT NULL ,
`userFk` int(10) unsigned DEFAULT NULL, `userFk` int(10) unsigned DEFAULT NULL,
`creationDate` timestamp NULL DEFAULT current_timestamp(), `creationDate` timestamp NULL DEFAULT current_timestamp(),
`userName` varchar(30) NOT NULL,
`ip` varchar(100) CHARACTER SET utf8mb3 COLLATE utf8mb3_general_ci NOT NULL, `ip` varchar(100) CHARACTER SET utf8mb3 COLLATE utf8mb3_general_ci NOT NULL,
`owner` tinyint(1) DEFAULT 1,
KEY `userFk` (`userFk`), KEY `userFk` (`userFk`),
CONSTRAINT `signInLog_ibfk_1` FOREIGN KEY (`userFk`) REFERENCES `user` (`id`) ON DELETE CASCADE ON UPDATE CASCADE CONSTRAINT `signInLog_ibfk_1` FOREIGN KEY (`userFk`) REFERENCES `user` (`id`) ON DELETE CASCADE ON UPDATE CASCADE
); );

View File

@ -26,6 +26,14 @@
}, },
"ip": { "ip": {
"type": "string" "type": "string"
},
"userName": {
"type": "string"
},
"owner": {
"type": "boolean",
"required": true,
"default": true
} }
}, },
"relations": { "relations": {