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

Update oc_id gems #1518

Merged
merged 4 commits into from
Jul 10, 2018
Merged

Update oc_id gems #1518

merged 4 commits into from
Jul 10, 2018

Conversation

markan
Copy link
Contributor

@markan markan commented Jul 5, 2018

Update oc_id gems, particularly doorkeeper.

@markan markan requested a review from a team July 5, 2018 18:00
@markan markan force-pushed the ma/oc-id-updates branch from 4c276ed to 2fc543c Compare July 8, 2018 05:24
Copy link
Contributor

@stevendanna stevendanna left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me. We have some basic oc_id pedant tests here: https://github.com/chef/chef-server/blob/master/oc-chef-pedant/spec/api/oc_id_spec.rb that we should make sure are passing.

@markan markan force-pushed the ma/oc-id-updates branch from 4bb8633 to 85cfc1c Compare July 8, 2018 18:55
@@ -413,7 +413,7 @@ GEM
activesupport (>= 4.2)
spring-commands-rspec (1.0.4)
spring (>= 0.9.1)
sprockets (2.12.4)
Copy link
Member

Choose a reason for hiding this comment

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

I just got a security alert on this, with a fix in version update to 2.12.5 so 👍 for timing :D

robbkidd and others added 4 commits July 9, 2018 18:58
2.0 changed the `doorkeeper_for` controller method to a `before_action`.
Update the UsersController.

`link_to` and `form_for` were using the array method of modifying the
URL helper to use based on object class. The forms were updated to use
specific URL path helpers because the Application object has a namespace
that confuses the helpers (e.g. Oauth::Doorkeeper::Application).

Migration added to add a scopes column to oauth_applications. This
feature is generally about authz and oc-id currently only really
provides authn. Opted to add the migration to keep the database scheme
in line with the attributes Doorkeeper expects will be on its models.

Doorkeeper initializer updated based on template from the gem.
Restricted the grant flows to the two types Chef services use.

Signed-off-by: Robb Kidd <[email protected]>
The backward incompatible changes[1] do not apply to oc-id.

* oc-id doesn't use a mongodb backend.
* oc-id doesn't use either of the doorkeeper_*_render_options methods.
* oc-id doesn't use application-specific scopes (despite adding the
  column to the database to support them) so the change in precedence
    behavior does not apply.

[1] https://github.com/doorkeeper-gem/doorkeeper/blob/v3.1.0/NEWS.md#300-rc2

Doorkeeper initializer updated based on template from the gem.

Signed-off-by: Robb Kidd <[email protected]>
---
The backward incompatible changes[1][2] do not apply to oc-id.

* oc-id is currently at Rails 4.2, ok to drop support for 4.1.
* oc-id is currently running Ruby 2.4, ok to drop support for 2.0.
* oc-id does not enable `reuse_access_token`, so token valid timing
  change shouldn't affect it.
  * oc-id already stores times in UTC.

[1] https://github.com/doorkeeper-gem/doorkeeper/releases/tag/v4.0.0.rc1
[2] https://github.com/doorkeeper-gem/doorkeeper/releases/tag/v4.0.0.rc3

Doorkeeper initializer updated based on template from the gem.

Signed-off-by: Robb Kidd <[email protected]>
active_record 4.2.8 pins this but doesn't manifest this in the gemspec
for some reason, so we explicitly pin pg to ~> 0.15

Signed-off-by: Mark Anderson <[email protected]>
@markan markan force-pushed the ma/oc-id-updates branch from 85cfc1c to 1395f39 Compare July 10, 2018 02:00
@markan markan merged commit ccf71d3 into master Jul 10, 2018
@markan markan deleted the ma/oc-id-updates branch July 10, 2018 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants