Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security] Bump doorkeeper from 4.2.6 to 4.4.0 #2847

Merged
merged 7 commits into from
Aug 7, 2018

Conversation

dependabot-preview[bot]
Copy link
Contributor

@dependabot-preview dependabot-preview bot commented Jul 18, 2018

DO NOT MERGE YET - merging / migrating this PR will break authentication for some API clients (gavity spy user promotion)

TODO

  • Confirm with Scotty from Gravity Spy that the user promotion code uses the default python client authentication flows or they have updated to a custom Oauth application for client credentials flow
  • Add new / modify migration to set specific apps to confidential = false to ensure viable authentication (PFE clients, Python API and gravity spy clients at the very least) as well as any implicit flow apps (has valid redirects) to be non-confidential.

This may impact authentication for a variety of applications in the wild, any strange auth errors should look to the confidential setting implications first.

Bumps doorkeeper from 4.2.6 to 4.4.0. This update includes security fixes.

Vulnerabilities fixed

Sourced from The Ruby Advisory Database.

Doorkeeper gem does not revoke token for public clients
Any OAuth application that uses public/non-confidential authentication when
interacting with Doorkeeper is unable to revoke its tokens when calling the
revocation endpoint.

A bug in the token revocation API would cause it to attempt to authenticate
the public OAuth client as if it was a confidential app. Because of this, the
token is never revoked.

The impact of this is the access or refresh token is not revoked, leaking
access to protected resources for the remainder of that token's lifetime.

... (truncated)

Patched versions: >= 4.4.0; >= 5.0.0.rc2
Unaffected versions: < 4.2.0

Release notes

Sourced from doorkeeper's releases.

v4.4.0

v4.3.2

v4.3.1

v4.3.0

Changelog

Sourced from doorkeeper's changelog.

4.4.0

4.3.2

4.3.1

4.3.0

Commits
  • 16e76e6 Merge pull request #1120 from f3ndot/backport-cve-2018-1000211
  • 35cd855 Disable confidential button if not supported, fix test coverage
  • d3fb696 Fix embarassing typo. Freeze heredoc constant
  • bd7bd3f Add news entry on this update
  • 4ecb0a2 [Lint] long lines, heredocs, other stylistic things
  • 3aebb59 Bump version to 4.4.0
  • 8e846f9 Warn developers when security migration not run
  • d6b56a9 Move warning into a constant for other uses
  • 36dc99b Add non-breaking backwards compatibility for 4.x and CVE-2018-1000211
  • 337d4c2 Use Application#confidential? to determine revocation auth eligibility
  • Additional commits viewable in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot ignore this [patch|minor|major] version will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
  • @dependabot use these labels will set the current labels as the default for future PRs for this repo and language
  • @dependabot use these reviewers will set the current reviewers as the default for future PRs for this repo and language
  • @dependabot use these assignees will set the current assignees as the default for future PRs for this repo and language
  • @dependabot badge me will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot dashboard:

  • Update frequency (including time of day and day of week)
  • Automerge options (never/patch/minor, and dev/runtime dependencies)
  • Pull request limits (per update run and/or open at any time)
  • Out-of-range updates (receive only lockfile updates, if desired)
  • Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

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] <support@dependabot.com>
@camallen camallen force-pushed the dependabot/bundler/doorkeeper-4.4.0 branch from ca975f4 to f953770 Compare July 26, 2018 12:59
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
camallen added 3 commits July 26, 2018 16:47
some of our apps (first_party password / session grants and implicit) will require the non-default value of confidential = false
ensure the spec testing devise session to token via password grant (#101) uses a non-confidential application
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).
.where("redirect_uri !~* ?", 'auth/.+/callback') # not the omniauth server apps
.update_all(non_confidential_opts)

#2. set the first party apps that aren't already set (PFE / python client)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Layout/LeadingCommentSpace: Missing space after #.

camallen added 2 commits July 27, 2018 13:32
avoid changing other apps that may be using client_id & client_secrets to authenticate, only update the first party apps we know about.
@camallen camallen requested review from marten and zwolf July 27, 2018 14:52
@dependabot-preview
Copy link
Contributor Author

A newer version of doorkeeper exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged.

@marten
Copy link
Contributor

marten commented Jul 30, 2018

I would love a summary of what changes and what's causing the incompatibility specifically. For instance, how would I tell if Caesar is going to be affected?

@camallen
Copy link
Contributor

I would love a summary of what changes and what's causing the incompatibility specifically.

Doorkeeper wasn't previously revoking tokens for non-confidential apps (see the Vulnerabilities fixed link in the PR description). The latest gem updates fixed this issue so public apps can now revoke tokens.

All implicit / native apps have to be set to non-confidential as that is how they work. Our first_party apps (PFE, python client) that convert devise sessions into oauth tokens also needed to be set to non-confidential as they supply a client_id but not a client_secret. I've audited all the apps we have (staging & production) and set up the migrations accordingly.

For instance, how would I tell if Caesar is going to be affected?

If your app us using an implicit grant / custom scheme (native) to get a token then it'll need it's non-confidential attribute set via the migration / UI to work properly. If you can keep a secret via your server and authentication using a client_id & client_secret then your app will be set to confidential via the default value in the migration and will continue to work accordingly.

@marten
Copy link
Contributor

marten commented Jul 30, 2018

Thanks, this "confidentiality" was a new concept but not explained. I get it now.

@camallen camallen merged commit f633f95 into master Aug 7, 2018
@camallen camallen deleted the dependabot/bundler/doorkeeper-4.4.0 branch August 7, 2018 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants