Skip to content

Future robust password handling #123

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

Open
calebbourg opened this issue May 2, 2025 · 0 comments
Open

Future robust password handling #123

calebbourg opened this issue May 2, 2025 · 0 comments
Labels
enhancement Improves existing functionality or feature security Any feature or fix relating to software security

Comments

@calebbourg
Copy link
Collaborator

Solidify our password security based on #117 (comment)

  • Error Handling: Completely agree that the Other use here does not give a lot of information. I've intentionally deferred adding more descriptive variants to these errors as I would like to take a step back at some point and take inventory of all of the uses Other and see what other appropriate categories of errors there are. I wanted to avoid adding ad-hoc variants "kitchen sink" style. I'd like to address that as a separate PR and issue.

  • Verification of current_password: I double checked this and our use of password_auth ensures OWASP recommended cryptography as stated in their docs:

Behind the scenes the crate uses the multi-algorithm support in the password-hash crate to support multiple password hashing algorithms simultaneously. By default it supports Argon2 (using the latest OWASP recommended parameters 8), but it can also optionally support PBKDF2 and scrypt by enabling crate features.

  • Handling of Removed Fields and Error Handling for Field Removal: This is handled both upstream and downstream. Our params check will fail prior to this step if these fields are not present. If this function returns an error, that will be transformed downstream to an error response to the client. This, as far as I have thought through it, should not panic.

  • Password Hashing: This is handled by password_auth

  • Insertion of Hashed Password into params: Excellent point. We store this in the data structure as that is the contract between the domain layer and the entity_api layer. I think I would prefer some sort of global filtering of passwords from logs over handling it this way. Especially if this approach would require breaking these abstraction boundaries.

  • Lack of Rate Limiting and Audit Logging: Love these. I think we should think through how we might implement in a way that's usable across the web interface.

  • Validation of New Password: Let's discuss what our requirements would be here.

  • Potential for Overwriting Fields: This is worth a larger discussion as well as the same issue exists across the entire web interface. We should look into how seaORM or Axum recommends sanitization.

@calebbourg calebbourg added enhancement Improves existing functionality or feature security Any feature or fix relating to software security labels May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves existing functionality or feature security Any feature or fix relating to software security
Projects
Status: No status
Development

No branches or pull requests

1 participant