Add signed ids to Active Record #39313

dhh · 2020-05-17T00:53:29Z

Add support for finding records based on signed ids, which are tamper-proof, verified ids that can be set to expire and scoped with a purpose. This is particularly useful for things like password reset or email verification, where you want the bearer of the signed id to be able to interact with the underlying record, but usually only within a certain time period.

signed_id = User.first.signed_id expires_in: 15.minutes, purpose: :password_reset

User.find_signed signed_id # => nil, since the purpose does not match

travel 16.minutes
User.find_signed signed_id # => nil, since the signed id has expired

travel_back
User.find_signed signed_id, purpose: :password_reset # => User.first

User.find_signed! "bad data" # => ActiveSupport::MessageVerifier::InvalidSignature

Add support for finding records based on signed ids, which are tamper-proof, verified ids that can be set to expire and scoped with a purpose. This is particularly useful for things like password reset or email verification, where you want the bearer of the signed id to be able to interact with the underlying record, but usually only within a certain time period.

HarlemSquirrel · 2020-05-17T03:32:44Z

This is cool. Would it be useful to have an option to self-destruct after n uses? Maybe an email link for verification that is one and done.

activerecord/CHANGELOG.md

dhh · 2020-05-17T03:41:53Z

This is cool. Would it be useful to have an option to self-destruct after n uses? Maybe an email link for verification that is one and done.

Just make the verification idempotent and you're good. These signed ids carry no state. That's their advantage over explicit tokens. So they can't have any self-destruct setup.

Co-authored-by: Santiago Bartesaghi <sbartesaghi@hotmail.com>

activerecord/lib/active_record/signed_id.rb

Co-authored-by: प्रथमेश Sonpatki <csonpatki@gmail.com>

sogamoso

Would it make sense to add a couple of tests for the bang version of the method?

activerecord/CHANGELOG.md

activerecord/lib/active_record/signed_id.rb

uxxman · 2020-05-17T10:01:59Z

First of all great addition 👍 currently I am doing all the same things using JWTs, where the payload has class, id and scope (same as purpose). Would love to have it provided by rails out-of-box.

The current implementation of SignedId doesn't handle one time usage cases, like password reset token should automatically expire if used one time. JWTs have an IssuedAt (iat) attribute that I use for this specific purpose. May be we can add an issued_at timestamp in SignedID as well and verify it against a provided timestamp field (updated_at or password_changed_at) and flag the token as expired if isssued_at < password_changed_at. This way we can handle one time usage cases as well. Just a suggestion :)

activerecord/lib/active_record/signed_id.rb

Co-authored-by: Marc Best <marcbest123@gmail.com>

kaspth

Sign me up 😄🙌

activerecord/lib/active_record/signed_id.rb

activerecord/test/cases/signed_id_test.rb

swanson · 2020-05-17T16:15:58Z

Is this different than: https://github.com/rails/globalid#signed-global-ids ?

This API seems nicer to me, but I'm confused about how this might work with GlobalId or what the main differences are. When would you recommend using one over the other?

Co-authored-by: David McCoy <davidmccoy@users.noreply.github.com>

dhh · 2020-05-17T17:38:34Z

@swanson GlobalID is for polymorphism. When the passed ID might respond to any number of classes. This is for when it's not that. We can use a much simpler API. So: Use signed_id when you're only referring to a single concrete class, use gid when referring to any number of classes.

@kaspth

(Which helps justify the lack of a default expiration date, cc @kaspth)

@kaspth

h/t @kaspth

kaspth

Some final nits. Dig the purpose versioning explanation, hadn't spotted that usage idea, so cool that we clarified — and the verifier exposure ✌️

activerecord/test/cases/signed_id_test.rb

Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>

dhh · 2020-05-17T17:59:05Z

Thanks for the review @kaspth! Looking good now ✌️

crawler · 2020-05-23T18:02:51Z

@dhh This is useful and elegant looking feature, but i noticed that the .find_signed method maybe needs to use a .primary_key instead of the :id Symbol, as the key to find_by parameter. Please take look at the two changed lines of code in my request 😃 #39404
Thank you!

The signed id feature introduced in #39313 can cause loading issues since it may try to generate a key before the secret key base has been set. To prevent this wrap the secret initialization in a lambda.

ksylvest · 2020-06-15T21:03:21Z

