valorin / pwned-validator

Super simple Laravel Validator for checking password via the Pwned Passwords service of Have I Been Pwned
MIT License
384 stars 24 forks source link

Validator not working? #14

Closed kenneth-youngLA closed 3 years ago

kenneth-youngLA commented 3 years ago

Hi, I'm trying to use this dependency, but it doesn't seem to do anything for me. This is my code:

protected function validator(array $data){ return Validator::make($data, [ 'email' => 'required|string|email|max:255', 'password' => 'required|string|min:12|max:128|pwned:3', ]); }

`$validation = $this->validator($request->all());

    if ($validation->fails()){
        return response(['errors' => $validation->errors()->all()]);
    }

    $loginData = $request->validate([
        'email' => 'email|required',
        'password' => 'required'
    ]);

    if (!auth()->attempt($loginData)) {
        return response(['message' => 'Invalid Credentials']);
    }

    $accessToken = auth()->user()->createToken('authToken')->accessToken;

    return response(['user' => auth()->user(), 'accessToken' => $accessToken]);`

I tried testing with postman, and the login works even though I am using a password that I checked is breached over 600 times through the pwned website.

What am I doing wrong?

valorin commented 3 years ago

Your code is slightly hard to read given the formatting stuffed up at he start, however nothing jumps out at me immediately.

I'd suggest debugging first of your other validator rules are being checked - try intentionally breaking the other rules - i.e. send a non-email username, or a password longer than 128 chars. See if you can get it to fail on anything else.

Or maybe dump the password string value to confirm it's being passed in correctly - some extra characters could be ending up on it, causing it to be unique, rather than the 600-times breached one.

Nothing else jumps out at me at this point for why it might not be working.

kenneth-youngLA commented 3 years ago

First off, I appreciate you taking the time to reply to me :)

Maybe a gist is more readable

gist code

Non email rule correctly gives error response image

Password with 128+ characters correctly gives error response image

I also tried with no password and correctly said that it required a password.

Here is the password I entered and it logs me in, even with the validator

image

kenneth-youngLA commented 3 years ago

I have been working on figuring out a solution all day, and what I came up with was creating a rule myself using your same code to log out data and see what was happening... Basically, the curl GET request to the pwned API always came up empty no matter what. I changed from curl to guzzle and removed the carbon cache and I started to get replies from the API. I also had to get an ssl certification. Hope this helps debugging

valorin commented 3 years ago

Ah, interesting. The fact that raw cURL didn't work but Guzzle did, and the Certificate suggests your server might be missing some dependencies to support the HTTPS connection to Pwned Passwords out-of-the-box. Guzzle must include some extras to make the requests more reliable. I didn't use Guzzle to avoid version conflicts, but I didn't consider curl to have more limited support. 🤔

I'd suggest talking to your web host about it. Maybe make a demo script that runs curl against the API and shows the empty response and take it to your host. They should be able to figure out what's going on with the server.

kenneth-youngLA commented 3 years ago

I'm currently running the code on localhost, I haven't hosted it anywhere yet. Guzzle uses curl under the box so I'm not even sure myself what's happening there.

Thank you for your time and for making this library :3

valorin commented 3 years ago

That's just weird then. Localhost dev usually has all the latest requirements. 😕 Sorry I couldn't be more help debugging the issue.

Happy for me to close this one?

kenneth-youngLA commented 3 years ago

Sure. I hope this helps others as well :)