Update signing logic for correct custom token format#23
Update signing logic for correct custom token format#23kosukesaigusa wants to merge 4 commits intofirebase:mainfrom
Conversation
|
Hello! Sorry for the delay. We'd need tests for this. Could you add those? Otherwise LGTM |
| final privateKeyDER = base64Decode(privateKeyString); | ||
|
|
||
| final asn1Parser = ASN1Parser(Uint8List.fromList(privateKeyDER)); | ||
| final topLevelSequence = asn1Parser.nextObject() as ASN1Sequence; |
There was a problem hiding this comment.
Too many casts and ! in there. Could you remove those? Such as by using is and != null
We'd also need tests for when those values are invalid
There was a problem hiding this comment.
Thank you! I'd like you to give some advice about this point!
I'm thinking to improve the code like the following to reduce as casting and !:
crypto_signer.dart
RSAPrivateKey _parsePrivateKeyFromPem() {
final privateKeyString = credential.privateKey
.replaceAll('-----BEGIN PRIVATE KEY-----', '')
.replaceAll('-----END PRIVATE KEY-----', '')
.replaceAll('\n', '');
final privateKeyDER = base64Decode(privateKeyString);
final asn1Parser = ASN1Parser(Uint8List.fromList(privateKeyDER));
final topLevelSequence = asn1Parser.nextObject();
if (topLevelSequence is! ASN1Sequence) {
throw const FormatException('Invalid ASN1 format for private key');
}
final privateKeyOctet = topLevelSequence.elements?[2];
if (privateKeyOctet is! ASN1OctetString) {
throw const FormatException('Invalid ASN1 format for private key');
}
final privateKeyParser = ASN1Parser(privateKeyOctet.valueBytes);
final privateKeySequence = privateKeyParser.nextObject();
if (privateKeySequence is! ASN1Sequence) {
throw const FormatException('Invalid ASN1 format for private key');
}
final modulus = privateKeySequence.elements?[1];
final exponent = privateKeySequence.elements?[3];
final p = privateKeySequence.elements?[4];
final q = privateKeySequence.elements?[5];
if (modulus is! ASN1Integer ||
exponent is! ASN1Integer ||
p is! ASN1Integer ||
q is! ASN1Integer) {
throw const FormatException('Invalid ASN1 format for private key');
}
return RSAPrivateKey(
modulus.integer!,
exponent.integer!,
p.integer,
q.integer,
);
}and tried to write the unit test for success case like the following:
crypto_signer_test.dart
test('parsePrivateKeyFromPem should parse valid private key', () {
final credential = Credential.fromServiceAccountParams(
clientId: 'id',
privateKey: _fakeRSAKey,
email: 'email',
);
final app = FirebaseAdminApp.initializeApp('project-id', credential);
final signer = cryptoSignerFromApp(app);
expect(signer, isA<CryptoSigner>());
expect(signer.algorithm, 'RS256');
final buffer = Uint8List.fromList(List.generate(32, (i) => i));
expect(() async => signer.sign(buffer), returnsNormally);
});However, when I try to write test to confirm the code throwing the FormatException, I need to give invalid private key string to Credential.fromServiceAccountParams, and it throws Invalid DER encoding error before reaching _ServiceAccountSigner.sign (_ServiceAccountSigner._parsePrivateKeyFromPem) method.
I think _ServiceAccountSigner is private, so unit test has to be started with the public cryptoSignerFromApp function, which is expected to given FirebaseAdminApp instance.
I appreciate if you have any ideas about it!
There was a problem hiding this comment.
One way to do it is to make _ServiceAccountSigner "public", but have export ... hide ServiceAccountSigner.
Make sure to make ServiceAccountSigner with @internal as you do so.
|
Thank you for your comment! I will add tests and update the code based on your feedback! |
PR with fix in upstream: firebase#23
|
Any updates on this? This lib is still not working with createCustomToken |
| final key = utf8.encode(credential.privateKey); | ||
| final hmac = Hmac(sha256, key); | ||
| final digest = hmac.convert(buffer); | ||
| final rsaPrivateKey = _parsePrivateKeyFromPem(); |
There was a problem hiding this comment.
Just a suggestion: https://github.com/jonasroussel/dart_jsonwebtoken comes with a convenient RSAPrivateKey(String pem) constructor. Could this be used instead of all the ASN1 code?
There was a problem hiding this comment.
This seems to do the trick (it passes the test in crypto_signer_test.dart):
return JWTAlgorithm.RS256.sign(RSAPrivateKey(credential.privateKey), buffer);
|
(I'm adding tests to this, and will merge) |
|
I ended-up switching to using Jose for implementing sign-in when working on app_check. This fixes the issue |
Background
Create custom token here:
Then, try to use the custom token to sign in Firebase Auth with on Flutter client app:
The exception like the following is thrown:
If I use the custom token created by JS (TS) SDK by the same project service account on my Flutter client app, it successfully signs in.
I noticed the length of custom tokens are different from each other.
Then this PR fixes the RSA-SHA256 signing logic using pointycastle package instead of crypto package.
After the change,
I am so excited by and curious about this project, enabling us to write Firebase Admin SDK server-side code by Dart!
Thank you so much for developing such a wonderful project!