Expose Retry-After header on all ACME responses (#264)

* Expose Retry-After header on errors and resources

* move parser to http middleware and always cast to time like certbot

* fix tests

* add a new field that always casts to a time object instead of changing original

* Add bigdecimal to the development gemspec

* Rebase: Ari improvements complete (#269)

* ARI related improvements

* Fix ARI tests: extract replaces from order response, re-record cassettes with Pebble

- Add :replaces to extract_attributes in attributes_from_order_response
  (bug: replaces was sent in requests but never parsed from responses)
- Re-record VCR cassettes against Pebble with real ARI data:
  - renewal_info_supported: real suggested window + retry-after
  - new_order_with_replaces: order response includes replaces field
  - new_order_already_replaced: real 409 alreadyReplaced error
- Update certificate_chain.pem fixture with cert from Pebble
- Remove pending() from replaces tests in order_spec and renewal_info_spec

---------

Co-authored-by: Ben Burkert <ben@benburkert.com>
Co-authored-by: Nick Silverman <nckslvrmn@gmail.com>

* test fixes

---------

Co-authored-by: Charles Barbier <unixcharles@gmail.com>
Co-authored-by: Charles Barbier <github@charlesbarbier.com>
Co-authored-by: Ben Burkert <ben@benburkert.com>

Author: 3 people

acme-client.gemspec

@@ -18,7 +18,7 @@ Gem::Specification.new do |spec|
 
   spec.add_development_dependency 'rake', '~> 13.0'
   spec.add_development_dependency 'rspec', '~> 3.9'
-  spec.add_development_dependency 'vcr', '~> 2.9'
+  spec.add_development_dependency 'vcr', '~> 6.0'
   spec.add_development_dependency 'bigdecimal'
   spec.add_development_dependency 'webmock', '~> 3.8'
   spec.add_development_dependency 'webrick', '~> 1.7'

lib/acme/client.rb

@@ -326,15 +326,20 @@ def attributes_from_order_response(response)
     )
 
     attributes[:url] = response.headers[:location] if response.headers[:location]
+    attributes[:retry_after] = response.headers['retry-after'] if response.headers['retry-after']
     attributes
   end
 
   def attributes_from_authorization_response(response)
-    extract_attributes(response.body, :identifier, :status, :expires, :challenges, :wildcard)
+    attributes = extract_attributes(response.body, :identifier, :status, :expires, :challenges, :wildcard)
+    attributes[:retry_after] = response.headers['retry-after'] if response.headers['retry-after']
+    attributes
   end
 
   def attributes_from_challenge_response(response)
-    extract_attributes(response.body, :status, :url, :token, :type, :error, :validated)
+    attributes = extract_attributes(response.body, :status, :url, :token, :type, :error, :validated)
+    attributes[:retry_after] = response.headers['retry-after'] if response.headers['retry-after']
+    attributes
   end
 
   def attributes_from_renewal_info_response(response)

lib/acme/client/error.rb

@@ -1,16 +1,16 @@
 class Acme::Client::Error < StandardError
-  attr_reader :subproblems
-  attr_reader :acme_error_body
-
+  attr_reader :retry_after, :retry_after_time, :subproblems, :acme_error_body
 
   Subproblem = Struct.new(:type, :detail, :identifier, keyword_init: true) do
     def to_h
       { type: type, detail: detail, identifier: identifier }
     end
   end
 
-  def initialize(message = nil, acme_error_body: nil, subproblems: nil)
+  def initialize(message = nil, retry_after: nil, acme_error_body: nil, subproblems: nil)
     super(message)
+    @retry_after_time = Acme::Client::Util.parse_retry_after(retry_after)
+    @retry_after = @retry_after_time ? [(@retry_after_time - Time.now).ceil, 0].max : nil
     @acme_error_body = acme_error_body
     @subproblems = parse_subproblems(subproblems)
   end

lib/acme/client/error/rate_limited.rb

@@ -1,10 +1,15 @@
 class Acme::Client::Error::RateLimited < Acme::Client::Error::ServerError
-  attr_reader :retry_after
-
   DEFAULT_MESSAGE = 'Error message: urn:ietf:params:acme:error:rateLimited'
+  DEFAULT_RETRY_SECONDS = 10
 
-  def initialize(message = DEFAULT_MESSAGE, retry_after = 10, acme_error_body: nil, subproblems: nil)
-    super(message, acme_error_body: acme_error_body, subproblems: subproblems)
-    @retry_after = retry_after.nil? ? 10 : retry_after.to_i
+  def initialize(message = DEFAULT_MESSAGE, retry_after = nil, acme_error_body: nil, subproblems: nil)
+    retry_after_time = case retry_after
+                       when Time then retry_after
+                       when nil then Time.now + DEFAULT_RETRY_SECONDS
+                       else Acme::Client::Util.parse_retry_after(retry_after) || Time.now + DEFAULT_RETRY_SECONDS
+                       end
+    int_retry_after = retry_after.nil? ? DEFAULT_RETRY_SECONDS : [(retry_after_time - Time.now).ceil, 0].max
+    super(message, retry_after: int_retry_after, acme_error_body: acme_error_body, subproblems: subproblems)
+    @retry_after_time = retry_after_time
   end
 end

lib/acme/client/http_client.rb

@@ -101,12 +101,13 @@ def raise_on_not_found!
     end
 
     def raise_on_error!
+      retry_after = env.response_headers['retry-after']
       body = env.body.is_a?(Hash) ? env.body : nil
       subproblems = error_subproblems
       if error_class == Acme::Client::Error::RateLimited
-        raise error_class.new(error_message, env.response_headers['Retry-After'], acme_error_body: body, subproblems: subproblems)
+        raise error_class.new(error_message, retry_after, acme_error_body: body, subproblems: subproblems)
       end
-      raise error_class.new(error_message, acme_error_body: body, subproblems: subproblems)
+      raise error_class.new(error_message, retry_after: retry_after, acme_error_body: body, subproblems: subproblems)
     end
 
     def error_message

lib/acme/client/resources/authorization.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Acme::Client::Resources::Authorization
-  attr_reader :url, :identifier, :domain, :expires, :status, :wildcard
+  attr_reader :url, :identifier, :domain, :expires, :status, :wildcard, :retry_after, :retry_after_time
 
   def initialize(client, **arguments)
     @client = client
@@ -52,7 +52,8 @@ def to_h
       status: status,
       expires: expires,
       challenges: @challenges,
-      wildcard: wildcard
+      wildcard: wildcard,
+      retry_after: retry_after
     }
   end
 
@@ -69,13 +70,15 @@ def initialize_challenge(attributes)
     Acme::Client::Resources::Challenges.new(@client, **arguments)
   end
 
-  def assign_attributes(url:, status:, expires:, challenges:, identifier:, wildcard: false)
+  def assign_attributes(url:, status:, expires:, challenges:, identifier:, wildcard: false, retry_after: nil)
     @url = url
     @identifier = identifier
     @domain = identifier.fetch('value')
     @status = status
     @expires = expires
     @challenges = challenges
     @wildcard = wildcard
+    @retry_after = retry_after
+    @retry_after_time = Acme::Client::Util.parse_retry_after(retry_after)
   end
 end

lib/acme/client/resources/challenges/base.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Acme::Client::Resources::Challenges::Base
-  attr_reader :status, :url, :token, :error, :validated
+  attr_reader :status, :url, :token, :error, :validated, :retry_after, :retry_after_time
 
   def initialize(client, **arguments)
     @client = client
@@ -38,7 +38,7 @@ def typed_error
   end
 
   def to_h
-    { status: status, url: url, token: token, error: error, validated: validated }
+    { status: status, url: url, token: token, error: error, validated: validated, retry_after: retry_after }
   end
 
   private
@@ -49,11 +49,13 @@ def send_challenge_validation(url:)
     ).to_h
   end
 
-  def assign_attributes(status:, url:, token:, error: nil, validated: nil)
+  def assign_attributes(status:, url:, token:, error: nil, validated: nil, retry_after: nil)
     @status = status
     @url = url
     @token = token
     @error = error
     @validated = validated
+    @retry_after = retry_after
+    @retry_after_time = Acme::Client::Util.parse_retry_after(retry_after)
   end
 end

lib/acme/client/resources/order.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Acme::Client::Resources::Order
-  attr_reader :url, :status, :contact, :finalize_url, :identifiers, :authorization_urls, :expires, :certificate_url, :profile, :replaces
+  attr_reader :url, :status, :contact, :finalize_url, :identifiers, :authorization_urls, :expires, :certificate_url, :profile, :replaces, :retry_after, :retry_after_time
 
   def initialize(client, **arguments)
     @client = client
@@ -56,13 +56,14 @@ def to_h
       identifiers: identifiers,
       certificate_url: certificate_url,
       profile: profile,
-      replaces: replaces
+      replaces: replaces,
+      retry_after: retry_after
     }
   end
 
   private
 
-  def assign_attributes(url: nil, status:, expires:, finalize_url:, authorization_urls:, identifiers:, certificate_url: nil, profile: nil, replaces: nil) # rubocop:disable Layout/LineLength,Metrics/ParameterLists
+  def assign_attributes(url: nil, status:, expires:, finalize_url:, authorization_urls:, identifiers:, certificate_url: nil, profile: nil, replaces: nil, retry_after: nil) # rubocop:disable Layout/LineLength,Metrics/ParameterLists
     @url = url unless url.nil?
     @status = status
     @expires = expires
@@ -72,5 +73,7 @@ def assign_attributes(url: nil, status:, expires:, finalize_url:, authorization_
     @certificate_url = certificate_url
     @profile = profile
     @replaces = replaces
+    @retry_after = retry_after
+    @retry_after_time = Acme::Client::Util.parse_retry_after(retry_after)
   end
 end

lib/acme/client/resources/renewal_info.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Acme::Client::Resources::RenewalInfo
-  attr_reader :ari_id, :suggested_window, :explanation_url, :retry_after
+  attr_reader :ari_id, :suggested_window, :explanation_url, :retry_after, :retry_after_time
 
   def initialize(client, **arguments)
     @client = client
@@ -49,5 +49,6 @@ def assign_attributes(ari_id:, suggested_window:, explanation_url: nil, retry_af
     @suggested_window = suggested_window
     @explanation_url = explanation_url
     @retry_after = retry_after
+    @retry_after_time = Acme::Client::Util.parse_retry_after(retry_after)
   end
 end

lib/acme/client/util.rb

@@ -1,6 +1,24 @@
+require 'time'
+
 module Acme::Client::Util
   extend self
 
+  # Parses a Retry-After header value into a Time.
+  # RFC 7231 §7.1.3: the value is either delay-seconds or an HTTP-date.
+  # Returns a Time, or nil if the value is nil or unparseable.
+  def parse_retry_after(value)
+    return nil if value.nil?
+
+    value = value.to_s
+    Integer(value, 10).then { |seconds| Time.now + seconds }
+  rescue ArgumentError, RangeError
+    begin
+      Time.httpdate(value)
+    rescue ArgumentError
+      nil
+    end
+  end
+
   def urlsafe_base64(data)
     Base64.urlsafe_encode64(data).sub(/[\s=]*\z/, '')
   end