-
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
SSO + masterkey encryption issue for new users #24182
Comments
@schiesbn can you have a look ? Looks like it's expecting a user's private key even in master key mode ? |
related to SSO |
After enabling the master key the user needs to re-login in order to initialize the master key for the session. That's why this solves the issue:
Once #18410 is fixed we can invalidate all sessions after encryption was enabled and enforce a re-login. |
@schiesbn: Thanks for the reply. Not sure whether I understand correctly, ok a fix for #18410 would force existing users to re-login, but how about new users that are created after encryption was enabled, e.g. like in this issue? Would the internal error still show? Thanks :) |
I'd expect users created after that to work properly. If it doesn't, the encryption code needs to be fixed to not bother about user keys in master key mode. |
For new users a key should be written to the session as soon as the user log-in and a new session gets created. Therefore the login hooks need to be triggered on login, but I assume that this happens for all SSO back-ends... Can someone with access to such back-ends verify this? Maybe @butonic ? Thanks! |
@davitol can you help reproducing this issue ? |
@PVince81 I'm just talking with @schiesbn about it |
Thanks to @davitol I could test it with a SSO system (Shibboleth) but couldn't reproduce it. What we tried:
Everything worked fine. What user back-end do you use for SSO? |
In the initial app there is a link to a demo sso app. |
technically they all work the same and with #23903 users home dirs are now initialized properly. Try with and without that patch. |
@schiesbn : As @butonic suggested, I strongly recommend to use the IApacheBackend for testing. It has it's own dedicated methods like |
@GitHubUser4234 I will give it a try and see if I can re-produce it with this one. Still would be good to know what you use for SSO? Is it something you wrote by yourself or something provided and supported by us? |
@schiesbn : Great! 👍 Well, the SSO is based on client certificate authentication. When the Apache web server verified the client's certificate, the user is logged in automatically to ownCloud. The automatic login functionality is a feature supported by ownCloud's IApacheBackend API. However, the testing app above doesn't contain any logic for client authentication, it's just the simplest IApacheBackend implementation possible to facilitate reproducing the problem. |
CC @ChristophWurst I heard you were looking into apache auth stuff |
That's good to know! If I can't reproduce it today, maybe we can have a look together on Monday, @ChristophWurst .... But maybe it is all already solved until Monday 😉 |
I think it is obvious that it fails with your app. You just always return 'true' for isSessionActive() and then of course there is no session and if there is no session there can't be a private key. If, during login the ownCloud login hooks are triggered a session gets created and ownCloud copy the private key to the session. But this is something which never happens with your test plugin |
But I'm not really familiar with the IApacheBackend. I see that it was written by @DeepDiver1975. Maybe you can bring some light into this? How is it supposed to work? I don't see a method which could be used to trigger the login hooks. |
Would probably a bad idea to do it in |
This statement doesn't seems valid. When |
Ok, seems like I can re-produce it to some extend. But I get different error messages. In the log files I see nothing. In the Browser I get
But I also stepped through this with the debugger. The login hooks are called correctly and the encryption keys get initialized without a error. The error only happens after re re-direct to the files view. Will check if I can find out more. |
Just tried it without encryption and there I get exactly the same error. So I'm not sure if this is really related to encryption. Btw, also your work around "Delete browser cookies & close browser" doesn't work for me. @GitHubUser4234 do you have any chance to debug this? Any change to check if this method gets called: https://github.com/owncloud/core/blob/master/apps/encryption/hooks/userhooks.php#L154 and that in |
@schiesbn : Thanks for trying. You are using OC 8.2.1? I will try to debug according to your request tomorrow, I'm off today 😃 |
Yes, I tried it with 8.2.1 and followed exactly your steps. But as said, for me it also fails without encryption. Thanks for debugging it on your side! |
@schiesbn: I have just debugged it, the Did you install the patch of #23903 yet? If not, you will encounter the bug described in #23899 which also occurs without encryption. In this case, please try again with the patch, thanks. |
Ah, sorry... I missed the patch. Let's try me again 😄 |
It can be so easy, if the test environment is set-up correctly. See the pull request above. 😄 This should fix the issue. Can you try it? Thanks! |
Wow that was really easy 😄 I have applied it to lib/private/user.php (as I couldn't find a lib/private/legacy/user.php in my 8.2.1 installation?) and it works now, thanks a lot 😃 |
That's correct, we moved the file only on master. Happy to hear that it works for you! 😄 |
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 mentioned in #23899 , there is an internal error for new users logging in through SSO when masterkey encryption is enabled.
Steps to reproduce
Link to the testing app
The error goes away when "dep_tester123" repeats the SSO:
What happens in the GUI:
What happens in the logs:
Server configuration
Operating system: RHEL 5
Web server: Apache 2.2
Database: MySQL
PHP version: 5.6
ownCloud version: 8.2.1
Updated from an older ownCloud or fresh install: No
The text was updated successfully, but these errors were encountered: