Let's Discuss Best Practices

So being new to Yii and reading through the various docs to learn what I’m trying to achieve, and looking up examples/tuts I had this thought. What’s the best practice when it comes to models within the Yii framework.

What’s the best practice for handling forms? For instance when first installed, there’s a LoginForm model that handles log in. So with that in mind I’ve added a RegistrationForm model. It’s kind of small though, aside from the rules to make sure all data is valid, it only has one other function called ‘register’. The only purpose that function has is this


public function register () {

	if(!$this->tos_agreed) {

		return false;

	}

	// pretty sure this could be done way better, but it works. Lol

	$user = new User(['scenario' => User::SCENARIO_REGISTER]);

	

	$salt = User::generateToken();

		

		

	$user->username = $this->username;

	$user->password = User::encryptPassword($this->password, $salt);

	$user->salt = $salt;

	$user->email = $this->email;

	$user->last_login_on = new Expression('CURRENT_TIMESTAMP');

	$user->tos_agreed_on = new Expression('CURRENT_TIMESTAMP');

	$user->last_login_ip = Yii::$app->request->userIP;

	$user->accessToken = User::generateToken();

	

	$saved = $user->save();

	

	return $saved;

}

But the question is are these ‘form’ models even necessary/best practice? This could all technically just be done in the User model. Though I read on one of the doc pages that if processing needs to be done it should use another model.

So what’s the best practice between models and forms. For instance the User model, should there be an in between between that and the login / registration form? I have another form that allows users to submit data, but then later that data can be edited. Should it use a model for the form that calls the model that deals with the DB? If so that seems a little convoluted when it comes to re-populating the form field.

So thoughts & opinions?

I just use the User model directly. Registration = create user record.

Also I’m not sure what you’re doing with the salt/password stuff, but I hope you’re not implementing your own logic for that. Best to stick with known, secure functions.

http://www.yiiframework.com/doc-2.0/guide-security-passwords.html

(uses php’s password_hash() function)

See. That’s kind of what I was initially thinking, but I read somewhere in the docs that it says for processing a separate model should be used.

We generate a salt and the accessToken, and potentially any other tokens, via random_bytes, because PHP says it’s cryptographically secure.


bin2hex(random_bytes($bytes))

Then take that along with crypt type SHA512 and pass it with the password pass it to crypt. We don’t implement our own logic. Lol. That would be silly. We do keep the salts stored independently from the encrypted password though, that’s why it’s its own field and why we don’t use password_hash() because password_has() returns the salt with the password as a single string.

You do implement your own logic. Yii’s Security component uses random_bytes and bcrypt in the right way internally so it’s better to rely on that except if you see any flaws in it. Not sure why you’re storing salt separately. Doesn’t make sense to me.

I feel this is off track of the original point of the thread. Which is to discuss the application structure. Not why we store the salt separately or not use Yii’s password encryption (which I didn’t even know was a thing).

That’s good that Yii uses random_bytes and bcrypt. However, bcrypt and SHA512 are both fine for password encryption. We already have users on an active site. We’re switching from CI to Yii2, because after researching it, at this point Yii meets more of our needs than CI. At present we started to patch together a bunch of different libraries. It made more sense to switch to a framework that has a number of things built-in that we were looking at patching together. That said, all of our user passwords are already stored as SHA512. To switch it to bcrypt we would still have to have backwards compatibility to encrypt the password as we have it now then force them to change their password so that it’s encrypted with bcrypt. Right now our focus is switching to the Yii2 framework, not changing password encryption from one highly secure method to another highly secure method.

By implementing our own logic, I meant doing our own not secure, ‘secure’ encryption; more of a pseudo-secure encryption. Like where some people use a simple hash on the password or create their own logic for generating a hash.

That said. In regards to my first post. What do you think about the application structure? Our registration page for instance has a tos_agreed checkbox. However, we store the timestamp, tos_agreed_on, of when they agreed so we can reference which was the last ToS version they agreed to. We also add their IP, encrypt password, and whatnot. amnah recommended that it be done in the User model, however, that would mean we’d have to set all that from the controller, I feel that dirties the controller. What are your thoughts about it?

OK. Thanks for explanation. Still you have one false statement about SHA512 being OK for passwords. It’s not since it’s brute-forced at very good rates using GPUs which are cheap nowadays.

Regarding your original question, a separate form for sign up sounds good for this case.

Technically you can achieve it with a single User model by using scenarios. In practice, there are issues with such approach:

  1. If you have some fields in the Signup form (password, right?) which are not in the User model you have to add extra public properties to the User model.

These extra properties make sense only for one specific case and are confusing for the rest of User model usages.

  1. Model contains code which is not about the object itself i.e. User model should not handle Signup process.

Also you’re right that putting any of such logic into controller is a bad idea.

You are right. But where do we put logic ?

1 Like

I don’t disagree with not using bcrypt and at some point in the future once we shift the framework over to Yii I’m sure we can do that. It’s just not our focus right now.

In another example. We have a Theme model, users can submit themes and other users can use them. Themes require a display/preview image. With this we do a little more “processing” since we have to use the UploadFile, then we get the image filename, IP, and stuff along with the ThemeSubmitForm and set it on the Theme model. However, a theme can be edited. From what I can tell this means we would have to take the data from the Theme model and set it on the ThemeSubmitForm model to populate the fields vs generating the fields/values from the Theme model itself, yeah? It seems convoluted, unless I’m missing something.

So in your opinion forms that do processing should be handled by a different model and then set that data on the DB model to save it, as like what I demonstrate in my first post? I mean it’s not heavy data processing and it shouldn’t go in the controller. So I feel I’m doing it right, but at the same time re-populating form fields…

It’s not that much convoluted. Of course, there’s more code and more entities but the pro is that when you’ll have more use cases for themes your primary model would stay simple as well as the forms. If you’ll do everything in a single place it would escalate quickly in case of adding more use cases. Of course, if you’re absolutely sure that there’s only one use case and if you’re willing to refactor immediately when another use case is introduced then it’s fine to have everything in a single place.

I like this explanation, it makes sense. I think for the most part it’ll be good to keep all/most forms attached to a *Form model then. Plus I’m not a big fan of regularly needing to refactor something. It’s a bit draining to keep working on the same component over and over.