Skip to content

Commit

Permalink
Add non-breaking backwards compatibility for 4.x and CVE-2018-1000211
Browse files Browse the repository at this point in the history
Instructs users via a post-install message on what they should do. If they ignore and do not run the migration generator, they'll simply not take advatange of the security fix (Doorkeeper will continue to assume all apps are confidential).
  • Loading branch information
Justin Bull committed Jul 11, 2018
1 parent 337d4c2 commit 36dc99b
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 0 deletions.
22 changes: 22 additions & 0 deletions doorkeeper.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,26 @@ Gem::Specification.new do |s|
s.add_development_dependency "generator_spec", "~> 0.9.3"
s.add_development_dependency "rake", ">= 11.3.0"
s.add_development_dependency "rspec-rails"

s.post_install_message = %q{
WARNING: This is a security release that addresses token revocation not working for public apps (CVE-2018-1000211)
There is no breaking change in this release, however to take advantage of the security fix you must:
1. Run `rails generate doorkeeper:add_client_confidentiality` for the migration
2. Review your OAuth apps and determine which ones exclusively use public grant flows (eg implicit)
3. Update their `confidential` column to `false` for those public apps
This is a backported security release.
For more information:
* https://github.com/doorkeeper-gem/doorkeeper/pull/1119
* https://github.com/doorkeeper-gem/doorkeeper/issues/891
}
end
8 changes: 8 additions & 0 deletions lib/doorkeeper/orm/active_record/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ def self.authorized_for(resource_owner)
where(id: resource_access_tokens.select(:application_id).distinct)
end

# Fallback to existing, default behaviour of assuming all apps to be
# confidential if the migration hasn't been run
def confidential
self.class.column_names.include?('confidential') ? super : true
end

alias_method :confidential?, :confidential

private

def generate_uid
Expand Down
27 changes: 27 additions & 0 deletions lib/generators/doorkeeper/add_client_confidentiality_generator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
require 'rails/generators/active_record'

class Doorkeeper::AddClientConfidentialityGenerator < ::Rails::Generators::Base
include Rails::Generators::Migration
source_root File.expand_path('../templates', __FILE__)
desc 'Adds a migration to fix CVE-2018-1000211.'

def install
migration_template(
'add_confidential_to_application_migration.rb.erb',
'db/migrate/add_confidential_to_doorkeeper_application.rb',
migration_version: migration_version
)
end

def self.next_migration_number(dirname)
ActiveRecord::Generators::Base.next_migration_number(dirname)
end

private

def migration_version
if ActiveRecord::VERSION::MAJOR >= 5
"[#{ActiveRecord::VERSION::MAJOR}.#{ActiveRecord::VERSION::MINOR}]"
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class AddConfidentialToDoorkeeperApplication < ActiveRecord::Migration<%= migration_version %>
def change
add_column(
:oauth_applications,
:confidential,
:boolean,
null: false,
default: true # maintaining backwards compatibility: require secrets
)
end
end

0 comments on commit 36dc99b

Please sign in to comment.