POST request to create link with incorrect password returns 400 instead of 403

[jesmrec]

Describe the bug

If clients create a password for the link that does not match the policy, request returns 400 Bad Request instead of 403 Forbidden

Steps to reproduce

1. Create a new link using a mobile client with a password that does not match the given policy

that curl simulates the operation

curl -s -k -u <user:pwd> 'https://<url>/ocs/v2.php/apps/files_sharing/api/v1/shares' -X POST -H 'content-type: application/x-www-form-urlencoded' -H 'Origin: https://<url>' -H 'Connection: keep-alive' -H 'Referer: https://<url>/files/spaces/personal/<username>?fileId=dcfd1520-b2b7-4d5c-a557-947dc830bb3f%24f88aac90-8060-4b6b-9d7b-3ec40d77462d%21f88aac90-8060-4b6b-9d7b-3ec40d77462d&tiles-size=1&files-spaces-generic-view-mode=resource-table&items-per-page=100' -H 'Cookie: occelzrainqf=1f1olng1ce1ldouujsmk2k9ta6' -H 'Sec-Fetch-Dest: empty' -H 'Sec-Fetch-Mode: cors' -H 'Sec-Fetch-Site: same-origin' --data-raw 'shareType=3&path=%2FDocuments&space_ref=dcfd1520-b2b7-4d5c-a557-947dc830bb3f%24f88aac90-8060-4b6b-9d7b-3ec40d77462d!781f56fa-e351-4d60-aaf3-8ed428db3c96&permissions=1&password=a'   | xmllint --format -
<?xml version="1.0" encoding="UTF-8"?>

Expected behavior

Request is correct sintactically and semantically, so 400 is not proper error code. oC10 returns 403 under same context. Apart of that, returns failure instead of error

<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>failure</status> <==== oC10 returns "failure" instead of error
    <statuscode>403</statuscode>  
    <message>At least 8 characters are required
at least 1 uppercase letters are required
at least 1 numbers are required
at least 1 special characters are required  !"#$%&amp;'()*+,-./:;&lt;=&gt;?@[\]^_`{|}~</message>
  </meta>
</ocs>

Actual behavior

<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>400</statuscode>
    <message>At least 8 characters are required
at least 1 uppercase letters are required
at least 1 numbers are required
at least 1 special characters are required  !"#$%&amp;'()*+,-./:;&lt;=&gt;?@[\]^_`{|}~</message>
  </meta>
</ocs>

Setup

version: "3.7"

services:
  ocis:
    image: owncloud/ocis:5.0.0-beta.1
    ports:
      - 9200:9200
      - 9215:9215
    environment:
      OCIS_INSECURE: "true"
      OCIS_URL: "<url>"
      IDM_CREATE_DEMO_USERS: "true"
      IDM_ADMIN_PASSWORD: "admin"
      PROXY_ENABLE_BASIC_AUTH: "true"
    restart: "no"
    entrypoint:
      - /bin/sh
    # run ocis init to initialize a configuration file with random secrets
    # it will fail on subsequent runs, because the config file already exists
    # therefore we ignore the error and then start the ocis server
    command: ["-c", "ocis init || true; ocis server"]

Additional context

Add any other context about the problem here.

[michaelstingl]

 <status>failure</status> <==== oC10 returns "failure" instead of error
     <statuscode>403</statuscode>

In other places, oCIS returns <status>error</status> too:

• [FEATURE REQUEST] User feedback for link passwords according to password policy rules (exposed via ocs/v1.php/cloud/capabilities) ios-app#1015 (comment)

[micbar]

This is weird. 403 is not the suitable response IMO. The creation of the share is not forbidden.

either it is a 400 bad request or 412 precondition failed

[jesmrec]

either it is a 400 bad request or 412 precondition failed

technically, i agree. But, whatever the resolution, the response over the same effect should be the same in both backends, otherwise clients will have to double logic to handle the same error.

[micbar]

Ok, we need to check with @janackermann and @2403905

can we make the ocs response „bug compatible“ with oc10?

[phil-davis]

https://github.com/owncloud/password_policy/blob/master/tests/acceptance/features/apiUpdateShare/publicShareLinkUpdateMinimumLength.feature#L35

With the password_policy app and oC10 core, if an attempt is made to update the link password with one that does not meet the policy, then the OCS status is 400

But when first creating a link, if the password does not meet the policy, then the OCS status is 403

  Scenario: user creates a public link share with password that is too short
    Given the administrator has enabled the minimum characters password policy
    And the administrator has set the minimum characters required to "10"
    And these users have been created with default attributes and small skeleton files:
      | username | password   |
      | Alice    | 1234567890 |
    And user "Alice" has uploaded file with content "Alice file" to "/randomfile.txt"
    When user "Alice" creates a public link share using the sharing API with settings
      | path     | randomfile.txt |
      | password | ABCabc1        |
    Then the OCS status code should be "403"
    And the HTTP status code should be "200"

That seems a bit strange to have 403 on attempted link creation, and 400 on attempted link update.

[micbar]

Clarification:

• OCS should be returning a response like oc10
• Sharing NG should return 400 as it does today.

[micbar]

@jesmrec Is that a release blocker for 5.0.0?

We will switch to Sharing NG Asap after 5.0.0 has been released.

[jesmrec]

I'd not block 5.0.0 for this. Will sharing NG fix this, then?

[micbar]

Sharing NG is a new graph Api part.

[jesmrec]

i know. The point is if sharing NG will return the correct code instead of the current sharing API. If it is, problem will be fixed by integrating sharing NG in clients. If not, problem will be moved to sharing NG.

[micbar]

SharingNG already returns a 400 status code with an "invalidRequest" error code in the json body.

[micbar]

@michaelstingl @jesmrec Do yo agree with a "won't fix" for OCS?

[michaelstingl]

@michaelstingl @jesmrec Do yo agree with a "won't fix" for OCS?

@jesmrec causes real problems?

[jesmrec]

@jesmrec causes real problems?

i don't think so. I will do some QA in client side when the local check over policies is done to be sure about this.

[micbar]

@jesmrec any updates?

[jesmrec]

Our first current implementation for the oCIS password policy gets the 400 as error for a wrong password. That means, if backend team finally changes the error code from 400 to other code that fits better with the wrong password scenario (f. ex. 406 or 412), keep us updated with such change to be aligned.

[micbar]

Thanks. There are no changes on OCS planned.

We will focus only on LibreGraph in the future.