implement verify method #33
Conversation
|
|
|
Will it be merged in next release ? Just tried it from commit ref and it is working! |
|
I'm in holiday at the moment. I'll look at this when I'm back from Japan. |
|
Suure, no worries. Have fun there! 🇯🇵 |
|
Hi @rrousselGit , I hope you're enjoying your trip to Japan! Just wanted to gently remind you about this, awaiting your review when you have a chance. |
|
Looks like this needs testing. My apologies, I should've raised this sooner. I'll review other PRs and fix other bugs in the meantime. If it's still around by then, I'll consider adding tests for this myself :) |
| token, | ||
| SecretKey(''), | ||
| ); | ||
| JWT.decode(token); |
| 'The provided token has expired. Get a fresh token from your client app and try again.', | ||
| ); | ||
| } on JWTException catch (e) { | ||
| if (e.message.startsWith(jwtCallbackErrorPrefix)) { |
There was a problem hiding this comment.
We shouldn't depend on the english message.
There was a problem hiding this comment.
And then, how about simplifying this error handling for now?
Just like:
...
} on JWTExpiredException {
throw JwtError(
JwtErrorCode.tokenExpired,
'The provided token has expired. Get a fresh token from your client app and try again.',
);
} on JWTException catch (e) {
// Just throw JwtErrorCode.unknown for now.
throw JwtError(JwtErrorCode.unknown, e.message);
}
...
enum JwtErrorCode {
invalidArgument('invalid-argument'),
...
// add this enum field
unknown('unknown');
...
}
There was a problem hiding this comment.
Sure. I shouldn't have done that indeed.
There was a problem hiding this comment.
Hello!
I would appreciate it if you could take a moment to check these comments and share your thoughts.
|
This has been sorted in a different PR. Thanks for your work though and sorry for the slow review! |
Fixed issue: Auth#verifyIdToken always throws unimplemented error #20