Encrypt authorization metadata#6947
Conversation
|
@ahukkanen I think the JSON field is queried in some authorization examples when matching authorizations by postal code. I think the idea was to encrypt only what was needed, manually, but leave the JSON field accessible to have useful clear values there. |
|
@mrcasals Postal code would count as a personal information. |
|
And I know the information is queried in some queries, they just have to be changed. Can you talk to @andreslucena about this? |
|
Hi @ahukkanen this is a possibly breaking change for multiple verification systems. Is there a related issue or meta-decidim proposal? |
|
@tramuntanal No, there's no Metadecidim proposal. Can you please talk to @andreslucena about this? |
|
I've already talked with @andreslucena , ok @ahukkanen we'll review |
tramuntanal
left a comment
There was a problem hiding this comment.
Hi @ahukkanen ,
I've left a couple of questions.
Also I'm noting that this PR will break code in other repos that rely on querying custom attributes in the metadata field like:
- https://github.com/AjuntamentdeBarcelona/decidim-barcelona/blob/e15d1e7cb4845aa73dcfea080113f58c052e3c12/decidim-stats/app/queries/decidim/stats/performers/age_group.rb#L28
- https://github.com/AjuntamentdeBarcelona/decidim-barcelona/blob/e15d1e7cb4845aa73dcfea080113f58c052e3c12/decidim-stats/app/queries/decidim/stats/performers/district.rb#L27
- https://github.com/AjuntamentdeBarcelona/decidim-barcelona/blob/e15d1e7cb4845aa73dcfea080113f58c052e3c12/decidim-stats/app/queries/decidim/stats/performers/gender.rb#L27
Maybe we can create an extras field of type jsonb to persist complementary data used for querying. Although we're opening the door to persist private data unencrypted.
What are your thoughts @decidim/product ?
.../app/commands/decidim/verifications/id_documents/admin/confirm_user_offline_authorization.rb
Show resolved
Hide resolved
...pp/controllers/decidim/verifications/id_documents/admin/pending_authorizations_controller.rb
Show resolved
Hide resolved
|
@tramuntanal I would personally like to avoid adding a possibility to store sensitive data unencrypted. I would not suggest storing strongly identifiable data in plain text to the database but in case you really have to, couldn't you use the |
|
Indeed @ahukkanen , I agree that it is best to have personal data encrypted, no doubt! My worries are about breaking code and other modules. |
|
@tramuntanal I have added a CHANGELOG note. |
🎩 What? Why?
The authorization metadata can contain sensitive personal information and it should be therefore encrypted.
This PR implements a new concern that provides automated encryption for active record attributes so that they will be stored encrypted in the database but when accessed through the code, they are available as plain text.
This is applied to the metadata attributes in the
Decidim::Authorizationrecord.The change is transparent and should not affect any Decidim APIs or code relying on the metadata. The only implication to existing code can be if the JSON object has been queried directly through the database. Now that the values are encrypted in the database, these kinds of queries no longer work.
Testing
Decidim::Authorization.create!(name: "testing", metadata: { foo: "bar" }, decidim_user_id: 1, unique_id: "uniqid", granted_at: DateTime.now)Decidim::Authorization.lastDecidim::Authorization.last.metadata📋 Checklist
🚨 Please review the guidelines for contributing to this repository.
docs/.