fix Android 13 write permissions

[EYALIN]

in Android 13 we don't need to ask for permissions

Platforms affected

Android

Motivation and Context

Closes #606
Closes #577

Description

Testing

Checklist

• I've run the tests to see all new and existing tests pass
• I added automated test coverage as appropriate for this change
• Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
• If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
• I've updated the documentation if necessary

[EYALIN]

@breautek the errors here are not related to my commits (IOS for example? )
can someone merge it?

[MauriceFrank]

@EYALIN DO you have any experience with the corodva-plugin-saf-mediastore?

I need to write media files into the gallery(external storage) on Android 13 but so far I have no success with the saf-mediastore plugin. Any alternatives?

I would be glad for any help ,thank you!

[EYALIN]

@MauriceFrank i'm using the photo library plugin, you can take my fork:
https://github.com/EYALIN/cordova-plugin-photo-library

[alexisGandar]

@EYALIN the fix works great thank you, can someone merge it ?

[ath0mas]

@EYALIN can you please correct the indentation.

Also, remove the private field permissions which is never used (lines 102-116).

And why not completly removing this whole methods hasWritePermission and getWritePermission? since cordova-plugin-file v8 requires cordova-android 12+ with support for Android 24+, while WRITE_EXTERNAL_STORAGE is not supported since 19.

[breautek]

And why not completly removing this whole methods hasWritePermission and getWritePermission? since cordova-plugin-file v8 requires cordova-android 12+ with support for Android 24+, while WRITE_EXTERNAL_STORAGE is not supported since 19.

WRITE_EXTERNAL_STORAGE is still used between API 24-28. It's not used with Scoped Storage mechanism.

Between API-24-28, the permission is not required for your app's external data directory, but is required to write to external shared directories (e.g. the Downloads directly).

But there are other significant issues with using the external filesystem on API 29+ that makes using a filesystem oriented API not feasible and I feel like we should just simply drop support for it in favour of media store plugin that interfaces with the native MediaStore instead. If we do go that route, then we can drop the permissions and probably simplify the file plugin significantly.

[ath0mas]

To me, hasReadPermission and hasWritePermission should be left unchanged, because they already return the correct status for *_EXTERNAL_STORAGE permissions - if considering it strictly.

And, as I work on PR #597, I like the idea that this plugin should concentrate on file operations among the app dirs only, default authorized without any permission ; and that trying to read-write outside should be done using other plugins, to follow the concepts and isolation forced by Google for file-sharing between apps or publicly, relying on SAF, MediaStore, ContentProvider, etc.

---

For more tech review:

• getWritePermission( is supposed to continue through its callbackContext param, and permission process, and so doing nothing in the if you added seems wrong

[pixellet14]

WRITE_EXTERNAL_STORAGE has been depreciated for API levels 32+

[breautek]

I think this can be rewritten as to invert the condition so we don't have an empty code block.

Suggested change

	if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
	} else {
	PermissionHelper.requestPermission(this, requestCode, Manifest.permission.WRITE_EXTERNAL_STORAGE);
	}
	if (android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) {
	PermissionHelper.requestPermission(this, requestCode, Manifest.permission.WRITE_EXTERNAL_STORAGE);
	}

[erisu]

getWritePermission is already wrapped by a condition that requires needPermission to be true, before it is called.

needPermission should then hit this condition:

else if(permissionType == WRITE && hasWritePermission()) {
            return false;
        }

Which should return false once the hasWritePermission method is fixed.

I think the only changes needed in this PR should be:

    private boolean hasWritePermission() {
        return android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU
            ? true
            : PermissionHelper.hasPermission(this, Manifest.permission.WRITE_EXTERNAL_STORAGE);
    }

[erisu]

@EYALIN Can you review the above messages and make the updates?

I believe you do not need to modify the getWritePermission method, as I previously mentioned the reasons why.
Only hasWritePermission needs to be updated. The code could be simplified with what I provided in my previous message.

Also, can you rebase your branch against with Cordova's main branch? There are conflicts.

And include Norman's suggestion about including a comment explaining about the true return.

// Starting with API 33, requesting WRITE_EXTERNAL_STORAGE is an auto permission rejection

[breautek]

Suggested change

	return true;
	// Starting with API 33, requesting WRITE_EXTERNAL_STORAGE is an auto permission rejection
	// Apps now have read-write access to it's own (created) files, so we can assume `true`, however not all files are readable.
	// See https://github.com/apache/cordova-plugin-file#androids-external-storage-quirks for more details.
	return true;

[breautek]

Just to explain why we are returning true for future contributors/code maintainers