@W-19345684: [iOS][QRCode] Shows Unsupported URL on FreshLogin using QRCode - #3901
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## dev #3901 +/- ##
==========================================
+ Coverage 63.06% 63.28% +0.22%
==========================================
Files 251 251
Lines 22467 22469 +2
==========================================
+ Hits 14168 14220 +52
+ Misses 8299 8249 -50
🚀 New features to boost your workflow:
|
| BOOL isUsingWebServerAuthentication = [[SalesforceSDKManager sharedManager] useWebServerAuthentication]; | ||
| // Second, switch authentication based on the in-use Salesforce Identity API UI Bridge front-door URL and code verifier. | ||
| if (self.frontdoorBridgeLoginOverride.frontdoorBridgeUrl) { | ||
| isUsingWebServerAuthentication = self.frontdoorBridgeLoginOverride.codeVerifier; |
There was a problem hiding this comment.
codeVerifier isn't a BOOL? The previous !self.frontdoorBridgeLoginOverride.codeVerifier call just checked if it was nil.
So if self.frontdoorBridgeLoginOverride.codeVerifier assigned to a BOOL just checks if it's nil what is the change in logic here? It looks the same to me.
There was a problem hiding this comment.
Since this is Obj-C, any pointer value (for a string in this case) can scale as a BOOL. nil = NO and any value = YES.
The key difference is that the non-nil (YES) value for the object in frontdoorBridgeLoginOverride.frontdoorBridgeUrl is checked first. That prevents the case I was seeing in the app at runtime where setting useWebServerAuthentication = NO would accidentally force user-agent handling for web-server-flow QR log in URLs. We vetted really similar logic in the Android app and this also works live in the iOS app as well, so we have those results to guide the runtime review.
There was a problem hiding this comment.
Would it be more readable with
// If frontdoorBridgeUrl is present, use codeVerifier to decide; otherwise, fall back to useWebServerAuthentication
if (self.frontdoorBridgeLoginOverride.frontdoorBridgeUrl // Check if an override is provided
? self.frontdoorBridgeLoginOverride.codeVerifier // If yes, only proceed if it's a web server flow (has codeVerifier)
: [[SalesforceSDKManager sharedManager] useWebServerAuthentication] // If no override, use default SDK setting
)
{
[self handleWebServerResponse:url]; // Web server flow/URLs with query string parameters.
} else {
[self handleUserAgentResponse:url]; // User agent flow/URLs with the fragment component.
}There was a problem hiding this comment.
That wouldn't compile, but this slight change will. Note the ternary operator needs both results to be of the same type, so it can't scale the Int as a BOOL like the previous version did or the compiler produces:
Incompatible operand types ('NSString * _Nullable' and 'int')
if (self.frontdoorBridgeLoginOverride.frontdoorBridgeUrl // Check if an override is provided
? self.frontdoorBridgeLoginOverride.codeVerifier != nil // If yes, only proceed if it's a web server flow (has codeVerifier)
: [[SalesforceSDKManager sharedManager] useWebServerAuthentication] // If no override, use default SDK setting
)
{
[self handleWebServerResponse:url]; // Web server flow/URLs with query string parameters.
} else {
[self handleUserAgentResponse:url]; // User agent flow/URLs with the fragment component.
}
There was a problem hiding this comment.
I do like the syntax candy, but I found this version just a little harder to process just from the code since it's more terse. It works though, so I tried massaging the comments a bit to see if that helped get the best of it all. Take a look at this 👉🏻 8141639
…QRCode (Code Review Updates)
1add4ee
into
forcedotcom:dev
🥁 Ready For Review 🎸
This simple fix makes the iOS side of QR Code Log In override the user-agent or web server flows just as Android now does. The error in the video was caused by the web server flow oriented QR code getting handled like a user agent one.