Clean up / refactor this store() method I'm new to Laravel and trying to figure out how I can reduce/refactor this store() call in my SessionController.

Basically, the SessionController is used to authenticate a user against an LDAP server (using wells/l4-ldap-ntlm instead of the default auth driver)

On LDAP auth success, it then checks if the user has an account/profile for this webapp. If they do not, it creates it. If they already have one, it just logs them in (user/profile only created on first login).

If LDAP auth fails, it redirects back to the login page.

I know this is a lengthy method for a controller, so I want to know the best practices for:


Refactoring this code out, so the controller is cleaner, and adhering to the "single responsibility" rule.
I need to use LDAP just for authentication, but I also want to store information for this webapp that is not stored in LDAP. Right now I am creating a User and a Profile model, but I don't know a good way to "link" them to the LDAP authenticated user once logged in.




public function store()
{
    // store credentials
    $input = Input::all();

    // attempt authentication
    $attempt = Auth::attempt([
        'username' =&gt; $input['user'],
        'password' =&gt; $input['pass']
    ]);

    if ($attempt) {
        // LDAP auth success
        $un   = Auth::user()-&gt;username;
        $role = Auth::user()-&gt;type;

        // check if user exists, creating a user and profile if they do not.
        try {
            $user = User::whereUsername($un)-&gt;firstOrFail();
        } 
        catch (Illuminate\Database\Eloquent\ModelNotFoundException $e) {
            $user = User::create(['username'=&gt;$un]);
            $user-&gt;roles()-&gt;attach( $role );
            $user-&gt;createProfile();
        }

        // redirect
        return Redirect::intended('/profile')
                        -&gt;with('flash_message', 'welcome, ' . $un);
    }
    else {
        // LDAP auth failed.
        return Redirect::back()
                        -&gt;with('flash_message', 'The credentials you entered were invalid. Please try again.')
                        -&gt;withInput();
    }

}
Okay, let me get this right (I have no experience working with an LDAP): The Auth class you're first using for $attempt is actually the LDAP library you're implementing?

I don't think I understand the logic in your code. You attempt to log in to the user, and if they successfully login, you check to see if the user exists? I must be misunderstanding something.

Well, assuming you know how exactly this works, I'll just point out a couple refactor points I can see:


In terms of breaking this apart into responsibilities: your method handles both authentication and registration. I can imagine registration being handled in another method.
I doubt you need to flash forward this entire parameter: 'welcome, ' . $un. Have your Blade file check for the flash and display "welcome, ". Since you're only sending one, just pass through $un. Either that or make it an array and pass two values: type of flash, and message.
Speaking of $un. There are better variable names, give yours some meaning and quality to them.
You can manage to un-nest your code a little:



if ($attempt) {
    ...
} else {
    return Redirect...
}



This piece could really just be:

if (!$attempt) {
    return Redirect...
}

// *What used to be inside the if*