activerecord/lib/active_record/signed_id.rb

+
+      # :nodoc:
+      def combine_signed_id_purposes(purpose)
+        [ name.underscore, purpose.to_s ].compact_blank.join("/")


Using combined_signed_id_purpose can generate identical 'purposes'. For example:

[User.name.underscore, "profile"].compact.join('/') # user/profile

[User::Profile.name.underscore, nil].compact.join('/') # user/profile

Does the possible overlap present enough of a concern to be nervous? I believe this'd mean:

user = User.find_by(name: '') signed_id = user.signed_id(purpose: 'profile') User::Profile.find_signed!(signed_id) # returns the profile with the same ID as the found user

If so - does it make sense to move the name into the key used via the signed_id_verifier (i.e. each model has a separate key). Happy to open a PR if so.

rmkni · 2020-07-28T17:49:26Z

Hello,
Is it possible to use signed_ids to secure ids in URL?
Instead of having GET object/12345 exposed, could it be possible to sign ids to prevent request forgery attacks?
Is there any performance issue using find_signed instead of find_by?

Want to make sure that those methods work on relation and return expected result when retation has or doesn't have any records. Those methods are delegated by https://github.com/rails/rails/blob/7cb451346618811796efce1f8a2bf576b8e4999c/activerecord/lib/active_record/relation/delegation.rb#L21, https://github.com/rails/rails/blob/7cb451346618811796efce1f8a2bf576b8e4999c/activerecord/lib/active_record/relation/delegation.rb#L95-L114, https://github.com/rails/rails/blob/7cb451346618811796efce1f8a2bf576b8e4999c/activerecord/lib/active_record/relation/delegation.rb#L56-L78 as I understand. Related to rails#39313

This scenario should not happen and now it raises a `ActiveSupport::MessageVerifier::InvalidSignature` error. Could be related to this new feature in 6.1 https://blog.saeloun.com/2020/05/20/rails-6-1-adds-support-for-signed-ids-to-active-record.html rails/rails#39313 And various issues that were supposedly fixed rails/rails#41233

dommmel · 2022-03-22T22:55:39Z

The current implementation of SignedId doesn't handle one time usage cases, like password reset token should automatically expire if used one time.

@uxxman For that use case you could simply put the current password in the purpose

User.first.signed_id expires_in: 15.minutes, purpose: User.first.password_digest

uxxman · 2022-03-23T08:21:08Z

@dommmel this won't work when you have to find the user by signed_id find_signed since we cannot know what purpose to use when finding

dommmel · 2022-03-23T08:40:21Z

@uxxman You're right. That won't work as such.

I guess you could use a token that combines the above with another (static) signed id and then split them up again later, using the first part for finding and the second for additional verification.

Then again, there might be problems with that as well. I'll stop commenting with half baked ideas now ;-)

uxxman · 2022-03-23T08:48:33Z

@dommmel if you are also looking for a solution for this you can checkout this pull request #44189

rails-bot bot added the activerecord label May 17, 2020

dhh added 2 commits May 16, 2020 17:56

Don't expire by default

56adf09

Remove redundant begin block

a8e6c87

santib reviewed May 17, 2020

View reviewed changes

activerecord/CHANGELOG.md Outdated Show resolved Hide resolved

Update activerecord/CHANGELOG.md

37789c9

Co-authored-by: Santiago Bartesaghi <sbartesaghi@hotmail.com>

prathamesh-sonpatki reviewed May 17, 2020

View reviewed changes

activerecord/lib/active_record/signed_id.rb Outdated Show resolved Hide resolved

Update activerecord/lib/active_record/signed_id.rb

83e95fa

Co-authored-by: प्रथमेश Sonpatki <csonpatki@gmail.com>

sogamoso approved these changes May 17, 2020

View reviewed changes

activerecord/CHANGELOG.md Show resolved Hide resolved

marcbest reviewed May 17, 2020

View reviewed changes

activerecord/lib/active_record/signed_id.rb Outdated Show resolved Hide resolved

marcbest reviewed May 17, 2020

View reviewed changes

activerecord/lib/active_record/signed_id.rb Outdated Show resolved Hide resolved

garethrees mentioned this pull request May 17, 2020

