18

I am a home hobbyist and am studying Laravel, currently in version 5.3. I am using a Mac, neither homestead nor vagrant.

I'm currently working on a website that uses a login and a register system to create users.

I've used php artisan migrate to manipulate my database locally.

Screen Shot 2016-12-27 at 4.18.38 PM.png

As listed below, it has three fields, namely:

  • Email
  • Username
  • Password

I have a User model (users.php):

<?php

namespace blog;

use Illuminate\Notifications\Notifiable;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Contracts\Auth\Authenticatable;

class User extends Model implements Authenticatable {
    use \Illuminate\Auth\Authenticatable;

    use Notifiable;

    protected $fillable = [
        'username', 'email', 'password',
    ];

}

And also, a UserController class (UserController.php):

<?php

namespace blog\Http\Controllers;

use Auth;
use blog\User;
use Illuminate\Http\Request;

class UserController extends Controller {

    public function postRegister(Request $request) {
        $username = $request['username'];
        $email = $request['email'];
        $password = bcrypt($request['password']);

        $user = new User();
        $user->email = $email;
        $user->username = $username;
        $user->password = $password;

        $user->save();

        return redirect()->route('login');        
    }

    public function postLogin(Request $request) {

        $credentials = [
            'username' => $request['username'],
            'password' => $request['password'],
        ];

        if(Auth::attempt($credentials)) {
            return redirect()->route('dashboard');       
        }

        return 'Failure'; 
    }
}

?>

As you can see, I am using bcrypt() as my hashing method.

However, this problem is, it will always result to a failure.

Screen Shot 2016-12-27 at 4.41.38 PM.png

I have checked the following links:

P.S. These links seem very hard to follow as I do not utilize the Input class.

Glorfindel
  • 21,988
  • 13
  • 81
  • 109
