Open immerda opened 3 years ago
A first potential issue we identified with this approach is that enabling encryption breaks compatibility with pre-existing app passwords. If encryption is enabled and a user accesses the server with an app password, then their keys are initialized without the secret, which makes them unreadable.
Therefore it is important to deauthenticate all app tokens (truncate authtoken table) and deauthenticate all web sessions before enabling encryption. Imho this could be explained with proper documentation, as it is probably only for people who know what they do anyway.
Otoh we could also try to block key creation with empty password when the feature is enabled.
Hi @ChristophWurst I hope you don't mind that I @ you on this thread. It seems you might be the person that could provide some guidance around the pieces of code that this issue and its PRs are touching upon. We are quite motivated to put in some more work and iron out the wrinkles, if there is some indication that upstream adoption is possible. Thanks already...
I'm not familiar with encryption code, sorry, I can't help here.
Hello @immerda
On our NC 25 installation with user key encryption enabled, I'm trying to setup OpenID or SAML to our FusionAuth SSO but I don't manage to do the right setup. I've just managed to use some OIDC app but it doesn't seem to use this system since on a newly registered user I got the private key warning (the same you got on a password change on a regular user).
I don't know how to make sure I use the right app so I'd be glad to know which one can make it work properly :(
Thanks in advance !
How to use GitHub
Is your feature request related to a problem? Please describe. Currently the default encryption plugin cannot have user-keys with single sign on, such as SAML. As described here the problem is that NC does not have access to the password, that is used to secure the users private key.
Describe the solution you'd like In our setting, our SAML identity provider can already safely store and provide per-user secrets derived from the users password. Therefore the obvious solution seems to to let the authentication backend provide a stable per-user secret which can be used in lieu of the actual password.
Describe alternatives you've considered A more flexible alternative would be to let the keystore use a generic and configurable "user secret provider". Different authentication backends could support different strategies for providing such a secret. However, my knowledge of the Nextcloud codebase is not good enough to implement this.
Additional context I submitted two PR which implement this strategy.
Update: these patches are now in https://github.com/nextcloud/server/pull/27929 and https://github.com/nextcloud/user_saml/pull/537
I am not quite sure TBH what the approach is for changing an interface. There might be other implementations of the
Authentication\IApacheBackend
interface. And also the change must be synchronized between server and saml plugin.We are currently running this patch in our testing environment. We can successfully create users, login via browsers and app, and normally use all features. The configuration has encryption enabled and the master-key disabled. We confirmed that files are correctly encrypted and decrypted.