[feature/password-policy] Password Policy support

[felix-schwarz]

Description

This PR implements password policy support throughout the iOS client app, including:

• an extensible password policy system based on rules, policies and validation reports with verbose error reporting for
  • characters and character sets
  • lengths
  • byte counts
• the generation of password policies based on server capabilities
• a default password policy for servers that do not provide a password policy
• a password generator based on password policy rules using "cryptographically secure random bytes"
• a password composer for entering, editing and generating passwords with instant rule verification and feedback
• one-tap password generation based on a server's password policy within Public Link creation
• sharing of combined public link URL and password to the clipboard, Messages, Mail and more via the system share sheet directly after link generation, like f.ex.:

Photos (https://demo.owncloud.org/s/D3WkWZOW8BUjeKr) | Password: 46CPou|#Pw5.

Related Issue

#973

Screenshots (if appropriate):

Inline Generator button	Password Composer	Password Composer	Share Sheet

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[jesmrec]

Some preliminary inputs meanwhile the Code Review finishes:

• I miss some feedback message when the user clicks on Copy (just next to Generate). Every action should have a reaction, otherwise the user does not know if the performed action is correct or not. Password is correctly copied, but a message like Copied to clipboard would be welcome

• I would avoid empty conditions to build password policy like:

Those conditions will be always green, they are no real requirements.

• If i set more digits than the allowed, i see a condition like Longer than 72 bytes in utf-8 encoding that is not fulfilled just with the 73th character. i know that this is related with the string encoding, but, how does it overflows the limit? could you ellaborate?

• How does it work in oC10? i've seen it enabled in servers that does not return values in capabilities

[felix-schwarz]

Thanks for the feedback @jesmrec!

• I miss some feedback message when the user clicks on Copy (just next to Generate). Every action should have a reaction, otherwise the user does not know if the performed action is correct or not. Password is correctly copied, but a message like Copied to clipboard would be welcome

I added a message with 98de65c.

• I would avoid empty conditions to build password policy like:

Those conditions will be always green, they are no real requirements.

Good point. The code generating the policy from capabilities now no longer includes rules with a zero length requirement.

• If i set more digits than the allowed, i see a condition like Longer than 72 bytes in utf-8 encoding that is not fulfilled just with the 73th character. i know that this is related with the string encoding, but, how does it overflows the limit? could you ellaborate?

My choice of wording here was off by 1. It should be At most rather than Less than of course.

• How does it work in oC10? i've seen it enabled in servers that does not return values in capabilities

For OC10 or if no policy is provided, a default policy is used: https://github.com/owncloud/ios-sdk/blob/3096454383cdfb24a11aa6b8a84b53c853308ed7/ownCloudSDK/Password%20Policy/OCPasswordPolicy%2BDefault.m#L27

[jesmrec]

@felix-schwarz this is how password policy works in oC10:

Open an admin account (list of files) > Top Left hamburger button > Market

Look for Password Policy in the lit of plugins and install it

Then, in oC10's admin dashboard, you'll find the setup for the password policy. Tick the conditions to fulfill and click on Save (this is important, no autosave available in the feature):

capabilities endpoint will return the following JSON object:

"password_policy": {
                    "password_requirements": {
                        "configuration": {
                            "lower_case": {
                                "generate_from": {
                                    "characters": "abcdefghijklmnopqrstuvwxyz"
                                },
                                "minimum": 1
                            },
                            "numbers": {
                                "generate_from": {
                                    "characters": "1234567890"
                                },
                                "minimum": 1
                            },
                            "special_characters": {
                                "generate_from": {
                                    "any": true
                                },
                                "minimum": 1
                            },
                            "upper_case": {
                                "generate_from": {
                                    "characters": "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
                                },
                                "minimum": 1
                            }
                        },
                        "minimum_characters": 8
                    }
                }
            },

as you can see, nothing to do with oCIS response. It may not worth to implement support for this if oC10 is not going to be supportable in the st, but this is up to you.

[jesmrec]

Let's QA here...

[jesmrec]

(1) [FIXED]

A little UX improvement:

Create and Share buttons should be disabled if password is mandatory and was not set yet. An error missing required password is displayed, but i think it's more friendly to allow it when it's really posible.

Indeed, this is the current behaviour for permission selection

iPhoneXR v17.4.1 b4ab539bd

[jesmrec]

(2) [FIXED]

In a non-oCIS server, the Generate button is there but no action happens.

video taken with demo.owncloud.com (not sure if the clicks are visible, but they are there)

https://github.com/owncloud/ios-app/assets/14894746/86b64691-fba4-4dc4-b500-9d53d189aca5

Should be either actioned or removed.

iPhoneXR v17.4.1 b4ab539bd

[jesmrec]

(3) [FIXED]

Look at the following steps:

1. Create a new link with password (no matter which password or how to generate it)
2. Open the link to edit
3. Remove the existing password with the x
4. Click on Generate

Current:

At this point, there is no way to recover the automatically generated password, because the edit mode lacks of Share option (create mode does). Just Save Changes and password is missing.

Set option allows it.

Expected:

After generation of new password, there should be a way to get it to be shared. Options that come to my mind:

• automatic copy to the clipboard after clicking on Generate
• other button called Share link in the creation mode

iPhoneXR v17.4.1 b4ab539bd

[jesmrec]

(4) [WONT FIX]

Just a tiny detail about wording:

The Share button on the bottom of the screen does two actions:

• First, save the link
• Then, shares the link by displaying the share sheet

I'd call the button Create & Share, maybe less confusing. Also, affecting the suggestion in (3) about the posibility of creating new button in edit mode.

iPhoneXR v17.4.1 b4ab539bd

[felix-schwarz]

@jesmrec Thanks for spotting all of this. Let me go through (1) - (4) in chronological order: (1) The UI now respects if a password is required. It disables the respective buttons if no password is set and also adds a ⚠️ if no password has been set. (2) That was a consequence of OC10 using the same name space for password policies (PPs), but a different format, so that PPs derived from capabilities ended up nonsensical, preventing the generator from generating a password. I've added basic support for OC10 PPs now, and also some logic checks to ensure incomplete PPs are brought up to a minimum standard (minimum number of characters). That should fix the issue. (3) I've added a Copy button next to the password. I want to avoid an automatic copy to clipboard because it could overwrite something important the user has stored there at the time. Adding a Share button here creates a UI issue insofar as it could only be used if the password has been re-generated and remain grayed out otherwise, possibly confusing users. (4) I initially called it Create & Share but reduced it to Share after I realized that meant running out of space in certain localizations, which would require a different button layout.

[jesmrec]

(1) -> was the improvement pushed? i still see the same behaviour as before (as ebad324e):

https://github.com/owncloud/ios-app/assets/14894746/62ce0157-2f15-455b-a077-f67e23dee1fc

(2) -> fixed. Just as a comment, the applied policy is not standard with minimum chars, it applies all existing criteria. That's OK for me (enforces security)

(3) -> fixed

(4) -> OK

So, (1) may need other review, the others are OK

[felix-schwarz]

@hosy Thanks for the feedback. I fully agree that characters would be more intuitive here, yet the criteria from ocis really is about byte count which can vary per character depending on what character is entered.

From the docs:

Note that a password can have a maximum length of 72 bytes. Depending on the alphabet used, a character is encoded by 1 to 4 bytes, defining the maximum length of a password indirectly. While US-ASCII will only need one byte, Latin alphabets and also Greek or Cyrillic ones need two bytes. Three bytes are needed for characters in Chinese, Japanese and Korean etc.

[felix-schwarz]

@jesmrec The issue here was that capabilities indicated files_sharing.public.password.enforced as false whereas the values for files_sharing.public.password.enforced_for values were true.

Other than ocis, OC10 reports enforced is true in case any of the enforced_for cases are true. This makes sense insofar as ocis added a new internal permission (called Invited people in the UI), for which a password can't be set, but for which a password would be required if files_sharing.public.password.enforced is true.

I've changed the implementation to enforce a password if the selected permissions match a respective enforced_for case for which true (enforcement) is indicated. In my testing, this worked well for all roles/permissions, including Invited people.

Please run another test to verify you now get the same results.

[jesmrec]

Tested for the following cases:

• [X] Enforce password protection for read-only links
• [X] Enforce password protection for read + write links
• [X] Enforce password protection for read + write + delete links
• [X] Enforce password protection for upload-only (File Drop) links

[jesmrec]

I've changed the implementation to enforce a password if the selected permissions match a respective enforced_for case for which true (enforcement) is indicated. In my testing, this worked well for all roles/permissions, including Invited people.

That means that you only mind the enforced_for and reject the enforced, right @felix-schwarz ?

In my test server (oCIS 5.0.0), i'm getting:

"password": {
                            "enforced": false,
                            "enforced_for": {
                                "read_only": true,
                                "read_write": true,
                                "read_write_delete": true,
                                "upload_only": true
                            }
                        },

with such setup, the viewer, uploaded, contributor and editor roles are password-enforced but the invited persons (this one requires login, so there is an existing security layer). Is that expected then? so enforced and enforced_for don't seem to be related

[felix-schwarz]

@jesmrec If enforced is true, the app requires a password for all links.

If enforced is false, the app goes on to check if any of the enforced_for cases that are true apply to permissions of the selected role. If there's a match, it requires a password for these.

For Invited people links, according to the text in the screenshot from ocis-web above, passwords aren't supported by the server. Whereas all other roles for links use a combination of read, create, update, delete permission, the invited people role uses a new internal permission (and only that).

So far as I understand it then, the server picks values for enforced and enforced_for wisely:

• by sending false for enforced, capabilities don't indicate a general requirement for passwords for links. That allows invited people links to be created without password.
• for all other roles, the password requirement is enforced by enforced_for cases, as all of them target combinations of permissions used by the other roles only (read, create, update, delete). The invited people role on the other hand only uses the internal permission, so isn't covered by enforced_for at all.

Hope that clarifies it.

[jesmrec]

Ok, that covers all the cases i tested in advance. We can move forward this one

Approved