From af8c7b9900a8dbf233aa11ce4446e522172ff277 Mon Sep 17 00:00:00 2001 From: Eric House Date: Tue, 29 Nov 2022 15:57:36 -0800 Subject: [PATCH] remove notion of banned permissions --- .../eehouse/android/xw4/BoardDelegate.java | 44 ++---- .../android/xw4/ConnViaViewLayout.java | 21 +-- .../android/xw4/GameConfigDelegate.java | 3 +- .../android/xw4/GamesListDelegate.java | 30 ---- .../android/xw4/InviteChoicesAlert.java | 10 +- .../org/eehouse/android/xw4/NBSProto.java | 2 +- .../java/org/eehouse/android/xw4/Perms23.java | 137 +++++++++--------- .../app/src/main/res/values/strings.xml | 27 ++-- 8 files changed, 101 insertions(+), 173 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 fc364a02e..814f84ec8 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 @@ -1197,17 +1197,11 @@ public class BoardDelegate extends DelegateBase showInviteChoicesThen(); break; case INVITE_SMS_DATA: - Perms23.Perm[] perms = (Perms23.Perm[])params[2]; - if ( Perms23.bannedWithWorkaround( m_activity, perms ) ) { + if ( Perms23.haveNBSPerms( m_activity ) ) { int nMissing = (Integer)params[0]; SentInvitesInfo info = (SentInvitesInfo)params[1]; launchPhoneNumberInvite( nMissing, info, RequestCode.SMS_DATA_INVITE_RESULT ); - } else if ( Perms23.anyBanned( m_activity, perms ) ) { - makeOkOnlyBuilder( R.string.sms_banned_ok_only ) - .setActionPair(Action.PERMS_BANNED_INFO, - R.string.button_more_info ) - .show(); } break; default: @@ -1282,10 +1276,8 @@ public class BoardDelegate extends DelegateBase RequestCode.BT_INVITE_RESULT ); break; case SMS_DATA: - Perm[] perms = { Perm.SEND_SMS, Perm.RECEIVE_SMS }; - Perms23.tryGetPerms( this, perms, R.string.sms_invite_rationale, - Action.INVITE_SMS_DATA, m_mySIS.nMissing, - info, perms ); + Perms23.tryGetPerms( this, Perms23.NBS_PERMS, R.string.sms_invite_rationale, + Action.INVITE_SMS_DATA, m_mySIS.nMissing, info ); break; case RELAY: case MQTT: @@ -2390,42 +2382,28 @@ public class BoardDelegate extends DelegateBase final StartAlertOrder thisOrder = StartAlertOrder.NBS_PERMS; if ( alertOrderAt( thisOrder ) // already asked? && m_summary.conTypes.contains( CommsConnType.COMMS_CONN_SMS ) ) { - Perm[] nbsPerms = { Perm.SEND_SMS, Perm.RECEIVE_SMS }; - if ( Perms23.havePermissions( m_activity, nbsPerms ) ) { + if ( Perms23.haveNBSPerms( m_activity ) ) { // We have them or a workaround; cool! proceed alertOrderIncrIfAt( thisOrder ); } else { - // Make sure these can be treated the same!!! - Assert.assertTrue( nbsPerms.length == 2 && - nbsPerms[0].isBanned(m_activity) - == nbsPerms[1].isBanned(m_activity) ); - m_permCbck = new Perms23.PermCbck() { @Override - public void onPermissionResult( boolean allGood, - Map perms ) + public void onPermissionResult( boolean allGood ) { if ( allGood ) { // Yay! nothing to do alertOrderIncrIfAt( thisOrder ); } else { - boolean banned = Perm.SEND_SMS.isBanned(m_activity); - int explID = banned - ? R.string.banned_nbs_perms : R.string.missing_sms_perms; - DlgDelegate.Builder builder = - makeConfirmThenBuilder( Action.DROP_SMS_ACTION, explID ); - if ( banned ) { - builder.setActionPair( Action.PERMS_BANNED_INFO, - R.string.button_more_info ) - .setNAKey( R.string.key_na_sms_banned ) - ; - } - builder.setNegButton( R.string.remove_sms ) + int explID = Perms23.NBSPermsInManifest( m_activity ) + ? R.string.missing_sms_perms + : R.string.variant_missing_nbs; + makeConfirmThenBuilder( Action.DROP_SMS_ACTION, explID ) + .setNegButton( R.string.remove_sms ) .show(); } } }; - new Perms23.Builder( nbsPerms ) + new Perms23.Builder( Perms23.NBS_PERMS ) .asyncQuery( m_activity, m_permCbck ); } } else { 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 598ecad35..6ae2bbd4e 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 @@ -176,14 +176,9 @@ public class ConnViaViewLayout extends LinearLayout { keyID = R.string.key_na_comms_relay; break; case COMMS_CONN_SMS: - if ( Perms23.havePermissions( getContext(), - Perms23.Perm.SEND_SMS, - Perms23.Perm.RECEIVE_SMS ) - || !Perms23.Perm.SEND_SMS.isBanned(getContext()) ) { + if ( Perms23.haveNBSPerms( getContext() ) ) { msgID = R.string.not_again_comms_sms; keyID = R.string.key_na_comms_sms; - } else { - msgID = R.string.sms_banned_ok_only; } break; case COMMS_CONN_BT: @@ -203,13 +198,13 @@ public class ConnViaViewLayout extends LinearLayout { break; } - DlgDelegate.Builder builder = 0 != keyID - ? m_dlgDlgt.makeNotAgainBuilder( keyID, msgID ) - : m_dlgDlgt.makeOkOnlyBuilder( msgID ) - .setActionPair( Action.PERMS_BANNED_INFO, - R.string.button_more_info ) - ; - builder.show(); + if ( 0 != msgID ) { + DlgDelegate.Builder builder = 0 != keyID + ? m_dlgDlgt.makeNotAgainBuilder( keyID, msgID ) + : m_dlgDlgt.makeOkOnlyBuilder( msgID ) + ; + builder.show(); + } } } } diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameConfigDelegate.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameConfigDelegate.java index 58e90099f..519dee6fc 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameConfigDelegate.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameConfigDelegate.java @@ -890,7 +890,8 @@ public class GameConfigDelegate extends DelegateBase private void showConnAfterCheck() { - if ( null == SMSPhoneInfo.get( m_activity ) ) { + if ( Perms23.NBSPermsInManifest( m_activity ) + && null == SMSPhoneInfo.get( m_activity ) ) { Perms23.tryGetPermsNA( this, Perms23.Perm.READ_PHONE_STATE, R.string.phone_state_rationale, R.string.key_na_perms_phonestate, 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 f70a5cdc0..e648c69cb 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 @@ -1042,20 +1042,6 @@ public class GamesListDelegate extends ListDelegateBase updateField(); - // RECEIVE_SMS is required now (Oreo/SDK_26) but wasn't - // before. There's logic elsewhere to ask for it AND SEND_SMS, but if - // the user's already granted SEND_SMS we can get RECEIVE_SMS just by - // 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(m_activity) ) { - if ( Perms23.havePermissions( m_activity, Perm.SEND_SMS ) ) { - Perms23.tryGetPerms( this, Perm.RECEIVE_SMS, 0, Action.SKIP_CALLBACK ); - } - } else if ( isFirstLaunch ) { - warnSMSBannedIf(); - } - if ( false ) { Set dupModeGames = DBUtils.getDupModeGames( m_activity ).keySet(); long[] asArray = new long[dupModeGames.size()]; @@ -1144,22 +1130,6 @@ public class GamesListDelegate extends ListDelegateBase } } - private void warnSMSBannedIf() - { - if ( !Perms23.havePermissions( m_activity, Perm.SEND_SMS, Perm.RECEIVE_SMS ) - && Perm.SEND_SMS.isBanned(m_activity) ) { - int smsGameCount = DBUtils.countOpenGamesUsingNBS( m_activity ); - if ( 0 < smsGameCount ) { - makeNotAgainBuilder( R.string.key_notagain_nbsGamesOnUpgrade, - R.string.not_again_nbsGamesOnUpgrade, - smsGameCount ) - .setActionPair( Action.PERMS_BANNED_INFO, - R.string.button_more_info ) - .show(); - } - } - } - private void moveGroup( long groupID, boolean moveUp ) { m_adapter.moveGroup( groupID, moveUp ); 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 028e36ebb..51c48fffa 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 @@ -227,14 +227,8 @@ public class InviteChoicesAlert extends DlgDelegateAlert R.string.qrcode_invite_expl ); break; case SMS_DATA: - if ( !Perms23.havePermissions( activity, Perm.SEND_SMS, Perm.RECEIVE_SMS ) - && Perm.SEND_SMS.isBanned(activity) ) { - builder = activity - .makeOkOnlyBuilder( R.string.sms_banned_ok_only ) - .setActionPair( Action.PERMS_BANNED_INFO, - R.string.button_more_info ) - ; - } else if ( ! XWPrefs.getNBSEnabled( getContext() ) ) { + if ( Perms23.NBSPermsInManifest( activity ) + && ! XWPrefs.getNBSEnabled( getContext() ) ) { builder = activity .makeConfirmThenBuilder( Action.ENABLE_NBS_ASK, R.string.warn_sms_disabled ) diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/NBSProto.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/NBSProto.java index de29f03ee..79832bb47 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/NBSProto.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/NBSProto.java @@ -330,7 +330,7 @@ public class NBSProto { Context context = XWApp.getContext(); boolean success = false; if ( XWPrefs.getNBSEnabled( context ) - && !Perms23.Perm.SEND_SMS.isBanned( context ) ) { + && Perms23.haveNBSPerms( context ) ) { // Try send-to-self if ( XWPrefs.getSMSToSelfEnabled( context ) ) { 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 0bc1068f0..265cadd20 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,6 +23,7 @@ import android.Manifest; import android.app.Activity; import android.content.Context; import android.content.pm.PackageManager; +import static android.content.pm.PackageManager.PERMISSION_GRANTED; import android.os.Build; import androidx.core.app.ActivityCompat; import androidx.core.content.ContextCompat; @@ -56,10 +57,6 @@ public class Perms23 { private Perm(String str) { m_str = str; } public String getString() { return m_str; } - public boolean isBanned( Context context ) - { - return !permInManifest( context, this ); - } public static Perm getFor( String str ) { Perm result = null; for ( Perm one : Perm.values() ) { @@ -101,8 +98,18 @@ public class Perms23 { return result; } + static boolean permsInManifest( Context context, Perm[] perms ) + { + boolean result = true; + for ( int ii = 0; result && ii < perms.length; ++ii ) { + result = permInManifest( context, perms[ii] ); + } + + return result; + } + public interface PermCbck { - void onPermissionResult( boolean allGood, Map perms ); + void onPermissionResult( boolean allGood ); } public interface OnShowRationale { void onShouldShowRationale( Set perms ); @@ -148,13 +155,14 @@ public class Perms23 { List askStrings = new ArrayList<>(); for ( Perm perm : m_perms ) { String permStr = perm.getString(); - boolean haveIt = perm.isBanned(activity) || PackageManager.PERMISSION_GRANTED + boolean inManifest = permInManifest( activity, perm ); + boolean haveIt = inManifest + && PERMISSION_GRANTED == ContextCompat.checkSelfPermission( activity, permStr ); - if ( !haveIt ) { + if ( !haveIt && inManifest ) { // do not pass banned perms to the OS! They're not in // AndroidManifest.xml so may crash on some devices - Assert.assertFalse( perm.isBanned(activity) ); askStrings.add( permStr ); if ( null != m_onShow ) { @@ -167,14 +175,7 @@ public class Perms23 { if ( haveAll ) { if ( null != cbck ) { - Map map = new HashMap<>(); - boolean allGood = true; - for ( Perm perm : m_perms ) { - boolean banned = perm.isBanned(activity); - map.put( perm, !banned ); - allGood = allGood & !banned; - } - callOPR( cbck, allGood, map ); + callOPR( cbck, true ); } } else if ( 0 < needShow.size() && null != m_onShow ) { // Log.d( TAG, "calling onShouldShowRationale()" ); @@ -222,11 +223,8 @@ public class Perms23 { private void doIt( boolean showRationale ) { Set validPerms = new HashSet<>(); - Set bannedPerms = new HashSet<>(); for ( Perm perm : m_perms ) { - if ( perm.isBanned(m_delegate.getActivity()) ) { - bannedPerms.add( perm ); - } else { + if ( permInManifest(m_delegate.getActivity(), perm ) ) { validPerms.add( perm ); } } @@ -234,9 +232,6 @@ public class Perms23 { if ( 0 < validPerms.size() ) { doItAsk( validPerms, showRationale ); } - if ( 0 < bannedPerms.size() ) { - postNeg(); - } } private void doItAsk( Set perms, boolean showRationale ) @@ -259,8 +254,7 @@ public class Perms23 { } builder.asyncQuery( m_delegate.getActivity(), new PermCbck() { @Override - public void onPermissionResult( boolean allGood, - Map permsMap ) { + public void onPermissionResult( boolean allGood ) { if ( Action.SKIP_CALLBACK != m_action ) { if ( allGood ) { m_delegate.onPosButton( m_action, m_params ); @@ -383,15 +377,13 @@ public class Perms23 { String[] perms, int[] granteds ) { // Log.d( TAG, "gotPermissionResult(%s)", perms.toString() ); - Map result = new HashMap<>(); boolean shouldResend = false; boolean allGood = true; for ( int ii = 0; ii < perms.length; ++ii ) { Perm perm = Perm.getFor( perms[ii] ); - Assert.assertTrue( !perm.isBanned(context) || ! BuildConfig.DEBUG ); - boolean granted = PackageManager.PERMISSION_GRANTED == granteds[ii]; + Assert.assertTrueNR( permInManifest( context, perm ) ); + boolean granted = PERMISSION_GRANTED == granteds[ii]; allGood = allGood && granted; - result.put( perm, granted ); // Hack. If SMS has been granted, resend all moves. This should be // replaced with an api allowing listeners to register @@ -412,7 +404,7 @@ public class Perms23 { PermCbck cbck = s_map.remove( code ); if ( null != cbck ) { - callOPR( cbck, allGood, result ); + callOPR( cbck, allGood ); } } @@ -421,52 +413,60 @@ public class Perms23 { boolean result = true; for ( int ii = 0; result && ii < perms.length; ++ii ) { Perm perm = perms[ii]; - boolean thisResult; - if ( perm.isBanned(context) ) { - thisResult = bannedWithWorkaround( context, perm ); - } else { - thisResult = PackageManager.PERMISSION_GRANTED - == ContextCompat.checkSelfPermission( XWApp.getContext(), - perm.getString() ); - } + boolean thisResult = permInManifest( context, perm ) + && PERMISSION_GRANTED + == ContextCompat.checkSelfPermission( XWApp.getContext(), + perm.getString() ); result = result && thisResult; } return result; } - static boolean anyBanned( Context context, Perms23.Perm... perms ) + static final Perm[] NBS_PERMS = { Perm.SEND_SMS, Perm.RECEIVE_SMS, }; + + static boolean haveNBSPerms( Context context ) { - boolean anyBanned = false; - for ( int ii = 0; !anyBanned && ii < perms.length; ++ii ) { - anyBanned = perms[ii].isBanned( context ); - } - return anyBanned; + return havePermissions( context, NBS_PERMS ); } - static boolean bannedWithWorkaround( Context context, Perms23.Perm... perms ) + static boolean NBSPermsInManifest( Context context ) { - boolean allBanned = true; - boolean workaroundKnown = true; - for ( Perms23.Perm perm : perms ) { - allBanned = allBanned && perm.isBanned(context); - - switch ( perm ) { - case SEND_SMS: - case RECEIVE_SMS: - workaroundKnown = false; - break; - default: - Log.e( TAG, "bannedWithWorkaround(): unexpected perm %s", perm ); - Assert.failDbg(); - break; - } - } - - boolean result = allBanned && workaroundKnown; - Log.d( TAG, "bannedWithWorkaround() => %b", result ); - return result; + return permsInManifest( context, NBS_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( context ); + // } + // return anyBanned; + // } + + // static boolean bannedWithWorkaround( Context context, Perms23.Perm... perms ) + // { + // boolean allBanned = true; + // boolean workaroundKnown = true; + // for ( Perms23.Perm perm : perms ) { + // allBanned = allBanned && perm.isBanned(context); + + // switch ( perm ) { + // case SEND_SMS: + // case RECEIVE_SMS: + // workaroundKnown = false; + // break; + // default: + // Log.e( TAG, "bannedWithWorkaround(): unexpected perm %s", perm ); + // Assert.failDbg(); + // break; + // } + // } + + // boolean result = allBanned && workaroundKnown; + // Log.d( TAG, "bannedWithWorkaround() => %b", result ); + // return result; + // } + // If two permission requests are made in a row the map may contain more // than one entry. private static int s_nextRecord = 0; @@ -478,11 +478,10 @@ public class Perms23 { return code; } - private static void callOPR( PermCbck cbck, boolean allGood, - Map map ) + private static void callOPR( PermCbck cbck, boolean allGood ) { - Log.d( TAG, "callOPR(): passing %s to %s", map, cbck ); - cbck.onPermissionResult( allGood, map ); + Log.d( TAG, "callOPR(): passing %b to %s", allGood, cbck ); + cbck.onPermissionResult( allGood ); } } diff --git a/xwords4/android/app/src/main/res/values/strings.xml b/xwords4/android/app/src/main/res/values/strings.xml index 0604a0db2..db0e470d1 100644 --- a/xwords4/android/app/src/main/res/values/strings.xml +++ b/xwords4/android/app/src/main/res/values/strings.xml @@ -2340,18 +2340,15 @@ via Data SMS but CrossWords does not have permission to do so. You can still open the game, but it may not be able to send or receive moves.\n\nYou can re-open it to be asked for permission again. Or - you can remove the Data SMS communication setting. - The Google Play Store - version of CrossWords is no longer allowed to support the - Play-by-Data-SMS feature. - \n\n - You have %1$d open games that use this feature. They will not be - able to send or receive moves via Data SMS as long as you are using - the Google Play version of CrossWords, or until I can figure out a - workaround that meets Store policies. - \n\n - You can read more using the button below. - CrossWords needs access to + you can remove the Data SMS communication setting. + + This game is configured to + communicate via Data SMS, but the Google Play Store version of + CrossWords is no longer allowed to support the Play-by-Data-SMS + feature. \n\n You can still open the game, but it may not be able + to send or receive moves. + + CrossWords needs access to temporary storage to keep what you’re about to download. @@ -2416,12 +2413,6 @@ • Launch CrossWords on the other device\n • If all else fails, reboot this device\n - - The Google Play version of CrossWords no longer supports invitations or play via data SMS. - Read more - This game is set up to communicate via data SMS, but apps from the Google Play Store are no longer allowed to do so (with rare exceptions). You can still open the game, but it may not be able to send or receive moves. -\n -\nYou can remove the data SMS communication setting, or use the “Read more” button to check progress on the development of an alternative. You can look up words BEFORE they’re committed as moves—by long-tapping, same as committed words.\n\nUse this feature to check the validity of words you’re