-
Notifications
You must be signed in to change notification settings - Fork 40
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
Update signing logic for correct custom token format #23
base: main
Are you sure you want to change the base?
Update signing logic for correct custom token format #23
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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: invertase#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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to do the trick (it passes the test in crypto_signer_test.dart
):
return JWTAlgorithm.RS256.sign(RSAPrivateKey(credential.privateKey), buffer);
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!