Add per login host oauth configuration. #2808
Conversation
Generated by 🚫 Danger |
|
|
||
| // Choose front door bridge use by verifying intent data and such that only front door bridge URLs with matching consumer keys are used. | ||
| val uiBridgeApiParametersFrontDoorBridgeUrlMismatchedConsumerKey = uiBridgeApiParametersConsumerKey != null && uiBridgeApiParametersConsumerKey != viewModel.bootConfig.remoteAccessConsumerKey | ||
| val uiBridgeApiParametersFrontDoorBridgeUrlMismatchedConsumerKey = uiBridgeApiParametersConsumerKey != null && uiBridgeApiParametersConsumerKey != viewModel.oAuthConfig.redirectUri |
There was a problem hiding this comment.
I don't understand the meaning of uiBridgeApiParametersConsumerKey != viewModel.oAuthConfig.redirectUri
| * Asynchronously retrieves the app config for the specified login host. If not set the values | ||
| * found in the BootConfig file will be used for all servers. | ||
| */ | ||
| var appConfigForLoginHost: suspend (server: String) -> OAuthConfig = { |
There was a problem hiding this comment.
For instance, if SApp goes to some end point and decides it should not override the default consumer key it doesn't have to figure out how to return the default config, it can simply return nil and let MSDK uses the default.
There was a problem hiding this comment.
I had considered that, which is why I made the OAuthConfig bootconfig constructor public for them to fall back to. But I suppose allowing null is easier so it can update to allow that.
| disabledContentColor = colorScheme.surfaceVariant, | ||
| ), | ||
| enabled = validInput, | ||
| onClick = { |
There was a problem hiding this comment.
On iOS, the view does not interact with SalesforceSDKManager (or anything else) directly. Instead it is passed a onUseConfig at initialization and uses it when the button is clicked. See here.
There was a problem hiding this comment.
I could extract out the SalesforceSDKManager but I don't really see the point. If it was used more than once it would make sense to extract it out, but the BootConfig class is not dynamic on Android.
I really do no like the idea of modifying the BootConfig class to change dynamically at runtime when it's purpose is represent a file that we do not write to. I also do not see the point in modifying BootConfig is is far simpler code-wise (and easier to understand IMO) to have a debug runtime config that takes president.
There was a problem hiding this comment.
From Login Options, are you exclusively using debugOverrideAppConfig?
There was a problem hiding this comment.
Hmm, so it's not really exercising (testing) the code path that internal app will use?
There was a problem hiding this comment.
There is only one path. From getAuthorizationUrl:
oAuthConfig = if (isDebugBuild && debugOverrideAppConfig != null) {
debugOverrideAppConfig!!
} else {
appConfigForLoginHost(server) ?: OAuthConfig(bootConfig)
}
debugOverrideAppConfig and appConfigForLoginHost are only ever used in this one place. oAuthConfig is the single source of truth.
… manifest to ensure they cannot be used in production.
Generated by 🚫 Danger |
…oid into dynamic_oauth_config
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #2808 +/- ##
=============================================
+ Coverage 43.68% 62.10% +18.42%
- Complexity 2061 2791 +730
=============================================
Files 214 215 +1
Lines 16931 16993 +62
Branches 2453 2474 +21
=============================================
+ Hits 7396 10554 +3158
+ Misses 8629 5264 -3365
- Partials 906 1175 +269
🚀 New features to boost your workflow:
|
2 participants
This PR adds a new
appConfigForLoginHostlambda toSalesforceSDKManagerthat allows the app to supply a consumer key, redirect and scopes per login server. This functionality is also utilized by the newly wired up "Override Boot Config" section of the debugLoginOptionsActivity.Apologies that this PR has become enormous, but the vast majority of it is test code.