Skip to content

Commit 288ee1a

Browse files
DaneEverittDaneEveritt
committed
Improved TOTp handling in login.
Cleaned up the code a bit, also checks TOTP before attemping to verify user. This addresses the potential for an attacker to try at a password and/or confirm that the password is correct unless they have a valid TOTP code for the request. A failed TOTP response will trigger a throttle count on the login as well.
1 parent 7345385 commit 288ee1a

5 files changed

Lines changed: 113 additions & 101 deletions

File tree

‎app/Http/Controllers/Auth/AuthController.php‎

Lines changed: 69 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44

55
use Pterodactyl\Models\User;
66

7-
use Validator;
87
use Auth;
8+
use Alert;
9+
use Validator;
910

1011
use Pterodactyl\Http\Controllers\Controller;
1112
use PragmaRX\Google2FA\Google2FA;
@@ -28,73 +29,6 @@ class AuthController extends Controller
2829

2930
use AuthenticatesAndRegistersUsers, ThrottlesLogins;
3031

31-
/**
32-
* Handle a login request to the application.
33-
*
34-
* @param \Illuminate\Http\Request $request
35-
* @return \Illuminate\Http\Response
36-
*/
37-
public function postLogin(Request $request)
38-
{
39-
$this->validate($request, [
40-
$this->loginUsername() => 'required', 'password' => 'required',
41-
]);
42-
43-
$throttles = $this->isUsingThrottlesLoginsTrait();
44-
45-
if ($throttles && $this->hasTooManyLoginAttempts($request)) {
46-
return $this->sendLockoutResponse($request);
47-
}
48-
49-
$credentials = $this->getCredentials($request);
50-
51-
if (Auth::attempt($credentials, $request->has('remember'))) {
52-
if(User::select('id')->where('email', $request->input('email'))->where('use_totp', 1)->exists()) {
53-
$validator = Validator::make($request->all(), [
54-
'totp_token' => 'required|numeric'
55-
]);
56-
57-
if($validator->fails()) {
58-
Auth::logout();
59-
return redirect('auth/login')->withErrors($validator)->withInput();
60-
}
61-
62-
$google2fa = new Google2FA();
63-
64-
if($google2fa->verifyKey(User::where('email', $request->input('email'))->first()->totp_secret, $request->input('totp_token'))) {
65-
return $this->handleUserWasAuthenticated($request, $throttles);
66-
} else {
67-
Auth::logout();
68-
$validator->errors()->add('field', trans('validation.welcome'));
69-
return redirect('auth/login')->withErrors($validator)->withInput();
70-
}
71-
} else {
72-
return $this->handleUserWasAuthenticated($request, $throttles);
73-
}
74-
}
75-
76-
if ($throttles) {
77-
$this->incrementLoginAttempts($request);
78-
}
79-
80-
return redirect($this->loginPath())
81-
->withInput($request->only($this->loginUsername(), 'remember'))
82-
->withErrors([
83-
$this->loginUsername() => $this->getFailedLoginMessage(),
84-
]);
85-
}
86-
87-
/**
88-
* Check if the provided user has TOTP enabled.
89-
*
90-
* @param \Illuminate\Http\Request $request
91-
* @return \Illuminate\Http\Response
92-
*/
93-
public function checkTotp(Request $request)
94-
{
95-
return response()->json(User::select('id')->where('email', $request->input('email'))->where('use_totp', 1)->first());
96-
}
97-
9832
/**
9933
* Post-Authentication redirect location.
10034
*
@@ -121,7 +55,7 @@ public function checkTotp(Request $request)
12155
*
12256
* @var integer
12357
*/
124-
protected $maxLoginAttempts = 5;
58+
protected $maxLoginAttempts = 3;
12559

12660
/**
12761
* Create a new authentication controller instance.
@@ -162,4 +96,70 @@ protected function create(array $data)
16296
]);
16397
}
16498

99+
/**
100+
* Handle a login request to the application.
101+
*
102+
* @param \Illuminate\Http\Request $request
103+
* @return \Illuminate\Http\Response
104+
*/
105+
public function postLogin(Request $request)
106+
{
107+
108+
$this->validate($request, [
109+
'email' => 'required|email',
110+
'password' => 'required',
111+
]);
112+
113+
$throttled = $this->isUsingThrottlesLoginsTrait();
114+
if ($throttled && $this->hasTooManyLoginAttempts($request)) {
115+
return $this->sendLockoutResponse($request);
116+
}
117+
118+
$G2FA = new Google2FA();
119+
$user = User::select('use_totp', 'totp_secret')->where('email', $request->input($this->loginUsername()))->first();
120+
121+
// Verify TOTP Token was Valid
122+
if($user->use_totp === 1) {
123+
if(!$G2FA->verifyKey($user->totp_secret, $request->input('totp_token'))) {
124+
125+
if ($throttled) {
126+
$this->incrementLoginAttempts($request);
127+
}
128+
129+
Alert::danger(trans('auth.totp_failed'))->flash();
130+
return redirect()->route('auth.login')->withInput($request->only('email', 'remember'));
131+
132+
}
133+
}
134+
135+
// Attempt to Login
136+
if (Auth::attempt([
137+
'email' => $request->input('email'),
138+
'password' => $request->input('password')
139+
], $request->has('remember'))) {
140+
return $this->handleUserWasAuthenticated($request, $throttled);
141+
}
142+
143+
if ($throttled) {
144+
$this->incrementLoginAttempts($request);
145+
}
146+
147+
return redirect()->route('auth.login')
148+
->withInput($request->only('email', 'remember'))
149+
->withErrors([
150+
'email' => $this->getFailedLoginMessage(),
151+
]);
152+
}
153+
154+
/**
155+
* Check if the provided user has TOTP enabled.
156+
*
157+
* @param \Illuminate\Http\Request $request
158+
* @return \Illuminate\Http\Response
159+
*/
160+
public function checkTotp(Request $request)
161+
{
162+
return response()->json(User::select('id')->where('email', $request->input('email'))->where('use_totp', 1)->first());
163+
}
164+
165165
}

