From 62c02920451d778779e24dc88506d73e24d12b05 Mon Sep 17 00:00:00 2001 From: Eric House Date: Sun, 10 Mar 2019 19:15:30 -0700 Subject: [PATCH] check if permission in manifest I got bitten when creating a new variant leaving the permission out of the manifest but using a config flag to claim it was there. Better to have just one place to configure that. So check for presence in the manifest and, for now, assert that that matches the isBanned flag. Soon the isBanned flag can be removed. --- .../eehouse/android/xw4/BoardDelegate.java | 7 +- .../android/xw4/ConnViaViewLayout.java | 2 +- .../android/xw4/GamesListDelegate.java | 4 +- .../android/xw4/InviteChoicesAlert.java | 2 +- .../java/org/eehouse/android/xw4/Perms23.java | 70 +++++++++++++++---- .../org/eehouse/android/xw4/SMSService.java | 2 +- 6 files changed, 64 insertions(+), 23 deletions(-) diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/BoardDelegate.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/BoardDelegate.java index 231b4a747..63a6f247d 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/BoardDelegate.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/BoardDelegate.java @@ -1211,7 +1211,7 @@ public class BoardDelegate extends DelegateBase SentInvitesInfo info = (SentInvitesInfo)params[1]; launchPhoneNumberInvite( nMissing, info, InviteMeans.SMS_DATA, RequestCode.SMS_DATA_INVITE_RESULT ); - } else if ( Perms23.anyBanned( perms ) ) { + } else if ( Perms23.anyBanned( m_activity, perms ) ) { makeOkOnlyBuilder( R.string.sms_banned_ok_only ) .setActionPair(new ActionPair( Action.PERMS_BANNED_INFO, R.string.button_more_info ) ) @@ -2309,7 +2309,8 @@ public class BoardDelegate extends DelegateBase } else { // Make sure these can be treated the same!!! Assert.assertTrue( nbsPerms.length == 2 && - nbsPerms[0].isBanned() == nbsPerms[1].isBanned() ); + nbsPerms[0].isBanned(m_activity) + == nbsPerms[1].isBanned(m_activity) ); m_permCbck = new Perms23.PermCbck() { @Override @@ -2320,7 +2321,7 @@ public class BoardDelegate extends DelegateBase // Yay! nothing to do alertOrderIncrIfAt( thisOrder ); } else { - boolean banned = Perm.SEND_SMS.isBanned(); + boolean banned = Perm.SEND_SMS.isBanned(m_activity); int explID = banned ? R.string.banned_nbs_perms : R.string.missing_sms_perms; DlgDelegate.ConfirmThenBuilder builder = diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/ConnViaViewLayout.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/ConnViaViewLayout.java index 0da7591ab..1d49a026c 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/ConnViaViewLayout.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/ConnViaViewLayout.java @@ -151,7 +151,7 @@ public class ConnViaViewLayout extends LinearLayout { if ( Perms23.havePermissions( getContext(), Perms23.Perm.SEND_SMS, Perms23.Perm.RECEIVE_SMS ) - || !Perms23.Perm.SEND_SMS.isBanned() ) { + || !Perms23.Perm.SEND_SMS.isBanned(getContext()) ) { msgID = R.string.not_again_comms_sms; keyID = R.string.key_na_comms_sms; } else { diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GamesListDelegate.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GamesListDelegate.java index 49a826319..eac7f44b3 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GamesListDelegate.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GamesListDelegate.java @@ -995,7 +995,7 @@ public class GamesListDelegate extends ListDelegateBase // asking (OS will grant without user interaction) since they're in // the same group. So just do it now. This code can be removed // later... - if ( !Perm.RECEIVE_SMS.isBanned() ) { + if ( !Perm.RECEIVE_SMS.isBanned(m_activity) ) { if ( Perms23.havePermissions( m_activity, Perm.SEND_SMS ) ) { Perms23.tryGetPerms( this, Perm.RECEIVE_SMS, 0, Action.SKIP_CALLBACK ); } @@ -1067,7 +1067,7 @@ public class GamesListDelegate extends ListDelegateBase private void warnSMSBannedIf() { if ( !Perms23.havePermissions( m_activity, Perm.SEND_SMS, Perm.RECEIVE_SMS ) - && Perm.SEND_SMS.isBanned() ) { + && Perm.SEND_SMS.isBanned(m_activity) ) { int smsGameCount = DBUtils.countOpenGamesUsingNBS( m_activity ); if ( 0 < smsGameCount ) { String msg = LocUtils.getString( m_activity, diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/InviteChoicesAlert.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/InviteChoicesAlert.java index 9e5492a3b..13ed6ff32 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/InviteChoicesAlert.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/InviteChoicesAlert.java @@ -117,7 +117,7 @@ public class InviteChoicesAlert extends DlgDelegateAlert { break; case SMS_DATA: if ( !Perms23.havePermissions( activity, Perm.SEND_SMS, Perm.RECEIVE_SMS ) - && Perm.SEND_SMS.isBanned() ) { + && Perm.SEND_SMS.isBanned(activity) ) { activity .makeOkOnlyBuilder( R.string.sms_banned_ok_only ) .setActionPair(new ActionPair( Action.PERMS_BANNED_INFO, diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/Perms23.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/Perms23.java index 9e08c5e50..a6f669d63 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/Perms23.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/Perms23.java @@ -23,7 +23,9 @@ import android.Manifest; import android.app.Activity; import android.app.AlertDialog; import android.content.Context; +import android.content.pm.PackageInfo; import android.content.pm.PackageManager; +import android.content.pm.PermissionInfo; import android.os.Build; import android.support.v4.app.ActivityCompat; import android.support.v4.content.ContextCompat; @@ -32,6 +34,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Set; @@ -56,13 +59,21 @@ public class Perms23 { private String m_str; private boolean m_banned; private Perm(String str) { this(str, false); } - private Perm(String str, boolean banned) { + private Perm( String str, boolean banned ) { m_str = str; m_banned = banned; } public String getString() { return m_str; } - public boolean isBanned() { return m_banned; } + public boolean isBanned( Context context ) + { + // PENDING... Once this has been here for a week or so, remove + // SMS_BANNED. That way absence of the permission from a variant's + // manifest is the only way being banned is expressed. It sucks + // keeping two things in sync. + Assert.assertFalse( m_banned == permInManifest( context, this ) ); + return m_banned; + } public static Perm getFor( String str ) { Perm result = null; for ( Perm one : Perm.values() ) { @@ -75,6 +86,35 @@ public class Perms23 { } } + private static Map sManifestMap = new HashMap<>(); + private static boolean permInManifest( Context context, Perm perm ) + { + boolean result = false; + if ( sManifestMap.containsKey( perm ) ) { + result = sManifestMap.get( perm ); + } else { + PackageManager pm = context.getPackageManager(); + try { + String[] pis = pm + .getPackageInfo( BuildConfig.APPLICATION_ID, + PackageManager.GET_PERMISSIONS ) + .requestedPermissions; + if ( pis == null ) { + Assert.assertFalse( BuildConfig.DEBUG ); + } else { + String manifestName = perm.getString(); + for ( int ii = 0; !result && ii < pis.length; ++ii ) { + result = pis[ii].equals( manifestName ); + } + } + } catch( PackageManager.NameNotFoundException nnfe ) { + Log.e(TAG, "permInManifest() nnfe: %s", nnfe.getMessage()); + } + sManifestMap.put( perm, result ); + } + return result; + } + public interface PermCbck { void onPermissionResult( boolean allGood, Map perms ); } @@ -83,7 +123,7 @@ public class Perms23 { } public static class Builder { - private Set m_perms = new HashSet(); + private Set m_perms = new HashSet<>(); private OnShowRationale m_onShow; public Builder(Set perms) { @@ -117,18 +157,18 @@ public class Perms23 { Log.d( TAG, "asyncQuery(%s)", m_perms ); boolean haveAll = true; boolean shouldShow = false; - Set needShow = new HashSet(); + Set needShow = new HashSet<>(); - ArrayList askStrings = new ArrayList(); + List askStrings = new ArrayList<>(); for ( Perm perm : m_perms ) { String permStr = perm.getString(); - boolean haveIt = perm.isBanned() || PackageManager.PERMISSION_GRANTED + boolean haveIt = perm.isBanned(activity) || PackageManager.PERMISSION_GRANTED == ContextCompat.checkSelfPermission( activity, permStr ); if ( !haveIt ) { // do not pass banned perms to the OS! They're not in // AndroidManifest.xml so may crash on some devices - Assert.assertFalse( perm.isBanned() ); + Assert.assertFalse( perm.isBanned(activity) ); askStrings.add( permStr ); if ( null != m_onShow && ActivityCompat @@ -146,7 +186,7 @@ public class Perms23 { Map map = new HashMap<>(); boolean allGood = true; for ( Perm perm : m_perms ) { - boolean banned = perm.isBanned(); + boolean banned = perm.isBanned(activity); map.put( perm, !banned ); allGood = allGood & !banned; } @@ -197,7 +237,7 @@ public class Perms23 { Set validPerms = new HashSet<>(); Set bannedPerms = new HashSet<>(); for ( Perm perm : m_perms ) { - if ( perm.isBanned() ) { + if ( perm.isBanned(m_delegate.getActivity()) ) { bannedPerms.add( perm ); } else { validPerms.add( perm ); @@ -323,7 +363,7 @@ public class Perms23 { info.handleButton( positive ); } - private static Map s_map = new HashMap(); + private static Map s_map = new HashMap<>(); public static void gotPermissionResult( Context context, int code, String[] perms, int[] granteds ) { @@ -333,7 +373,7 @@ public class Perms23 { boolean allGood = true; for ( int ii = 0; ii < perms.length; ++ii ) { Perm perm = Perm.getFor( perms[ii] ); - Assert.assertTrue( !perm.isBanned() || ! BuildConfig.DEBUG ); + Assert.assertTrue( !perm.isBanned(context) || ! BuildConfig.DEBUG ); boolean granted = PackageManager.PERMISSION_GRANTED == granteds[ii]; allGood = allGood && granted; result.put( perm, granted ); @@ -367,7 +407,7 @@ public class Perms23 { for ( int ii = 0; result && ii < perms.length; ++ii ) { Perm perm = perms[ii]; boolean thisResult; - if ( perm.isBanned() ) { + if ( perm.isBanned(context) ) { thisResult = bannedWithWorkaround( context, perm ); } else { thisResult = PackageManager.PERMISSION_GRANTED @@ -379,11 +419,11 @@ public class Perms23 { return result; } - static boolean anyBanned( Perms23.Perm... perms ) + static boolean anyBanned( Context context, Perms23.Perm... perms ) { boolean anyBanned = false; for ( int ii = 0; !anyBanned && ii < perms.length; ++ii ) { - anyBanned = perms[ii].isBanned(); + anyBanned = perms[ii].isBanned( context ); } return anyBanned; } @@ -393,7 +433,7 @@ public class Perms23 { boolean allBanned = true; boolean workaroundKnown = true; for ( Perms23.Perm perm : perms ) { - allBanned = allBanned && perm.isBanned(); + allBanned = allBanned && perm.isBanned(context); switch ( perm ) { case SEND_SMS: diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/SMSService.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/SMSService.java index 60351ddf0..52b2d9e1f 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/SMSService.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/SMSService.java @@ -516,7 +516,7 @@ public class SMSService extends XWJIService { short nbsPort = getNBSPort(); try { SmsManager mgr = SmsManager.getDefault(); - boolean useProxy = Perms23.Perm.SEND_SMS.isBanned() + boolean useProxy = Perms23.Perm.SEND_SMS.isBanned( this ) && NBSProxy.isInstalled( this ); PendingIntent sent = useProxy ? null : makeStatusIntent( MSG_SENT ); PendingIntent delivery = useProxy ? null : makeStatusIntent( MSG_DELIVERED );