Expiration for signed global ids.

[kaspth]

Adds #27.

expires_in

This change adds a class and instance level expires_in setting, which signed global ids use to assign an expiration date to itself.
The instance level option takes precedence to the class level option.

An application configuration point is provided to set the class level expires_in.

expires_at

An instance level option to provide an explicit expiration date is also added. This takes precedence to any of the expires_in options.

Passing nil to either of the instance level expiration options turns off expiration checking. I.e. it creates a signed global id that won't expire.

[kaspth]

Because I resolve this to setting the expires_at attribute, this makes the passed in value a static setting. The expiration date is set to one month in the future of when this app was booted.

[dhh]

Does that mean that the expiry will keep getting smaller if you don't restart the server? It should probably be 1.month per generation. Not a static value.

On Aug 22, 2014, at 8:39, Kasper Timm Hansen notifications@github.com wrote:

In lib/global_id/railtie.rb:

@@ -15,6 +15,9 @@ class Railtie < Rails::Railtie # :nodoc:
app.config.global_id.app ||= app.railtie_name.remove('application').gsub!('','-')
GlobalID.app = app.config.global_id.app

•  app.config.global_id.expires_in ||= 1.month
  

  Because I resolve this to setting the expires_at attribute, this makes the passed in value a static setting. The expiration date is set to one month in the future of when this app was booted.

—
Reply to this email directly or view it on GitHub.

[jeremy]

Right: it's a duration. When we generate new signed Global IDs, they'll use that default duration to set their expires_at: expires_in.from_now

[kaspth]

Sorry, my original comment made it sound like the static value was a good thing, which it isn't.
I have a newer version which fixes this. I just need to write some more tests, then I'll push it.

[chancancode]

cc @jeremy @rafaelfranca @xuchu this is related to rails/rails#16462, we should probably make sure our overall approach is not too different so we can eventually migrate this to use the one provided by AS in the future :)

[chancancode]

@xuchu you should probably help review this one :)

[jeremy]

@chancancode @xuchu Yes indeed! See #27 for motivation and relationship with that PR 😁

[chancancode]

This should probably be a specific error. Perhaps should share a super class with the "invalid signature" case even. @xuchu @rafaelfranca @jeremy thoughts? (we need to decide that for our side too)

[kaspth]

@xuchu raises an ExpiredMessage exception.

Perhaps we could subclass that and create a ExpiredSignedGlobalID?

[tony612]

I think you'd better solve the conflicts ASAP cause there'll be many conflicts probably 😏

[tony612]

Shouldn't it be expires_at - Time.now? And when expires_at is nil, Time.now seems not an expected value for expires_in, right?

[kaspth]

Hold off with the review for now, I've moved away from this approach. This line has been ✂️

[tony612]

OK:+1:

[kaspth]

Actually let me push what I have now. I still need tests and it hasn't been rebased yet.

Den 22/08/2014 kl. 14.37 skrev Tony Han notifications@github.com:

In lib/global_id/signed_global_id.rb:

@@ -21,23 +21,48 @@ def pick_verifier(options)
end
end

• def expires_in

•  expires_at ? Time.now - expires_at : Time.now
  

  OK

—
Reply to this email directly or view it on GitHub.

Kasper

[kaspth]

All right, I think that's it. I'd love for #31 to be merged first, then I'll rebase and update this PR.

I've added a ExpiredMessage exception to bridge the compatibility between this and rails/rails#16462.
So to integrate with that change, we can just remove #raise_if_expired and use ActiveSupport::PersishableEnvelope::ExpiredMessage instead.

❤️

[jeremy]

Merged #31 - thank you @kaspth !

[kaspth]

You're welcome, @jeremy. I've rebased and updated this PR.