Skip to content

Commit

Permalink
Merge branch 'dev' into shahzaibj/msal-issue-1864-fix
Browse files Browse the repository at this point in the history
  • Loading branch information
shahzaibj committed Jan 27, 2024
2 parents 897af40 + d2dd9bb commit f2acb07
Show file tree
Hide file tree
Showing 8 changed files with 512 additions and 15 deletions.
1 change: 1 addition & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
V.Next
---------
- [PATCH] Fix MSAL Issue 1864 (#2280)
- [MINOR] Add AccountManagerBackupIpcStrategyTargetingSpecificBrokerApp (#2294)
- [MINOR] Adding additional fields to JwtRequestBody for sending PRTv4 requests (#2289)
- [MINOR] Update SignIn/Signup parameters classes for Native Auth(#2284)
- [MINOR] Add IsQrPinAvailableCommand, controllers behavior and define constants for the isQrAvailable API (#2219)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1532,8 +1532,6 @@ public static final class BrokerContentProvider {
* place, so that we only need to make updates in one location, and it's clearer what we need
* to do when adding new APIs.
*/
@Getter
@Accessors(prefix = "m")
@AllArgsConstructor
public enum API {
MSAL_HELLO(MSAL_HELLO_PATH, null, VERSION_3),
Expand Down Expand Up @@ -1568,15 +1566,27 @@ public enum API {
/**
* The content provider path that the API exists behind.
*/
private String mPath;
private final String mPath;
/**
* The broker-host-to-broker protocol version that the API requires.
*/
private String mBrokerVersion;
private final String mBrokerVersion;
/**
* The msal-to-broker version that the API requires.
*/
private String mMsalVersion;
private final String mMsalVersion;

public String getPath(){
return mPath;
}

public String getBrokerVersion(){
return mBrokerVersion;
}

public String getMsalVersion(){
return mMsalVersion;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@

/**
* A strategy for communicating with the targeted broker via AccountManager's addAccount().
* This will only communicate to the owner of com.microsoft.workaccount account type.
* <p>
* NOTE: SuppressLint is added because this API requires MANAGE_ACCOUNTS for API<= 22.
* AccountManagerUtil.canUseAccountManagerOperation() will validate that.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
// Copyright (c) Microsoft Corporation.
// All rights reserved.
//
// This code is licensed under the MIT License.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files(the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions :
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
package com.microsoft.identity.common.internal.broker.ipc

import android.accounts.AccountManager
import android.accounts.AuthenticatorDescription
import android.content.Context
import android.os.Bundle
import com.microsoft.identity.common.exception.BrokerCommunicationException
import com.microsoft.identity.common.internal.broker.BrokerData
import com.microsoft.identity.common.internal.broker.BrokerValidator
import com.microsoft.identity.common.internal.broker.IBrokerValidator
import com.microsoft.identity.common.internal.util.AccountManagerUtil
import com.microsoft.identity.common.internal.util.ProcessUtil
import com.microsoft.identity.common.logging.Logger

/**
* An IPC strategy that utilizes AccountManager.addAccount().
*
* The receiving end of this class is Broker's AccountAuthenticatorForBrokerDiscovery.
* Each broker apps would need to wire that class up to AccountManager manually,
* with a unique account type.
* (see getAccountTypeForEachPackage() and
* https://developer.android.com/training/sync-adapters/creating-authenticator)
*
* As of Jan 23, this class (and AccountAuthenticatorForBrokerDiscovery) only supports
* Broker Discovery and Passthrough mode.
*
* NOTE: This is not an official IPC Mechanism, therefore it is meant to be used as a backup only.
* (Apparently, it's more resilient than Content Provider and Bound Service under certain scenarios).
**/
class AccountManagerBackupIpcStrategyTargetingSpecificBrokerApp
internal constructor (private val accountTypeForEachPackage: Map<String, String>,
private val sendRequestViaAccountManager: (accountType: String, bundle: Bundle) -> Bundle,
private val getAccountManagerApps: () -> Array<AuthenticatorDescription>,
private val brokerValidator: IBrokerValidator) : IIpcStrategy {
companion object {
val TAG = AccountManagerBackupIpcStrategyTargetingSpecificBrokerApp::class.simpleName

// Content of the bundle to be sent.
/** Key associated to Content Provider path of [BrokerOperationBundle.Operation] **/
const val CONTENT_PROVIDER_PATH_KEY = "CONTENT_PROVIDER_PATH"

/** Key associated to request bundle **/
const val REQUEST_BUNDLE_KEY = "REQUEST_BUNDLE"

// Account types associated to each broker apps.
/** Account type for Link To Windows (defined in Broker's res/xml/ltw_account_manager_passthrough_backup.xml) **/
internal const val LTW_BACKUP_IPC_ACCOUNT_MANAGER_ACCOUNT_TYPE = "com.microsoft.ltwpassthroughbackup"

/** Account type for Company Portal (defined in Broker's res/xml/cp_account_manager_passthrough_backup.xml) **/
internal const val CP_BACKUP_IPC_ACCOUNT_MANAGER_ACCOUNT_TYPE = "com.microsoft.cppassthroughbackup"

/** Account type for Authenticator (defined in Broker's res/xml/authapp_account_manager_passthrough_backup.xml) **/
internal const val AUTHAPP_BACKUP_IPC_ACCOUNT_MANAGER_ACCOUNT_TYPE = "com.microsoft.authapppassthroughbackup"

/**
* Returns AccountManager account types associated to each Broker app packages.
**/
internal fun getAccountTypeForEachPackage(): Map<String, String>{
val result = mapOf(
BrokerData.debugMockLtw.packageName to LTW_BACKUP_IPC_ACCOUNT_MANAGER_ACCOUNT_TYPE,
BrokerData.prodLTW.packageName to LTW_BACKUP_IPC_ACCOUNT_MANAGER_ACCOUNT_TYPE,

BrokerData.debugMockCp.packageName to CP_BACKUP_IPC_ACCOUNT_MANAGER_ACCOUNT_TYPE,
BrokerData.prodCompanyPortal.packageName to CP_BACKUP_IPC_ACCOUNT_MANAGER_ACCOUNT_TYPE,

BrokerData.debugMockAuthApp.packageName to AUTHAPP_BACKUP_IPC_ACCOUNT_MANAGER_ACCOUNT_TYPE,
BrokerData.prodMicrosoftAuthenticator.packageName to AUTHAPP_BACKUP_IPC_ACCOUNT_MANAGER_ACCOUNT_TYPE,
)

// This will throw an error if any PROD apps doesn't implement this mechanism.
// (To make sure we don't miss this if we have to add another broker app in the future).
BrokerData.prodBrokers.forEach {
assert(result.containsKey(it.packageName))
}

return result
}

/**
* Gets an instance of this class.
* This will return null if AccountManager cannot be used as an IPC mechanism.
* */
fun tryCreateInstance(context: Context):
AccountManagerBackupIpcStrategyTargetingSpecificBrokerApp?{
if (!AccountManagerUtil.canUseAccountManagerOperation(
context,
setOf(LTW_BACKUP_IPC_ACCOUNT_MANAGER_ACCOUNT_TYPE,
CP_BACKUP_IPC_ACCOUNT_MANAGER_ACCOUNT_TYPE,
AUTHAPP_BACKUP_IPC_ACCOUNT_MANAGER_ACCOUNT_TYPE)
)){
return null
}

val accountManager = AccountManager.get(context)

return AccountManagerBackupIpcStrategyTargetingSpecificBrokerApp(
accountTypeForEachPackage = getAccountTypeForEachPackage(),
sendRequestViaAccountManager = { accountType, requestBundle ->
accountManager.addAccount(
accountType,
null,
null,
requestBundle,
null,
null,
ProcessUtil.getPreferredHandler()
).result
},
getAccountManagerApps = {
accountManager.authenticatorTypes
},
brokerValidator = BrokerValidator(context))
}
}

@Throws(BrokerCommunicationException::class)
override fun communicateToBroker(bundle: BrokerOperationBundle): Bundle? {
val methodTag = "$TAG:communicateToBroker"
val targetPackageName = bundle.targetBrokerAppPackageName

val accountType = accountTypeForEachPackage.getOrElse(targetPackageName) {
throw BrokerCommunicationException(
BrokerCommunicationException.Category.OPERATION_NOT_SUPPORTED_ON_CLIENT_SIDE,
type,
"AccountManagerBackupIpcStrategy doesn't recognize $targetPackageName as a broker",
null
)
}

// check if the account type owner actually belongs to the corresponding broker app.
validateTargetApp(targetPackageName, accountType)

// Pack the request bundle.
// Changing this format is a breaking change
// as the receiving end might be on an older broker version (therefore doesn't recognize the change)
val requestBundle = Bundle().apply {
putParcelable(REQUEST_BUNDLE_KEY, bundle.bundle)
putString(CONTENT_PROVIDER_PATH_KEY, bundle.operation.contentApi.path)
}

return try {
val result = sendRequestViaAccountManager(accountType, requestBundle)
result
} catch (e: Throwable) {
Logger.error(methodTag, e.message, e)
// Technically... this might NOT be a connection error.
// AccountManager returns both connection error and legit failure
// (i.e. not supported, bad request) as AuthenticatorException...
// It also has no error code. The only difference would be in the error message.
throw BrokerCommunicationException(
BrokerCommunicationException.Category.CONNECTION_ERROR,
type,
"AccountManager failed to respond - ${e.message}",
e
)
}
}

/**
* Check if the App currently associated to the given account type is
* actually a valid broker app.
**/
@Throws(BrokerCommunicationException::class)
private fun validateTargetApp(
targetPackageName: String,
accountType: String,
) {
val targetAppInfo = try {
getAccountManagerApps().first {
it.packageName == targetPackageName && it.type == accountType
}
} catch (t: Throwable) {
throw BrokerCommunicationException(
BrokerCommunicationException.Category.OPERATION_NOT_SUPPORTED_ON_SERVER_SIDE,
type,
"$targetPackageName doesn't support account manager backup ipc for Broker Discovery.",
null)
}

if (!brokerValidator.isValidBrokerPackage(targetAppInfo.packageName)) {
throw BrokerCommunicationException(
BrokerCommunicationException.Category.OPERATION_NOT_SUPPORTED_ON_SERVER_SIDE,
type,
"${targetAppInfo.packageName} is not a valid broker app",
null
)
}
}

override fun getType(): IIpcStrategy.Type {
return IIpcStrategy.Type.ACCOUNT_MANAGER_ADD_ACCOUNT
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@
public class BrokerOperationBundle {
private static final String TAG = BrokerOperationBundle.class.getSimpleName();

@Getter
@Accessors(prefix = "m")
public enum Operation {
MSAL_HELLO(API.MSAL_HELLO, BrokerAccountManagerOperation.HELLO),
MSAL_GET_INTENT_FOR_INTERACTIVE_REQUEST(API.ACQUIRE_TOKEN_INTERACTIVE, BrokerAccountManagerOperation.GET_INTENT_FOR_INTERACTIVE_REQUEST),
Expand Down Expand Up @@ -80,6 +78,15 @@ public enum Operation {

final API mContentApi;
final String mAccountManagerOperation;

public API getContentApi(){
return mContentApi;
}

public String getAccountManagerOperation(){
return mAccountManagerOperation;
}

Operation(API contentApi, String accountManagerOperation) {
this.mContentApi = contentApi;
this.mAccountManagerOperation = accountManagerOperation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import static com.microsoft.identity.common.internal.broker.ipc.BrokerOperationBundle.Operation.MSAL_SSO_TOKEN;
import static com.microsoft.identity.common.internal.controllers.BrokerOperationExecutor.BrokerOperation;
import static com.microsoft.identity.common.internal.util.StringUtil.isEmpty;
import static com.microsoft.identity.common.java.AuthenticationConstants.Broker.BROKER_ACCOUNT_TYPE;
import static com.microsoft.identity.common.java.AuthenticationConstants.LocalBroadcasterAliases.RETURN_BROKER_INTERACTIVE_ACQUIRE_TOKEN_RESULT;
import static com.microsoft.identity.common.java.AuthenticationConstants.LocalBroadcasterFields.REQUEST_CODE;
import static com.microsoft.identity.common.java.AuthenticationConstants.LocalBroadcasterFields.RESULT_CODE;
Expand Down Expand Up @@ -117,6 +118,7 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -200,7 +202,9 @@ static List<IIpcStrategy> getIpcStrategies(final Context applicationContext,
strategies.add(new BoundServiceStrategy<>(client));
}

if (AccountManagerUtil.canUseAccountManagerOperation(applicationContext)) {
if (AccountManagerUtil.canUseAccountManagerOperation(applicationContext,
Collections.singleton(BROKER_ACCOUNT_TYPE)))
{
sb.append("AccountManagerStrategy.");
strategies.add(new AccountManagerAddAccountStrategy(applicationContext));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@

package com.microsoft.identity.common.internal.util;

import static com.microsoft.identity.common.java.AuthenticationConstants.Broker.BROKER_ACCOUNT_TYPE;

import android.app.admin.DevicePolicyManager;
import android.content.Context;
import android.content.pm.PackageManager;
Expand All @@ -35,6 +33,8 @@

import com.microsoft.identity.common.logging.Logger;

import java.util.Set;

public final class AccountManagerUtil {
private static final String TAG = AccountManagerUtil.class.getSimpleName();

Expand All @@ -44,9 +44,13 @@ private AccountManagerUtil() {
}

/**
* To verify if the caller can use to AccountManager to use broker.
* To verify if the caller can use to AccountManager to communicate to broker.
*
* @param context an Android context.
* @param accountTypes AccountManager account type to check (if they're being controlled by MDM).
*/
public static boolean canUseAccountManagerOperation(final Context context) {
public static boolean canUseAccountManagerOperation(final Context context,
final Set<String> accountTypes) {
final String methodTag = TAG + ":canUseAccountManagerOperation:";

if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
Expand All @@ -63,9 +67,10 @@ public static boolean canUseAccountManagerOperation(final Context context) {
if (devicePolicyManager != null) {
final String[] accountTypesWithManagementDisabled = devicePolicyManager.getAccountTypesWithManagementDisabled();
if (accountTypesWithManagementDisabled != null) {
for (final String accountType : accountTypesWithManagementDisabled) {
if (BROKER_ACCOUNT_TYPE.equalsIgnoreCase(accountType)) {
Logger.verbose(methodTag, "Broker account type is disabled by MDM.");
for (final String disabledAccountType : accountTypesWithManagementDisabled) {
if (accountTypes.contains(disabledAccountType)) {
Logger.info(methodTag, "Account type " + disabledAccountType +
" is disabled by MDM.");
return false;
}
}
Expand Down
Loading

0 comments on commit f2acb07

Please sign in to comment.