-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[Auth] Forward secure coding calls for TOTPMultiFactorInfo #13592
Conversation
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.
Despite being marked unavailable, this class's init?(coder:)
is actually called when totp is registered:
coder.encode(enrolledFactors, forKey: kEnrolledFactorsCodingKey) |
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.
Thanks!
Will need to be cherry-picked to 11.2 branch as well.
I'm not sure now is the time to address it, but |
/// This is the totp info for the second factor. | ||
let totpInfo: NSObject? |
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.
Typing this with a more specific type (dictionary) added a more code to cast it during encoding and decoding. I stepped back and see that this property is never read from (it's only read from from the AuthProtoMFAEnrollment.swift
class). I think it's better to just remove it as it's addition is more confusing.
I was able to successfully enroll via TOTP in the sample app using these changes. |
Fix #13591
totpInfo
property because it's not used. The property onAuthProtoMFAEnrollment
is enough (e.g.git grep "\.totpInfo" -- FirebaseAuth/Sources/Swift
.firebase-ios-sdk/FirebaseAuth/Sources/Swift/Backend/RPC/Proto/AuthProtoMFAEnrollment.swift
Line 19 in ed4269b