-
Notifications
You must be signed in to change notification settings - Fork 393
Oauth scopes are now optional in bootconfig file #2778
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -116,6 +116,18 @@ static BootConfig getHybridBootConfig(Context ctx, String assetFilePath) { | |
| return hybridBootConfig; | ||
| } | ||
|
|
||
| /** | ||
| * Gets a native boot config instance from XML resources. | ||
| * Package-private for testing purposes. | ||
| * @param ctx The context used to read XML resources. | ||
| * @return A BootConfig representing the native boot config object. | ||
| */ | ||
| static BootConfig getNativeBootConfig(Context ctx) { | ||
| BootConfig nativeBootConfig = new BootConfig(); | ||
| nativeBootConfig.readFromXML(ctx); | ||
| return nativeBootConfig; | ||
| } | ||
|
|
||
| /** | ||
| * Validates a boot config's inputs against basic sanity tests. | ||
| * @param config The BootConfig instance to validate. | ||
|
|
@@ -180,7 +192,9 @@ public JSONObject asJSON() { | |
| JSONObject config = new JSONObject(); | ||
| 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) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oauth scopes are optional so might not be there. |
||
| config.put(OAUTH_SCOPES, new JSONArray(Arrays.asList(oauthScopes))); | ||
| } | ||
| config.put(IS_LOCAL, isLocal); | ||
| config.put(START_PAGE, startPage); | ||
| config.put(ERROR_PAGE, errorPage); | ||
|
|
@@ -224,7 +238,12 @@ private void readFromXML(Context ctx) { | |
| final Resources res = ctx.getResources(); | ||
| remoteAccessConsumerKey = res.getString(R.string.remoteAccessConsumerKey); | ||
| oauthRedirectURI = res.getString(R.string.oauthRedirectURI); | ||
| oauthScopes = res.getStringArray(R.array.oauthScopes); | ||
| try { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oauth scopes are optional so might not be there. |
||
| oauthScopes = res.getStringArray(R.array.oauthScopes); | ||
| } catch (Resources.NotFoundException e) { | ||
| // oauthScopes is optional, leave it as null | ||
| oauthScopes = null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -237,11 +256,18 @@ private void parseBootConfig(JSONObject config) { | |
| // Required fields. | ||
| remoteAccessConsumerKey = config.getString(REMOTE_ACCESS_CONSUMER_KEY); | ||
| oauthRedirectURI = config.getString(OAUTH_REDIRECT_URI); | ||
| final JSONArray jsonScopes = config.getJSONArray(OAUTH_SCOPES); | ||
| oauthScopes = new String[jsonScopes.length()]; | ||
| for (int i = 0; i < oauthScopes.length; i++) { | ||
| oauthScopes[i] = jsonScopes.getString(i); | ||
|
|
||
| // Optional oauthScopes field. | ||
| if (config.has(OAUTH_SCOPES)) { | ||
| final JSONArray jsonScopes = config.getJSONArray(OAUTH_SCOPES); | ||
| oauthScopes = new String[jsonScopes.length()]; | ||
| for (int i = 0; i < oauthScopes.length; i++) { | ||
| oauthScopes[i] = jsonScopes.getString(i); | ||
| } | ||
| } else { | ||
| oauthScopes = null; | ||
| } | ||
|
|
||
| isLocal = config.getBoolean(IS_LOCAL); | ||
| startPage = config.getString(START_PAGE); | ||
| errorPage = config.getString(ERROR_PAGE); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| { | ||
| "remoteAccessConsumerKey": "3MVG9Iu66FKeHhINkB1l7xt7kR8czFcCTUhgoA8Ol2Ltf1eYHOU4SqQRSEitYFDUpqRWcoQ2.dBv_a1Dyu5xa", | ||
| "oauthRedirectURI": "testsfdc:///mobilesdk/detect/oauth/done", | ||
| "oauthScopes": [], | ||
| "isLocal": true, | ||
| "startPage": "index.html", | ||
| "errorPage": "error.html", | ||
| "shouldAuthenticate": true, | ||
| "attemptOfflineLoad": true | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "remoteAccessConsumerKey": "3MVG9Iu66FKeHhINkB1l7xt7kR8czFcCTUhgoA8Ol2Ltf1eYHOU4SqQRSEitYFDUpqRWcoQ2.dBv_a1Dyu5xa", | ||
| "oauthRedirectURI": "testsfdc:///mobilesdk/detect/oauth/done", | ||
| "isLocal": true, | ||
| "startPage": "index.html", | ||
| "errorPage": "error.html", | ||
| "shouldAuthenticate": true, | ||
| "attemptOfflineLoad": true | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -298,14 +298,35 @@ private void tryScopes(String[] scopes, String expectedScopeParamValue) throws U | |
| } | ||
| } | ||
|
|
||
| private void tryLoginHint(String loginHint, String expectedLoginHintParamValue) throws URISyntaxException { | ||
| String callbackUrl = "sfdc://callback"; | ||
|
|
||
| // Test with loginHint provided | ||
| URI authorizationUrl = OAuth2.getAuthorizationUrl(true, true, new URI(TestCredentials.LOGIN_URL), | ||
| TestCredentials.CLIENT_ID, callbackUrl, null, loginHint, null, "some-challenge", null); | ||
| HttpUrl url = HttpUrl.get(authorizationUrl); | ||
| boolean loginHintFound = false; | ||
| for (int i = 0, size = url.querySize(); i < size; i++) { | ||
| if (url.queryParameterName(i).equalsIgnoreCase("login_hint")) { | ||
| loginHintFound = true; | ||
| Assert.assertEquals("Wrong login hint value", expectedLoginHintParamValue, url.queryParameterValue(i)); | ||
| break; | ||
| } | ||
| } | ||
| if (expectedLoginHintParamValue == null) { | ||
| Assert.assertFalse("Login hint found when not expected", loginHintFound); | ||
| } else { | ||
| Assert.assertTrue("No login hint param found in query", loginHintFound); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Testing getAuthorizationUrl with scopes. | ||
| * | ||
| * @throws URISyntaxException See {@link URISyntaxException}. | ||
| */ | ||
| @Test | ||
| public void testGetAuthorizationUrlWithScopes() throws URISyntaxException { | ||
|
|
||
| //verify basic scopes present | ||
| tryScopes(new String[]{"foo", "bar"}, "bar foo refresh_token"); | ||
|
|
||
|
|
@@ -317,7 +338,52 @@ public void testGetAuthorizationUrlWithScopes() throws URISyntaxException { | |
|
|
||
| //empty scopes -- should not find scopes | ||
| tryScopes(new String[] {}, null); | ||
|
|
||
| //null scopes -- should not find scopes | ||
| tryScopes(null, null); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Making sure null scopes are handled - null scopes can now happen with the change to BootConfig.java |
||
| } | ||
|
|
||
| /** | ||
| * Testing getAuthorizationUrl with loginHint parameter. | ||
| * | ||
| * @throws URISyntaxException See {@link URISyntaxException}. | ||
| */ | ||
| @Test | ||
| public void testGetAuthorizationUrlWithLoginHint() throws URISyntaxException { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated to test - to improve coverage of the getAuthorizationUrl() method. |
||
| //verify basic login hint present | ||
| tryLoginHint("[email protected]", "[email protected]"); | ||
|
|
||
| //empty login hint -- should not find login hint | ||
| tryLoginHint("", null); | ||
|
|
||
| //null login hint -- should not find login hint | ||
| tryLoginHint(null, null); | ||
| } | ||
|
|
||
| /** | ||
| * Testing getAuthorizationUrl with custom displayType. | ||
| * | ||
| * @throws URISyntaxException See {@link URISyntaxException}. | ||
| */ | ||
| @Test | ||
| public void testGetAuthorizationUrlWithCustomDisplayType() throws URISyntaxException { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated to test - to improve coverage of the getAuthorizationUrl() method. |
||
| String callbackUrl = "sfdc://callback"; | ||
| String customDisplayType = "page"; | ||
|
|
||
| URI authorizationUrl = OAuth2.getAuthorizationUrl(true, true, new URI(TestCredentials.LOGIN_URL), | ||
| TestCredentials.CLIENT_ID, callbackUrl, null, null, customDisplayType, "some-challenge", null); | ||
| HttpUrl url = HttpUrl.get(authorizationUrl); | ||
|
|
||
| boolean displayFound = false; | ||
| for (int i = 0, size = url.querySize(); i < size; i++) { | ||
| if (url.queryParameterName(i).equalsIgnoreCase("display")) { | ||
| displayFound = true; | ||
| Assert.assertEquals("Wrong display value", customDisplayType, url.queryParameterValue(i)); | ||
| break; | ||
| } | ||
| } | ||
| Assert.assertTrue("display parameter should be present", displayFound); | ||
| } | ||
|
|
||
|
|
||
| /** | ||
|
|
||
This file was deleted.
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.
Used by new tests that check bootconfig from xml.
Uh oh!
There was an error while loading. Please reload this page.
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.
Is is possible that customers get this confused with the 'Native Login' feature?
Edit: That probably doesn't make sense.