Fix document management permission enforcement.

Allow both explicit holders of the MANAGE_DOCUMENTS permission and
those holding Uri grants to perform management tasks.

Extend grants for newly created documents when caller doesn't have
permission.  Revoke grants when deleting documents.

Test now writes actual content into picked file.  Workaround updated
flags for Drive app.

Bug: 10623211
Change-Id: Ia8e90b33e0fac8294b2cacb96d083c43fdf75aab

diff --git a/core/java/android/provider/DocumentsProvider.java b/core/java/android/provider/DocumentsProvider.java
index 09f4866..07cb2a9 100644 (file)
--- a/core/java/android/provider/DocumentsProvider.java
+++ b/core/java/android/provider/DocumentsProvider.java
@@ -28,11 +28,13 @@ import android.content.ContentValues;
 import android.content.Context;
 import android.content.Intent;
 import android.content.UriMatcher;
+import android.content.pm.PackageManager;
 import android.content.pm.ProviderInfo;
 import android.content.res.AssetFileDescriptor;
 import android.database.Cursor;
 import android.graphics.Point;
 import android.net.Uri;
+import android.os.Binder;
 import android.os.Bundle;
 import android.os.CancellationSignal;
 import android.os.ParcelFileDescriptor;
@@ -40,6 +42,8 @@ import android.os.ParcelFileDescriptor.OnCloseListener;
 import android.provider.DocumentsContract.Document;
 import android.util.Log;
 
+import com.android.internal.util.ArrayUtils;
+
 import libcore.io.IoUtils;
 
 import java.io.FileNotFoundException;
@@ -328,16 +332,24 @@ public abstract class DocumentsProvider extends ContentProvider {
     @Override
     public final Bundle callFromPackage(
             String callingPackage, String method, String arg, Bundle extras) {
+        final Context context = getContext();
+
         if (!method.startsWith("android:")) {
             // Let non-platform methods pass through
             return super.callFromPackage(callingPackage, method, arg, extras);
         }
 
-        // Require that caller can manage given document
         final String documentId = extras.getString(Document.COLUMN_DOCUMENT_ID);
         final Uri documentUri = DocumentsContract.buildDocumentUri(mAuthority, documentId);
-        getContext().enforceCallingOrSelfUriPermission(
-                documentUri, Intent.FLAG_GRANT_WRITE_URI_PERMISSION, method);
+
+        // Require that caller can manage given document
+        final boolean callerHasManage =
+                context.checkCallingOrSelfPermission(android.Manifest.permission.MANAGE_DOCUMENTS)
+                == PackageManager.PERMISSION_GRANTED;
+        if (!callerHasManage) {
+            getContext().enforceCallingOrSelfUriPermission(
+                    documentUri, Intent.FLAG_GRANT_WRITE_URI_PERMISSION, method);
+        }
 
         final Bundle out = new Bundle();
         try {
@@ -345,14 +357,26 @@ public abstract class DocumentsProvider extends ContentProvider {
                 final String mimeType = extras.getString(Document.COLUMN_MIME_TYPE);
                 final String displayName = extras.getString(Document.COLUMN_DISPLAY_NAME);
 
-                // TODO: issue Uri grant towards calling package
-                // TODO: enforce that package belongs to caller
                 final String newDocumentId = createDocument(documentId, mimeType, displayName);
                 out.putString(Document.COLUMN_DOCUMENT_ID, newDocumentId);
 
+                // Extend permission grant towards caller if needed
+                if (!callerHasManage) {
+                    final Uri newDocumentUri = DocumentsContract.buildDocumentUri(
+                            mAuthority, newDocumentId);
+                    context.grantUriPermission(callingPackage, newDocumentUri,
+                            Intent.FLAG_GRANT_READ_URI_PERMISSION
+                            | Intent.FLAG_GRANT_WRITE_URI_PERMISSION
+                            | Intent.FLAG_PERSIST_GRANT_URI_PERMISSION);
+                }
+
             } else if (METHOD_DELETE_DOCUMENT.equals(method)) {
-                final String docId = extras.getString(Document.COLUMN_DOCUMENT_ID);
-                deleteDocument(docId);
+                deleteDocument(documentId);
+
+                // Document no longer exists, clean up any grants
+                context.revokeUriPermission(documentUri, Intent.FLAG_GRANT_READ_URI_PERMISSION
+                        | Intent.FLAG_GRANT_WRITE_URI_PERMISSION
+                        | Intent.FLAG_PERSIST_GRANT_URI_PERMISSION);
 
             } else {
                 throw new UnsupportedOperationException("Method not supported " + method);

diff --git a/packages/DocumentsUI/src/com/android/documentsui/TestActivity.java b/packages/DocumentsUI/src/com/android/documentsui/TestActivity.java
index 2405cb5..7b7c3d5 100644 (file)
--- a/packages/DocumentsUI/src/com/android/documentsui/TestActivity.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/TestActivity.java
@@ -33,10 +33,14 @@ import libcore.io.IoUtils;
 import libcore.io.Streams;
 
 import java.io.InputStream;
+import java.io.OutputStream;
 
 public class TestActivity extends Activity {
     private static final String TAG = "TestActivity";
 
+    private static final int CODE_READ = 42;
+    private static final int CODE_WRITE = 43;
+
     private TextView mResult;
 
     @Override
@@ -49,10 +53,10 @@ public class TestActivity extends Activity {
         view.setOrientation(LinearLayout.VERTICAL);
 
         final CheckBox multiple = new CheckBox(context);
-        multiple.setText("\nALLOW_MULTIPLE\n");
+        multiple.setText("ALLOW_MULTIPLE");
         view.addView(multiple);
         final CheckBox localOnly = new CheckBox(context);
-        localOnly.setText("\nLOCAL_ONLY\n");
+        localOnly.setText("LOCAL_ONLY");
         view.addView(localOnly);
 
         Button button;
@@ -70,7 +74,7 @@ public class TestActivity extends Activity {
                 if (localOnly.isChecked()) {
                     intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);
                 }
-                startActivityForResult(intent, 42);
+                startActivityForResult(intent, CODE_READ);
             }
         });
         view.addView(button);
@@ -89,7 +93,7 @@ public class TestActivity extends Activity {
                 if (localOnly.isChecked()) {
                     intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);
                 }
-                startActivityForResult(intent, 42);
+                startActivityForResult(intent, CODE_READ);
             }
         });
         view.addView(button);
