-
Notifications
You must be signed in to change notification settings - Fork 363
Add secure-userstore-using-hashing #5669
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
Conversation
WalkthroughAdds a new documentation page describing how to secure JDBC user stores with hashing (SHA, MD5, PLAIN_TEXT, PBKDF2, BCRYPT), including configuration examples for primary and secondary stores, and updates mkdocs navigation to reference the new file. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🪛 LanguageToolen/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md[grammar] ~82-~82: Use a hyphen to join words. (QB_NEW_EN_HYPHEN) [style] ~146-~146: Using many exclamation marks might seem excessive (in this case: 9 exclamation marks for a text that’s 3811 characters long) (EN_EXCESSIVE_EXCLAMATION) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
en/identity-server/next/docs/deploy/configure/user-stores/secure-userstore-using-bcrypt.md (1)
41-41: Link format inconsistency: consider using relative paths with template variables.The link on line 41 uses an absolute URL format (
https://is.docs.wso2.com/en/latest/...), whereas the companion documentation filesecuring-a-user-store-with-hashing.mduses template variables ({{base_path}}/...). For consistency and maintainability, consider aligning the link format with the project's documentation standards. Verify with your documentation build system whether one format is preferred.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
en/identity-server/next/docs/deploy/configure/user-stores/secure-userstore-using-bcrypt.md(1 hunks)en/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md(1 hunks)en/identity-server/next/mkdocs.yml(1 hunks)
🧰 Additional context used
🪛 LanguageTool
en/identity-server/next/docs/deploy/configure/user-stores/secure-userstore-using-bcrypt.md
[grammar] ~91-~91: Use a hyphen to join words.
Context: ... accounts of the user store using [admin initiated password reset](https://is.doc...
(QB_NEW_EN_HYPHEN)
en/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md
[style] ~78-~78: Using many exclamation marks might seem excessive (in this case: 6 exclamation marks for a text that’s 3620 characters long)
Context: .../configure-secondary-user-stores). !!! Note "Existing user stores" - Y...
(EN_EXCESSIVE_EXCLAMATION)
[grammar] ~82-~82: Use a hyphen to join words.
Context: ... accounts of the user store using [admin initiated password reset]({{base_path}}/...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (2)
en/identity-server/next/mkdocs.yml (1)
846-846: Navigation entry is appropriately structured and placed.The new entry correctly links to the hashing documentation and maintains consistency with the existing navigation hierarchy and formatting conventions.
en/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md (1)
1-169: Well-structured documentation with comprehensive hashing configuration guidance.This new documentation page provides a solid foundation for understanding and configuring password hashing algorithms in WSO2 Identity Server. The progression from algorithm overview to specific configuration steps for both primary and secondary stores is clear and logical. The side-by-side configuration comparison for PBKDF2 and BCRYPT is particularly helpful for users choosing between algorithms.
en/identity-server/next/docs/deploy/configure/user-stores/secure-userstore-using-bcrypt.md
Outdated
Show resolved
Hide resolved
en/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md
Show resolved
Hide resolved
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
en/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md (1)
82-82: Apply hyphenation to compound adjective.This is a duplicate of a previously flagged issue. The phrase "admin initiated password reset" should use a hyphen as a compound adjective.
Apply this diff to fix the grammar:
- - Trigger password reset for all accounts of the user store using [admin initiated password reset]({{base_path}}/guides/users/manage-users#reset-the-users-password). + - Trigger password reset for all accounts of the user store using [admin-initiated password reset]({{base_path}}/guides/users/manage-users#reset-the-users-password).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
en/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md(1 hunks)en/identity-server/next/mkdocs.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- en/identity-server/next/mkdocs.yml
🧰 Additional context used
🪛 LanguageTool
en/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md
[grammar] ~82-~82: Use a hyphen to join words.
Context: ... accounts of the user store using [admin initiated password reset]({{base_path}}/...
(QB_NEW_EN_HYPHEN)
[style] ~145-~145: Using many exclamation marks might seem excessive (in this case: 9 exclamation marks for a text that’s 3810 characters long)
Context: ...al hashing part is done. !!! Note NIST recommends `PBKDF2WithHma...
(EN_EXCESSIVE_EXCLAMATION)
🔇 Additional comments (2)
en/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md (2)
1-180: Verify reviewer feedback about missing table description columns and notes.The PR summary indicates two issues from reviewer sadilchamishka: (1) missing "description" column in a table, and (2) missing note under PBKDF2 Parameters section.
Examining the current code:
- The secondary user store configuration table (lines 87-112) has a "Description" column
- The PBKDF2 Parameters section (lines 146-151) contains a note block explaining pseudo-random function options
However, please verify with the reviewer's screenshots whether these issues have been addressed or if additional changes are required, as they may be referring to specific table formatting differences not visible in the plain HTML structure.
10-39: Fix HTML structure: missing closing</td>tags in the hashing algorithms table.Multiple rows in the table are missing closing
</td>tags, which breaks the table rendering:
- Line 20-21: SHA row is missing
</td>after the long description- Line 27-29: PLAIN_TEXT row is missing
</td>for the first column- Lines 31-33: PBKDF2 row is missing
</td>tags- Lines 35-37: BCRYPT row is missing
</td>tagsApply this diff to fix the table:
<tr class="odd"> <td>SHA</td> <td> Uses SHA digest method (SHA-1 or SHA-256). If you enter only SHA for the PasswordDigest property, it is interpreted as SHA-1. Note that SHA-256 is the default digest method used by WSO2 IS for JDBC user stores if no value is explicitly configured. + </td> </tr> <tr class = "odd"> <td>MD5</td> <td>Uses MD5 digest method</td> </tr> <tr class="even"> + <td>PLAIN_TEXT</td> <td>Stores passwords in plain text</td> + </td> </tr> <tr class="even"> + <td>PBKDF2</td> <td>A modern, NIST-recommended key derivation function that reduces brute-force attack risks.</td> + </td> </tr> <tr class="even"> + <td>BCRYPT</td> <td>A modern password hashing function with built-in salting and adaptive cost factor.</td> + </td> </tr>Likely an incorrect or invalid review comment.
en/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md
Show resolved
Hide resolved
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.
Actionable comments posted: 1
♻️ Duplicate comments (2)
en/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md (2)
82-82: Hyphenate the compound adjective for grammatical correctness.Line 82 uses "admin initiated password reset" but the compound adjective should be hyphenated as "admin-initiated password reset."
Apply this diff to fix the grammar:
- - Trigger password reset for all accounts of the user store using [admin initiated password reset]({{base_path}}/guides/users/manage-users#reset-the-users-password). + - Trigger password reset for all accounts of the user store using [admin-initiated password reset]({{base_path}}/guides/users/manage-users#reset-the-users-password).
120-145: Fix missing closing</tr>tag in PBKDF2 Parameters table.The last row of the PBKDF2 Parameters table (lines 139–143 for the
pbkdf2.prfparameter) is missing its closing</tr>tag before the table closes at line 145.Apply this diff to fix the table structure:
<tr> <td><code>pbkdf2.prf</code></td> <td>Pseudo-Random Function </td> <td><code>PBKDF2WithHmacSHA256</code></td> <td>The key component of the PBKDF2 hashing algorithm in which the actual hashing part is done.</td> + </tr> </table>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
en/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
en/identity-server/next/docs/deploy/configure/user-stores/securing-a-user-store-with-hashing.md
[grammar] ~82-~82: Use a hyphen to join words.
Context: ... accounts of the user store using [admin initiated password reset]({{base_path}}/...
(QB_NEW_EN_HYPHEN)
[style] ~146-~146: Using many exclamation marks might seem excessive (in this case: 9 exclamation marks for a text that’s 3811 characters long)
Context: ... part is done. !!! Note NIST recommends `PBKDF2WithHma...
(EN_EXCESSIVE_EXCLAMATION)


Purpose
This PR adds comprehensive documentation for Hashing support in WSO2 Identity Server.
The new documentation includes:
Related Issues
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.