Upgrade Doorkeeper to 4.4.3
What does this MR do?
Updates the doorkeeper gem to 4.4.3 so that we can address security issue CVE-2018-1000211.
This issue involves token revocation not working for public apps.
The version migration notes are available here: https://github.com/doorkeeper-gem/doorkeeper/wiki/Migration-from-old-versions#api-changes-4
Completing this upgrade will also enable us to complete the upgrade of the
doorkeeper-openid_connect
gem #27383 (closed)
Steps
-
Run rails generate doorkeeper:add_client_confidentiality
for the migration -
Review your OAuth apps and determine which ones exclusively use public grant flows (eg implicit) -
Update their confidential
column tofalse
for those public apps- This has been addressed by first setting all the existing applications to
confidential: false
, and then setting the column default totrue
, as suggested in this comment on the original MR: gitlab-foss!21576 (comment 100133446)
- This has been addressed by first setting all the existing applications to
Output of bundle update
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
rails generate doorkeeper:add_client_confidentiality
Migration generated by running class AddConfidentialToDoorkeeperApplication < ActiveRecord::Migration[5.2]
def change
add_column(
:oauth_applications,
:confidential,
:boolean,
null: false,
default: true # maintaining backwards compatibility: require secrets
)
end
end
rails db:migrate:up VERSION=20191127163053
Running == 20191127163053 AddConfidentialToDoorkeeperApplication: migrating ===========
-- transaction_open?()
-> 0.0000s
-- execute("SET statement_timeout TO 0")
-> 0.0005s
-- transaction()
-- add_column(:oauth_applications, :confidential, :boolean, {:default=>nil})
-> 0.0018s
-- change_column_default(:oauth_applications, :confidential, false)
-> 0.0036s
-> 0.0064s
-- columns(:oauth_applications)
-> 0.0014s
-- transaction_open?()
-> 0.0000s
-- exec_query("SELECT COUNT(*) AS count FROM \"oauth_applications\"")
-> 0.0021s
-- exec_query("SELECT \"oauth_applications\".\"id\" FROM \"oauth_applications\" ORDER BY \"oauth_applications\".\"id\" ASC LIMIT 1")
-> 0.0004s
-- exec_query("SELECT \"oauth_applications\".\"id\" FROM \"oauth_applications\" WHERE \"oauth_applications\".\"id\" >= 1 ORDER BY \"oauth_applications\".\"id\" ASC LIMIT 1 OFFSET 1")
-> 0.0003s
-- execute("UPDATE \"oauth_applications\" SET \"confidential\" = FALSE WHERE \"oauth_applications\".\"id\" >= 1 AND \"oauth_applications\".\"id\" < 2")
-> 0.0005s
-- exec_query("SELECT \"oauth_applications\".\"id\" FROM \"oauth_applications\" WHERE \"oauth_applications\".\"id\" >= 2 ORDER BY \"oauth_applications\".\"id\" ASC LIMIT 1 OFFSET 1")
-> 0.0003s
-- execute("UPDATE \"oauth_applications\" SET \"confidential\" = FALSE WHERE \"oauth_applications\".\"id\" >= 2 AND \"oauth_applications\".\"id\" < 3")
-> 0.0005s
-- exec_query("SELECT \"oauth_applications\".\"id\" FROM \"oauth_applications\" WHERE \"oauth_applications\".\"id\" >= 3 ORDER BY \"oauth_applications\".\"id\" ASC LIMIT 1 OFFSET 1")
-> 0.0003s
-- execute("UPDATE \"oauth_applications\" SET \"confidential\" = FALSE WHERE \"oauth_applications\".\"id\" >= 3 AND \"oauth_applications\".\"id\" < 4")
-> 0.0004s
-- exec_query("SELECT \"oauth_applications\".\"id\" FROM \"oauth_applications\" WHERE \"oauth_applications\".\"id\" >= 4 ORDER BY \"oauth_applications\".\"id\" ASC LIMIT 1 OFFSET 1")
-> 0.0003s
-- execute("UPDATE \"oauth_applications\" SET \"confidential\" = FALSE WHERE \"oauth_applications\".\"id\" >= 4 AND \"oauth_applications\".\"id\" < 5")
-> 0.0004s
-- exec_query("SELECT \"oauth_applications\".\"id\" FROM \"oauth_applications\" WHERE \"oauth_applications\".\"id\" >= 5 ORDER BY \"oauth_applications\".\"id\" ASC LIMIT 1 OFFSET 1")
-> 0.0003s
-- execute("UPDATE \"oauth_applications\" SET \"confidential\" = FALSE WHERE \"oauth_applications\".\"id\" >= 5")
-> 0.0004s
-- change_column_null(:oauth_applications, :confidential, false)
-> 0.0005s
-- execute("RESET ALL")
-> 0.0002s
-- change_column_default(:oauth_applications, :confidential, true)
-> 0.0025s
== 20191127163053 AddConfidentialToDoorkeeperApplication: migrated (0.0192s) ==
rails db:migrate:down VERSION=20191127163053
Running == 20191127163053 AddConfidentialToDoorkeeperApplication: reverting ===========
-- remove_column(:oauth_applications, :confidential)
-> 0.0015s
== 20191127163053 AddConfidentialToDoorkeeperApplication: reverted (0.0016s) ==
Updates
The following areas have been updated so that users can manage the confidentiality of the applications
UI Updates
Admin Area
User Settings
API Updates
Add confidential
optionally to the create query params - will default to true
if not supplied:
POST /api/v4/applications?name=test&redirect_uri=http://localhost:3000/redirect&scopes=api&confidential=false
Add confidential
to the API response:
{
"id": 7,
"application_id": "cfa8b5d03509016bbca0...",
"application_name": "test",
"callback_url": "http://localhost:3000/redirect",
"confidential": true,
"secret": "b471b8502f339dde9a0fabe..."
}
Database checklist
-
Conforms to the database guides
When adding migrations:
-
Updated db/schema.rb
-
Added a down
method so the migration can be reverted -
Added the output of the migration(s) to the MR body -
Added tests for the migration in spec/migrations
if necessary (e.g. when migrating data) -
Added rollback procedure. Include either a rollback procedure or description how to rollback changes
When adding or modifying queries to improve performance:
-
Included data that shows the performance improvement, preferably in the form of a benchmark -
Included the output of EXPLAIN (ANALYZE, BUFFERS)
of the relevant queries
When adding foreign keys to existing tables:
-
Included a migration to remove orphaned rows in the source table before adding the foreign key -
Removed any instances of dependent: ...
that may no longer be necessary
When adding tables:
-
Ordered columns based on the Ordering Table Columns guidelines -
Added foreign keys to any columns pointing to data in other tables -
Added indexes for fields that are used in statements such as WHERE
,ORDER BY
,GROUP BY
, andJOIN
s
When removing columns, tables, indexes or other structures:
-
Removed these in a post-deployment migration -
Made sure the application no longer uses (or ignores) these structures
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team