‎app/Http/Routes/AuthRoutes.php‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,22 @@ class AuthRoutes {
1010

1111
public function map(Router $router) {
1212
$router->group(['prefix' => 'auth'], function () use ($router) {
13+
1314
$router->get('login', [ 'as' => 'auth.login', 'uses' => 'Auth\AuthController@getLogin' ]);
14-
$router->post('login/totp', [ 'as' => 'auth.login.totp', 'uses' => 'Auth\AuthController@checkTotp' ]);
15-
$router->post('login', [ 'as' => 'auth.login.submit', 'uses' => 'Auth\AuthController@postLogin' ]);
15+
$router->post('login', [ 'uses' => 'Auth\AuthController@postLogin' ]);
16+
$router->post('login/totp', [ 'uses' => 'Auth\AuthController@checkTotp' ]);
17+
1618

1719
$router->get('password', [ 'as' => 'auth.password', 'uses' => 'Auth\PasswordController@getEmail' ]);
1820
$router->post('password', [ 'as' => 'auth.password.submit', 'uses' => 'Auth\PasswordController@postEmail' ], function () {
1921
return redirect('auth/password')->with('sent', true);
2022
});
21-
23+
$router->post('password/verify', [ 'uses' => 'Auth\PasswordController@postReset' ]);
2224
$router->get('password/verify/{token}', [ 'as' => 'auth.verify', 'uses' => 'Auth\PasswordController@getReset' ]);
23-
$router->post('password/verify', [ 'as' => 'auth.verify.submit', 'uses' => 'Auth\PasswordController@postReset' ]);
2425

2526
$router->get('logout', [ 'as' => 'auth.logout', 'uses' => 'Auth\AuthController@getLogout' ]);
27+
2628
});
2729
}
2830

29-
}
31+
}

‎resources/lang/en/auth.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,6 @@
2121
'sendlink' => 'Send Password Reset Link',
2222
'emailsent' => 'Your password reset email is on its way.',
2323
'remeberme' => 'Remeber Me',
24+
'totp_failed' => 'The TOTP token provided was invalid. Please ensure that the token generated by your device was valid.'
2425

2526
];

‎resources/lang/en/strings.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
'password' => 'Password',
1313
'email' => 'Email',
1414
'whoops' => 'Whoops',
15-
'failed' => 'Your request could not be processed. Please try again later.',
1615
'success' => 'Success',
1716
'location' => 'Location',
1817
'node' => 'Node',

‎resources/views/auth/login.blade.php‎

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@
2424
</ul>
2525
</div>
2626
@endif
27+
@foreach (Alert::getMessages() as $type => $messages)
28+
@foreach ($messages as $message)
29+
<div class="alert alert-{{ $type }} alert-dismissable" role="alert">
30+
<button type="button" class="close" data-dismiss="alert" aria-label="Close"><span aria-hidden="true">&times;</span></button>
31+
{{ $message }}
32+
</div>
33+
@endforeach
34+
@endforeach
2735
<div class="form-group">
2836
<label for="email" class="control-label">{{ trans('strings.email') }}</label>
2937
<div>
@@ -76,33 +84,35 @@
7684
<div class="col-md-3"></div>
7785
<script type="text/javascript">
7886
$(document).ready(function() {
79-
$("#login-form").one("submit", function(event) {
87+
$('#login-form').submit(function (event) {
8088
event.preventDefault();
81-
var check_email = $("#email").val();
82-
$.ajax({
83-
type: 'POST',
84-
url: '/auth/login/totp',
85-
data: {
86-
email: check_email,
87-
_token: '{!! csrf_token() !!}'
88-
}
89-
}).done(function(data) {
90-
if (typeof data.id !== 'undefined') {
91-
$("#openTOTP").modal('show');
92-
$('#openTOTP').on('shown.bs.modal', function() {
93-
$("#totp_token").focus();
94-
});
95-
} else {
96-
$("#login-form").submit();
97-
}
98-
}).fail(function(jqXHR) {
99-
alert("{{ trans('strings.failed') }}");
100-
});
101-
});
102-
$("#totp-form").submit(function() {
103-
$('#login-form :input').not(':submit').clone().hide().appendTo('#totp-form');
104-
return true;
105-
});
89+
var check_email = $('#email').val();
90+
$.ajax({
91+
type: 'POST',
92+
url: '/auth/login/totp',
93+
headers: {
94+
'X-CSRF-TOKEN': '{{ csrf_token() }}'
95+
},
96+
data: {
97+
email: check_email
98+
}
99+
}).done(function (data) {
100+
if (typeof data.id !== 'undefined') {
101+
$('#openTOTP').modal('show');
102+
$('#openTOTP').on('shown.bs.modal', function() {
103+
$('#totp_token').focus();
104+
});
105+
} else {
106+
$('#login-form').submit();
107+
}
108+
}).fail(function (jqXHR) {
109+
alert('Unable to validate potential TOTP need.');
110+
console.error(jqXHR);
111+
});
112+
});
113+
$('#totp-form').submit(function () {
114+
return $('#login-form :input').not(':submit').clone().hide().appendTo('#totp-form');
115+
});
106116
});
107117
</script>
108118
@endsection

0 commit comments

Comments
 (0)