Skip to content

Comments

Feat/Sha256 password import#301

Merged
marcosmartini merged 2 commits intomainfrom
Feat/Hashed-passwords
Dec 20, 2024
Merged

Feat/Sha256 password import#301
marcosmartini merged 2 commits intomainfrom
Feat/Hashed-passwords

Conversation

@clairekinde11
Copy link
Contributor

@clairekinde11 clairekinde11 commented Dec 19, 2024

Added additional supported method for importing passwords.

Summary by CodeRabbit

  • New Features

    • Updated password hashing methods to include sha256.
    • Introduced new fields for password security: salt, salt_position, and salt_format.
  • Documentation

    • Enhanced clarity and organization of the import process for users.
    • Expanded instructions for migrating to Kinde for user authentication.
    • Updated notes on bcrypt hash compatibility and handling of sha256 hashes.

Added additional supported method for importing passwords.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 19, 2024

Caution

Review failed

The pull request is closed.

Walkthrough

This pull request updates the documentation for bulk user import in Kinde, focusing on password hashing and security. The changes introduce support for sha256 hashing and add new fields like salt, salt_position, and salt_format to provide more flexible password encryption options. The documentation has been reorganized to clarify password data requirements, with specific notes about bcrypt hash compatibility and the handling of sha256 hashes.

Changes

File Change Summary
src/content/docs/manage-users/add-and-edit/import-users-in-bulk.mdx - Added sha256 to supported hashing methods
- Introduced new password-related fields: salt, salt_position, salt_format
- Updated bcrypt hash compatibility note
- Added aside explaining sha256 hash handling
- Improved documentation clarity and organization
src/content/docs/get-started/switch-to-kinde/switch-to-kinde-for-user-authentication.mdx - Expanded password data section with new fields: salt, salt_position, salt_format
- Updated hashing method options to include sha256
- Clarified handling of salt for sha256
- Adjusted bcrypt hash compatibility note
- Enhanced clarity on user data preparation

Possibly related PRs

Suggested reviewers

  • oliwolff1
  • marcosmartini

Poem

🐰 Hashing secrets, salt and might,
Importing users with cryptic delight
SHA256 joins the dance
Bcrypt bows with a knowing glance
Security's rabbit leaps with glee! 🔐


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f19f89f and 431eecb.

📒 Files selected for processing (1)
  • src/content/docs/get-started/switch-to-kinde/switch-to-kinde-for-user-authentication.mdx (1 hunks)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Dec 19, 2024

Deploying kinde-docs-previews with  Cloudflare Pages  Cloudflare Pages

Latest commit: 431eecb
Status: ✅  Deploy successful!
Preview URL: https://faf02abc.kinde-docs-previews.pages.dev
Branch Preview URL: https://feat-hashed-passwords.kinde-docs-previews.pages.dev

View logs

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (3)
src/content/docs/manage-users/add-and-edit/import-users-in-bulk.mdx (3)

102-104: Enhance salt-related documentation with examples

The salt-related fields are well-defined but would benefit from concrete examples to aid implementation.

Consider adding examples:

- `salt` - extra characters added to passwords to make them stronger
- `salt_position` - position of salt in password string. E.g. prefix (before) or suffix (after).
- `salt_format` - format of the salt, e.g. hex, string, etc.
+ `salt` - extra characters added to passwords to make them stronger (e.g., "x4vF9")
+ `salt_position` - position of salt in password string: "prefix" (salt + password) or "suffix" (password + salt)
+ `salt_format` - format of the salt: "hex" for hexadecimal (e.g., "6164616D") or "string" for plain text
🧰 Tools
🪛 LanguageTool

[style] ~102-~102: Consider using a shorter alternative to avoid wordiness.
Context: ... extra characters added to passwords to make them stronger - salt_position - position of salt in...

(MADE_IT_JJR)


106-110: Clarify bcrypt variant compatibility

The bcrypt variant note should be expanded to cover all common variants for completeness.

Consider adding information about other variants:

<Aside title="bcrypt $2b variant support:">
-  Please note if you are importing bcrypt hashes with the $2b variant, Kinde will substitute this for the $2a variant. These are interchangeable as long as you were not running OpenBSD at the time the hashes were generated.
+  Please note if you are importing bcrypt hashes with the $2b variant, Kinde will substitute this for the $2a variant. Common bcrypt variants include:
+  - $2a$ - The original bcrypt algorithm (supported)
+  - $2b$ - Minor update to fix a wrap-around bug (automatically converted to $2a)
+  - $2y$ - Format used by PHP (not currently supported)
+  
+  Note: The $2b to $2a conversion is safe unless the hashes were specifically generated on OpenBSD.
</Aside>

124-124: Enhance hashing method table with security information

The table should include security recommendations to help users make informed decisions.

Consider expanding the table:

