How to get previous value in onSaving()

[Aranir]

The onSaving() method receives two objects:

An instance and a changeSet. The problem I have is that the instance already contains the modified values which will be applied through the changeSet.

For only changing the (hashed and salted) passwords if they got modified, I would need to know the current (preSave) value of the (hashed and salted) password, compare it to the new password (from the change set) and then encrypt the new one if, and only if it got modified.

Currently I don't see a way how to retrieve the preSave value of the instance in the onSaving() function (as the _original value is private).

Any help would be appreciated.

[notheotherben]

Hi @Aranir,

As you mentioned, the _original value would be the right way to access it, however that's currently part of the private API. That being said, I don't know if your use case should need access to the original value.

To explain what I mean, let me go into the three different approaches you can take advantage of to deal with this.

Option 1: Use Mutator Methods

This is my personal approach when it comes to passwords which are hashed and salted, specifically because it prevents the password from being stored a defined variable in clear-text at any point. Granted, the password is still available in a closure throughout the execution of your methods, however it is far less likely to be accidentally leaked this way.

interface UserDoc {
  _id?: string;
  email: string;
  passwordHash: string;
}

class User extends Iridium.Instance<UserDoc, User> {
  @Iridium.ObjectID
  _id: string;

  @Iridium.Property(String)
  email: string;

  @Iridium.Property(String)
  passwordHash: string;

  checkPassword(password) {
    // Check whether the password matches the hash
    return bcrypt.compareSync(password, this.passwordHash);
  }

  changePassword(newPass) {
   this.passwordHash = bcrypt.hashSync(newPass, null);
  }
}

This approach, combined with some more restrictive validation rules on the passwordHash property, helps ensure that you don't risk exposing your password in logs, API responses etc by accident. It also does away with the need to do transparent conversion of your password, which I enjoy from the "no magic" perspective.

Option 2: Rely on a Transform

Property transforms in Iridium are executed as part of the getter/setter on an object. This means that you can rather easily use them to perform conversion of the values automatically on assignment to the property. I tend to avoid this approach since it falls into the "magic" category and doesn't make it immediately obvious what is happening, but if you're after a solution which allows you to assign new passwords to the field and have them persisted as hashes, this might be your best option.

interface UserDoc {
  _id?: string;
  email: string;
  passwordHash: string;
}

class User extends Iridium.Instance<UserDoc, User> {
  @Iridium.ObjectID
  _id: string;

  @Iridium.Property(String)
  email: string;

  @Iridium.Property(String)
  @Iridium.Transform(value => value, value => bcrypt.hashSync(value, null))
  passwordHash: string;

  checkPassword(password) {
    // Check whether the password matches the hash
    return bcrypt.compareSync(password, this.passwordHash);
  }
}

This approach takes advantage of the fact that the transform will only be executed when you assign a new value, however you need to be careful that you don't instance.passwordHash = instance.passwordHash or you might end up with some broken login credentials.

Option 3: Use the onSaving hook and the changes set

Iridium's instance save() method is actually pretty smart about how it builds up the changes object, comparing the old and new values to identify when they have changed and only including them when they have. As a result, you should be able to simply test for the presence of the $set.passwordHash key in changes.

interface UserDoc {
  _id?: string;
  email: string;
  passwordHash: string;
}

class User extends Iridium.Instance<UserDoc, User> {
  @Iridium.ObjectID
  _id: string;

  @Iridium.Property(String)
  email: string;

  @Iridium.Property(String)
  passwordHash: string;

  checkPassword(password) {
    // Check whether the password matches the hash
    return bcrypt.compareSync(password, this.passwordHash);
  }

  static onSaving(instance, changes) {
    if (changes.$set && changes.$set.passwordHash)
      changes.$set.passwordHash = bcrypt.hashSync(changes.$set.passwordHash, null)
  }
}

This approach takes advantage of the fact that $set.passwordHash will only be present if the password hash has changed (comparing _original to _modified). It then updates the changes object with the hashed password instead and allows the operation to continue on its way.

I don't particularly like this solution since at any point you don't know whether instance.passwordHash is a hash or the actual password, making for a very difficult to reason about security model and potentially leading to things like the checkPassword() method breaking.

That's just me $0.02, feel free to ask if you have any other questions.

Regards,
Benjamin

[Aranir]

Thank you Benjamin, your answer was very helpful!.

Yes i can see those are viable options.

I still think that the original values of the entry being modified would be a helpful value to have for certain situations.

Is there a specific reason why it would be harmful to have the previous value available in the onSaving function?

[notheotherben]

Hi Aranir,
I'm terribly sorry for not getting back to your last question, I had completely forgotten about this thread.

The major issue is that, for consistency's sake, it would be necessary to implement a variant of the current .document property. That has a very good chance of breaking other people's code if we choose a name that anybody else is already using. Alternatively, we could try and include the modified and original fields as part of the arguments received by onSaving(), however that only applies to changes that were made via the property settings, calls to save(changes) would then have strange results.

Similarly, those internal APIs aren't guaranteed to remain the same - the moment we expose their data via external API endpoints we constrain how much flexibility we have to make changes to the underlying data structures (for performance, consistency or functionality reasons).

I'll definitely keep your request in mind for future versions and see if I can find a nice way of handling your requirements though.