Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

in 'Getting Started' – let's NOT hash the password (it's already hashed) #5429

Closed
emjayess opened this issue Jan 23, 2024 · 3 comments
Closed
Assignees

Comments

@emjayess
Copy link

Bug discovered, here:

\App\Models\User::creating(function ($entry) {
$entry->password = \Hash::make($entry->password);
});

Summary and findings

☝🏼 In the Getting Started card guide include, (3.) prescribes this extra hashing of $entry->password inside of setupCreateOperation().

In trying and testing this, I found that a user created while this code was in place could not, in fact, log in using the provided password.

Upon further testing and inspection of both $entry->password and also request('password'), what seems to be the case is that $entry->password has already been hashed (by laravel?), and so invoking \Hash::make() on this value is hashing a hash, thus breaking this user's ability to log in with the originally provided password.

passwords

Proposed resolution:

Simply remove this recommendation or prescribed code block

  • or, annotate somehow if it is specific to the version of Laravel... is it needed in Laravel 9 and below, but not in Laravel 10??

Php, Laravel, and Backpack version(s) context / report =>

### PHP VERSION:
PHP 8.3.1 (cli) (built: Dec 21 2023 17:49:39) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.3.1, Copyright (c) Zend Technologies
    with Zend OPcache v8.3.1, Copyright (c), by Zend Technologies

### LARAVEL VERSION:
10.39.0.0

### BACKPACK PACKAGE VERSIONS:
backpack/backupmanager: v5.0.0
backpack/basset: 1.2.2
backpack/crud: 6.5.1
backpack/generators: v4.0.2
backpack/theme-coreuiv4: 1.1.1
Copy link

welcome bot commented Jan 23, 2024

Hello there! Thanks for opening your first issue on this repo!

Just a heads-up: Here at Backpack we use Github Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.

Backpack communication channels:

  • Bug Reports, Feature Requests - Github Issues (here);
  • Quick help (How do I do X) - Gitter Chatroom;
  • Long questions (I have done X and Y and it won't do Z wtf) - Stackoverflow, using the backpack-for-laravel tag;
  • Showing off something you've made, asking for opinion on Backpack/Laravel matters - Reddit;

Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.

Thank you!

--
Justin Case
The Backpack Robot

@karandatwani92
Copy link
Contributor

Hey @emjayess

Thanks for reporting your findings & proposing a solution. I appreciate your efforts and time.
My will colleague @pxpm will review it.

Cheers!

@pxpm
Copy link
Contributor

pxpm commented May 20, 2024

Sorry @emjayess it took me so much time to get back here, I totally missed this. 😞

I've just fixed it in backpack/crud:6.7.14

Thanks for the heads up 🙏

@pxpm pxpm closed this as completed May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

3 participants