[fnf#60] Project invites mysociety/alaveteli#5657

Merged

davidmccoy reviewed May 17, 2020

View reviewed changes

activerecord/lib/active_record/signed_id.rb Outdated Show resolved Hide resolved

dhh and others added 2 commits May 17, 2020 07:00

Update activerecord/lib/active_record/signed_id.rb

df5c511

Co-authored-by: Marc Best <marcbest123@gmail.com>

Update activerecord/lib/active_record/signed_id.rb

a988faa

Co-authored-by: Marc Best <marcbest123@gmail.com>

kaspth reviewed May 17, 2020

View reviewed changes

dhh and others added 3 commits May 17, 2020 10:33

Test bang method

dc99ade

Update activerecord/lib/active_record/signed_id.rb

1fd15f4

Co-authored-by: David McCoy <davidmccoy@users.noreply.github.com>

Merge branch 'signed-id' of github.com:rails/rails into signed-id

c0733ed

dhh added 2 commits May 17, 2020 10:44

Explain using purpose for versioning

a110368

(Which helps justify the lack of a default expiration date, cc @kaspth)

Expose the direct verifier instance used

26282a8

h/t @kaspth

kaspth approved these changes May 17, 2020

View reviewed changes

activerecord/test/cases/signed_id_test.rb Outdated Show resolved Hide resolved

activerecord/test/cases/signed_id_test.rb Outdated Show resolved Hide resolved

dhh and others added 2 commits May 17, 2020 10:57

Forgot the bang!

5a1aa30

Update activerecord/test/cases/signed_id_test.rb

2bdcdc4

Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>

dhh deleted the signed-id branch May 17, 2020 18:19

crawler mentioned this pull request May 23, 2020

Take a primay_key in count in the ActiveRecord::SignedId.find_signed #39404

Merged

pixeltrix mentioned this pull request May 28, 2020

Make signed_id_verifier_secret lazily evaluated #39464

Merged

ksylvest reviewed Jun 15, 2020

View reviewed changes

ecylmz mentioned this pull request Jul 29, 2020

Telefon doğrulamasıyla parola sıfırlama işlemi yap omu/nokul#1939

Merged

9 tasks

bogdanvlviv mentioned this pull request Oct 27, 2020

Test find_signed/! on Relation #40467

Merged

onerinas mentioned this pull request Apr 17, 2021

Any plans on using signed ids? abevoelker/devise-passwordless#2

Closed

jonathanhefner mentioned this pull request Nov 22, 2021

Add :expires_with option to signed_id #43620

Closed

This was referenced Oct 5, 2023

#signed_id outputs URL unsafe strings #49505

Closed

Ensure #signed_id outputs url_safe strings #49507

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add signed ids to Active Record #39313

Add signed ids to Active Record #39313

dhh commented May 17, 2020

HarlemSquirrel commented May 17, 2020

dhh commented May 17, 2020

sogamoso left a comment

uxxman commented May 17, 2020

kaspth left a comment

swanson commented May 17, 2020 •

edited

Loading

dhh commented May 17, 2020

kaspth left a comment

dhh commented May 17, 2020

crawler commented May 23, 2020

ksylvest Jun 15, 2020

rmkni commented Jul 28, 2020

dommmel commented Mar 22, 2022 •

edited

Loading

uxxman commented Mar 23, 2022

dommmel commented Mar 23, 2022

uxxman commented Mar 23, 2022

Add signed ids to Active Record #39313

Add signed ids to Active Record #39313

Conversation

dhh commented May 17, 2020

HarlemSquirrel commented May 17, 2020

dhh commented May 17, 2020

sogamoso left a comment

Choose a reason for hiding this comment

uxxman commented May 17, 2020

kaspth left a comment

Choose a reason for hiding this comment

swanson commented May 17, 2020 • edited Loading

dhh commented May 17, 2020

kaspth left a comment

Choose a reason for hiding this comment

dhh commented May 17, 2020

crawler commented May 23, 2020

ksylvest Jun 15, 2020

Choose a reason for hiding this comment

rmkni commented Jul 28, 2020

dommmel commented Mar 22, 2022 • edited Loading

uxxman commented Mar 23, 2022

dommmel commented Mar 23, 2022

uxxman commented Mar 23, 2022

swanson commented May 17, 2020 •

edited

Loading

dommmel commented Mar 22, 2022 •

edited

Loading