feat(auth): Add Argon2 Hashing Algorithm support - #637
Conversation
|
Looks good to me. |
lahirumaramba
left a comment
There was a problem hiding this comment.
Thank you, Sam! Looks pretty good! Left a question on abstraction.
| } | ||
| } | ||
|
|
||
| public enum Argon2HashType { |
There was a problem hiding this comment.
I wonder if this could use another layer of abstraction. Similar to what we have done with Hmac and RepeatableHash. We could introduce an abstract class for ARGON2Hash and implementations for ARGON2d, ARGON2i, and ARGON2id. WDYT?
There was a problem hiding this comment.
I think that's unnecessary here. For HMAC, it makes sense to add a layer of abstraction since the algorithms are identified as different from one another (HMAC_SHA256 HMAC_SHA1 HMAC_MD5, etc). Here ARGON2 is the algorithm, the hash-types are an additional configuration for the same algorithm.
There was a problem hiding this comment.
I see. That makes sense. Thanks, Sam!
| VERSION_10, | ||
| VERSION_13 |
There was a problem hiding this comment.
They are the supported versions, as per the API: https://cloud.google.com/identity-platform/docs/reference/rest/v1/Argon2Parameters#version
lahirumaramba
left a comment
There was a problem hiding this comment.
LGTM!
Please get a TW to take a quick look at the docs before we merge this. Thanks!
…Parameters` field
Hey there! So you want to contribute to a Firebase SDK?
Before you file this pull request, please read these guidelines:
Discussion
Testing
API Changes
RELEASE NOTE: Added
Argon2hashing algorithm support in theimportUsers()API.