Encrypted Property is not encrypted

[matt-tolliday-iss]

Describe the bug A property that is marked as Encrypted is not being encrypted in the database.

.AddEncryptedProperty(p => p.Password)

[Column("Password")] public string Password { get; set; }

Does anything else need configuring to make sure this works?

How will un-encrypted values be treated if this is fixed and left enabled?

Steps To Reproduce Steps to reproduce the behavior:

1. .AddEncryptedProperty(p => p.Password)
2. Perform action against the entity (e.g. repo.Save(entity);)
3. Check database
4. Not encypted

Expected behavior Property is encrypted seamlessly in the database

Screenshots If applicable, add screenshots to help explain your problem.

Environment (please complete the following information):

• Umbraco Version10.3.2
• Product Version 1.6.1

[mattbrailsford]

Hi @matt-tolliday-iss

Did you attempt to save the entity before checking the database? The encryption / decryption only occurs when persisting / fetching the value from the database so simply configuring an encrypted property does not automatically encrypt existing values.

[matt-tolliday-iss]

Hi @mattbrailsford - yes this configuration is now in production and entities are being saved/retrieved - I can see the un-encrypted values in the database (unfortunately).

[mattbrailsford]

So we basically wrap the IDataProtectionProvider interface calling Protect / Unprotect on it. I'd be suprised if that wasn't working though as it's basically used by Umbraco for a bunch of other things too so I'd expect those to fail too 🤔

[matt-tolliday-iss]

Any way I could check if those are failing? I'm not aware that we have done anything with overriding that provider at all

[mattbrailsford]

I'm not quite sure.

Are you using a custom repository? or the default one? Not that I think this should matter 🤔

[matt-tolliday-iss]

Its using the default one. Are there any logs I could locate?

[mattbrailsford]

Oh, wait. Are you modifying the entity yourself and calling save via the API?

It looks like the encryption only takes place in the mapping logic which would suggest I may have only implemented it for the back office UI 🤦‍♂️

[matt-tolliday-iss]

Yeah intensively using the API to do things manually! It's a very bespoke site

[mattbrailsford]

Ok, I think that's the problem. I think I've put the logic in the wrong place and it potentially needs moving to be inside the repository.

Right now I think it's only encrypting if it is saved via the back office

[mattbrailsford]

Right, so I have a 1.6.3-beta0002 on our unstable feed at https://nuget.outfield.digital/unstable/v3/index.json which moves the encryption/decryption process out of the UI view model mappers and into the base repository implementations which should mean this now occurs even outside of UI based updates.

If you want to give this a try and see if it resolves your issue, that would be great.

[matt-tolliday-iss]

Thanks Matt, that's magic - before we pull this and test - can I check what will happen to values that are unencrypted in the database field at the moment, when we enable the encryption, will that cause an issue or would they be read okay, and encrypted back on save?

[mattbrailsford]

Good question.

Encrypted values I believe should be ok, but it may error when fetching values that aren't encrypted 🤔

If the later is the case, I'm not sure what we can do other than a try catch

[matt-tolliday-iss]

I guess there's two thoughts here:

1) if the value is encrypted somehow in another mechanism/another key that the one its trying to use, so you cannot trust that the value is actually just a "raw" value to re-encrypt it at the point of failure, then making any adjustment feels super risky to the integrity of the data value on read out

2) could you not just treat it as the raw value on read, display it in the UI for the user as raw etc, but then encrypt it on save of the entity?

[mattbrailsford]

Number 2 is really what we are doing now.

Essentially any GET method in the repo post processes the object and any property said to be encrypted is decrypted. When saving, just before the save occurs we encrypt the vaule, and then after the save decrypt it again, so from an API perspective, it always appears unencrypted.

But yea, the problem will be if the value has been encrypted in some alternative way. For this we'd probably have to make the security helper swapable so you could control the encryption method.

But the other thing is still true in that, any property that is configured as encrypted, Konstrukt will assume any value in the DB is encrytped and attempt to decrypt it, and vica versa, any value on passed on the object is assume to be unencrypted and at point of save, it is encrypted. We don' t really have a way to test "is this value encrypted?"

[matt-tolliday-iss]

I guess selfishly as long as the read "works" then my scenario will be okay - which I suppose for adding the encrypted property onto a configuration after a collection has been used for a while would work, possibly that is the majority of cases here.

Changing encryption method would be quite niche, perhaps?

[mattbrailsford]

Ok, there should be a 1.6.3-beta0007 on there shortly that has a swapable security helper, and it also performs the decryption in a try catch so if it fails decryption, it will just return the original string.

Not ideal from a performance perspective I guess, but if the goal of implementing encrypted properties is for everyhing to become encrypted then I guess eventually there shouldn't be any errors.

[matt-tolliday-iss]

Hi @mattbrailsford - updated to the beta0007 - get this error on startup - i havent changed any configuration around the security helper - should I have?

This was thrown when initialising a repo:

this._repo = factory.GetRepository<EntityDto, int>();

"Object reference not set to an instance of an object."

at Konstrukt.Persistence.DefaultKonstruktRepositoryProxy2..ctor(DefaultKonstruktRepository repository) at Konstrukt.Persistence.KonstruktRepositoryFactory.GetRepository[TEntity,TId](String collectionAlias) at Client.Umbraco.Web.Infrastructure.HostedServices.Service..ctor(IRuntimeState runtimeState, ILogger1 logger, IKonstruktRepositoryFactory factory, IOptions1 globalSettings, ISendGridClient emailSender, ICoreScopeProvider scopeProvider) in..." `

[mattbrailsford]

@matt-tolliday-iss good spot, and nope, that shouldn't require you to change anything.

I've located the issue and submitted a fix. There should be a beta0008 on that feed shortly.

[matt-tolliday-iss]

That's worked nicely for me - create entity via api/repo encrypted fine, and a previously unencrypted property loaded entity - then encrypted the unencrypted value on save of entity in back office.

Nice one thanks @mattbrailsford

[mattbrailsford]

Excellent.

Thanks for testing @matt-tolliday-iss 👍

[mattbrailsford]

Fixed in 1.6.3