-  | Hashing method | Salt     | Salt position             |
-  | -------------- | -------- | ------------------------- |
-  | md5            | Optional | required if salt included |
-  | bcrypt         |          |                           |
-  | crypt          | Optional |                           |
-  | wordpress      | Optional |                           |
-  | sha256         | Optional | required if salt included |
+  | Hashing method | Salt     | Salt position             | Security Level | Recommendation |
+  | -------------- | -------- | ------------------------- | -------------- | -------------- |
+  | bcrypt         |          |                           | High          | Recommended    |
+  | sha256         | Optional | required if salt included | Medium-High   | Use with salt  |
+  | crypt          | Optional |                           | Medium        | Legacy only    |
+  | wordpress      | Optional |                           | Medium        | Legacy only    |
+  | md5            | Optional | required if salt included | Low           | Not recommended|
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 820a6a0 and f19f89f.

📒 Files selected for processing (1)
  • src/content/docs/manage-users/add-and-edit/import-users-in-bulk.mdx (1 hunks)
🧰 Additional context used
🪛 LanguageTool
src/content/docs/manage-users/add-and-edit/import-users-in-bulk.mdx

[uncategorized] ~101-~101: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...hm used to encrypt the user’s password. Currently crypt, bcrypt, sha256, **md...

(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)


[style] ~102-~102: Consider using a shorter alternative to avoid wordiness.
Context: ... extra characters added to passwords to make them stronger - salt_position - position of salt in...

(MADE_IT_JJR)

Comment on lines +112 to +116
<Aside title="sha256 support:">

Provide the hash in hex format. Import the salt using the `salt` column. For the `salt_format`, specify how the salt should be interpreted: e.g. **hex** for a hex-encoded string (68656c6c6f for hello). By default, the salt is treated as a plain string, and escape sequences (like \n or \v) are treated as literal characters.

</Aside>
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add SHA256 implementation examples

The SHA256 documentation should include specific examples of hash generation and salt combination.

Consider adding implementation details:

<Aside title="sha256 support:">
-  Provide the hash in hex format. Import the salt using the `salt` column. For the `salt_format`, specify how the salt should be interpreted: e.g. **hex** for a hex-encoded string (68656c6c6f for hello). By default, the salt is treated as a plain string, and escape sequences (like \n or \v) are treated as literal characters.
+  SHA256 implementation details:
+  1. Hash format: Provide the hash in lowercase hex format (64 characters)
+  2. Salt handling:
+     - For prefix salt: SHA256(salt + password)
+     - For suffix salt: SHA256(password + salt)
+  
+  Example:
+  ```csv
+  email,hashed_password,hashing_method,salt,salt_position,salt_format
+  user@example.com,5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8,sha256,mysalt,suffix,string
+  ```
+  This example represents: SHA256("password" + "mysalt")
</Aside>


- `hashed_password` - the user’s password encrypted using a hashing method or algorithm.
- `hashing_method` - the name of the algorithm used to encrypt the user’s password. Currently **crypt**, **bcrypt**, **md5**, and **wordpress** are supported. [Contact us](https://kinde-21631392.hs-sites.com/en-au/feature-request) if you need a different method.
- `hashing_method` - the name of the algorithm used to encrypt the user’s password. Currently **crypt**, **bcrypt**, **sha256**, **md5**, and **wordpress** are supported. [Contact us](https://kinde-21631392.hs-sites.com/en-au/feature-request) if you need a different method.
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add security recommendations for hashing methods

While the documentation accurately lists the supported hashing methods, it should include security recommendations to guide users toward more secure choices.

Consider reordering the methods by security strength and adding a security note:

-Currently **crypt**, **bcrypt**, **sha256**, **md5**, and **wordpress** are supported.
+Currently supported hashing methods (ordered by security strength):
+- **bcrypt** (recommended)
+- **sha256** with salt
+- **crypt**
+- **wordpress**
+- **md5** (not recommended for new implementations due to known vulnerabilities)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- `hashing_method` - the name of the algorithm used to encrypt the users password. Currently **crypt**, **bcrypt**, **sha256**, **md5**, and **wordpress** are supported. [Contact us](https://kinde-21631392.hs-sites.com/en-au/feature-request) if you need a different method.
- `hashing_method` - the name of the algorithm used to encrypt the user's password. Currently supported hashing methods (ordered by security strength):
- **bcrypt** (recommended)
- **sha256** with salt
- **crypt**
- **wordpress**
- **md5** (not recommended for new implementations due to known vulnerabilities)
[Contact us](https://kinde-21631392.hs-sites.com/en-au/feature-request) if you need a different method.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~101-~101: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...hm used to encrypt the user’s password. Currently crypt, bcrypt, sha256, **md...

(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)

Copy link
Contributor

@oliwolff1 oliwolff1 left a comment

Choose a reason for hiding this comment

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

I can't comment on whether the content is good - I think someone who knows SHA256 would need to comment on this - but it looks good to me.

I will approve for now

@oliwolff1
Copy link
Contributor

oliwolff1 commented Dec 20, 2024

@oliwolff1 oliwolff1 self-requested a review December 20, 2024 05:02
Copy link
Contributor

@oliwolff1 oliwolff1 left a comment

Choose a reason for hiding this comment

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

@marcosmartini marcosmartini merged commit e748ff6 into main Dec 20, 2024
4 checks passed
@marcosmartini marcosmartini deleted the Feat/Hashed-passwords branch December 20, 2024 05:57
@coderabbitai coderabbitai bot mentioned this pull request Jan 17, 2025
@coderabbitai coderabbitai bot mentioned this pull request Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants