Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: u&p service hashes passwords #20147

Merged
merged 6 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 21 additions & 4 deletions packages/plugins/users-permissions/server/services/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const bcrypt = require('bcryptjs');
const urlJoin = require('url-join');

const { sanitize } = require('@strapi/utils');
const { toNumber, getOr } = require('lodash/fp');
const { getService } = require('../utils');

const USER_MODEL_UID = 'plugin::users-permissions.user';
Expand All @@ -27,18 +28,34 @@ module.exports = ({ strapi }) => ({
},

/**
* Promise to search count users
* Hashes password fields in the provided values object if they are present.
* It checks each key in the values object against the model's attributes and
* hashes it if the attribute type is 'password',
*
* @return {Promise}
* @param {object} values - The object containing the fields to be hashed.
* @return {object} The values object with hashed password fields if they were present.
*/
async ensureHashedPasswords(values) {
const attributes = strapi.getModel(USER_MODEL_UID).attributes;

for (const key in values) {
if (attributes[key] && attributes[key].type === 'password') {
// Check if a custom encryption.rounds has been set on the password attribute
const rounds = toNumber(getOr(10, 'encryption.rounds', attributes[key]));
values[key] = await bcrypt.hash(values[key], rounds);
}
}

return values;
},

/**
* Promise to add a/an user.
* @return {Promise}
*/
async add(values) {
return strapi.db.query(USER_MODEL_UID).create({
data: values,
data: await this.ensureHashedPasswords(values),
populate: ['role'],
});
},
Expand All @@ -52,7 +69,7 @@ module.exports = ({ strapi }) => ({
async edit(userId, params = {}) {
return strapi.db.query(USER_MODEL_UID).update({
where: { id: userId },
data: params,
data: await this.ensureHashedPasswords(params),
populate: ['role'],
});
},
Expand Down
13 changes: 12 additions & 1 deletion tests/api/plugins/users-permissions/content-api/auth.test.api.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const bcrypt = require('bcryptjs');

const { createStrapiInstance } = require('api-tests/strapi');
const { createRequest } = require('api-tests/request');
const { createAuthenticatedUser } = require('../utils');
Expand All @@ -20,7 +22,7 @@ const internals = {

const data = {};

describe.skip('Auth API', () => {
describe('Auth API', () => {
beforeAll(async () => {
strapi = await createStrapiInstance({ bypassAuth: false });

Expand Down Expand Up @@ -111,6 +113,15 @@ describe.skip('Auth API', () => {
},
});

// check that password was hashed
const user = await strapi.db.query('plugin::users-permissions.user').findOne({
where: {
email: internals.user.email.toLowerCase(),
},
});
expect(bcrypt.compareSync(internals.newPassword, user.password)).toBe(true);

// check results
expect(res.statusCode).toBe(200);
expect(res.body).toMatchObject({
jwt: expect.any(String),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

// Test a simple default API with no relations

const bcrypt = require('bcryptjs');
const { createStrapiInstance } = require('api-tests/strapi');
const { createContentAPIRequest } = require('api-tests/request');

Expand Down Expand Up @@ -73,6 +74,15 @@ describe('Users API', () => {
body: user,
});

// check that password was hashed
const userDb = await strapi.db.query('plugin::users-permissions.user').findOne({
where: {
email: user.email,
},
});

expect(bcrypt.compareSync(user.password, userDb.password)).toBe(true);

expect(res.statusCode).toBe(201);
expect(res.body).toMatchObject({
username: user.username,
Expand Down