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

Chore: Update the people.email to use Fields.EmailEncrypted #285

Closed
3 tasks
nelsonic opened this issue Mar 9, 2023 · 4 comments
Closed
3 tasks

Chore: Update the people.email to use Fields.EmailEncrypted #285

nelsonic opened this issue Mar 9, 2023 · 4 comments
Assignees
Labels
chore a tedious but necessary task often paying technical debt T2h Time Estimate 2 Hours tech-debt A feature/requirement implemented in a sub-optimal way & must be re-written technical A technical issue that requires understanding of the code, infrastructure or dependencies

Comments

@nelsonic
Copy link
Member

nelsonic commented Mar 9, 2023

As noted in #284 sadly, by default the mix phx.gen.auth generator
does not setup any protection for personal data in the database. 😢
Email addresses are stored as plaintext:

mix-phx-gen-auth-people-table-email-plaintext

Similarly the people_tokens table stores email addresses as plaintext in the sent_to column:

people_token-email-plaintext

This is obviously undesirable. 🙃
This is a privacy/security issue waiting to become a scandal!

Todo

  • Re-add fields to the newly re-created auth (Phoenix 1.7) app
  • Update the people and people_tokens schemas to use Fields.EmailEncrypted
  • Update tests as required

This shouldn't take very long but allocating T2h to allow for documenting the steps

@nelsonic nelsonic added chore a tedious but necessary task often paying technical debt technical A technical issue that requires understanding of the code, infrastructure or dependencies T2h Time Estimate 2 Hours tech-debt A feature/requirement implemented in a sub-optimal way & must be re-written labels Mar 9, 2023
@nelsonic nelsonic self-assigned this Mar 9, 2023
@nelsonic
Copy link
Member Author

nelsonic commented Mar 9, 2023

Getting the following error:

15:06:13.691 [info] alter table people
** (Postgrex.Error) ERROR 42804 (datatype_mismatch) column "email" cannot be cast automatically to type bytea

    hint: You might need to specify "USING email::bytea".

@nelsonic
Copy link
Member Author

nelsonic commented Mar 9, 2023

Sadly, having the person.email and person_tokens.sent_to in plaintext is relied upon by the following query:

def verify_email_token_query(token, context) do
case Base.url_decode64(token, padding: false) do
{:ok, decoded_token} ->
hashed_token = :crypto.hash(@hash_algorithm, decoded_token)
days = days_for_context(context)
query =
from token in token_and_context_query(hashed_token, context),
join: person in assoc(token, :person),
where: token.inserted_at > ago(^days, "day") and token.sent_to == person.email,
select: person
{:ok, query}
# phx.gen.auth boilerplate code not yet covered by tests ...
# coveralls-ignore-start
:error ->
:error
# coveralls-ignore-stop
end
end

I don't see us using this function in our auth implementation
because this is the flow/journey where people click on a link in an email to verify their email address
and we are definitely not doing email verification with links.

see: #223 (comment)

So I'm going to explore removing this function and seeing what the ramifications are ... 💭

@nelsonic nelsonic changed the title Chore: Update the people and people_tokens to use fields Chore: Update the people.email to use Fields.EmailEncrypted Mar 9, 2023
nelsonic added a commit that referenced this issue Mar 9, 2023
…cy/security #285 also comment out all insecure/unused code - to be removed shortly
nelsonic added a commit that referenced this issue Mar 9, 2023
…cy/security #285 also comment out all insecure/unused code - to be removed shortly
@nelsonic
Copy link
Member Author

Following the changes made in: 2bbba99

If we now run a query to view the data in the people table:

SELECT id, email, inserted_at FROM people;

We see that the email is now a binary blob
image

Registration and Login still works as expected:
image

But now the personal data captured in registration is stored encrypted at rest the way it should be. 🔐

@nelsonic
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore a tedious but necessary task often paying technical debt T2h Time Estimate 2 Hours tech-debt A feature/requirement implemented in a sub-optimal way & must be re-written technical A technical issue that requires understanding of the code, infrastructure or dependencies
Projects
Status: Done
Development

No branches or pull requests

1 participant