JWT Improvement Part 2 #721
Conversation
1. Call onTokenRegistrationFailed in more circumstances and rename it 2. Provide an AuthFailure object to onTokenRegistrationFailed
| public final String userKey; | ||
|
|
||
| /** the authToken which casued the failure */ | ||
| public final String failedAuthToken; |
There was a problem hiding this comment.
yes we pass null when creating object in case of null.
| this.failedRequestTime = failedRequestTime; | ||
| this.failureReason = failureReason; | ||
| } | ||
| } |
There was a problem hiding this comment.
| } | |
| } | |
| enum class AuthFailureReason { | |
| INVALID_USER, | |
| INVALID_AUTH_TOKEN, | |
| AUTH_TOKEN_EXPIRED, | |
| UNKNOWN | |
| } |
There was a problem hiding this comment.
Some other enum to consider can be:
//ERROR_CODE_JWT_USER_IDENTIFIERS_MISMATCHED
//"JWT payload requires a value for userId or email" - JWT_PAYLOAD_MISSING_VALID_USER_KEY
//"Email could not be found" - JWT_PAYLOAD_MISSING_VALID_USER_KEY
//ERROR_CODE_INVALID_JWT_PAYLOAD
//"JWT token is expired" - AUTH_TOKEN_EXPIRED
//"exp must be less than 1 year from iat" - AUTH_TOKEN_EXPIRATION_INVALID
//"JWT is invalid" - JWT_SIGNATURE_INVALID
//"JWT format is invalid" - JWT_FORMAT_INVALID
//"JWT token has been invalidated" - JWT_TOKEN_INVALIDATED
//"Invalid payload" - JWT_PAYLOAD_INVALID
//null - JWT_NULL
There was a problem hiding this comment.
Yes enum sounds good. Thanks for the suggestion.
I hope, all the text error messages for JWT are covered here, because in case of unrelated message we won't be able to return the error message as we are using enum now instead of string.
| public final long failedRequestTime; | ||
|
|
||
| /** indicates a reason for failure */ | ||
| public final String failureReason; |
| @@ -101,7 +101,7 @@ private void handleAuthTokenSuccess(String authToken, IterableHelper.SuccessHand | |||
|
|
|||
| private void handleAuthTokenFailure(Throwable throwable) { | |||
| IterableLogger.e(TAG, "Error while requesting Auth Token", throwable); | |||
| authHandler.onTokenRegistrationFailed(throwable); | |||
| handleAuthFailure(null, IterableConstants.AUTH_TOKEN_GENERATION_ERROR); | |||
There was a problem hiding this comment.
Use of constant highly appreciated. However suggesting above to have set of enum for varied JWT failures. What do you think about it?
| @@ -118,7 +118,7 @@ public void queueExpirationRefresh(String encodedJWT) { | |||
| } | |||
| } catch (Exception e) { | |||
| IterableLogger.e(TAG, "Error while parsing JWT for the expiration", e); | |||
| authHandler.onTokenRegistrationFailed(new Throwable("Auth token decode failure. Scheduling auth token refresh in 10 seconds...")); | |||
| handleAuthFailure(encodedJWT, IterableConstants.AUTH_TOKEN_PAYLOAD_INVALID); | |||
There was a problem hiding this comment.
Same as above. Having enum will help developers type cast the reasons into specific enums and take actions easily
| @@ -135,6 +135,22 @@ void reSyncAuth() { | |||
| } | |||
| } | |||
|
|
|||
| void handleAuthFailure(String authToken, String failureReason) { | |||
| authHandler.onAuthFailure(new AuthFailure(getEmailOrUserId(), authToken, IterableUtil.currentTimeMillis(), failureReason)); | |||
There was a problem hiding this comment.
Great use of currentTime function to tell exactly when the failure occurred.
| public static final String AUTH_TOKEN_NULL = "AUTH_TOKEN_NULL"; | ||
| public static final String AUTH_TOKEN_GENERATION_ERROR = "AUTH_TOKEN_GENERATION_ERROR"; | ||
|
|
||
| public static final String AUTH_TOKEN_GENERIC_ERROR = "AUTH_TOKEN_GENERIC_ERROR"; | ||
| public static final String AUTH_TOKEN_EXPIRED = "AUTH_TOKEN_EXPIRED"; | ||
| public static final String AUTH_TOKEN_EXPIRATION_INVALID = "AUTH_TOKEN_EXPIRATION_INVALID"; | ||
| public static final String AUTH_TOKEN_SIGNATURE_INVALID = "AUTH_TOKEN_SIGNATURE_INVALID"; | ||
| public static final String AUTH_TOKEN_FORMAT_INVALID = "AUTH_TOKEN_FORMAT_INVALID"; | ||
| public static final String AUTH_TOKEN_INVALIDATED = "AUTH_TOKEN_INVALIDATED"; | ||
| public static final String AUTH_TOKEN_PAYLOAD_INVALID = "AUTH_TOKEN_PAYLOAD_INVALID"; | ||
| public static final String AUTH_TOKEN_USER_KEY_INVALID = "AUTH_TOKEN_USER_KEY_INVALID"; | ||
|
|
There was a problem hiding this comment.
Suggested above to have it as Enum for failure reason. Open to suggestions too
馃敼 Jira Ticket(s) if any
鉁忥笍 Description