[Feature request] Force security if not protected #4087

manuelplazaspalacio · 2023-06-28T06:40:35Z

Related Issues

App: #4061

Library PR (if needed):

Added changelog files for the fixed issues in folder changelog/unreleased. More info here

QA

#4087 (comment)

(1) Basic behaviour [Feature request] Force security if not protected #4087 (comment) [FIXED]
(2) lock_enforce not followed [Feature request] Force security if not protected #4087 (comment) [FIXED]

JuancaG05

Some changes suggested here @manuelplazaspalacio!

.idea/codeStyles/Project.xml

changelog/unreleased/4087

owncloudApp/src/main/java/com/owncloud/android/extensions/ActivityExt.kt

owncloudApp/src/main/java/com/owncloud/android/utils/MdmConfigurations.kt

.../java/com/owncloud/android/presentation/viewmodels/settings/SettingsSecurityViewModelTest.kt

JuancaG05

Last changes here 😁 good job @manuelplazaspalacio!

JuancaG05 · 2023-07-07T12:59:12Z

owncloudApp/src/main/java/com/owncloud/android/extensions/ActivityExt.kt

@@ -285,39 +286,46 @@ fun Activity.hideSoftKeyboard() {
 fun Activity.checkPasscodeEnforced(securityEnforced: SecurityEnforced) {
    val sharedPreferencesProvider = OCSharedPreferencesProvider(this)

+    // If device protection is false, launch the previous behaviour (check the lockEnforced).
+    // If device protection is true, check the lockEnforced only if device is not secure.
+    val deviceProtection: Boolean = !this.resources.getBoolean(R.bool.device_protection) || !isDeviceSecure()


device_protection should be retrieved from the MDM provider since we have the option there 👍

JuancaG05 · 2023-07-07T13:17:16Z

.../java/com/owncloud/android/presentation/viewmodels/settings/SettingsSecurityViewModelTest.kt

So, regarding the new methods, we are testing the method isSecurityEnforcedEnabled, so tests should always start by is security enforced enabled and not by is security enforced disabled, we are mixing here the name of the method to test with the locked enforced value.
I would propose a new naming for all these tests:
is security enforced enabled {true|false} - ok - device {not} secure {no} device protection {no} lock enforced
So that we know which method we are testing (is security enforced enabled), the expected result of this test case ({true|false}), and the value of each of the 3 parameters involved (device {not} secure {no} device protection {no} lock enforced).

jesmrec · 2023-07-11T10:07:13Z

tested with <bool name="device_protection">true</bool> to check new behaviours.

Fresh installation

lock_enforced is 0

	Device protected	Device not protected
App not protected	Passcode not asked and enable in Settings ✅	Passcode must be asked and not removable. Behaviour like `lock_enforced = 1` ✅

lock_enforced is not 0 (passcode enforced)

	Device protected	Device not protected
App not protected	Passcode is enforced and must be asked and not allowed to remove ✅	Passcode asked and not removable following behaviour of `lock_enforced` ✅

Status changes (no lock_enforce)

Test Case	Steps	Expected Result	Result
Set device protection I	1. App not protected 2. Device not protected 3. Add protection to device 4. Open app	App does not ask to create new passcode	✅
Set device protection II	1. App protected 2. Device not protected 3. Add protection to device 4. Open app	App does not ask to create new passcode. Passcode is removable from `Settings`	✅
Unset device protection I	1. App not protected 2. Device protected 3. Remove protection from device 4. Open app	App asks to create new passcode/pattern. Passcode/Pattern is not removable from `Settings`	✅
Unset device protection II	1. App protected 2. Device protected 3. Remove protection from device 4. Open app	App does not ask to create new passcode. Passcode is not removable from `Settings`	✅
Set app protection I	1. App not protected 2. Device not protected 3 Open app	App asks to create new passcode. Passcode is not removable from `Settings`	✅
Set app protection II	1. App not protected 2. Device protected 3. Add protection to app 4. Open app	App does not ask to create new passcode. Passcode is removable from `Settings`	✅
Unset app protection I	1. App protected 2. Device protected 3. Remove protection from app 4. Open app	App does not ask to create new passcode	✅
Unset app protection II	1. App protected 2. Device not protected 3. Remove protection from app	Passcode is not removable from `Settings`	✅

Summarizing:

lock_enforced prevails over device_protection
If lock is enforced (!=0), the protection condition is always fulfilled (app protected || device protected || both protected) and the value of device_protection does not matter.
If lock is not enforced (==0), we have to assure always that the protection condition is fulfilled if device_protection is true.

jesmrec · 2023-07-11T10:26:12Z

(1) [FIXED]

I was not able to reproduce the base case

Remove all the security methods in device, so screen is never locked and content is accesible without entering pin, fingerprint, face and so on
Set device_protection to true
Install the app from scratch

Current: Security method is not asked after first open, user can enter URL and work with the app without security in app and device

Expected: device_protection as true should not allow users to use the app without app security methods.

Pixel2 Android11
c8261845d

jabarros

LGTM

Is in vacation

jesmrec · 2023-08-02T15:50:13Z

(2) [FIXED]

Set

<bool name="device_protection">true</bool>
<integer name="lock_enforced">3</integer>

then, install the app from scratch

Current: app is asking users to choose pattern or passcode
Expected: lock_enforced is 3, so pattern is enforced. Pattern enabled prevails.

Pixel 2 Android 11
c65e7f8

Adding device to security checks. Modifying and adding tests for new security checks.

Adding comments to devices protection checks. Changing device protection messages.

Obtaining the device protection from the MdmProvider.

jesmrec · 2023-08-03T09:36:00Z

Approved. Ready to go

manuelplazaspalacio added a commit that referenced this pull request Jun 28, 2023

Adding changelog #4087.

9dd6a43

manuelplazaspalacio requested a review from JuancaG05 June 28, 2023 06:55

manuelplazaspalacio self-assigned this Jun 28, 2023

manuelplazaspalacio added a commit that referenced this pull request Jun 28, 2023

Adding changelog #4087.

3ca6d23

manuelplazaspalacio force-pushed the feature/force_security_if_not_protected branch from 29be800 to b889f1b Compare June 28, 2023 07:05

manuelplazaspalacio linked an issue Jun 28, 2023 that may be closed by this pull request

[FEATURE REQUEST] Force security if device is not protected #4061

Closed

9 tasks

manuelplazaspalacio force-pushed the feature/force_security_if_not_protected branch from d24fb0c to 73d6f63 Compare June 28, 2023 08:55

JuancaG05 requested changes Jun 29, 2023

View reviewed changes

manuelplazaspalacio requested a review from JuancaG05 July 5, 2023 12:42

manuelplazaspalacio force-pushed the feature/force_security_if_not_protected branch 3 times, most recently from 5cfb2c7 to 420a37b Compare July 7, 2023 12:48

JuancaG05 previously requested changes Jul 7, 2023

View reviewed changes

jesmrec approved these changes Jul 10, 2023

View reviewed changes

manuelplazaspalacio force-pushed the feature/force_security_if_not_protected branch from c826184 to 7805174 Compare August 1, 2023 08:17

jabarros approved these changes Aug 2, 2023

View reviewed changes

manuelplazaspalacio and others added 8 commits August 3, 2023 10:31

Checking if device is secure.

8da40a1

Adding device to security checks. Modifying and adding tests for new security checks.

Solving error when checking device protection.

54ce109

Adding comments to devices protection checks. Changing device protection messages.

Changing comments

4ac46c2

Changing test namings

ae353c7

Obtaining the device protection from the MdmProvider.

Calens changelog updated

e859765

Changed logic to show security and modify passcode settings.

4ddbf0f

Modifying test with the new logic.

ea9551f

Showing selected enforced security when device_protection to true.

bad94b8

manuelplazaspalacio force-pushed the feature/force_security_if_not_protected branch from 0a936e8 to bad94b8 Compare August 3, 2023 08:32

jesmrec added the Approved by qa label Aug 3, 2023

manuelplazaspalacio merged commit f585ec4 into master Aug 3, 2023
5 checks passed

manuelplazaspalacio deleted the feature/force_security_if_not_protected branch August 3, 2023 10:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature request] Force security if not protected #4087

[Feature request] Force security if not protected #4087

manuelplazaspalacio commented Jun 28, 2023 •

edited by jesmrec

JuancaG05 left a comment

JuancaG05 left a comment

JuancaG05 Jul 7, 2023

JuancaG05 Jul 7, 2023

jesmrec commented Jul 11, 2023 •

edited

jesmrec commented Jul 11, 2023 •

edited

jabarros left a comment

jesmrec commented Aug 2, 2023 •

edited

jesmrec commented Aug 3, 2023

[Feature request] Force security if not protected #4087

[Feature request] Force security if not protected #4087

Conversation

manuelplazaspalacio commented Jun 28, 2023 • edited by jesmrec

Related Issues

QA

JuancaG05 left a comment

Choose a reason for hiding this comment

JuancaG05 left a comment

Choose a reason for hiding this comment

JuancaG05 Jul 7, 2023

Choose a reason for hiding this comment

JuancaG05 Jul 7, 2023

Choose a reason for hiding this comment

jesmrec commented Jul 11, 2023 • edited

jesmrec commented Jul 11, 2023 • edited

(1) [FIXED]

jabarros left a comment

Choose a reason for hiding this comment

jesmrec commented Aug 2, 2023 • edited

(2) [FIXED]

jesmrec commented Aug 3, 2023

manuelplazaspalacio commented Jun 28, 2023 •

edited by jesmrec

jesmrec commented Jul 11, 2023 •

edited

jesmrec commented Jul 11, 2023 •

edited

jesmrec commented Aug 2, 2023 •

edited