Oauth scopes are now optional in bootconfig file #2778
Conversation
Added tests in BootConfigTest to make sure empty or missing scopes are gracefully handled both when the config is XML and JSON NB: I converted BootConfigTest to Kotlin to be able to use mockk - which was needed for the tests that read the config from the XML. Also added tests in OAuth2Test for getAuthorizationUrl I was initially focused on scopes, but ended adding tests for other parameters for completeness
| * @param ctx The context used to read XML resources. | ||
| * @return A BootConfig representing the native boot config object. | ||
| */ | ||
| static BootConfig getNativeBootConfig(Context ctx) { |
There was a problem hiding this comment.
Used by new tests that check bootconfig from xml.
There was a problem hiding this comment.
Is is possible that customers get this confused with the 'Native Login' feature?
Edit: That probably doesn't make sense.
| config.put(REMOTE_ACCESS_CONSUMER_KEY, remoteAccessConsumerKey); | ||
| config.put(OAUTH_REDIRECT_URI, oauthRedirectURI); | ||
| config.put(OAUTH_SCOPES, new JSONArray(Arrays.asList(oauthScopes))); | ||
| if (oauthScopes != null) { |
There was a problem hiding this comment.
Oauth scopes are optional so might not be there.
| remoteAccessConsumerKey = res.getString(R.string.remoteAccessConsumerKey); | ||
| oauthRedirectURI = res.getString(R.string.oauthRedirectURI); | ||
| oauthScopes = res.getStringArray(R.array.oauthScopes); | ||
| try { |
There was a problem hiding this comment.
Oauth scopes are optional so might not be there.
| * @throws URISyntaxException See {@link URISyntaxException}. | ||
| */ | ||
| @Test | ||
| public void testGetAuthorizationUrlWithLoginHint() throws URISyntaxException { |
There was a problem hiding this comment.
Unrelated to test - to improve coverage of the getAuthorizationUrl() method.
| * @throws URISyntaxException See {@link URISyntaxException}. | ||
| */ | ||
| @Test | ||
| public void testGetAuthorizationUrlWithCustomDisplayType() throws URISyntaxException { |
There was a problem hiding this comment.
Unrelated to test - to improve coverage of the getAuthorizationUrl() method.
| tryScopes(new String[] {}, null); | ||
|
|
||
| //null scopes -- should not find scopes | ||
| tryScopes(null, null); |
There was a problem hiding this comment.
Making sure null scopes are handled - null scopes can now happen with the change to BootConfig.java
| validateBootConfig(config, "Validation should fail with relative unauthenticatedStartPage value.") | ||
| } | ||
|
|
||
| fun testBootConfigJsonWithOauthScopes() { |
There was a problem hiding this comment.
NB: Only the tests named test*OauthScopes are new.
2 participants
Added tests in BootConfigTest to make sure empty or missing scopes are gracefully handled both when the config is XML and JSON
NB: I converted BootConfigTest to Kotlin to be able to use mockk - which was needed for the tests that read the config from the XML.
Also added tests in OAuth2Test for getAuthorizationUrl.
I was initially focused on scopes, but ended adding tests for other parameters also for completeness.