Programming_Duders
  • 291
  • 2
  • 3
  • 14
  • Laravel comes with its own [authentication controller](https://laravel.com/docs/5.3/authentication). Why don't you use it instead of writing your own? – sepehr Dec 27 '16 at 10:16
  • Your route 'login' is with GET method? Can you please attach your routes? – Marc Garcia Dec 27 '16 at 11:47

3 Answers3

14

The problem is with the way you're redirecting the user to login route after the registration. You're falsely assuming that the $request data will be accompanied with the redirect.

Let's assume this scenario: A request gets dispatched to the postRegister method with name, email and password fields. The controller creates the user and saves it into the database. Then it redirects the user, who is not yet authenticated, to the login route. The postLogin method gets triggered, but this time with no request data. As a result, Auth::attempt($credentials) fails and you get that nasty Failure on screen.

If you add a dd($credentials) right after you create the array, you'll see that it has no values:

public function postLogin(Request $request)
{
    $credentials = [
        'username' => $request['username'],
        'password' => $request['password'],
    ];

    // Dump data
    dd($credentials);

    if (Auth::attempt($credentials)) {
        return redirect()->route('dashboard');
    }

    return 'Failure';
}

It will return something like this:

array:2 [
  "username" => null
  "password" => null
]

You cannot redirect with custom request data (unless with querystring which is part of the URL), not matter what. It's not how HTTP works. Request data aside, you can't even redirect with custom headers.

Now that you know what's the root of your problem, let's see what are the options to fix it.

1. Redirect with flashed data

In case you want to preserve this structure, you need to flash the request data of postRegister() into the session (which is persistent between requests) and then retrieve it in the postLogin() method using Session facade, session() helper or the actual Illuminate\Session\SessionManager class.

Here's what I mean:
(I slightly modified your code; dropped extra variables, made it a lil bit cleaner, etc.)

public function postRegister(Request $request)
{
    // Retrieve all request data including username, email & password.
    // I assume that the data IS validated.
    $input = $request->all();

    // Hash the password
    $input['password'] = bcrypt($input['password']);

    // Create the user
    User::create($input);

    // Redirect
    return redirect()
        // To the route named `login`
        ->route('login')

        // And flash the request data into the session,
        // if you flash the `$input` into the session, you'll
        // get a "Failure" message again. That's because the 
        // password in the $input array is already hashed and 
        // the attempt() method requires user's password, not 
        // the hashed copy of it. 
        //
        ->with($request->only('username', 'password'));
}

public function postLogin(Request $request)
{
    // Create the array using the values from the session
    $credentials = [
        'username' => session('username'),
        'password' => session('password'),
    ];

    // Attempt to login the user
    if (Auth::attempt($credentials)) {
        return redirect()->route('dashboard');
    }

    return 'Failure';
}

I strongly recommend you against using this approach. This way the implementation of postLogin() method which is supposed to be responsible to login users gets coupled with session data which is not good. This way, you're not able to use postLogin independently from the postRegister.

2. Login the user right after the registration

This is a slightly better solution; If you decided that you need to log in the user right after the registration, why not just doing that?

Note that Laravel's own authentication controller does it automatically.

By the way, here's what I mean:
(Ideally this should be broken down into multiple methods, just like Laravel's own authentication controller. But it's just an example to get you started.)

public function postRegister(Request $request)
{
    $input = $request->all();

    $input['password'] = bcrypt($input['password']);

    User::create($input);

    // event(UserWasCreated::class);

    if (Auth::attempt($request->only('username', 'password'))) {
        return redirect()
            ->route('dashboard')
            ->with('Welcome! Your account has been successfully created!');
    }

    // Redirect
    return redirect()
        // To the previous page (probably the one generated by a `getRegister` method)
        ->back()
        // And with the input data (so that the form will get populated again)
        ->withInput();
}

But still, it's far from perfect! There are many other ways to tackle this. One could be using events, throwing exceptions on failure and redirecting using custom exceptions. But I'm not gonna explore them as there's already a solution perfectly designed for this.

If you want to write your own authentication controller, that's fine. You'll learn a lot along the way. But I strongly suggest reading Laravel's own authentication code, especially RegistersUsers and AuthenticatesUsers traits in order to learn from it.

And one more note; you don't need that Illuminate\Auth\Authenticatable trait in your User model as it's already extending Authenticatable which use that trait.

Community
  • 1
  • 1
sepehr
  • 17,110
  • 7
  • 81
  • 119
  • Many many thanks for everything, although solution 1 doesn't really turn out that well, and I'm facing quite a number of errors for solution 2... :( – Programming_Duders Dec 28 '16 at 16:01
  • You're welcome. What type of errors are you encountering? – sepehr Dec 28 '16 at 16:06
  • I still get a 'failure' for solution 1 (flashing), and when I did "dd" credentials was working for postRegister() but becomes null when I do it in postLogin() – Programming_Duders Dec 28 '16 at 16:07
  • That is awesome! :) – Programming_Duders Dec 28 '16 at 16:11
  • You were right; there was an issue with the first solution's code, I fixed it and explained the issue with code comments. Please test it again. The second one was working just fine. Regarding that `dd` thing, that's expected behavior; there's no request data in `postLogin`, so it's all `null`. I explained why in the answer. – sepehr Dec 28 '16 at 16:23
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/131806/discussion-between-programming-duders-and-sepehr). – Programming_Duders Dec 29 '16 at 13:46
  • what about if i use different mysql table name or model for login – Salim Murshed May 09 '22 at 15:04
4

You should hash your password everytime you insert a row bcrypt(pass). Auth::attempt assumes that the password being retrieved from the database is hashed

vachina
  • 43
  • 4
-2

Auth::attempt uses \Hash::make($someString) to generate the hash. You should use this as well in order to generate same hashes from same strings (I assume the seed is different than with bcrypt() function).

So change this line:

$password = bcrypt($request['password']);

To:

$password = \Hash::make($request['password']);
sepehr
  • 17,110
  • 7
  • 81
  • 119
DevK
  • 9,597
  • 2
  • 26
  • 48
  • This was awesome, but it still didn't solve my problem, sadly... :( – Programming_Duders Dec 27 '16 at 10:06
  • Hmm, did you try creating new user after changing the code? And are you sure the users are getting saved to the database? – DevK Dec 27 '16 at 10:08
  • Yes, I had. Users are saved in the database, I'll post an update in the question above... :) – Programming_Duders Dec 27 '16 at 10:10
  • 3
    Both are the same. The [`bcrypt` function](https://github.com/laravel/framework/blob/66976ba3f559ee6ede4cc865ea995996cd42ee1b/src/Illuminate/Foundation/helpers.php#L192) is just a helper for the same functionality. – sepehr Dec 27 '16 at 10:13
  • Oh okay. Then I have no idea what it could be. – DevK Dec 27 '16 at 10:19