Skip to content
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

Merged
merged 5 commits into from
Sep 11, 2018
Merged

Use Laravel authentication #8702

merged 5 commits into from
Sep 11, 2018

Conversation

murrant
Copy link
Member

@murrant murrant commented May 12, 2018

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:

  • Implement token auth for API
  • Remember Me (updated and tested)
  • Two Factor Auth
  • Unauth status screen
  • Remove legacy includes
  • API check auth
  • Remove unneeded legacy code
  • Test each auth type (simple tests done for mysql, ldap, and active directory)
  • General Testing

Auth types tested:

  • Mysql
  • Active Directory
  • LDAP
  • Radius
  • HTTP
  • HTTP-LDAP
  • HTTP-AD
  • SSO

What to test:

  • login/logout
  • user management (if applicable)
  • api token creation
  • api access / full usage
  • other auth related things
  • 2FA

@murrant murrant self-assigned this May 12, 2018
@murrant murrant added this to TODO in Laravel Conversion Items via automation May 12, 2018
@murrant murrant force-pushed the laravel-auth branch 2 times, most recently from 77ce538 to 9bac825 Compare May 12, 2018 04:02
@murrant murrant moved this from TODO to In Progress in Laravel Conversion Items May 12, 2018
@laf
Copy link
Member

laf commented May 13, 2018

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 Unauthenticated and nothing more.

Had to delete the librenms_session cookie to allow me to get back to the login page. Hitting /logout didn't make a difference.

@laf
Copy link
Member

laf commented May 13, 2018

Second thing is the API no longer works, Get:

<!DOCTYPE html>
<html>
    <head>
        <meta charset="UTF-8" />
        <meta http-equiv="refresh" content="0;url=https://web01.lathwood.uk/login" />

        <title>Redirecting to https://web01.lathwood.uk/login</title>
    </head>
    <body>
        Redirecting to <a href="https://web01.lathwood.uk/login">https://web01.lathwood.uk/login</a>.
    </body>
</html>

@murrant murrant force-pushed the laravel-auth branch 2 times, most recently from bdca9dc to efeca09 Compare May 15, 2018 03:30
@murrant murrant changed the title Use Laravel authentication - WIP WIP - Use Laravel authentication May 15, 2018
@murrant murrant force-pushed the laravel-auth branch 2 times, most recently from df78f74 to 6c1201b Compare May 31, 2018 05:05
@murrant murrant changed the title WIP - Use Laravel authentication Use Laravel authentication May 31, 2018
@murrant murrant added WebUI Needs Testing Waiting for others to verify that the code functions properly. labels May 31, 2018
@murrant
Copy link
Member Author

murrant commented Jun 1, 2018

Ready for testing. Still have some small issues to work out but I think it is all there.

@theherodied
Copy link
Contributor

