Skip to content

Commit db1b0e0

Browse files
authored
Merge branch 'dev' into zhipan/onboarding-telemetry-ipc-v2
2 parents 24f7ddd + 62b7c33 commit db1b0e0

8 files changed

Lines changed: 556 additions & 2 deletions

File tree

.github/copilot-instructions.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,11 @@ See [changelog.txt](../changelog.txt) for the changelog format.
554554

555555
## 11. Dependencies & Versioning
556556

557+
When any change is made to a `common/build.gradle` file or the shared dependency versions file at `gradle/versions.gradle` that adds a new dependency or changes a dependency version, always display the following warning message:
558+
> ⚠️ **Warning:** Changes detected ⚠️
559+
>
560+
> Please follow the recommendations in [Adding or Updating Gradle Dependencies](https://eng.ms/docs/microsoft-security/identity/entra-developer-application-platform/auth-client/authn-sdk-msal-android/android-auth-libraries/how-tos/adding-or-updating-gradle-dependencies).
561+
557562
Flag:
558563
- Security library downgrade.
559564
- Major upgrade without referenced release notes.
@@ -674,4 +679,4 @@ Always cite specific code and give a minimal, actionable fix; use an assumption
674679

675680
---
676681

677-
Thank you for contributing to this project!
682+
Thank you for contributing to this project!

azure-pipelines/pull-request-validation/build-consumers.yml

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,37 @@ stages:
132132
tasks: msal:testLocalDebugUnitTest -Plabtest -PlabSecret=$(LabVaultAppCert) -ProbolectricSdkVersion=${{variables.robolectricSdkVersion}} -PmockApiUrl=$(MOCK_API_URL) -PnativeAuthConfigString=$(NATIVE_AUTH_CONFIG_STRING)
133133
jdkArchitecture: x64
134134
jdkVersionOption: "1.17"
135+
# msalautomationapp
136+
- job: msalAutomationAppValidation
137+
displayName: MSAL Automation App
138+
dependsOn:
139+
- setupBranch
140+
variables:
141+
commonBranch: $[ dependencies.setupBranch.outputs['setvarStep.commonBranch'] ] # map in the variable
142+
condition: and( succeeded(), not(contains(variables['prLabels'], variables['skipConsumerValidationLabel'])) )
143+
steps:
144+
- checkout: msal
145+
displayName: Checkout msal repository
146+
clean: true
147+
submodules: recursive
148+
persistCredentials: True
149+
- task: CmdLine@2
150+
displayName: Checkout common submodule $(commonBranch)
151+
inputs:
152+
script: |
153+
git fetch
154+
git checkout $(commonBranch)
155+
git pull
156+
git status
157+
git rev-parse HEAD
158+
workingDirectory: $(Agent.BuildDirectory)/s/common
159+
- template: ../templates/steps/automation-cert.yml
160+
- task: Gradle@3
161+
displayName: Assemble msalautomationapp
162+
inputs:
163+
jdkArchitecture: x64
164+
jdkVersionOption: "1.17"
165+
tasks: clean msalautomationapp:assembleLocal
135166
# broker
136167
- job: brokerValidation
137168
displayName: Broker

changelog.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
vNext
22
----------
33
- [MINOR] Add onboarding telemetry blob fields to BrokerRequest/BrokerResult and command parameters for client↔broker IPC transport
4+
- [PATCH] Handle app_link Intent redirection by validating broker install links and rejecting unsupported redirect URIs with appropriate error responses (#3102)
45
- [PATCH] Extend filter-then-clone optimization to load() and getIdTokensForAccountRecord() in MsalOAuth2TokenCache: when ENABLE_FILTER_THEN_CLONE_IN_MEMORY_CACHE flight is enabled, skip clone-all preload and call direct flight-gated overloads that clone only matching credentials; add new getCredentialsFilteredBy overload with kid support (#3100)
56
- [PATCH] Move Multiple Listening apps check to the authorization layer (#3070)
67
- [PATCH] Edge TB: Fix lookup mode (#3108)
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// All rights reserved.
3+
//
4+
// This code is licensed under the MIT License.
5+
//
6+
// Permission is hereby granted, free of charge, to any person obtaining a copy
7+
// of this software and associated documentation files(the "Software"), to deal
8+
// in the Software without restriction, including without limitation the rights
9+
// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
10+
// copies of the Software, and to permit persons to whom the Software is
11+
// furnished to do so, subject to the following conditions :
12+
//
13+
// The above copyright notice and this permission notice shall be included in
14+
// all copies or substantial portions of the Software.
15+
//
16+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
// THE SOFTWARE.
23+
package com.microsoft.identity.common.java.providers
24+
25+
import com.microsoft.identity.common.java.util.CommonURIBuilder
26+
import org.apache.hc.core5.http.NameValuePair
27+
import java.net.URISyntaxException
28+
29+
/**
30+
* Strict allowlist validator for the `app_link` query-parameter value carried on
31+
* a broker-installation redirect URI (`msauth://...?app_link=<url>`).
32+
*
33+
* The classifier in [RawAuthorizationResult.getResultCodeFromFinalRedirectUri] uses this to
34+
* decide whether to return [RawAuthorizationResult.ResultCode.BROKER_INSTALLATION_TRIGGERED]
35+
* (which downstream Android sinks turn into `startActivity(ACTION_VIEW, app_link)`).
36+
*
37+
* The Microsoft identity service (eSTS) only ever emits one of three concrete
38+
* `app_link` values today:
39+
* 1. `https://play.google.com/store/apps/details?id=com.azure.authenticator`
40+
* 2. `https://play.google.com/store/apps/details?id=com.microsoft.windowsintune.companyportal`
41+
* 3. `https://go.microsoft.com/fwlink/?linkid=2134649` (China Company Portal)
42+
* Each may carry an optional `referrer` parameter set by the server.
43+
*/
44+
object BrokerInstallLinkValidator {
45+
46+
private const val SCHEME_HTTPS = "https"
47+
private const val HOST_PLAY = "play.google.com"
48+
private const val HOST_FWLINK = "go.microsoft.com"
49+
private const val PATH_PLAY = "/store/apps/details"
50+
private const val PATH_FWLINK = "/fwlink"
51+
private const val PATH_FWLINK_TRAILING = "/fwlink/"
52+
53+
private const val PARAM_ID = "id"
54+
private const val PARAM_LINKID = "linkid"
55+
private const val PARAM_REFERRER = "referrer"
56+
57+
private val ALLOWED_PACKAGE_IDS = setOf(
58+
"com.azure.authenticator",
59+
"com.microsoft.windowsintune.companyportal"
60+
)
61+
62+
private val ALLOWED_FWLINK_IDS = setOf("2134649")
63+
64+
/**
65+
* @return `true` iff [url] decodes to one of the allowlisted broker-install
66+
* destinations defined above; `false` for any other input (including null,
67+
* blank, malformed, or attacker-controlled values).
68+
*/
69+
@JvmStatic
70+
fun isSafeBrokerInstallLink(url: String?): Boolean {
71+
if (url.isNullOrBlank()) return false
72+
73+
val builder: CommonURIBuilder = try {
74+
CommonURIBuilder(url)
75+
} catch (e: URISyntaxException) {
76+
return false
77+
}
78+
79+
// Scheme must be exactly https (case-insensitive).
80+
if (!SCHEME_HTTPS.equals(builder.scheme, ignoreCase = true)) return false
81+
82+
// Reject embedded credentials, fragments, and non-default ports.
83+
if (builder.userInfo != null) return false
84+
if (builder.fragment != null) return false
85+
if (builder.port != -1) return false
86+
87+
val host = builder.host ?: return false
88+
val path = builder.path ?: return false
89+
90+
// Use getQueryParams() from CommonURIBuilder to parse query parameters.
91+
// toUniqueParamMap returns null if any key appears more than once, defending
92+
// against parameter-smuggling attacks such as ?id=safe&id=evil.
93+
val params = toUniqueParamMap(builder.queryParams) ?: return false
94+
95+
return when {
96+
HOST_PLAY.equals(host, ignoreCase = true) -> isValidPlayLink(path, params)
97+
HOST_FWLINK.equals(host, ignoreCase = true) -> isValidFwlink(path, params)
98+
else -> false
99+
}
100+
}
101+
102+
private fun isValidPlayLink(path: String, params: Map<String, String>): Boolean {
103+
if (path != PATH_PLAY) return false
104+
val id = params[PARAM_ID] ?: return false
105+
if (!ALLOWED_PACKAGE_IDS.any { it.equals(id, ignoreCase = true) }) return false
106+
return hasOnlyAllowedExtras(params, PARAM_ID)
107+
}
108+
109+
private fun isValidFwlink(path: String, params: Map<String, String>): Boolean {
110+
if (path != PATH_FWLINK && path != PATH_FWLINK_TRAILING) return false
111+
val linkId = params[PARAM_LINKID] ?: return false
112+
if (linkId !in ALLOWED_FWLINK_IDS) return false
113+
return hasOnlyAllowedExtras(params, PARAM_LINKID)
114+
}
115+
116+
private fun hasOnlyAllowedExtras(params: Map<String, String>, requiredKey: String): Boolean {
117+
for (key in params.keys) {
118+
if (key == requiredKey) continue
119+
if (key == PARAM_REFERRER) continue
120+
return false
121+
}
122+
return true
123+
}
124+
125+
/**
126+
* Converts a [NameValuePair] list (from [CommonURIBuilder.getQueryParams]) into a map.
127+
*
128+
* Returns `null` if any key appears more than once — this defends against
129+
* parameter-smuggling attacks such as `?id=safe&id=evil` where a permissive
130+
* parser might pick the wrong value.
131+
*/
132+
private fun toUniqueParamMap(pairs: List<NameValuePair>): Map<String, String>? {
133+
val out = LinkedHashMap<String, String>()
134+
for (pair in pairs) {
135+
if (out.containsKey(pair.name)) return null
136+
out[pair.name] = pair.value ?: ""
137+
}
138+
return out
139+
}
140+
}

common4j/src/main/com/microsoft/identity/common/java/providers/RawAuthorizationResult.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,8 @@ public static RawAuthorizationResult fromRedirectUri(@NonNull final String redir
193193
.resultCode(getResultCodeFromFinalRedirectUri(uri))
194194
.authorizationFinalUri(uri)
195195
.build();
196+
} catch (final ClientException e) {
197+
return fromException(e);
196198
} catch (final URISyntaxException e) {
197199
return fromException(new ClientException(ClientException.MALFORMED_URL,
198200
"Failed to parse redirect URL", e));
@@ -217,14 +219,22 @@ public static RawAuthorizationResult fromPropertyBag(@NonNull final PropertyBag
217219
.build();
218220
}
219221

220-
private static ResultCode getResultCodeFromFinalRedirectUri(@NonNull final URI uri) throws URISyntaxException {
222+
private static ResultCode getResultCodeFromFinalRedirectUri(@NonNull final URI uri) throws URISyntaxException, ClientException {
221223
final String methodTag = TAG + "getResultCodeFromFinalRedirectUri";
222224
final Map<String, String> parameters = UrlUtil.getParameters(uri);
223225

224226
if (REDIRECT_PREFIX.equalsIgnoreCase(uri.getScheme())) {
225227
// i.e. (Browser) msauth://com.msft.identity.client.sample.local/1wIqXSqBj7w%2Bh11ZifsnqwgyKrY%3D?wpj=1&username=idlab1%40msidlab4.onmicrosoft.com&app_link=https%3a%2f%2fplay.google.com%2fstore%2fapps%2fdetails%3fid%3dcom.azure.authenticator
226228
// (WebView) msauth://wpj/?username=idlab1%40msidlab4.onmicrosoft.com&app_link=https%3a%2f%2fplay.google.com%2fstore%2fapps%2fdetails%3fid%3dcom.azure.authenticator%26referrer%3dcom.msft.identity.client.sample.local
227229
if (parameters.containsKey(APP_LINK_KEY)) {
230+
// Only the eSTS-emitted Play Store and China fwlink targets are accepted;
231+
// anything else is rejected as an unsupported redirect (UNSUPPORTED_URL).
232+
final String appLink = parameters.get(APP_LINK_KEY);
233+
if (!BrokerInstallLinkValidator.isSafeBrokerInstallLink(appLink)) {
234+
Logger.warn(methodTag, "Rejected app_link that is not on the broker-install allowlist.");
235+
throw new ClientException(ClientException.UNSUPPORTED_URL,
236+
"app_link rejected by allowlist");
237+
}
228238
Logger.info(methodTag, "Return to caller with BROWSER_CODE_WAIT_FOR_BROKER_INSTALL, and waiting for result.");
229239
return ResultCode.BROKER_INSTALLATION_TRIGGERED;
230240
}

0 commit comments

Comments
 (0)