-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
External Storage Interface: Selecting Username and Password Options #16305
Comments
I guess this only applies to system-wide mount points, the ones that admin setup for all users ? The tricky part here is that if the admin sets up a system mount point to "User entered", then we need to find a place where to store the user entered password. In any case, it looks like it's a good opportunity to move the ext storage configuration from "mount.json" to a proper database structure. Here is the ticket about revamping the data structure for external storages: #11261 |
Allowing to select an authentication type also means that every storage backend needs to be adapted to support any of these. I'd suggest having each backend implement specific methods from which the username/password can be injected from outside by whichever auth mechanism was selected. CC @Xenopathic @icewind1991 for additional input |
@PVince81 That's the BackendService work I started over at #15914. I got a bit sidetracked with exams between then and now... Feel free to push to that branch, anyone who wants to help. I was trying to think of the best way to design the OOP structure for authentication methods. The best I've come up with so far: Interfaces for each authentication type (e.g. IPassword), implemented by different classes (e.g. Auth\UsernamePassword, Auth\OCPassword) and registered with a AuthenticationMethodService or equivalent. Then a backend defines which interface(s) it supports, and it gets passed the selected authentication object through $params['authObject'] or something. This is the Strategy pattern. Only issue with this is that the AuthenticationMethodService will have to iterate through all registered authentication types to find which match a backend's defined interfaces, unless we can store a map of authentication interfaces to authentication implementations. |
Cool, thanks for the info. |
Self assigning as part of #15914 |
So, session-based credentials and basic username/password are currently already implemented. Next, I'll implement store-in-DB user credentials, followed by the shared credentials. |
@MTRichards can you elaborate the workflow for shared login ? Who does what when ? "Using credentials entered in the external storage line when the storage is initial configured" |
Hello @PVince81
That is the idea. Perhaps oddly worded...but this is the single account shared for all users indicated for that external storage. |
@MTRichards there's a problem with "User Login, store in Session": it makes it impossible to run any cron tasks on that storage (occ files:scan or others) because the credentials won't be available. So either we should get rid of that option and favor "store in DB" or add a big warning that tells that some important functionality might not be available. |
No choice but to keep it. Storing passwords in databases is a big no go for many of our customers, so for them we need to be crystal clear on the options and the implications. @PVince81 won't the scan on access setting in the config file pick up these files, albeit quite slowly and when something is accessed? @carlaschroder |
@MTRichards scan on access through web UI or sync client will work. The problem is for any cron job or CLI tool that requires access to the storage, like the "occ files:scan" command. |
Got it. If the user is not in with an active session, or even if they are (since we can't access the session veriable) we can't access the storage from any ownCloud server Cron initiated job. |
Thanks @icewind1991 |
|
@MTRichards looks like we missed the option "Global credentials" in this list ? |
We have global credentials at admin level (unique) and user level (unique per user). Both can be shared between different mount points. From files_external perspective has no sense to be the same credentials for every backend. I think that should be different for each backend. |
If we want to have this different for each backend, it means we need a more complex form that lets you map credentials to every backend, with only one set of credentials allowed for each one. |
I'm afraid so. The other solution, is to change this in the migration script, to adapt old config to the new available. |
IMO it actually does make sense to have Global Credentials affect all storages. The primary usecase for it is when there are many storages that are all accessed with the same username/password, hence the network is running an identity server. It is highly likely that the identity server is connected to all services available on the network, regardless of the exact backend (SFTP, SMB, WebDAV etc), so they all use the same credentials -> Global Credentials. In the home user scenario, Global Credentials won't be that useful anyway |
Global credentials was simply meant as a shortcut for an admin so they didn't have to type the credentials in more than once. Use case was this: Admin enters global credentials, and then on a per mount point basis they select if the global credentials should be used in the drop down credentials list, or something else. Use case for users: They enter the global credentials once in the personal page, and then can select on the mounts whether to use these global credentials or some other form of authentication. Just to be clear, global credentials are a convenience and nothing more - you can get to the same result by just typing in user names. So do we need a better form, or just this as an option in the dropdown list of credential options? |
Perhaps there is a way to avoid having a separate authentication mechanism (= mroe complexity) and have something which solves the problem in a different way? The problem is that an admin/user must type in a username/password for each individual storage, which becomes problematic when there are many storages that need configured. What about, when a 'Username/password' is filled for the first time in a session, and a second one is created, then that username/password is copied to the second storage as a default value? So the flow would be: admin/user creates a storage, sets username/password on it. Admin/user creates a second storage, username and password fields already set to the same as for the first storage (modifyable if necessary). That still gives the benefit of reduced typing for much lower complexity. @MTRichards @PVince81 @DeepDiver1975 thoughts? |
That would make changing the credentials hard |
@Xenopathic I think as an admin I would find this behavior strange, as if it was a bug or the browser remembered the login like it happened sometimes. Having a field to storage global credentials is fine but the question is whether it's global to all backends (at least those supporting "username and password") or whether it should be per-backend. Also another question technically is where to store this, should we reuse the table we just created for credentials or use a new one ? We probably also need new REST endpoints to be able to read/write these. |
@PVince81 see #16305 (comment) We can store the information in the CredentialsManager |
Discussed with @MTRichards and @jmaciasportela, the decision is to do the following:
|
Raised here: #21991 I've also updated the main ticket to add a bullet point for it. |
All authentication backends were added. |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
As an admin, I want to be able to select one of four credential options so that when users log into ownCloud, the appropriate authentication mechanism is used for external storage connections.
The four are:
Acceptance Criteria:
Edit by @PVince81: added the new wording
The text was updated successfully, but these errors were encountered: