Migrate advanced external storage backends to new registration API [part 3] #18441
Conversation
RobinMcCorkell
force-pushed
the
ext-backends.advanced
branch
2 times, most recently
from
10fd293
to
5059d46
Compare
The Rackspace/OpenStack differences have been split into separate auth mechanisms, with correct legacy migration
SMB_OC has been merged with SMB, via the identifier aliases mechanism. Legacy migration is done to the Session Credentials password mechanism
The SFTP backend now supports public key authentication alongside password authentication.
VisibilityTrait -> PermissionsTrait
PermissionsTrait stores two sets of data, $permissions and
$allowedPermissions (analogous to $visibility and $allowedVisibility of
VisibilityTrait). Each set is a map of user type ('admin' or 'personal')
to permissions (mounting permission, create permission).
The result is that a backend can now be restricted for creation, while
still allowing it to be mounted. This is useful for deprecating backends
or auth mechanisms, preventing new storages being created, while still
allowing existing storages to be mounted.
Backend and auth mechanism permissions are checked on storage creation, both for personal storages and for admin storages
RobinMcCorkell
force-pushed
the
ext-backends.advanced
branch
from
5059d46
to
0b97a05
Compare
|
The visibility stuff has been augmented to a proper permissions model, similar to UNIX ACLs. It allows the deprecated storages to still be mounted, but not to be created (by a user or by the admin). |
|
smb lost the "username as share" option |
|
code looks good otherwise, havent tested |
|
@icewind1991 SMB never had the 'Username as share' option, only SMB_OC (which still has it, for existing storages). It can be replicated by putting '$user' in the share field though. |
|
Code looks good, needs testing |
|
🚀 Test PASSed.🚀 |
|
A new inspection was created. |
|
Good thing I tested the legacy migration... All 3 backends now correctly migrate to the new system - OpenStack with the 'API Key' set becomes an OpenStack with Rackspace authentication, OpenStack without 'API Key' becomes OpenStack with OpenStack authentication, SMB_OC is migrated to SMB with Session credentials (the typo was with 'Username as share', which now also works correctly). SFTP_Key correctly migrates to SFTP with RSA Public Key authentication, and it works both before and after. Generation of RSA keys works too with the new authentication mechanism. Please review @PVince81 @MorrisJobke @nickvergessen @DeepDiver1975 @icewind1991 |
| * For user type constants, see BackendService::USER_* | ||
| * For permission constants, see BackendService::PERMISSION_* | ||
| */ | ||
| trait PermissionsTrait { |
There was a problem hiding this comment.
Why do we need such complex permissions ?
There was a problem hiding this comment.
Mainly for deprecation capabilities. In the current example, we need a way to stop users creating SMB_OC (it's deprecated), but still mount and modify existing ones. Then, the admin might want to disable mounting certain storages, so that's removing the mount and create permissions.
Basically I thought: UNIX extended permissions are well designed, so lets just emulate that.
|
looks good 👍 |
| } | ||
|
|
||
| public function manipulateStorageConfig(StorageConfig &$storage) { | ||
| $auth = new RSACrypt(); |
There was a problem hiding this comment.
We need a different RSACrypt object for each storage, so injection wouldn't work (there's also no Factory available)
|
👍 otherwise |
|
Core and common external tests pass, and SFTP tests pass (except for two failures with SFTP, but they also occur on master). Everything works when tested manually, and two reviewers -> merge |
Migrate advanced external storage backends to new registration API [part 3]
This PR migrates the following external storage backends:
Please review @PVince81 @DeepDiver1975 @MorrisJobke @icewind1991
cc @jmaciasportela for an example of how registration works.
Replaces #18245