Two Factor Authentication endpoints #51
Conversation
…n endpoint implementation
RaimundasSakalauskas
left a comment
RaimundasSakalauskas
left a comment
There was a problem hiding this comment.
Seems great. I might add "getMFAToken" to iOS too as this seems like a nice helper method.
jensck
left a comment
jensck
left a comment
There was a problem hiding this comment.
If iOS has a getMFAToken(), either we should have it, too, or we should eliminate it from both.
I lean toward the latter. Do we really want this as public API? How would it be used for anything but the login code that we're already providing?
| * | ||
| * @return server-provided multi-factor auth token or null | ||
| */ | ||
| public String getMfaToken() { |
There was a problem hiding this comment.
We're already using GSON everywhere else. Why the inconsistency?
There was a problem hiding this comment.
Adding a specialized auth method onto a generic API exception doesn't feel great to me. Could this specialized logic be handled elsewhere (somewhere that isn't exposed as public API surface)?
There was a problem hiding this comment.
Note 'loadServerErrorMsg' method, we infact do not use gson here, so this is consistent with the rest of the class.
There was a problem hiding this comment.
I believe it has value as public API, we could however handle it elsewhere or have child exception, something like 'ParticleLoginException'.
|
Just copying what I said on slack regarding mfa getter "I would say - yes, if we have public API for login why wouldn't we have public mfa token getter it's part of login flow after all. Shouldn't user be able to implement same behavior as our own tinker app?" |
|
Moved MFA getter to new ParticleLoginException (inherits from ParticleCloudException), this way we won't expose getter with exceptions unrelated to login. |
|
+1 for the specialized |
Implemented calls to 2fa endpoints + wrapper method to authenticate by passing in mfa & otp.