feat(security): implement screenshot protection across the application #3130
feat(security): implement screenshot protection across the application #3130AnuragMishra-akm wants to merge 4 commits intoopenMF:developmentfrom
Conversation
- added FLAG_SECURE to MainActivity for global protection on Android - created SecureScreen KMP utility in core:ui module - applied screenshot prevention to sensitive screens including login, passcode, account details, and notifications
Disables the oss-licenses-plugin task and sets a default artifact attribute to resolve the "Multiple artifacts exist" build failure caused by Gradle 8.11 and the multiplatform shared module.
feat(security): implement screenshot protection across the application
📝 WalkthroughWalkthroughAdds a multiplatform SecureScreen composable (expect/actual) with an Android implementation that manages FLAG_SECURE, integrates SecureScreen into LoginScreen and MainActivity, and adds a Gradle subprojects block to adjust/disable OssLicenses tasks and artifact resolution. Changes
Sequence Diagram(s)sequenceDiagram
participant Compose as Composable (SecureScreen)
participant Context as LocalContext / Activity
participant Window as Window
participant Counter as secureScreenCount
Compose->>Context: Resolve Activity from context wrappers
Context-->>Compose: Activity with window
Compose->>Counter: increment()
Note right of Counter: count++
Compose->>Window: set FLAG_SECURE
Window-->>Compose: FLAG_SECURE applied
Note over Compose,Counter: Composable active
Compose->>Counter: onDispose -> decrement()
Note right of Counter: count--
alt count <= 0
Compose->>Window: clear FLAG_SECURE
Window-->>Compose: FLAG_SECURE cleared
Counter->>Counter: reset to 0
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/utils/SecureScreen.kt (1)
14-19: Make the common contract explicitly best-effort.The KDoc reads as if
SecureScreen()always blocks capture, but the desktop/native actuals are no-ops. That makes it easy for shared UI to assume protection exists on unsupported targets. Please document the unsupported platforms here, or expose capability so callers can branch explicitly.📝 Suggested documentation update
/** - * A composable that enables screenshot protection for the current screen. - * On Android, this adds WindowManager.LayoutParams.FLAG_SECURE to the window. + * Requests screenshot protection for the current screen on platforms that support it. + * + * Android: adds WindowManager.LayoutParams.FLAG_SECURE to the hosting window. + * Desktop/native: currently no-op. */ `@Composable` expect fun SecureScreen()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/utils/SecureScreen.kt` around lines 14 - 19, The KDoc and expect declaration for SecureScreen currently imply it always blocks screen capture, but actual implementations on desktop/native are no-ops; update the common contract to state this is a best-effort protection and explicitly list unsupported targets (e.g., desktop / native) in the KDoc for SecureScreen, or add a capability check API (e.g., SecureScreen.isSupported(): Boolean) alongside the expect fun SecureScreen() so callers can branch when protection is unavailable; reference the SecureScreen expect function and the new isSupported capability in your change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build.gradle.kts`:
- Around line 74-77: The current tasks.configureEach block indiscriminately
disables any task whose name contains "OssLicensesTask", which breaks OSS
license metadata for all modules; instead, restrict the disablement to the
specific failing module/task by checking the module (project) identity before
toggling the task. Modify the condition around the
name.contains("OssLicensesTask") check to also verify the project (e.g.,
project.name or project.path equals the specific module name) or target the
specific task by its exact name (using tasks.named("ExactOssLicensesTaskName")
or checking task.project.name) so only that one module/task is disabled while
leaving OssLicensesTask enabled elsewhere.
In `@cmp-shared/cmp_shared.podspec`:
- Line 53: Replace the hard-coded backslash path in spec.resources with a
platform-safe join: update the assignment to use Ruby's File.join (referencing
spec.resources) so the resource glob is built with File.join('build', 'compose',
'cocoapods', 'compose-resources') instead of the backslash string; this ensures
Dir.glob sees correct separators and the Compose assets are matched on all
platforms.
In
`@core/ui/src/androidMain/kotlin/org/mifos/mobile/core/ui/utils/SecureScreen.android.kt`:
- Around line 25-46: SecureScreen currently always increments secureScreenCount
and clears FLAG_SECURE on last dispose, which can remove the app-level
FLAG_SECURE set in MainActivity.onCreate(); change SecureScreen() so it only
marks/increments/clears when this instance actually enabled the flag: inside
SecureScreen() check window.attributes.flags &
WindowManager.LayoutParams.FLAG_SECURE == 0 before calling window.addFlags(...)
and set a local didSetFlag boolean; increment secureScreenCount only when
didSetFlag is true; onDispose only decrement secureScreenCount and call
window.clearFlags(...) if didSetFlag was true and secureScreenCount reaches 0,
leaving the app-level flag set by MainActivity.onCreate() untouched.
---
Nitpick comments:
In
`@core/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/utils/SecureScreen.kt`:
- Around line 14-19: The KDoc and expect declaration for SecureScreen currently
imply it always blocks screen capture, but actual implementations on
desktop/native are no-ops; update the common contract to state this is a
best-effort protection and explicitly list unsupported targets (e.g., desktop /
native) in the KDoc for SecureScreen, or add a capability check API (e.g.,
SecureScreen.isSupported(): Boolean) alongside the expect fun SecureScreen() so
callers can branch when protection is unavailable; reference the SecureScreen
expect function and the new isSupported capability in your change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f905f667-0919-4d67-8fcc-246821dcc126
📒 Files selected for processing (8)
build.gradle.ktscmp-android/src/main/kotlin/cmp/android/app/MainActivity.ktcmp-shared/cmp_shared.podspeccore/ui/src/androidMain/kotlin/org/mifos/mobile/core/ui/utils/SecureScreen.android.ktcore/ui/src/commonMain/kotlin/org/mifos/mobile/core/ui/utils/SecureScreen.ktcore/ui/src/desktopMain/kotlin/org/mifos/mobile/core/ui/utils/SecureScreen.desktop.ktcore/ui/src/nativeMain/kotlin/org/mifos/mobile/core/ui/utils/SecureScreen.native.ktfeature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginScreen.kt
| tasks.configureEach { | ||
| if (name.contains("OssLicensesTask", ignoreCase = true)) { | ||
| enabled = false | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Gradle files referencing the OSS licenses plugin/tasks:"
rg -n --glob '*.gradle*' -g '!**/build/**' 'oss.?licenses|OssLicenses'
echo
echo "Code or manifests referencing OSS notices surfaces:"
rg -n -g '!**/build/**' 'OssLicensesMenuActivity|open_source_licenses|oss_licenses'Repository: openMF/mifos-mobile
Length of output: 1015
Avoid disabling the OSS licenses plugin globally.
The codebase actively surfaces OSS notices via OssLicensesMenuActivity (used in core/ui and declared in the manifest), which depends on metadata generated by OssLicensesTask. Globally disabling this task prevents license data from being generated, causing users to see non-functional or missing OSS notices—a compliance regression. Scope the workaround to the specific failing module/task instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@build.gradle.kts` around lines 74 - 77, The current tasks.configureEach block
indiscriminately disables any task whose name contains "OssLicensesTask", which
breaks OSS license metadata for all modules; instead, restrict the disablement
to the specific failing module/task by checking the module (project) identity
before toggling the task. Modify the condition around the
name.contains("OssLicensesTask") check to also verify the project (e.g.,
project.name or project.path equals the specific module name) or target the
specific task by its exact name (using tasks.named("ExactOssLicensesTaskName")
or checking task.project.name) so only that one module/task is disabled while
leaving OssLicensesTask enabled elsewhere.
| } | ||
| ] | ||
| spec.resources = ['build/compose/cocoapods/compose-resources'] | ||
| spec.resources = ['build\compose\cocoapods\compose-resources'] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In a CocoaPods podspec on macOS, should spec.resourcespaths use forward slashes /File.join, and do Windows-style backslashes break resource glob matching?
💡 Result:
In CocoaPods podspecs on macOS, spec.resources paths should use forward slashes (/). These paths are Ruby file glob patterns processed by Ruby's Dir.glob, which expects forward slashes as path separators universally, including on Windows where backslashes act as escape characters in glob patterns and break matching. Use forward slashes directly (e.g., spec.resources = 'Resources//.png') or File.join for dynamic paths (e.g., spec.resources = File.join('Resources', '', '.png')), as File.join produces forward slashes on all platforms. Windows-style backslashes (e.g., Resources\\*.png) will break resource glob matching because Dir.glob interprets \ as escaping the next metacharacter, preventing proper pattern matching against actual filesystem paths (which Ruby normalizes to forward slashes internally). Official CocoaPods documentation and all examples use forward slashes exclusively. Ruby's cross-platform path handling ensures forward slashes work everywhere for globs.
Citations:
- 1: https://guides.cocoapods.org/syntax/podspec.html
- 2: https://stackoverflow.com/questions/18601721/include-files-recursively-in-cocoapods-podspec
- 3: https://stackoverflow.com/questions/18601721/include-files-recursively-in-cocoapods-podspec/59257783
- 4: https://stackoverflow.com/questions/28005339/ruby-dir-glob-issue-only-on-windows
- 5: https://stackoverflow.com/questions/20667542/ruby-dir-glob-not-returning-any-files
- 6: https://stackoverflow.com/questions/29357327/why-does-dirglob-not-populate-my-variable-on-windows
Use File.join for spec.resources to ensure proper glob matching.
The backslash path separators in spec.resources = ['build\compose\cocoapods\compose-resources'] will break glob matching. Ruby's Dir.glob interprets backslashes as escape characters in glob patterns, preventing the resource glob from matching the intended directory on all platforms. This will cause the Compose assets to be omitted from the iOS pod.
🔧 Proposed fix
- spec.resources = ['build\compose\cocoapods\compose-resources']
+ spec.resources = [File.join('build', 'compose', 'cocoapods', 'compose-resources')]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmp-shared/cmp_shared.podspec` at line 53, Replace the hard-coded backslash
path in spec.resources with a platform-safe join: update the assignment to use
Ruby's File.join (referencing spec.resources) so the resource glob is built with
File.join('build', 'compose', 'cocoapods', 'compose-resources') instead of the
backslash string; this ensures Dir.glob sees correct separators and the Compose
assets are matched on all platforms.
| private var secureScreenCount = 0 | ||
|
|
||
| @Composable | ||
| actual fun SecureScreen() { | ||
| val context = LocalContext.current | ||
| DisposableEffect(Unit) { | ||
| val activity = context.findActivity() | ||
| val window = activity?.window | ||
|
|
||
| if (window != null) { | ||
| secureScreenCount++ | ||
| window.addFlags(WindowManager.LayoutParams.FLAG_SECURE) | ||
| } | ||
|
|
||
| onDispose { | ||
| val activityOnDispose = context.findActivity() | ||
| val windowOnDispose = activityOnDispose?.window | ||
| if (windowOnDispose != null) { | ||
| secureScreenCount-- | ||
| if (secureScreenCount <= 0) { | ||
| windowOnDispose.clearFlags(WindowManager.LayoutParams.FLAG_SECURE) | ||
| secureScreenCount = 0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "MainActivity FLAG_SECURE setup:"
fd -g 'MainActivity.kt' -x sed -n '48,53p' {}
echo
echo "SecureScreen Android add/clear logic:"
fd -g 'SecureScreen.android.kt' -x sed -n '25,49p' {}Repository: openMF/mifos-mobile
Length of output: 1183
🏁 Script executed:
cat -n core/ui/src/androidMain/kotlin/org/mifos/mobile/core/ui/utils/SecureScreen.android.ktRepository: openMF/mifos-mobile
Length of output: 2341
🏁 Script executed:
rg "FLAG_SECURE" --type kotlin -B 2 -A 2Repository: openMF/mifos-mobile
Length of output: 2852
Don't clear the app-level FLAG_SECURE set by MainActivity.
MainActivity.onCreate() (line 52) sets FLAG_SECURE unconditionally for the entire app. This effect clears that flag as soon as the last SecureScreen() leaves composition, re-enabling screenshots throughout the app when navigating away from login/passcode/account screens. Only clear the flag if SecureScreen itself enabled it.
🔒 Proposed fix
private var secureScreenCount = 0
+private var initialSecureFlagState = false
`@Composable`
actual fun SecureScreen() {
val context = LocalContext.current
DisposableEffect(Unit) {
- val activity = context.findActivity()
- val window = activity?.window
+ val window = context.findActivity()?.window
if (window != null) {
- secureScreenCount++
- window.addFlags(WindowManager.LayoutParams.FLAG_SECURE)
+ if (secureScreenCount == 0) {
+ initialSecureFlagState =
+ (window.attributes.flags and WindowManager.LayoutParams.FLAG_SECURE) != 0
+ if (!initialSecureFlagState) {
+ window.addFlags(WindowManager.LayoutParams.FLAG_SECURE)
+ }
+ }
+ secureScreenCount++
}
onDispose {
- val activityOnDispose = context.findActivity()
- val windowOnDispose = activityOnDispose?.window
- if (windowOnDispose != null) {
- secureScreenCount--
- if (secureScreenCount <= 0) {
- windowOnDispose.clearFlags(WindowManager.LayoutParams.FLAG_SECURE)
- secureScreenCount = 0
+ if (window != null) {
+ secureScreenCount = (secureScreenCount - 1).coerceAtLeast(0)
+ if (secureScreenCount == 0) {
+ if (!initialSecureFlagState) {
+ window.clearFlags(WindowManager.LayoutParams.FLAG_SECURE)
+ }
+ initialSecureFlagState = false
}
}
}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/ui/src/androidMain/kotlin/org/mifos/mobile/core/ui/utils/SecureScreen.android.kt`
around lines 25 - 46, SecureScreen currently always increments secureScreenCount
and clears FLAG_SECURE on last dispose, which can remove the app-level
FLAG_SECURE set in MainActivity.onCreate(); change SecureScreen() so it only
marks/increments/clears when this instance actually enabled the flag: inside
SecureScreen() check window.attributes.flags &
WindowManager.LayoutParams.FLAG_SECURE == 0 before calling window.addFlags(...)
and set a local didSetFlag boolean; increment secureScreenCount only when
didSetFlag is true; onDispose only decrement secureScreenCount and call
window.clearFlags(...) if didSetFlag was true and secureScreenCount reaches 0,
leaving the app-level flag set by MainActivity.onCreate() untouched.
|
@AnuragMishra-akm Welcome to the community. Please give it a read before starting to contribute https://mifosforge.jira.com/wiki/spaces/MP/pages/4537024513/Welcome+to+the+Mifos+Mobile+Apps+Community. This tickets are yet to be properly groomed. You are free to take any OnDeck tickets across all projects. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cmp-shared/cmp_shared.podspec (1)
53-53:⚠️ Potential issue | 🟠 Major
spec.resourcespath separator is still unsafe for CocoaPods globbing.Line 53 uses backslashes; this has already been flagged and remains unresolved. Use
File.join(or forward slashes) so resources are reliably discovered.🔧 Proposed fix
- spec.resources = ['build\compose\cocoapods\compose-resources'] + spec.resources = [File.join('build', 'compose', 'cocoapods', 'compose-resources')]#!/bin/bash # Detect backslash-based podspec resource paths rg -n "spec\.resources\s*=\s*\['.*\\\\.*'\]" cmp-shared/cmp_shared.podspec🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmp-shared/cmp_shared.podspec` at line 53, spec.resources currently uses backslashes which break CocoaPods globbing; update the assignment to use forward slashes or File.join so the path is portable. Locate the spec.resources = ['build\compose\cocoapods\compose-resources'] line (symbol: spec.resources) and replace the backslash-separated literal with either a forward-slash string 'build/compose/cocoapods/compose-resources' or construct the path with File.join('build', 'compose', 'cocoapods', 'compose-resources') to ensure reliable resource discovery.
🧹 Nitpick comments (1)
cmp-shared/cmp_shared.podspec (1)
34-52: Add trailing comma to satisfy RuboCop array-literal convention.
spec.script_phasesis a multiline array and requires a trailing comma after the last element per RuboCop'sStyle/TrailingCommaInArrayLiteralrule.🔧 Proposed fix
- } + }, ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmp-shared/cmp_shared.podspec` around lines 34 - 52, The array assigned to spec.script_phases lacks the required trailing comma; update the array literal for spec.script_phases (the hash with :name => 'Build cmp_shared', :execution_position, :shell_path, :script) by adding a trailing comma after the final element so it conforms to RuboCop's Style/TrailingCommaInArrayLiteral rule; locate the spec.script_phases assignment and append the trailing comma immediately after the closing brace of the script hash.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmp-shared/cmp_shared.podspec`:
- Line 3: The podspec's declared version (spec.version in cmp_shared.podspec) is
0.0.1 but the committed lockfile (cmp-ios/Podfile.lock) pins cmp_shared as 1.0;
either update spec.version back to "1.0" in cmp_shared.podspec or, if 0.0.1 is
intentional, run pod install inside cmp-ios to regenerate cmp-ios/Podfile.lock
and commit that updated lockfile in the same PR so the podspec and Podfile.lock
stay consistent (ensure you verify the pod declaration pod 'cmp_shared', :path
=> '../cmp-shared' reflects the intended behavior).
---
Duplicate comments:
In `@cmp-shared/cmp_shared.podspec`:
- Line 53: spec.resources currently uses backslashes which break CocoaPods
globbing; update the assignment to use forward slashes or File.join so the path
is portable. Locate the spec.resources =
['build\compose\cocoapods\compose-resources'] line (symbol: spec.resources) and
replace the backslash-separated literal with either a forward-slash string
'build/compose/cocoapods/compose-resources' or construct the path with
File.join('build', 'compose', 'cocoapods', 'compose-resources') to ensure
reliable resource discovery.
---
Nitpick comments:
In `@cmp-shared/cmp_shared.podspec`:
- Around line 34-52: The array assigned to spec.script_phases lacks the required
trailing comma; update the array literal for spec.script_phases (the hash with
:name => 'Build cmp_shared', :execution_position, :shell_path, :script) by
adding a trailing comma after the final element so it conforms to RuboCop's
Style/TrailingCommaInArrayLiteral rule; locate the spec.script_phases assignment
and append the trailing comma immediately after the closing brace of the script
hash.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c6f3cb7d-6b7b-49a0-82a0-78997002db43
📒 Files selected for processing (1)
cmp-shared/cmp_shared.podspec
| Pod::Spec.new do |spec| | ||
| spec.name = 'cmp_shared' | ||
| spec.version = '1.0' | ||
| spec.version = '0.0.1' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify podspec version vs lockfile pinned version
rg -n "spec\.version" cmp-shared/cmp_shared.podspec
rg -n "cmp_shared \(" cmp-ios/Podfile.lockRepository: openMF/mifos-mobile
Length of output: 171
🏁 Script executed:
#!/bin/bash
# Get full context of cmp_shared in Podfile.lock
head -30 cmp-ios/Podfile.lock | cat -n
# Check if there's a Podfile in the repo
find . -name "Podfile" -type f | head -5Repository: openMF/mifos-mobile
Length of output: 482
🏁 Script executed:
#!/bin/bash
# Get the Podfile to see how cmp_shared is declared
if [ -f "cmp-ios/Podfile" ]; then
cat -n cmp-ios/Podfile | grep -A5 -B5 "cmp_shared" || echo "cmp_shared not found in Podfile"
fi
# Also check the full podspec to see version context
cat -n cmp-shared/cmp_shared.podspec | head -15Repository: openMF/mifos-mobile
Length of output: 1212
Version in podspec must match committed Podfile.lock.
Line 3 sets spec.version to 0.0.1, but cmp-ios/Podfile.lock currently pins cmp_shared (1.0). Since the Podfile declares pod 'cmp_shared', :path => '../cmp-shared' without a version constraint, CocoaPods reads the version from the podspec. The next pod install will regenerate the lockfile with version 0.0.1, causing lockfile churn and CI inconsistency.
Either update the version back to 1.0, or regenerate and commit cmp-ios/Podfile.lock with the new version in the same PR.
🔧 Proposed fix
- spec.version = '0.0.1'
+ spec.version = '1.0'Or, if 0.0.1 is intentional, run pod install in cmp-ios/ and commit the updated lockfile.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| spec.version = '0.0.1' | |
| spec.version = '1.0' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmp-shared/cmp_shared.podspec` at line 3, The podspec's declared version
(spec.version in cmp_shared.podspec) is 0.0.1 but the committed lockfile
(cmp-ios/Podfile.lock) pins cmp_shared as 1.0; either update spec.version back
to "1.0" in cmp_shared.podspec or, if 0.0.1 is intentional, run pod install
inside cmp-ios to regenerate cmp-ios/Podfile.lock and commit that updated
lockfile in the same PR so the podspec and Podfile.lock stay consistent (ensure
you verify the pod declaration pod 'cmp_shared', :path => '../cmp-shared'
reflects the intended behavior).
|
@AnuragMishra-akm Hello, thank you for your pull request. Currently, our developers and contributors are required to upload screenshots and videos showcasing their changes. Enabling this feature will prevent them from capturing screenshots. There is a field within the class that allows us to check if the user has enabled this setting. We need to extend this functionality across all platforms, not just Android. Therefore, please make this pull request a draft and enhance it accordingly. Inform us once all changes are complete and it is ready for review. |
|
and the purpose of the ticket is to stop screenshots on specific screens, not the entire application. |
3 participants
Fixes - (https://mifosforge.jira.com/browse/MM-573)(https://mifosforge.jira.com/browse/MM-569) [LOW] SEC-023: Add Screenshot Protection Description: Added the Screenshot prevention feature to the whole app. Now no one can take a screenshot. It runs on the device successfully.
./gradlew checkorci-prepush.shto make sure you didn't break anythingSummary by CodeRabbit
New Features
Chores