-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Use Laravel authentication #8702
Conversation
77ce538
to
9bac825
Compare
Ok, first thing from me is using Firefox (not tested on other browsers). Login either with or without remember me. Close the browser and reopen. Both times you will be faced with Had to delete the |
Second thing is the API no longer works, Get:
|
bdca9dc
to
efeca09
Compare
df78f74
to
6c1201b
Compare
Ready for testing. Still have some small issues to work out but I think it is all there. |
@murrant Using AD auth and getting `Unhandled MySQL Error [42S22] SQLSTATE[42S22]: Column not found: 1054 Unknown column 'auth_type' in 'where clause' |
Removed the PR and reapplied it. It's working now. |
Yeah, you will need to apply the sql schema update to use it. |
I'm not running mysql strict here but I get:
When trying to edit an existing user. |
Delete user redirects to dashboard: |
When logging in with read only user (with or without devices assigned), you see the yellow pop up of you have unpolled devices in the last 15 minutes. |
I've tried to test sso and not sure it is working. Following the test example from #7601 (comment) which I used when that auth type landed and it worked fine. |
@TheMysteriousX if you get chance, could you test this PR with SSO? |
2FA has some issues with adding+removing tokens right now. I've got it almost worked out, but I'm running into an issue involving the ajax request setResolution. |
Sure, I'll give it a try when I can. Once its merged I'll look at what hooks are available in Laravel itself too - I know it has some support for single sign on. |
Mysql error on edit fixed. I noticed something odd going on with dashboards... phpdebugbar I think, will check it later. |
@TheMysteriousX You didn't run composer install |
Is there a dependency that hasn't made it into composer.json?
|
@TheMysteriousX I think sometimes the autoloader just needs to be bumped. |
Apologies, I was unclear - validation passes and I've run composer install, and this still PR breaks my install regardless of auth method used. |
@TheMysteriousX I can't reproduce the issue. Please try |
No change I'm afraid - ~~~I might have a theory though. What happens if you set the following cookie manually and try to load LibreNMS with this patchset:~~~ Tried the inverse, no change - error occurs with or without foreign cookies. |
Just reproduced this using the librenms docker image here: https://hub.docker.com/r/jarischaefer/docker-librenms/
Error is exactly the same - librenms isn't even installed yet:
|
@TheMysteriousX There was an error in the exception handling, I hooked in at the wrong place and caused the unauthenticated exception to be handle incorrectly. (and once the session was created, it didn't trigger again) |
All working with fine for me with the latest changes. |
@TheMysteriousX what authentication methods did you test? Also a huge thanks for testing :) |
I tested MySQL and SSO - everything seemed to work as expected, including editing, creation, password resets and all that. |
039de03
to
df6f95e
Compare
Support legacy auth methods Always create DB entry for users (segregate by auth method) Port api auth to Laravel restrict poller errors to devices the user has access to Run checks on every page load. But set a 5 minute (configurable) timer. Only run some checks if the user is an admin Move toastr down a few pixels so it isn't as annoying. Fix menu not loaded on laravel pages when twofactor is enabled for the system, but disabled for the user. Add two missing menu entries in the laravel menu Rewrite 2FA code Simplify some and verify code before applying Get http-auth working Handle legacy $_SESSION differently. Allows Auth::once(), etc to work.
df6f95e
to
a6d564b
Compare
Authentication using LDAP not working here.
Although, LDAP server is OK, as you can see: ./scripts/auth_test.php -u user |
After latest changes I can see following in librenms.log: [2018-09-13 17:21:48] production.ERROR: 2 /opt/librenms/LibreNMS/Component.php:162 Illegal offset type I am using old auth_method. |
This PR has been locked as it is not the correct place to ask for help. For help please use our Discord server or the community site: http://docs.librenms.org/Support/FAQ/#faq3 |
Use Laravel for authentication, but call into legacy auth code for now.
Testers
If you would like to test this pull request then please run:
./scripts/github-apply 8702
and run ./build-base.php to update your db schema.TODO:
Auth types tested:
What to test: