Skip to content

Commit

Permalink
[Security] Bump doorkeeper from 4.2.6 to 4.4.0 (#2847)
Browse files Browse the repository at this point in the history
* [Security] Bump doorkeeper from 4.2.6 to 4.4.0

Bumps [doorkeeper](https://github.com/doorkeeper-gem/doorkeeper) from 4.2.6 to 4.4.0. **This update includes security fixes.**
- [Release notes](https://github.com/doorkeeper-gem/doorkeeper/releases)
- [Changelog](https://github.com/doorkeeper-gem/doorkeeper/blob/master/NEWS.md)
- [Commits](doorkeeper-gem/doorkeeper@v4.2.6...v4.4.0)

Signed-off-by: dependabot[bot] <[email protected]>

* add confidential column to oauth_applications

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:

    * doorkeeper-gem/doorkeeper#1119
    * doorkeeper-gem/doorkeeper#891

* add note on what this migration default value means

some of our apps (first_party password / session grants and implicit) will require the non-default value of confidential = false

* switch to non-confidential oauth application

ensure the spec testing devise session to token via password grant (#101) uses a non-confidential application

* add data migration for oauth confidential attribute

set confidential to false for implicit, native apps as well as any first party apps that use the password grant without supplying the client secret (PFE / python client).

* move comment to be more informative

* only change apps we know use non-confidential password flow

avoid changing other apps that may be using client_id & client_secrets to authenticate, only update the first party apps we know about.
  • Loading branch information
dependabot[bot] authored and camallen committed Aug 7, 2018
1 parent 2877cc4 commit f633f95
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 4 deletions.
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ gem 'aws-sdk', '~> 2.10'
gem "cellect-client", '~> 3.0.2'
gem 'dalli-elasticache'
gem 'devise', '~> 4.3'
gem 'doorkeeper', '~> 4.2'
gem 'doorkeeper', '~> 4.4'
gem 'doorkeeper-jwt', '~> 0.2.1'
gem 'httparty'
gem 'faraday', '~> 0.9'
Expand Down
4 changes: 2 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ GEM
diff-lcs (1.3)
domain_name (0.5.20170404)
unf (>= 0.0.5, < 1.0.0)
doorkeeper (4.2.6)
doorkeeper (4.4.0)
railties (>= 4.2)
doorkeeper-jwt (0.2.1)
jwt (~> 1.5.2, >= 1.5.2)
Expand Down Expand Up @@ -454,7 +454,7 @@ DEPENDENCIES
dalli-elasticache
database_cleaner (~> 1.7.0)
devise (~> 4.3)
doorkeeper (~> 4.2)
doorkeeper (~> 4.4)
doorkeeper-jwt (~> 0.2.1)
factory_bot_rails
faraday (~> 0.9)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
class AddConfidentialToDoorkeeperApplication < ActiveRecord::Migration
def change
add_column(
:oauth_applications,
:confidential,
:boolean,
null: false,
default: true # maintaining backwards compatibility: require secrets
)

# setting all the existing apps to confidential will break login for grant flows(password)
# that do not supply a client_secret, e.g. our main zooniverse.org UI and api clients
# these apps will require a confidential = false setting
# all implicit apps will require confidential = false as well
# apps that can keep secrets and use them to authenticate will require the default value
reversible do |dir|
dir.up do
non_confidential_opts = { confidential: false }

Doorkeeper::Application
.where("redirect_uri ~* ?", '://') # all implicit apps & native apps (protocol scheme in the redirect_uri)
.where("redirect_uri !~* ?", 'auth/.+/callback') # not the omniauth server apps
.update_all(non_confidential_opts)

# only change the apps we know use the password grant without a client_secret
known_non_confidential_first_party_app_names = [
"ZooniverseFirstParty - PFE",
"Panoptes python client - official",
"PFE Application"
]
Doorkeeper::Application
.first_party # all first_party apps (e.g. PFE, python client)
.where(name: known_non_confidential_first_party_app_names)
.where.not(confidential: false)
.update_all(non_confidential_opts)
end
end
end
end
5 changes: 4 additions & 1 deletion db/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,8 @@ CREATE TABLE public.oauth_applications (
owner_type character varying,
trust_level integer DEFAULT 1 NOT NULL,
default_scope character varying[] DEFAULT '{}'::character varying[],
scopes character varying DEFAULT ''::character varying NOT NULL
scopes character varying DEFAULT ''::character varying NOT NULL,
confidential boolean DEFAULT true NOT NULL
);


Expand Down Expand Up @@ -4156,3 +4157,5 @@ INSERT INTO schema_migrations (version) VALUES ('20180710151618');

INSERT INTO schema_migrations (version) VALUES ('20180724112620');

INSERT INTO schema_migrations (version) VALUES ('20180726133210');

1 change: 1 addition & 0 deletions spec/controllers/tokens_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
end

context "when supplied a valid users's devise session" do
let(:app) { create(:non_confidential_first_party_app, owner: owner) }
let(:req) do
@request.env['devise.mapping'] = Devise.mappings[:user]
sign_in owner
Expand Down
4 changes: 4 additions & 0 deletions spec/factories/applications.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@

factory :first_party_app do
trust_level 2

factory :non_confidential_first_party_app do
confidential false
end
end

factory :secure_app do
Expand Down

0 comments on commit f633f95

Please sign in to comment.