@murrant Using AD auth and getting `Unhandled MySQL Error [42S22] SQLSTATE[42S22]: Column not found: 1054 Unknown column 'auth_type' in 'where clause'

@theherodied
Copy link
Contributor

Removed the PR and reapplied it. It's working now.

@murrant
Copy link
Member Author

murrant commented Jun 8, 2018

Yeah, you will need to apply the sql schema update to use it.

@laf
Copy link
Member

laf commented Jun 10, 2018

I'm not running mysql strict here but I get:

Unhandled MySQL Error [23000] SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'can_modify_passwd' cannot be null

When trying to edit an existing user.

@laf
Copy link
Member

laf commented Jun 10, 2018

Delete user redirects to dashboard: deluser?action=del&id=30 - missing .php.

@laf
Copy link
Member

laf commented Jun 10, 2018

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.

@laf
Copy link
Member

laf commented Jun 10, 2018

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.

@laf
Copy link
Member

laf commented Jun 13, 2018

@TheMysteriousX if you get chance, could you test this PR with SSO?

@murrant
Copy link
Member Author

murrant commented Jun 13, 2018

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.

@TheMysteriousX
Copy link
Contributor

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.

@murrant
Copy link
Member Author

murrant commented Jun 14, 2018

Mysql error on edit fixed.
deleting user works for me, it should not end in .php
restrict polling alert to devices the user can access (or should it only be admins?) Also, should it be on every page like it is now?
I have not looked into sso or authorizers.

I noticed something odd going on with dashboards... phpdebugbar I think, will check it later.

@murrant
Copy link
Member Author

murrant commented Aug 28, 2018

@TheMysteriousX You didn't run composer install

@TheMysteriousX
Copy link
Contributor

TheMysteriousX commented Aug 28, 2018

Is there a dependency that hasn't made it into composer.json?

Component | Version
--------- | -------
LibreNMS  | 1.42.01-92-g29549f6
DB Schema | 261
PHP       | 7.1.18
MySQL     | 5.5.56-MariaDB
RRDTool   | 1.4.8
SNMP      | NET-SNMP 5.7.2
====================================

[OK]    Composer Version: 1.7.2
[OK]    Dependencies up-to-date.
[OK]    Database connection successful
[OK]    Database schema correct
[WARN]  Your local git branch is not master, this will prevent automatic updates.
	[FIX] You can switch back to master with git checkout master
[WARN]  Your local git contains modified files, this could prevent automatic updates.
	[FIX] You can fix this with ./scripts/github-remove
	Modified Files:
$ ./scripts/composer_wrapper.php install
> LibreNMS\ComposerHelper::preInstall
Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Nothing to install or update
Generating autoload files
> LibreNMS\ComposerHelper::postInstall
> Illuminate\Foundation\ComposerScripts::postInstall
> php artisan optimize
Generating optimized class loader
The compiled services file has been removed.

@murrant
Copy link
Member Author

murrant commented Aug 28, 2018

@TheMysteriousX I think sometimes the autoloader just needs to be bumped.

@TheMysteriousX
Copy link
Contributor

Apologies, I was unclear - validation passes and I've run composer install, and this still PR breaks my install regardless of auth method used.

@murrant
Copy link
Member Author

murrant commented Aug 29, 2018

@TheMysteriousX I can't reproduce the issue. Please try php artisan cache:clear.

@TheMysteriousX
Copy link
Contributor

TheMysteriousX commented Aug 29, 2018

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.

@TheMysteriousX
Copy link
Contributor

Just reproduced this using the librenms docker image here: https://hub.docker.com/r/jarischaefer/docker-librenms/

root@librenms:/opt/librenms# ./validate.php
====================================
Component | Version
--------- | -------
LibreNMS  | 1.42.01-97-ge603fcf
DB Schema | 263
PHP       | 7.2.8-1+ubuntu16.04.1+deb.sury.org+1
MySQL     | 5.6.41
RRDTool   | 1.5.5
SNMP      | NET-SNMP 5.7.3
====================================

[OK]    Composer Version: 1.7.2
[OK]    Dependencies up-to-date.
[OK]    Database connection successful
[OK]    Database schema correct
[WARN]  You have not added any devices yet.
	[FIX] You can add a device in the webui or with ./addhost.php
[WARN]  IPv6 is disabled on your server, you will not be able to add IPv6 devices.
[WARN]  Your local git branch is not master, this will prevent automatic updates.
	[FIX] You can switch back to master with git checkout master
[WARN]  Your local git contains modified files, this could prevent automatic updates.
	[FIX] You can fix this with ./scripts/github-remove
	Modified Files:

Error is exactly the same - librenms isn't even installed yet:

[2018-08-29 16:35:25] production.ERROR: Symfony\Component\Debug\Exception\FatalThrowableError: Type error: Argument 1 passed to Illuminate\Cookie\Middleware\EncryptCookies::encrypt() must be an instance of Symfony\Component\HttpFoundation\Response, instance of Wpb\String_Blade_Compiler\View given, called in /opt/librenms/vendor/laravel/framework/src/Illuminate/Cookie/Middleware/EncryptCookies.php on line 59 in /opt/librenms/vendor/laravel/framework/src/Illuminate/Cookie/Middleware/EncryptCookies.php:123
Stack trace:
#0 /opt/librenms/vendor/laravel/framework/src/Illuminate/Cookie/Middleware/EncryptCookies.php(59): Illuminate\Cookie\Middleware\EncryptCookies->encrypt(Object(Wpb\String_Blade_Compiler\View))
#1 /opt/librenms/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(148): Illuminate\Cookie\Middleware\EncryptCookies->handle(Object(Illuminate\Http\Request), Object(Closure))
#2 /opt/librenms/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#3 /opt/librenms/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(102): Illuminate\Routing\Pipeline->Illuminate\Routing\{closure}(Object(Illuminate\Http\Request))
#4 /opt/librenms/vendor/laravel/framework/src/Illuminate/Routing/Router.php(574): Illuminate\Pipeline\Pipeline->then(Object(Closure))
#5 /opt/librenms/vendor/laravel/framework/src/Illuminate/Routing/Router.php(533): Illuminate\Routing\Router->runRouteWithinStack(Object(Illuminate\Routing\Route), Object(Illuminate\Http\Request))
#6 /opt/librenms/vendor/laravel/framework/src/Illuminate/Routing/Router.php(511): Illuminate\Routing\Router->dispatchToRoute(Object(Illuminate\Http\Request))
#7 /opt/librenms/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(176): Illuminate\Routing\Router->dispatch(Object(Illuminate\Http\Request))
#8 /opt/librenms/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(30): Illuminate\Foundation\Http\Kernel->Illuminate\Foundation\Http\{closure}(Object(Illuminate\Http\Request))
#9 /opt/librenms/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(30): Illuminate\Routing\Pipeline->Illuminate\Routing\{closure}(Object(Illuminate\Http\Request))
#10 /opt/librenms/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(148): Illuminate\Foundation\Http\Middleware\TransformsRequest->handle(Object(Illuminate\Http\Request), Object(Closure))
#11 /opt/librenms/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#12 /opt/librenms/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(30): Illuminate\Routing\Pipeline->Illuminate\Routing\{closure}(Object(Illuminate\Http\Request))
#13 /opt/librenms/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(148): Illuminate\Foundation\Http\Middleware\TransformsRequest->handle(Object(Illuminate\Http\Request), Object(Closure))
#14 /opt/librenms/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#15 /opt/librenms/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ValidatePostSize.php(27): Illuminate\Routing\Pipeline->Illuminate\Routing\{closure}(Object(Illuminate\Http\Request))
#16 /opt/librenms/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(148): Illuminate\Foundation\Http\Middleware\ValidatePostSize->handle(Object(Illuminate\Http\Request), Object(Closure))
#17 /opt/librenms/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#18 /opt/librenms/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/CheckForMaintenanceMode.php(46): Illuminate\Routing\Pipeline->Illuminate\Routing\{closure}(Object(Illuminate\Http\Request))
#19 /opt/librenms/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(148): Illuminate\Foundation\Http\Middleware\CheckForMaintenanceMode->handle(Object(Illuminate\Http\Request), Object(Closure))
#20 /opt/librenms/vendor/laravel/framework/src/Illuminate/Routing/Pipeline.php(53): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#21 /opt/librenms/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(102): Illuminate\Routing\Pipeline->Illuminate\Routing\{closure}(Object(Illuminate\Http\Request))
#22 /opt/librenms/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(151): Illuminate\Pipeline\Pipeline->then(Object(Closure))
#23 /opt/librenms/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(116): Illuminate\Foundation\Http\Kernel->sendRequestThroughRouter(Object(Illuminate\Http\Request))
#24 /opt/librenms/html/index.php(53): Illuminate\Foundation\Http\Kernel->handle(Object(Illuminate\Http\Request))
#25 {main}

@murrant
Copy link
Member Author

murrant commented Aug 30, 2018

@TheMysteriousX
Thanks found the problem. When APP_DEBUG is disabled and there is no existing session and you visit a url besides /login, it will trigger.

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)

@TheMysteriousX
Copy link
Contributor

All working with fine for me with the latest changes.

TheMysteriousX
TheMysteriousX previously approved these changes Aug 30, 2018
@murrant
Copy link
Member Author

murrant commented Aug 30, 2018

@TheMysteriousX what authentication methods did you test? Also a huge thanks for testing :)

@murrant murrant mentioned this pull request Aug 30, 2018
1 task
@TheMysteriousX
Copy link
Contributor

I tested MySQL and SSO - everything seemed to work as expected, including editing, creation, password resets and all that.

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.
@murrant murrant merged commit 32a7c50 into librenms:master Sep 11, 2018
Laravel Conversion Items automation moved this from In Progress to Done Sep 11, 2018
@paulocoimbrati
Copy link
Contributor

paulocoimbrati commented Sep 12, 2018

Authentication using LDAP not working here.

Component Version
LibreNMS 1.43-57-g31e931d8e
DB Schema 267
PHP 7.0.27-0+deb9u1
MySQL 10.1.26-MariaDB-0+deb9u1
RRDTool 1.6.0
SNMP NET-SNMP 5.7.3

Although, LDAP server is OK, as you can see:

./scripts/auth_test.php -u user
Authenticate user user:
AUTH SUCCESS

@titanismo
Copy link

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
[2018-09-13 17:21:48] production.ERROR: 2 /opt/librenms/LibreNMS/Component.php:162 ksort() expects parameter 1 to be array, null given
[2018-09-13 17:21:48] production.ERROR: 2 /opt/librenms/LibreNMS/Component.php:163 Illegal offset type
[2018-09-13 17:21:48] production.ERROR: 2 /opt/librenms/LibreNMS/Component.php:163 ksort() expects parameter 1 to be array, null given

I am using old auth_method.

@murrant murrant deleted the laravel-auth branch September 13, 2018 14:52
@murrant
Copy link
Member Author

murrant commented Sep 13, 2018

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

@librenms librenms locked as off-topic and limited conversation to collaborators Sep 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Needs Testing Waiting for others to verify that the code functions properly. WebUI
Projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants