[ci skip] Improve ActiveSupport::MessageVerifier and ActiveRecord::SignedId docs #52064
Conversation
feliperaul
force-pushed
the
improve_message_verifier_and_signed_id_docs
branch
from
2ad38c6 to
d92e312
Compare
|
I don't think Encrypted Ids makes sense as a framework feature. Ids should not need to be encrypted, they don't leak any sensitive information. The reason why SignedId was added was to build features like password reset links, what would be the use case for encrypting the ids? |
The documentation on ActiveSupport::MessageVerifier used the “sensitive data” string as an example; that wording might induce the developer to think we’re dealing with encryption, while the payload is actually only Base64 encoded and is not protected at all. We also improve the documentation on ActiveRecord::SignedId, which uses MessageVerifier and thereby will also expose the ID as encoded cleartext, making explicit that it’s not encryption, only signing. Lastly, we refer the developer to MessageEncryptor if the payload needs to be encrypted.
rafaelfranca
force-pushed
the
improve_message_verifier_and_signed_id_docs
branch
from
d92e312 to
ad20d9e
Compare
|
@rafaelfranca I think IDs leak somewhat sensitive information, which is the precise count of records in your database at that moment. It can be used for competitive analysis (for instance, how many new users have my competitors onboarded in the last day/week/month). Take your password reset links for instance; if I create a new user and |
|
I don't think that justify the introduction of a new feature to Rails, how many records you have in a table (which not necessarily can be found using the ids). We need to define an unique use case for the future. If we really wanted to hide the ids Rails would never generate primary keys as integers. |
…d_signed_id_docs [ci skip] Improve ActiveSupport::MessageVerifier and ActiveRecord::SignedId docs
Motivation / Background
The documentation on
ActiveSupport::MessageVerifieruses “sensitive data” string as an example of a message to sign/verify; that wording might induce the developer to think we’re dealing with encryption, while the payload is actually only Base64 encoded so it's actually cleartext and can be decoded by anyone.Actually, the fact that the signed string is base64 encoded and that the
MessageVerifierworks with "keys" (the current docs even state about Key Rotation) might further cause the wrong impression in the developer that there's some encryption going on, whilst there is absolutely none.We add a reference to
MessageEncryptorto let the developer knows that, if he needs the payload to be encrypted, Rails already has that taken care of.Finally, we also improve the documentation on
ActiveRecord::SignedId, which usesMessageVerifierand thereby will also expose the ID as encoded cleartext; we make it explicit that it’s not encryption, only signing, so the ID, albeit tamper proof, will be able to be read in cleartext by anyone.Additional information
While working on this, I felt the desire to have
ActiveRecord::EncryptedId.It would behave just like
ActiveRecord::SignedIddoes, but it would useMessageEncryptorinstead ofMessageVerifier, so that the IDs would never be exposed.The API would respect the parity, so we would have
user.encrypted_idandUser.find_encrypted(encrypted_id).I recently opened issue #52063 to discuss the last missing piece that I would use to implement
ActiveRecord::EncryptedId; I know this PR is just regarding documentation, but if any members could indicate if a PR forActiveRecord::EncryptedIdhad a chance of being accepted I would start working on it as soon as I could spare the time.Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]