Can this class can be refactored to be more efficient? I've taken tips from other people who have already helped me refactor it a fair amount, but I was wondering if there was any last minute touch-ups that I could apply.
class LoginController extends Controller
{
public function getLoginView() {
return view('frontend.guest.login');
}
public function onLoginPost(Request $request) {
$validator = Validator::make($request->all(), [
'login_email' => 'required|email|exists:users,mail',
'login_password' => 'required'
]);
Session::put('last_message_for', 'login');
if ( $validator->fails()) {
return Redirect::back()->withErrors($validator->messages());
}
if (!Auth::attempt(['mail' => $request->input('login_email'), 'password' => $request->input('login_password')])) {
$this->addNewWebsiteLogin($request, User::where('mail', $request->input('login_email'))->pluck('id')->first(), "0");
return Redirect::back()->withMessage('Opps, you entered an incorrect login.')->withColor('warning');
}
$user = Auth::user();
$permissions = PlatformSetting::findSetting('website.login.required_permissions');
if (!$user->roleplayExists()) {
Auth::logout();
return Redirect::back()->withMessage('Opps, we suggest you contact support.')->withColor('warning');
}
if (strlen($permissions) > 0 && !$user->hasAnyPermissions($permissions)) {
Auth::logout();
return Redirect::back()->withMessage('Opps, you don\'t have permission to login.')->withColor('warning');
}
$this->addNewWebsiteLogin($request, $user->id, "1");
$country = $user->getCountry();
$is_different_ip = (!empty(Auth::user()->ip_last) && $user->ip_last != $request->ip());
if ($rpInfo->lock_account_on_different_ip && $is_different_ip) {
$user->lockAccount("Detected a login from a different IP address.");
}
$is_different_country = (!empty(Auth::user()->last_country) && Auth::user()->last_country != $country);
if ($rpInfo->lock_account_on_different_country && $is_different_country) {
$user->lockAccount("Detected a login from a different country.");
}
if (!$user->is_locked) {
if (($user->website_pin_selection == 'different_ip' && $is_different_ip) || ($user->website_pin_selection == 'different_country' && $is_different_country)) {
$user->pin_lock = '1';
}
else if ($user->website_pin_selection == 'every_login') {
$user->pin_lock = '1';
}
}
// We do this because otherwhys, when the legit owner logs in he'll have trouble.
// He'll probably get his account locked, because its set to the attackers country/ip.
// We'll set the ip_last and last_country on every pin successful enter, to be safe.
if (!$user->is_locked && !$user->pin_lock) {
$user->ip_last = $request->ip();
$user->last_country = $country;
}
$user->save();
return redirect()->route('frontend.user.home');
}
private function addNewWebsiteLogin(Request $request, $userId, $status) {
$agent = new Agent();
$loginRequest = new LoginRequest;
$loginRequest->user_id = $userId;
$loginRequest->password_tried = ($status == '0' ? $request->input('login_password') : '');
$loginRequest->request_ip = $request->ip();
$loginRequest->request_device = $agent->isDesktop() ? 'Desktop' : ($agent->isMobile() ? 'Mobile' : 'Tablet');
$loginRequest->request_system = $agent->platform() . ' ' . $agent->version($agent->platform());
$loginRequest->request_browser = $agent->browser();
$loginRequest->request_successful = $status;
$loginRequest->save();
}
}
I'm also a bit worried about this function, which is called in LoginController as it could be time consuming or it could be down:
public function getCountry() {
return json_decode(file_get_contents("http://freegeoip.net/json/" . request()->ip()))->country_name;
}
I'm not really sure what's best, return an empty country or have a backup API link or what. Can anyone suggest anything?