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.

Sign up for GitHub

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

Updated generatePassword function #9475

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Updated generatePassword function #9475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e968292
Updated generatePassword function
bisht-richa Nov 14, 2024
809ca08
Fixed change log file name
bisht-richa Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 29 additions & 11 deletions web/html/src/utils/functions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,15 @@ function sortByDate(aRaw: any, bRaw: any, columnKey: string, sortDirection: numb
aRaw[columnKey] === null
? null
: aRaw[columnKey] instanceof Date
? aRaw[columnKey]
: // eslint-disable-next-line local-rules/no-raw-date
? aRaw[columnKey]
: // eslint-disable-next-line local-rules/no-raw-date
new Date(aRaw[columnKey].replace(unparsableDateRegex, "$1"));
const bDate =
bRaw[columnKey] === null
? null
: bRaw[columnKey] instanceof Date
? bRaw[columnKey]
: // eslint-disable-next-line local-rules/no-raw-date
? bRaw[columnKey]
: // eslint-disable-next-line local-rules/no-raw-date
new Date(bRaw[columnKey].replace(unparsableDateRegex, "$1"));

const result = aDate > bDate ? 1 : aDate < bDate ? -1 : 0;
Expand All @@ -116,16 +116,34 @@ function capitalize(str: string): string {
}

function generatePassword(): string {
const length = Math.floor(Math.random() * 10) + 15;
// See https://stackoverflow.com/a/68617567/1470607
const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789:-_";
const array = new Uint32Array(charset.length);
window.crypto.getRandomValues(array);
const length = 24; // fixed length for better security
Copy link
Member

@Etheryte Etheryte Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not true? Using a fixed length reduces security, since it makes the space you need to brute force smaller. To exemplify, if we use only passwords of length 25, then there are X^25 combinations where X is the number of symbols in our charset, but if we allow for example passwords of length 15-25, then there are X^15 + X^16 + X^17 + ... + X^25 combinations.

const lowercase = "abcdefghijklmnopqrstuvwxyz";
const uppercase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
const numbers = "0123456789";
const specials = ":-_!@#$%^&*()"; // extended set of special characters
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity, why does this constant only include this limited amount of special chars?


// Helper function to get a random character from a given set
const allChars = lowercase + uppercase + numbers + specials;

// Ensure at least one character from each category
const getRandomChar = (charset) => charset[Math.floor(Math.random() * charset.length)];
Copy link
Member

@Etheryte Etheryte Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Math.random() is a pseudorandom number generator, I don't think we want to use it here. Similarly below in the sort function, I think this is not the way we want to approach this. The whole idea of using window.crypto.getRandomValues() is that we don't want to use Math.random().


let password = "";
for (let i = 0; i < length; i++) {
password += charset[array[i] % charset.length];
password += getRandomChar(lowercase);
password += getRandomChar(uppercase);
password += getRandomChar(numbers);
password += getRandomChar(specials);

const array = new Uint8Array(length - 4); // Remaining characters
window.crypto.getRandomValues(array);

for (let i = 0; i < array.length; i++) {
password += allChars[array[i] % allChars.length];
}

// Shuffle the password to avoid guaranteed character positions
password = password.split('').sort(() => 0.5 - Math.random()).join('');
Comment on lines +144 to +145
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For shuffling, you could also use the Fisher–Yates shuffle.


return password;
}

Expand Down
1 change: 1 addition & 0 deletions web/spacewalk-web.changes.bisht-richa.improvement-generatePassword-function
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Improvement in generatePassword function.
Loading