Bypassed Storage Permission Check for Shared Images

[doug-doug-doug]

The share intent provides files via content uris with temporary read permissions, so we do not need to obtain storage permissions

Description (required)

Fixes #6437

What changes did you make and why?

Modified UploadActivity.checkStoragePermissions to bypass checking for storage permission when receiving images from a share intent (and thus requesting storage permissions) as the share intent provides files via content uris with temporary read permissions, so we do not need to obtain storage permissions.

Tests performed (required)

Tested betaDebug on Medium Phone with API level 35.
Tested prodDebug on Medium Phone with API level 35.

---

Note: Please ensure that you have read CONTRIBUTING.md if this is your first pull request.

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Simplified the file sharing workflow—shared items are now processed immediately without requiring upfront storage permission requests.
  • Improved content sharing efficiency using temporary read access for shared files instead of persistent storage permissions.

[coderabbitai]

Walkthrough

The upload activity's permission check flow is modified to bypass storage permission requirements for share intents. When Intent.ACTION_SEND or ACTION_SEND_MULTIPLE is detected, the code now directly calls receiveSharedItems() using content URIs with temporary read permissions, skipping the standard permission gate that previously applied to all intents.

Changes

Cohort / File(s)	Summary
Share Intent Permission Bypass app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.kt	Added early-return branch in permission check path to detect share intents and process shared items directly without requiring storage permission checks; mirrors existing onCreate logic for consistency

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UploadActivity
    participant PermissionCheck
    participant SharedItemHandler

    User->>UploadActivity: Send/Share intent
    UploadActivity->>PermissionCheck: checkStoragePermissions()
    
    alt Share Intent Detected (NEW)
        PermissionCheck->>SharedItemHandler: receiveSharedItems()
        SharedItemHandler->>SharedItemHandler: Process via content URIs
        Note over SharedItemHandler: No storage permissions requested
    else Standard Intent (PREVIOUS)
        PermissionCheck->>PermissionCheck: hasAllPermissions or hasPartialAccess?
        PermissionCheck->>SharedItemHandler: receiveSharedItems()
    end
    
    SharedItemHandler->>User: Display selected items

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review the permission-check logic to ensure shared items are correctly identified and don't trigger unintended side effects
• Verify that the early-return path doesn't bypass critical initialization or validation steps needed for other intent types
• Confirm the consistency between the onCreate and checkStoragePermissions branches for share intent handling
• Test scenarios with partial permissions and shared content URIs to ensure temporary access is properly utilized

Poem

🐰 A share intent hops right through the gate,
No permission walls to hesitate!
Content URIs whisper soft and free,
Shared images dance with glee.
The burden lifted, the flow pristine—
Fast and fair as can be seen! 📸✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "Bypassed Storage Permission Check for Shared Images" directly describes the main change in the codebase. The summary confirms that the code now short-circuits permission checks for share intents (ACTION_SEND and ACTION_SEND_MULTIPLE) and immediately processes shared items via content URIs without requiring storage permissions. The title is concise, specific, and clearly communicates the primary change from the developer's perspective.
Linked Issues Check	✅ Passed	The linked issue #6437 describes a problem where users who share multiple images and select "Allow limited access" see 0 chosen images and are prompted to re-select them. The PR directly addresses this by bypassing storage permission checks for share intents, allowing shared items to be processed immediately via content URIs with temporary read permissions, which prevents the limited photo picker from appearing and preserves the originally shared selection. This implementation aligns perfectly with the expected behavior stated in the issue.
Out of Scope Changes Check	✅ Passed	The changes are confined to UploadActivity.kt and specifically target the checkStoragePermissions method to bypass permission checks for share intents. The modification mirrors existing early-return logic from the onCreate path and directly addresses the PR objective of bypassing storage permission checks for shared images. No unrelated changes or alterations to exported entities are present, and all modifications are focused on resolving the linked issue without introducing scope creep.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.kt (1)

317-325: Share intent permission bypass looks good; consider visibility state and refactoring.

The approach is sound—share intents provide temporary read permissions via content URIs, so bypassing storage permission checks is correct and should fix issue #6437.

Minor considerations:

1. Top card visibility: In the permission-granted path (line 330), cvContainerTopCard.visibility is set to VISIBLE before calling receiveSharedItems(). The new share-intent path skips this. If the default XML visibility is not VISIBLE, multi-file uploads might not display the top card correctly. Consider adding:

   binding.cvContainerTopCard.visibility = View.VISIBLE

   before calling receiveSharedItems() at line 323.

2. Code duplication: The intent action check at lines 322 duplicates the check at line 485 in receiveSharedItems(). Consider extracting a helper:

   private fun isShareIntent() = intent.action in listOf(Intent.ACTION_SEND, Intent.ACTION_SEND_MULTIPLE)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 63f621c and d51ddb9.

📒 Files selected for processing (1)

• app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.kt (1 hunks)

[RitikaPahwa4444]

Works great! :) Just a minor suggestion - I'll merge once addressed.

[github-actions]

✅ Generated APK variants!

• 🤖 Download betaDebugAPK
• 🤖 Download prodDebugAPK