From d5bb73f66754c9d02b237cb89b8e813f4ed0b086 Mon Sep 17 00:00:00 2001 From: Dave Santoro Date: Thu, 14 Jul 2011 12:23:15 -0700 Subject: [PATCH] Fix issue with profile checks during updateData(). We were doing a query with profiles included to determine the set of items to update, but that triggers a permission error if the calling app doesn't have READ_PROFILE permission (even if the rows being updated don't affect the profile). Bug 5028891 Change-Id: Id16d31d5d9d62dea2e62709c6ac03c1562a64438 --- .../providers/contacts/ContactsProvider2.java | 48 +++++++++++-------- .../contacts/ContactsProvider2Test.java | 22 +++++++++ 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/src/com/android/providers/contacts/ContactsProvider2.java b/src/com/android/providers/contacts/ContactsProvider2.java index e62aa87..f5ec5b2 100644 --- a/src/com/android/providers/contacts/ContactsProvider2.java +++ b/src/com/android/providers/contacts/ContactsProvider2.java @@ -3625,10 +3625,11 @@ private int updateData(Uri uri, ContentValues values, String selection, // so we don't need to worry about updating data we don't have permission to read. // This query will be allowed to return profiles, and we'll do the permission check // within the loop. - Cursor c = query(uri.buildUpon() + Cursor c = queryLocal(uri.buildUpon() .appendQueryParameter(ContactsContract.ALLOW_PROFILE, "1").build(), DataRowHandler.DataUpdateQuery.COLUMNS, - selection, selectionArgs, null); + selection, selectionArgs, null, -1 /* directory ID */, + true /* suppress profile check */); try { while(c.moveToNext()) { // Check profile permission for the raw contact that owns each data record. @@ -3962,15 +3963,15 @@ public Cursor query(Uri uri, String[] projection, String selection, String[] sel String directory = getQueryParameter(uri, ContactsContract.DIRECTORY_PARAM_KEY); if (directory == null) { return wrapCursor(uri, - queryLocal(uri, projection, selection, selectionArgs, sortOrder, -1)); + queryLocal(uri, projection, selection, selectionArgs, sortOrder, -1, false)); } else if (directory.equals("0")) { return wrapCursor(uri, queryLocal(uri, projection, selection, selectionArgs, sortOrder, - Directory.DEFAULT)); + Directory.DEFAULT, false)); } else if (directory.equals("1")) { return wrapCursor(uri, queryLocal(uri, projection, selection, selectionArgs, sortOrder, - Directory.LOCAL_INVISIBLE)); + Directory.LOCAL_INVISIBLE, false)); } DirectoryInfo directoryInfo = getDirectoryAuthority(directory); @@ -4120,8 +4121,9 @@ public void resetDirectoryCache() { } } - public Cursor queryLocal(Uri uri, String[] projection, String selection, String[] selectionArgs, - String sortOrder, long directoryId) { + private Cursor queryLocal(Uri uri, String[] projection, String selection, + String[] selectionArgs, String sortOrder, long directoryId, + boolean suppressProfileCheck) { if (VERBOSE_LOGGING) { Log.v(TAG, "query: " + uri); } @@ -4141,8 +4143,9 @@ public Cursor queryLocal(Uri uri, String[] projection, String selection, String[ case CONTACTS: { setTablesAndProjectionMapForContacts(qb, uri, projection); boolean existingWhere = appendLocalDirectorySelectionIfNeeded(qb, directoryId); - appendProfileRestriction(qb, uri, Contacts.IS_USER_PROFILE, existingWhere); - sortOrder = prependProfileSortIfNeeded(uri, sortOrder); + appendProfileRestriction(qb, uri, Contacts.IS_USER_PROFILE, existingWhere, + suppressProfileCheck); + sortOrder = prependProfileSortIfNeeded(uri, sortOrder, suppressProfileCheck); break; } @@ -4288,8 +4291,9 @@ public Cursor queryLocal(Uri uri, String[] projection, String selection, String[ } setTablesAndProjectionMapForContactsWithSnippet( qb, uri, projection, filterParam, directoryId); - appendProfileRestriction(qb, uri, Contacts.IS_USER_PROFILE, false); - sortOrder = prependProfileSortIfNeeded(uri, sortOrder); + appendProfileRestriction(qb, uri, Contacts.IS_USER_PROFILE, false, + suppressProfileCheck); + sortOrder = prependProfileSortIfNeeded(uri, sortOrder, suppressProfileCheck); break; } @@ -4710,7 +4714,8 @@ public Cursor queryLocal(Uri uri, String[] projection, String selection, String[ case RAW_CONTACTS: { setTablesAndProjectionMapForRawContacts(qb, uri); - appendProfileRestriction(qb, uri, RawContacts.RAW_CONTACT_IS_USER_PROFILE, true); + appendProfileRestriction(qb, uri, RawContacts.RAW_CONTACT_IS_USER_PROFILE, true, + suppressProfileCheck); break; } @@ -4728,7 +4733,8 @@ public Cursor queryLocal(Uri uri, String[] projection, String selection, String[ setTablesAndProjectionMapForData(qb, uri, projection, false); selectionArgs = insertSelectionArg(selectionArgs, String.valueOf(rawContactId)); qb.appendWhere(" AND " + Data.RAW_CONTACT_ID + "=?"); - appendProfileRestriction(qb, uri, RawContacts.RAW_CONTACT_IS_USER_PROFILE, true); + appendProfileRestriction(qb, uri, RawContacts.RAW_CONTACT_IS_USER_PROFILE, true, + suppressProfileCheck); break; } @@ -4780,7 +4786,8 @@ public Cursor queryLocal(Uri uri, String[] projection, String selection, String[ case DATA: { setTablesAndProjectionMapForData(qb, uri, projection, false); - appendProfileRestriction(qb, uri, RawContacts.RAW_CONTACT_IS_USER_PROFILE, true); + appendProfileRestriction(qb, uri, RawContacts.RAW_CONTACT_IS_USER_PROFILE, true, + suppressProfileCheck); break; } @@ -5844,8 +5851,8 @@ private boolean appendLocalDirectorySelectionIfNeeded(SQLiteQueryBuilder qb, lon } private void appendProfileRestriction(SQLiteQueryBuilder qb, Uri uri, String profileColumn, - boolean andRequired) { - if (!shouldIncludeProfile(uri)) { + boolean andRequired, boolean suppressProfileCheck) { + if (!shouldIncludeProfile(uri, suppressProfileCheck)) { qb.appendWhere((andRequired ? " AND (" : "") + profileColumn + " IS NULL OR " + profileColumn + "=0" @@ -5853,8 +5860,9 @@ private void appendProfileRestriction(SQLiteQueryBuilder qb, Uri uri, String pro } } - private String prependProfileSortIfNeeded(Uri uri, String sortOrder) { - if (shouldIncludeProfile(uri)) { + private String prependProfileSortIfNeeded(Uri uri, String sortOrder, + boolean suppressProfileCheck) { + if (shouldIncludeProfile(uri, suppressProfileCheck)) { if (TextUtils.isEmpty(sortOrder)) { return Contacts.IS_USER_PROFILE + " DESC"; } else { @@ -5864,12 +5872,12 @@ private String prependProfileSortIfNeeded(Uri uri, String sortOrder) { return sortOrder; } - private boolean shouldIncludeProfile(Uri uri) { + private boolean shouldIncludeProfile(Uri uri, boolean suppressProfileCheck) { // The user's profile may be returned alongside other contacts if it was requested and // the calling application has permission to read profile data. boolean profileRequested = readBooleanQueryParameter(uri, ContactsContract.ALLOW_PROFILE, false); - if (profileRequested) { + if (profileRequested && !suppressProfileCheck) { enforceProfilePermission(false); } return profileRequested; diff --git a/tests/src/com/android/providers/contacts/ContactsProvider2Test.java b/tests/src/com/android/providers/contacts/ContactsProvider2Test.java index f46436d..cdbf761 100644 --- a/tests/src/com/android/providers/contacts/ContactsProvider2Test.java +++ b/tests/src/com/android/providers/contacts/ContactsProvider2Test.java @@ -1685,6 +1685,28 @@ public void testInsertProfileDataRequiresWritePermission() { } } + public void testUpdateDataDoesNotRequireProfilePermission() { + mActor.removePermissions("android.permission.READ_PROFILE"); + mActor.removePermissions("android.permission.WRITE_PROFILE"); + + // Create a non-profile contact. + long rawContactId = createRawContactWithName("Domo", "Arigato"); + long dataId = getStoredLongValue(Data.CONTENT_URI, + Data.RAW_CONTACT_ID + "=? AND " + Data.MIMETYPE + "=?", + new String[]{String.valueOf(rawContactId), StructuredName.CONTENT_ITEM_TYPE}, + Data._ID); + + // Updates its name using a selection. + ContentValues values = new ContentValues(); + values.put(StructuredName.GIVEN_NAME, "Bob"); + values.put(StructuredName.FAMILY_NAME, "Blob"); + mResolver.update(Data.CONTENT_URI, values, Data._ID + "=?", + new String[]{String.valueOf(dataId)}); + + // Check that the update went through. + assertStoredValues(ContentUris.withAppendedId(Data.CONTENT_URI, dataId), values); + } + public void testQueryContactIncludeProfile() { ContentValues profileValues = new ContentValues(); long profileRawContactId = createBasicProfileContact(profileValues);