-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add initial seeding support using users #285
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
94ddd01
Add initial seeding support using users
Cruikshanks e4cbff5
Add package.json run script
Cruikshanks 290ac7e
Add default dev/test user password
Cruikshanks 2f23555
Add first seed file for users
Cruikshanks 79ba6c6
Add endpoint to trigger seed process
Cruikshanks adf87e4
Add some tests
Cruikshanks d6ca1ec
Merge branch 'main' into add-initial-seeding-users
Cruikshanks ce3fa1c
Get the alphabet correct!
Cruikshanks 987e539
Refactor insert where not exists pattern
Cruikshanks 39c4314
Refactor updating our seed data with ID's
Cruikshanks 19ce6b0
Sort out the order of functions
Cruikshanks fc001f9
Use our standard convention for module.exports
Cruikshanks 5183c45
Typo in comment
Cruikshanks a19577a
Implement suggestion
Cruikshanks a5bd591
Implement @Jozzey salt suggestion
Cruikshanks 1e9d9a1
Merge branch 'main' into add-initial-seeding-users
Cruikshanks File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
'use strict' | ||
|
||
/** | ||
* Runs the Knex seed process programmatically | ||
* @module SeedService | ||
*/ | ||
|
||
const { db } = require('../../../../db/db.js') | ||
|
||
/** | ||
* Triggers the Knex seed process programmatically | ||
* | ||
* This is the same as calling `knex seed:run` on the command line. Only we pull in `db.js` because that is our file | ||
* which sets up Knex with the right config and all our 'tweaks'. | ||
* | ||
* In this context you can read `db.seed.run()` as `knex.seed.run()`. | ||
* | ||
* See {@link https://knexjs.org/guide/migrations.html#seed-files | Seed files} for more details. | ||
* | ||
* Credit to {@link https://stackoverflow.com/a/53169879 | Programmatically run knex seed:run} | ||
*/ | ||
async function go () { | ||
await db.seed.run() | ||
} | ||
|
||
module.exports = { | ||
go | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,141 @@ | ||
'use strict' | ||
|
||
const bcrypt = require('bcryptjs') | ||
const { randomUUID } = require('crypto') | ||
|
||
const DatabaseConfig = require('../../config/database.config.js') | ||
|
||
const seedUsers = [ | ||
{ | ||
userName: '[email protected]', | ||
application: 'water_admin', | ||
group: 'super' | ||
}, | ||
{ | ||
userName: '[email protected]', | ||
application: 'water_admin', | ||
group: 'super' | ||
}, | ||
{ | ||
userName: '[email protected]', | ||
application: 'water_admin', | ||
group: 'environment_officer' | ||
}, | ||
{ | ||
userName: '[email protected]', | ||
application: 'water_admin', | ||
group: 'wirs' | ||
}, | ||
{ | ||
userName: '[email protected]', | ||
application: 'water_admin', | ||
group: 'billing_and_data' | ||
}, | ||
{ | ||
userName: '[email protected]', | ||
application: 'water_admin', | ||
group: 'psc' | ||
}, | ||
{ | ||
userName: '[email protected]', | ||
application: 'water_vml' | ||
}, | ||
{ | ||
userName: '[email protected]', | ||
application: 'water_vml' | ||
}, | ||
{ | ||
userName: '[email protected]', | ||
application: 'water_vml' | ||
} | ||
] | ||
|
||
async function seed (knex) { | ||
await _insertUsersWhereNotExists(knex) | ||
|
||
await _updateSeedUsersWithUserIdAndGroupId(knex) | ||
|
||
await _insertUserGroupsWhereNotExists(knex) | ||
} | ||
|
||
function _generateHashedPassword () { | ||
// 10 is the number of salt rounds to perform to generate the salt. The legacy code uses | ||
// const salt = bcrypt.genSaltSync(10) to pre-generate the salt before passing it to hashSync(). But this is | ||
// intended for operations where you need to hash a large number of values. If you just pass in a number bcrypt will | ||
// autogenerate the salt for you. | ||
// https://github.com/kelektiv/node.bcrypt.js#usage | ||
return bcrypt.hashSync(DatabaseConfig.defaultUserPassword, 10) | ||
} | ||
|
||
async function _groups (knex) { | ||
return knex('idm.groups') | ||
.select('groupId', 'group') | ||
} | ||
|
||
async function _insertUsersWhereNotExists (knex) { | ||
const password = _generateHashedPassword() | ||
|
||
for (const seedUser of seedUsers) { | ||
const existingUser = await knex('idm.users') | ||
.first('userId') | ||
.where('userName', seedUser.userName) | ||
.andWhere('application', seedUser.application) | ||
|
||
if (!existingUser) { | ||
await knex('idm.users') | ||
.insert({ | ||
userName: seedUser.userName, | ||
application: seedUser.application, | ||
password, | ||
userData: '{ "source": "Seeded" }', | ||
resetRequired: 0, | ||
badLogins: 0 | ||
}) | ||
} | ||
} | ||
} | ||
|
||
async function _insertUserGroupsWhereNotExists (knex) { | ||
const seedUsersWithGroups = seedUsers.filter((seedData) => seedData.group) | ||
|
||
for (const seedUser of seedUsersWithGroups) { | ||
const existingUserGroup = await knex('idm.userGroups') | ||
.select('userGroupId') | ||
.where('userId', seedUser.userId) | ||
.andWhere('groupId', seedUser.groupId) | ||
|
||
if (!existingUserGroup) { | ||
await knex('idm.userGroups') | ||
.insert({ | ||
userGroupId: randomUUID({ disableEntropyCache: true }), | ||
userId: seedUser.userId, | ||
groupId: seedUser.groupId | ||
}) | ||
} | ||
} | ||
} | ||
|
||
async function _updateSeedUsersWithUserIdAndGroupId (knex) { | ||
const users = await _users(knex) | ||
const groups = await _groups(knex) | ||
|
||
seedUsers.forEach((seedUser) => { | ||
const user = users.find(({ userName }) => userName === seedUser.userName) | ||
seedUser.userId = user.userId | ||
|
||
if (seedUser.group) { | ||
const userGroup = groups.find(({ group }) => group === seedUser.group) | ||
seedUser.groupId = userGroup.groupId | ||
} | ||
}) | ||
} | ||
|
||
async function _users (knex) { | ||
return knex('idm.users') | ||
.select('userId', 'userName') | ||
.whereJsonPath('userData', '$.source', '=', 'Seeded') | ||
} | ||
|
||
module.exports = { | ||
seed | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could destructure
defaultUserPassword
if we wanted seeing as it's the only bit ofDatabaseConfig
we use here? Not going to insist on it though 😁There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think (as in you have forced me to think why I did this!) when we are talking about a value as opposed to a function it's best to leave the object so you have some context when it appears in the code, for example, I'm using it in
Remove
DatabaseConfig
it starts looking a bit like it appears from nowhere.I am conscious they are both just objects declared at the top of the file.
Again, I'm not precious about it either 🤷 . But it feels like a 'convention-in-the-making'. So, one to think about and discuss on the Dev call in my absence!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we are having a bit of a discussion about this 😁 I was wondering why we are generating the
salt
separately? I think we get the same result if we just do thisreturn bcrypt.hashSync(defaultUserPassword, 10)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just following orders 🤷 https://github.com/kelektiv/node.bcrypt.js#to-hash-a-password-1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a look at Technique 2 in that readme 😉
I would guess that the technique to generate the
salt
separately would be more efficient if you were performing multiple hashes as you could reuse thesalt
to save a bit of CPU? Not fussed either way but thought I'd mention it since we were having a chat about that chunk of code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a valid point. I didn't dig that far because I was just trying to replicate what they had done so we generate passwords the rest of the service can recognise. But I think technique 2 makes the code just a bit simpler so leave this with me and I'll make the change.