fix: use URL-safe Base64 decoding for Firebase Scrypt password hash and salt
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
Firebase Scrypt password hash and salt decoding uses base64.StdEncoding.
When users are exported from Firebase, their PasswordHash may contain URL-safe Base64 characters (- and _). These characters cannot be decoded by base64.StdEncoding.DecodeString(), causing decoding errors.
Related issue: https://github.com/firebase/firebase-admin-go/issues/479
What is the new behavior?
- Changed PasswordHash and salt decoding to use base64.URLEncoding
-
signerKeyandsaltSeparatorcontinue to use base64.StdEncoding (as they are provided in standard Base64 format)
Test updates:
- Updated test data to use salt and hash containing URL-safe Base64 characters (_)
- Added negative test cases to verify that standard Base64 (containing
/) in salt and hash results in errors
Additional context
According to Firebase Admin SDK Issue https://github.com/firebase/firebase-admin-go/issues/479, when exporting user data from Firebase, the PasswordHash is encoded in URL-safe Base64, while the signerKey provided in the Firebase Console uses standard Base64. When importing users from Firebase Auth, the exported data contains URL-safe Base64 encoded values like:
{
"passwordHash": "cxbDzJu2W1DU20SXh--UNtR9Tu2tH2GSKQ91hGBAEN49OJJpuZbdhz-3GdYnBujQXxkXdqnhUehXVeLA6zoF-g==",
"passwordSalt": "G-s6NpQQJdtvlQ=="
}
Both passwordHash and passwordSalt contain - characters, which are URL-safe Base64 specific and cannot be decoded with standard Base64.
Instead of using Base64.URLEncoding this should normalize URL-encoded strings to regular ones (or vice versa). Clearly there's ones where regular URL encoding is used.
Pull Request Test Coverage Report for Build 20006493610
Warning: This coverage report may be inaccurate.
This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
- For more information on this, see Tracking coverage changes with pull request builds.
- To avoid this issue with future PRs, see these Recommended CI Configurations.
- For a quick fix, rebase this PR at GitHub. Your next report should be accurate.
Details
- 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage remained the same at 68.435%
| Totals | |
|---|---|
| Change from base Build 19871602451: | 0.0% |
| Covered Lines: | 14641 |
| Relevant Lines: | 21394 |
💛 - Coveralls
@hf thanks! I agreed. I pushed new commit d738166a21b53c1fb66850ed19e7c35f3126b567
@hf
I've updated the PR to normalize the encoding as you suggested. Could you take another look when you get a chance? Thanks!