@@ -108,7 +112,7 @@ public class TestActivity extends Activity {
                 if (localOnly.isChecked()) {
                     intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);
                 }
-                startActivityForResult(intent, 42);
+                startActivityForResult(intent, CODE_READ);
             }
         });
         view.addView(button);
@@ -129,7 +133,7 @@ public class TestActivity extends Activity {
                 if (localOnly.isChecked()) {
                     intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);
                 }
-                startActivityForResult(intent, 42);
+                startActivityForResult(intent, CODE_READ);
             }
         });
         view.addView(button);
@@ -146,7 +150,7 @@ public class TestActivity extends Activity {
                 if (localOnly.isChecked()) {
                     intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);
                 }
-                startActivityForResult(intent, 42);
+                startActivityForResult(intent, CODE_WRITE);
             }
         });
         view.addView(button);
@@ -165,7 +169,7 @@ public class TestActivity extends Activity {
                 if (localOnly.isChecked()) {
                     intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);
                 }
-                startActivityForResult(Intent.createChooser(intent, "Kittens!"), 42);
+                startActivityForResult(Intent.createChooser(intent, "Kittens!"), CODE_READ);
             }
         });
         view.addView(button);
@@ -178,20 +182,45 @@ public class TestActivity extends Activity {
 
     @Override
     protected void onActivityResult(int requestCode, int resultCode, Intent data) {
-        mResult.setText("resultCode=" + resultCode + ", data=" + String.valueOf(data));
-
-        final Uri uri = data != null ? data.getData() : null;
-        if (uri != null) {
-            InputStream is = null;
-            try {
-                is = getContentResolver().openInputStream(uri);
-                final int length = Streams.readFullyNoClose(is).length;
-                Log.d(TAG, "read length=" + length);
-            } catch (Exception e) {
-                Log.w(TAG, "Failed to read " + uri, e);
-            } finally {
-                IoUtils.closeQuietly(is);
+        mResult.setText(null);
+        String result = "resultCode=" + resultCode + ", data=" + String.valueOf(data);
+
+        if (requestCode == CODE_READ) {
+            final Uri uri = data != null ? data.getData() : null;
+            if (uri != null) {
+                InputStream is = null;
+                try {
+                    is = getContentResolver().openInputStream(uri);
+                    final int length = Streams.readFullyNoClose(is).length;
+                    result += "; read length=" + length;
+                } catch (Exception e) {
+                    result += "; ERROR";
+                    Log.w(TAG, "Failed to read " + uri, e);
+                } finally {
+                    IoUtils.closeQuietly(is);
+                }
+            } else {
+                result += "no uri?";
+            }
+        } else if (requestCode == CODE_WRITE) {
+            final Uri uri = data != null ? data.getData() : null;
+            if (uri != null) {
+                OutputStream os = null;
+                try {
+                    os = getContentResolver().openOutputStream(uri);
+                    os.write("THE COMPLETE WORKS OF SHAKESPEARE".getBytes());
+                } catch (Exception e) {
+                    result += "; ERROR";
+                    Log.w(TAG, "Failed to write " + uri, e);
+                } finally {
+                    IoUtils.closeQuietly(os);
+                }
+            } else {
+                result += "no uri?";
             }
         }
+
+        Log.d(TAG, result);
+        mResult.setText(result);
     }
 }

diff --git a/packages/DocumentsUI/src/com/android/documentsui/model/RootInfo.java b/packages/DocumentsUI/src/com/android/documentsui/model/RootInfo.java
index 189284b..a63f269 100644 (file)
--- a/packages/DocumentsUI/src/com/android/documentsui/model/RootInfo.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/model/RootInfo.java
@@ -52,6 +52,13 @@ public class RootInfo {
         root.summary = getCursorString(cursor, Root.COLUMN_SUMMARY);
         root.documentId = getCursorString(cursor, Root.COLUMN_DOCUMENT_ID);
         root.availableBytes = getCursorLong(cursor, Root.COLUMN_AVAILABLE_BYTES);
+
+        // TODO: remove this hack
+        if ("com.google.android.apps.docs.storage".equals(root.authority)) {
+            root.flags &= ~(Root.FLAG_PROVIDES_AUDIO | Root.FLAG_PROVIDES_IMAGES
+                    | Root.FLAG_PROVIDES_VIDEO);
+        }
+
         return root;
     }