fix: check for the identifier alias for the storage backend

[jvillafanez]

Description

Prevent local storage to be used as external if it isn't explicitly allowed.

Related Issue

• Fixes <issue_link>

Motivation and Context

How Has This Been Tested?

• test environment:
• test case 1:
• test case 2:
• ...

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item, see TEMPLATE

Notes:

This is likely the minimum change possible at the moment. There are no plans to add or modify the backend's aliases
or identifiers at the moment, but it could become unmanageable quickly.
We should consider to move and improve the check at some point.

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[DeepDiver1975]

• can you add unit tests?
• please add changelog items

[DeepDiver1975]

• I miss tests where local storage mounting is allowed.files_external_allow_create_new_local => true
• did you double check if the http status code change 422 -> 403 has any impact on the frontend. technically this is a breaking change to the api

[jvillafanez]

did you double check if the http status code change 422 -> 403 has any impact on the frontend. technically this is a breaking change to the api

I don't think it's possible to trigger the behavior from the web UI. If the backend isn't visible, it won't show up in the frontend and it won't be selectable.
I've also included the DAV storage to test and it has the same behavior: the DAV storage doesn't show up as selectable in the user's external storages. From the admin settings, the webdav checkbox isn't selected, and even though you can mark the checkbox and save it, it won't persist.

The change in the HTTP code should match the previous behavior: it returns a 403 if the user tries to create or update a backend that he isn't allowed to use. It also makes more sense to return a 403 error in those circumstances.

I miss tests where local storage mounting is allowed.files_external_allow_create_new_local => true

They should be covered. For the global / user storage controllers, the tests have the flag enabled by default, so all those tests run with that setting. The only exception are the "new" testCreateLocal and testCreateLocalClassname, which explicitly set the flag as false.

[DeepDiver1975]

They should be covered. For the global / user storage controllers, the tests have the flag enabled by default, so all those tests run with that setting. The only exception are the "new" testCreateLocal and testCreateLocalClassname, which explicitly set the flag as false.

looking at the changeset - I see files_external_allow_create_new_local only being removed from the code - is this setting actually still be checked anywhere?

[DeepDiver1975]

They should be covered.

I prefer 'are' over 'should' - assertion over assumption

[jvillafanez]

looking at the changeset - I see files_external_allow_create_new_local only being removed from the code - is this setting actually still be checked anywhere?

It's used in the settings page. The flag is in an awkward position... it doesn't allow the creation of the local storage from the frontend, but the admin can still create the local storage via API. Regular users won't be able to setup local storages anyway.

I'll try to add an isAllowedAdminBackend (similar to the isAllowedUserBackend) and check for the flag there. If not allowed, the backend visibility will be set to none at registration time (similar to what is done for regular users). I think we can go with that solution if it works.

[DeepDiver1975]

. it doesn't allow the creation of the local storage from the frontend, but the admin can still create the local storage via API.

just to be sure: the backend has to verify everything - ignore frontend beahavior.
finally: adding tests accordingly helps to verify this and to ensure that changes in the future will not fuck it up again ;-)