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

Feature/blowfish #1

Open
wants to merge 23 commits into
base: feature/Blowfish
Choose a base branch
from
Open

Feature/blowfish #1

wants to merge 23 commits into from

Conversation

lennartvdd
Copy link

This commit updates your branch to current mishamx/master and fixes some compatibility issues, adds security improvements and a small bugfixes. These commits should also make it easier for mishamx to merge your branch into his master, so they can be released to the publicly available version of Yii-user

Changes:

  • Replaced Randomness extension (Unix only) with a more generally compatible static function in the User model for Blowfish salt generation
  • Added $2y$ Blowfish mode to deal with potential high-bit attacks for PHP versions >= 5.3.7
  • Fix fatal error, getting property of non object, when WebUser->id not (yet?) set.
  • Generate URL safe activation key (activation fails when blowfish is used)

mishamx and others added 23 commits July 2, 2012 23:58
* Added a salt column to the user table.
* Adds a salt to users that have no salted password upon succesfull login.
* Salts are now more unique/random

* Bug fixed:
This commit fixes a bug where a password could by stored to the database
in plaintext format by the AdminController->actionUpdate() function,
when an admin would supply an unencrypted version of the old password
(effectively changing the password to itself, but skipping the encryption
methods).
Fix lastvisit_at update on login
…ted_passwords

Conflicts:
	controllers/RegistrationController.php
	models/User.php
Conflicts:
	UserModule.php
	components/UserIdentity.php
	controllers/RegistrationController.php
	models/User.php
	models/UserChangePassword.php
* Replaced Randomness extension (Unix only) with a more generally compatible static function in the User model for Blowfish salt generation
* Added $2y$ Blowfish mode to deal with potential high-bit attacks for PHP versions >= 5.3.7
* Fix a small typo causing fatal error:wq
@lennartvdd
Copy link
Author

Ow.. it seems I wasn't paying enough attention. My feature/salted_passwords has also been merged in this pull. Let me know if this is a problem and I'll give you a new pull request without it..

@Alan01252
Copy link
Owner

Thanks Lennartvss, I'll review asap.

@Alan01252
Copy link
Owner

Hi lennartvdd. Sorry for the delay, this is taking a little longer than I thought for two reasons.

  1. This is the first time I've done a pull request via github
  2. There's quite a few changes and as I use this fork in some of my live sites I want to make sure everything works before merge.

Apologies again for the delay.

@lennartvdd
Copy link
Author

No worries about the delay, it's your repo, so you have full right to with it what or when you want ;)

I just liked your blowfish implementation and hope to see it in the next release (if he ever merges any of the pull requests). This way should be easier for him. Just made the mistake of joinig this pull request with my feature/salt branch, which in retrospect is going to do the exact opposite of making it easy to merge them seperately.

The upside is that both are branches are compatible this way though..

Let me know if you have any more questions

if (isset($find)&&$find->status) {
$this->render('/user/message',array('title'=>UserModule::t("User activation"),'content'=>UserModule::t("You account is active.")));
} elseif(isset($find->activkey) && ($find->activkey==$activkey)) {
$find->activkey = UserModule::encrypting(microtime());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Todo: fix security risk here. Guessing the activation key becomes possible after the user activates his account if you have knowledge of when the account was last activated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants