Add disabled at (#101)

* Backfill tests

Structure the code slightly differently in order to facilitate testing,
and then test with a database and a full Rails stack for better
assurance we have things working correctly.

* Add usage of disabled_at to allow old keys

When old keys are used, and assuming the disabled_at field is set, this
allows those old keys to be successful, but warns that the key is
disabled and will be removed soon.

Use defaults for 6.0 so that test work for Rails 6 runner

Update the generators to reduce whitespace

Blank lines were generated by the generators in a few places, which
cause some style format validators to fail, such as rubocop.

* Update version to 4.2.0

Note that we are moving from 4.0.2 to 4.2.0. This is because there were
a set of RC builds and tags for 4.1.0 which never made it to a 4.1.0
release. Nevertheless, it seems prudent to move to a new minor revision.

Author: micahhainlinestitchfix

.gitignore

@@ -1,5 +1,6 @@
 pkg
 spec/reports
+spec/fake_app/log/
 .vimrc
 *.sw?
 .idea/

README.md

@@ -35,7 +35,7 @@ Then, set it up:
 
 ### Upgrading from an older version
 
-- When upgrading to version 4.0.0 you may now take advantage of an in-memory cache
+- When upgrading to version 4.0.0 and above you may now take advantage of an in-memory cache
 
 You can enabled it like so
 
@@ -46,18 +46,46 @@ Stitches.configure do |config|
 end
 ```
 
-- If you have a version lower than 3.3.0, you need to run two generators, one of which creates a new database migration on your
-  `api_clients` table:
+You can also set a leniency for disabled API keys, which will allow old API keys to continue to be used if they have a
+`disabled_at` field set as long as the leniency is not exceeded. Note that if the `disabled_at` field is not populated
+the behavior will remain the same as it always was, and the request will be denied when the `enabled` field is set to
+`true`. If Stitches allows a call due to leniency settings, a log message will be generated with a severity depending on
+how long ago the API key was disabled.
+
+```ruby
+Stitches.configure do |config|
+  config.disabled_key_leniency_in_seconds = 3 * 24 * 60 * 60 # Time in seconds, defaults to three days 
+  config.disabled_key_leniency_error_log_threshold_in_seconds = 2 * 24 * 60 * 60 # Time in seconds, defaults to two days 
+end
+```
+
+If a disabled key is used within the `disabled_key_leniency_in_seconds`, it will be allowed. 
+
+Anytime a disabled key is used a log will be generated. If it is before the 
+`disabled_key_leniency_error_log_threshold_in_seconds` it will be a warning log message, if it is after that, it will be
+an error message. `disabled_key_leniency_error_log_threshold_in_seconds` should never be a greater number than 
+`disabled_key_leniency_in_seconds`, as this provides an escallating series of warnings before finally disabling access.
+
+- If you are upgrading from a version older than 3.3.0 you need to run three generators, two of which create database
+  migrations on your `api_clients` table:
 
   ```
   > bin/rails generate stitches:add_enabled_to_api_clients
   > bin/rails generate stitches:add_deprecation
+  > bin/rails generate stitches:add_disabled_at_to_api_clients
   ```
 
-- If you have a version lower than 3.6.0, you need to run one generator:
+- If you are upgrading from a version between 3.3.0 and 3.5.0 you need to run two generators:
 
   ```
   > bin/rails generate stitches:add_deprecation
+  > bin/rails generate stitches:add_disabled_at_to_api_clients
+  ```
+
+- If you are upgrading from a version between 3.6.0 and 4.0.2 you need to run one generator:
+
+  ```
+  > bin/rails generate stitches:add_disabled_at_to_api_clients
   ```
 
 ## Example Microservice Endpoint

lib/stitches/add_disabled_at_to_api_clients_generator.rb

@@ -0,0 +1,18 @@
+require 'rails/generators'
+
+module Stitches
+  class AddDisabledAtToApiClientsGenerator < Rails::Generators::Base
+    include Rails::Generators::Migration
+
+    source_root(File.expand_path(File.join(File.dirname(__FILE__),"generator_files")))
+
+    def self.next_migration_number(path)
+      Time.now.utc.strftime("%Y%m%d%H%M%S")
+    end
+
+    desc "Upgrade your api_clients table so it includes the `disabled_at` field"
+    def update_api_clients_table
+      migration_template "db/migrate/add_disabled_at_to_api_clients.rb", "db/migrate/add_disabled_at_to_api_clients.rb"
+    end
+  end
+end

lib/stitches/allowlist_middleware.rb

@@ -3,15 +3,14 @@ module Stitches
   class AllowlistMiddleware
     def initialize(app, options={})
       @app           = app
-      @configuration = options[:configuration] ||  Stitches.configuration
-      @except        = options[:except]        || @configuration.allowlist_regexp
+      @configuration = options[:configuration]
+      @except        = options[:except]
 
-      unless @except.nil? || @except.is_a?(Regexp)
-        raise ":except must be a Regexp"
-      end
+      allowlist_regex
     end
+
     def call(env)
-      if @except && @except.match(env["PATH_INFO"])
+      if allowlist_regex && allowlist_regex.match(env["PATH_INFO"])
         @app.call(env)
       else
         do_call(env)
@@ -24,5 +23,20 @@ def do_call(env)
       raise 'subclass must implement'
     end
 
+    def configuration
+      @configuration || Stitches.configuration
+    end
+
+  private
+
+    def allowlist_regex
+      regex = @except || configuration.allowlist_regexp
+
+      if !regex.nil? && !regex.is_a?(Regexp)
+        raise ":except must be a Regexp"
+      end
+
+      regex
+    end
   end
 end

lib/stitches/api_client_access_wrapper.rb

@@ -2,26 +2,57 @@
 
 module Stitches::ApiClientAccessWrapper
 
-  def self.fetch_for_key(key)
+  def self.fetch_for_key(key, configuration)
     if cache_enabled
-      fetch_for_key_from_cache(key)
+      fetch_for_key_from_cache(key, configuration)
     else
-      fetch_for_key_from_db(key)
+      fetch_for_key_from_db(key, configuration)
     end
   end
 
-  def self.fetch_for_key_from_cache(key)
+  def self.fetch_for_key_from_cache(key, configuration)
     api_key_cache.getset(key) do
-      fetch_for_key_from_db(key)
+      fetch_for_key_from_db(key, configuration)
     end
   end
 
-  def self.fetch_for_key_from_db(key)
-    if ::ApiClient.column_names.include?("enabled")
-      ::ApiClient.find_by(key: key, enabled: true)
+  def self.fetch_for_key_from_db(key, configuration)
+    api_client = ::ApiClient.find_by(key: key)
+    return unless api_client
+
+    unless api_client.respond_to?(:enabled?)
+      logger.warn('api_keys is missing "enabled" column.  Run "rails g stitches:add_enabled_to_api_clients"')
+      return api_client
+    end
+
+    unless api_client.respond_to?(:disabled_at)
+      logger.warn('api_keys is missing "disabled_at" column.  Run "rails g stitches:add_disabled_at_to_api_clients"')
+    end
+
+    return api_client if api_client.enabled?
+
+    disabled_at = api_client.respond_to?(:disabled_at) ? api_client.disabled_at : nil
+    if disabled_at && disabled_at > configuration.disabled_key_leniency_in_seconds.seconds.ago
+      message = "Allowing disabled ApiClient: #{api_client.name} with key #{api_client.key} disabled at #{disabled_at}"
+      if disabled_at > configuration.disabled_key_leniency_error_log_threshold_in_seconds.seconds.ago
+        logger.warn(message)
+      else
+        logger.error(message)
+      end
+      return api_client
+    else
+      logger.error("Rejecting disabled ApiClient: #{api_client.name} with key #{api_client.key}")
+    end
+    nil
+  end
+
+  def self.logger
+    if defined?(StitchFix::Logger::LogWriter)
+      StitchFix::Logger::LogWriter
+    elsif defined?(Rails.logger)
+      Rails.logger
     else
-      ActiveSupport::Deprecation.warn('api_keys is missing "enabled" column.  Run "rails g stitches:add_enabled_to_api_clients"')
-      ::ApiClient.find_by(key: key)
+      ::Logger.new('/dev/null')
     end
   end
 
@@ -39,4 +70,4 @@ def self.api_key_cache
   def self.cache_enabled
     Stitches.configuration.max_cache_ttl.positive?
   end
-end
+end

lib/stitches/api_key.rb

@@ -25,12 +25,12 @@ class ApiKey < Stitches::AllowlistMiddleware
     def do_call(env)
       authorization = env["HTTP_AUTHORIZATION"]
       if authorization
-        if authorization =~ /#{@configuration.custom_http_auth_scheme}\s+key=(.*)\s*$/
+        if authorization =~ /#{configuration.custom_http_auth_scheme}\s+key=(.*)\s*$/
           key = $1
-          client = Stitches::ApiClientAccessWrapper.fetch_for_key(key)
+          client = Stitches::ApiClientAccessWrapper.fetch_for_key(key, configuration)
           if client.present?
-            env[@configuration.env_var_to_hold_api_client_primary_key] = client.id
-            env[@configuration.env_var_to_hold_api_client] = client
+            env[configuration.env_var_to_hold_api_client_primary_key] = client.id
+            env[configuration.env_var_to_hold_api_client] = client
             @app.call(env)
           else
             unauthorized_response("key invalid")
@@ -59,7 +59,7 @@ def rails_app_module
     def unauthorized_response(reason)
       status = 401
       body = "Unauthorized - #{reason}"
-      header = { "WWW-Authenticate" => "#{@configuration.custom_http_auth_scheme} realm=#{rails_app_module}" }
+      header = { "WWW-Authenticate" => "#{configuration.custom_http_auth_scheme} realm=#{rails_app_module}" }
       Rack::Response.new(body, status, header).finish
     end
 

lib/stitches/configuration.rb

@@ -15,8 +15,12 @@ def reset_to_defaults!
     @env_var_to_hold_api_client= NonNullString.new("env_var_to_hold_api_client","STITCHES_API_CLIENT")
     @max_cache_ttl = NonNullInteger.new("max_cache_ttl", 0)
     @max_cache_size = NonNullInteger.new("max_cache_size", 0)
+    @disabled_key_leniency_in_seconds = ActiveSupport::Duration.days(3)
+    @disabled_key_leniency_error_log_threshold_in_seconds = ActiveSupport::Duration.days(2)
   end
 
+  attr_accessor :disabled_key_leniency_in_seconds, :disabled_key_leniency_error_log_threshold_in_seconds
+
   # A RegExp that allows URLS around the mime type and api key requirements.
   # nil means that ever request must have a proper mime type and api key.
   attr_reader :allowlist_regexp

lib/stitches/generator_files/db/migrate/add_disabled_at_to_api_clients.rb

@@ -0,0 +1,5 @@
+class AddDisabledAtToApiClients < ActiveRecord::Migration<% if Rails::VERSION::MAJOR >= 5 %>[<%= Rails::VERSION::MAJOR %>.<%= Rails::VERSION::MINOR %>]<% end %>
+  def change
+    add_column :api_clients, :disabled_at, "timestamp with time zone", null: true
+  end
+end

lib/stitches/generator_files/db/migrate/add_enabled_to_api_clients.rb

@@ -1,8 +1,4 @@
-<% if Rails::VERSION::MAJOR >= 5 %>
-class AddEnabledToApiClients < ActiveRecord::Migration[<%= Rails::VERSION::MAJOR %>.<%= Rails::VERSION::MINOR %>]
-<% else %>
-class AddEnabledToApiClients < ActiveRecord::Migration
-<% end %>
+class AddEnabledToApiClients < ActiveRecord::Migration<% if Rails::VERSION::MAJOR >= 5 %>[<%= Rails::VERSION::MAJOR %>.<%= Rails::VERSION::MINOR %>]<% end %>
   def change
     add_column :api_clients, :enabled, :bool, null: false, default: true
     remove_index :api_clients, [:name ] # existing one would be unique

lib/stitches/generator_files/db/migrate/create_api_clients.rb

@@ -1,14 +1,11 @@
-<% if Rails::VERSION::MAJOR >= 5 %>
-class CreateApiClients < ActiveRecord::Migration[<%= Rails::VERSION::MAJOR %>.<%= Rails::VERSION::MINOR %>]
-<% else %>
-class CreateApiClients < ActiveRecord::Migration
-<% end %>
+class CreateApiClients < ActiveRecord::Migration<% if Rails::VERSION::MAJOR >= 5 %>[<%= Rails::VERSION::MAJOR %>.<%= Rails::VERSION::MINOR %>]<% end %>
   def change
     create_table :api_clients do |t|
       t.string :name, null: false
       t.column :key, "uuid default uuid_generate_v4()", null: false
       t.column :enabled, :bool, null: false, default: true
       t.column :created_at, "timestamp with time zone default now()", null: false
+      t.column :disabled_at, "timestamp with time zone", null: true
     end
     add_index :api_clients, [:name]
     add_index :api_clients, [:key], unique: true