forcedotcom / SalesforceMobileSDK-Android

Android SDK for Salesforce
Other
341 stars 388 forks source link

@W-16171409: [Android] Add QR Code Login Support in MSDK #2594

Closed JohnsonEricAtSalesforce closed 1 month ago

JohnsonEricAtSalesforce commented 3 months ago

🎸 Ready For Review! 🥁

This adds support for log in via log in QR codes generated with the UI Bridge API to MSDK. This completes a very detailed spike previously completed by @wmathurin and directly inherits from that codebase.

Before reviewing, I highly recommend reading this document and references in detail: 👉🏻 https://salesforce.quip.com/JXmvAwirhR3V

There's actually very little if any logic change from the spike code. What I did try to apply was my novice's eye to the topic so we can name and document the code such that it'll be a bit more approachable for those uninitiated to the UI Bridge API, how that is delivered via the QR code and the many touch points that has in MSDK's log in logic. I did the same in the companion pull request that introduces commented out content to our template guiding a developer through enabling this in a new app.

The companion template update is in https://github.com/forcedotcom/SalesforceMobileSDK-Templates/pull/417

JohnsonEricAtSalesforce commented 3 months ago

It's worth noting I did intend to include commits from the spike performed by @wmathurin. I rebased his branch on dev before adding my own commits. Plus, a squash will make this very tidy on dev when we're done while preserving his history here.

mobilesdk-bot commented 3 months ago
1 Error
:no_entry_sign: Tests have failed, see below for more information.
2 Warnings
:warning: libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/LoginActivity.kt#L209 - Using setJavaScriptEnabled can introduce XSS vulnerabilities into your application, review carefully
:warning: libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/OAuthWebviewHelper.kt#L215 - Using setJavaScriptEnabled can introduce XSS vulnerabilities into your application, review carefully

Tests:

Name Classname Time
testRestClientUnauthenticatedlientInfoAsync com.salesforce.androidsdk.rest.RestClientTest 10.236

Tests results for SalesforceSDK

Generated by :no_entry_sign: Danger

JohnsonEricAtSalesforce commented 3 months ago

Hold merging this to dev and the 12.1.0 codebase until we confirm the iOS version will also make it in time for the release.

codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 16.98113% with 44 lines in your changes missing coverage. Please review.

Project coverage is 57.10%. Comparing base (301e0f4) to head (6f0974e). Report is 72 commits behind head on dev.

Files with missing lines Patch % Lines
.../src/com/salesforce/androidsdk/ui/LoginActivity.kt 20.51% 27 Missing and 4 partials :warning:
...com/salesforce/androidsdk/ui/OAuthWebviewHelper.kt 0.00% 13 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #2594 +/- ## ============================================ + Coverage 51.41% 57.10% +5.69% - Complexity 1789 2424 +635 ============================================ Files 146 187 +41 Lines 12097 15204 +3107 Branches 1712 2135 +423 ============================================ + Hits 6220 8683 +2463 - Misses 5178 5615 +437 - Partials 699 906 +207 ``` | [Flag](https://app.codecov.io/gh/forcedotcom/SalesforceMobileSDK-Android/pull/2594/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=forcedotcom) | Coverage Δ | | |---|---|---| | [MobileSync](https://app.codecov.io/gh/forcedotcom/SalesforceMobileSDK-Android/pull/2594/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=forcedotcom) | `81.68% <ø> (?)` | | | [SalesforceHybrid](https://app.codecov.io/gh/forcedotcom/SalesforceMobileSDK-Android/pull/2594/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=forcedotcom) | `55.56% <ø> (ø)` | | | [SalesforceReact](https://app.codecov.io/gh/forcedotcom/SalesforceMobileSDK-Android/pull/2594/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=forcedotcom) | `52.36% <ø> (ø)` | | | [SalesforceSDK](https://app.codecov.io/gh/forcedotcom/SalesforceMobileSDK-Android/pull/2594/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=forcedotcom) | `42.24% <16.98%> (-0.81%)` | :arrow_down: | | [SmartStore](https://app.codecov.io/gh/forcedotcom/SalesforceMobileSDK-Android/pull/2594/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=forcedotcom) | `78.30% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=forcedotcom#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files with missing lines](https://app.codecov.io/gh/forcedotcom/SalesforceMobileSDK-Android/pull/2594?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=forcedotcom) | Coverage Δ | | |---|---|---| | [.../salesforce/androidsdk/app/SalesforceSDKManager.kt](https://app.codecov.io/gh/forcedotcom/SalesforceMobileSDK-Android/pull/2594?src=pr&el=tree&filepath=libs%2FSalesforceSDK%2Fsrc%2Fcom%2Fsalesforce%2Fandroidsdk%2Fapp%2FSalesforceSDKManager.kt&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=forcedotcom#diff-bGlicy9TYWxlc2ZvcmNlU0RLL3NyYy9jb20vc2FsZXNmb3JjZS9hbmRyb2lkc2RrL2FwcC9TYWxlc2ZvcmNlU0RLTWFuYWdlci5rdA==) | `53.35% <100.00%> (+0.08%)` | :arrow_up: | | [...com/salesforce/androidsdk/ui/OAuthWebviewHelper.kt](https://app.codecov.io/gh/forcedotcom/SalesforceMobileSDK-Android/pull/2594?src=pr&el=tree&filepath=libs%2FSalesforceSDK%2Fsrc%2Fcom%2Fsalesforce%2Fandroidsdk%2Fui%2FOAuthWebviewHelper.kt&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=forcedotcom#diff-bGlicy9TYWxlc2ZvcmNlU0RLL3NyYy9jb20vc2FsZXNmb3JjZS9hbmRyb2lkc2RrL3VpL09BdXRoV2Vidmlld0hlbHBlci5rdA==) | `10.28% <0.00%> (-0.15%)` | :arrow_down: | | [.../src/com/salesforce/androidsdk/ui/LoginActivity.kt](https://app.codecov.io/gh/forcedotcom/SalesforceMobileSDK-Android/pull/2594?src=pr&el=tree&filepath=libs%2FSalesforceSDK%2Fsrc%2Fcom%2Fsalesforce%2Fandroidsdk%2Fui%2FLoginActivity.kt&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=forcedotcom#diff-bGlicy9TYWxlc2ZvcmNlU0RLL3NyYy9jb20vc2FsZXNmb3JjZS9hbmRyb2lkc2RrL3VpL0xvZ2luQWN0aXZpdHkua3Q=) | `24.53% <20.51%> (-1.43%)` | :arrow_down: | ... and [49 files with indirect coverage changes](https://app.codecov.io/gh/forcedotcom/SalesforceMobileSDK-Android/pull/2594/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=forcedotcom)
mobilesdk-bot commented 1 month ago
1 Warning
:warning: No Lint Results.

Tests results for SalesforceHybrid

Generated by :no_entry_sign: Danger

mobilesdk-bot commented 1 month ago
1 Error
:no_entry_sign: Tests have failed, see below for more information.

Tests:

Name Classname Time
test[testCleanResyncGhosts] com.salesforce.androidsdk.reactnative.ReactMobileSyncTest 131.635

Tests results for SalesforceReact

Generated by :no_entry_sign: Danger

mobilesdk-bot commented 1 month ago
1 Warning
:warning: No Lint Results.

Tests results for MobileSync

Generated by :no_entry_sign: Danger