Configuration to enforce application scopes #1010

talklittle · 2018-01-03T04:50:19Z

houndci-bot · 2018-01-03T04:50:33Z

spec/requests/applications/applications_request_spec.rb

+
+      click_button 'Submit'
+      i_should_see 'Application created'
+      i_should_see 'My Application'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-01-03T04:50:33Z

spec/requests/applications/applications_request_spec.rb

+      fill_in 'doorkeeper_application[scopes]', with: ''
+
+      click_button 'Submit'
+      i_should_see 'Application created'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-01-03T04:50:34Z

spec/requests/applications/applications_request_spec.rb

+              with: 'https://example.com'
+      fill_in 'doorkeeper_application[scopes]', with: ''
+
+      click_button 'Submit'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-01-03T04:50:34Z

spec/requests/applications/applications_request_spec.rb

+      fill_in 'doorkeeper_application[name]', with: 'My Application'
+      fill_in 'doorkeeper_application[redirect_uri]',
+              with: 'https://example.com'
+      fill_in 'doorkeeper_application[scopes]', with: ''


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-01-03T04:50:34Z

spec/requests/applications/applications_request_spec.rb

+
+      fill_in 'doorkeeper_application[name]', with: 'My Application'
+      fill_in 'doorkeeper_application[redirect_uri]',
+              with: 'https://example.com'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-01-03T04:50:34Z

spec/requests/applications/applications_request_spec.rb

+      fill_in 'doorkeeper_application[name]', with: 'My Application'
+      fill_in 'doorkeeper_application[redirect_uri]',
+              with: 'https://example.com'
+      fill_in 'doorkeeper_application[scopes]', with: 'blahblah'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-01-03T04:50:34Z

spec/requests/applications/applications_request_spec.rb

+
+      fill_in 'doorkeeper_application[name]', with: 'My Application'
+      fill_in 'doorkeeper_application[redirect_uri]',
+              with: 'https://example.com'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-01-03T04:50:35Z

spec/requests/applications/applications_request_spec.rb

+      config_is_set('enforce_configured_scopes', true)
+
+      fill_in 'doorkeeper_application[name]', with: 'My Application'
+      fill_in 'doorkeeper_application[redirect_uri]',


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-01-03T04:50:35Z

spec/requests/applications/applications_request_spec.rb

+    scenario 'adding app validating bad scope' do
+      config_is_set('enforce_configured_scopes', true)
+
+      fill_in 'doorkeeper_application[name]', with: 'My Application'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-01-03T04:50:35Z

spec/requests/applications/applications_request_spec.rb

+    end
+
+    scenario 'adding app validating bad scope' do
+      config_is_set('enforce_configured_scopes', true)


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-01-03T04:55:27Z

spec/requests/applications/applications_request_spec.rb

-feature 'Adding applications' do
-  context 'in application form' do
+feature "Adding applications" do
+  context "in application form" do


Block has too many lines. [44/25]

houndci-bot · 2018-01-03T04:55:27Z

spec/requests/applications/applications_request_spec.rb


-feature 'Adding applications' do
-  context 'in application form' do
+feature "Adding applications" do


Block has too many lines. [46/25]

houndci-bot · 2018-01-03T04:55:27Z

spec/requests/applications/applications_request_spec.rb

@@ -1,94 +1,132 @@
-require 'spec_helper_integration'
+require "spec_helper_integration"


Missing magic comment # frozen_string_literal: true.

houndci-bot · 2018-01-03T04:55:27Z

lib/doorkeeper/models/application_mixin.rb

+      validates_each :scopes, if: :validate_configured_scopes? do |record, attr, value|
+        value_str = value.to_s
+        record.errors.add attr, :invalid_scope unless value_str.blank? or
+          ScopeChecker.valid?(value_str, Doorkeeper.configuration.scopes)


Align the operands of a condition in an unless statement spanning multiple lines.

houndci-bot · 2018-01-03T04:55:27Z

lib/doorkeeper/models/application_mixin.rb

@@ -14,6 +14,12 @@ module ApplicationMixin
      validates :uid, uniqueness: true
      validates :redirect_uri, redirect_uri: true

+      validates_each :scopes, if: :validate_configured_scopes? do |record, attr, value|
+        value_str = value.to_s
+        record.errors.add attr, :invalid_scope unless value_str.blank? or


Use || instead of or.

houndci-bot · 2018-01-03T04:55:27Z

lib/doorkeeper/models/application_mixin.rb

@@ -14,6 +14,12 @@ module ApplicationMixin
      validates :uid, uniqueness: true
      validates :redirect_uri, redirect_uri: true

+      validates_each :scopes, if: :validate_configured_scopes? do |record, attr, value|


Line is too long. [87/80]

houndci-bot · 2018-01-03T05:01:42Z

spec/requests/applications/applications_request_spec.rb

+
+      fill_in 'doorkeeper_application[name]', with: 'My Application'
+      fill_in 'doorkeeper_application[redirect_uri]',
+              with: 'https://example.com'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-01-03T05:01:42Z

spec/requests/applications/applications_request_spec.rb

+              with: 'https://example.com'
+      fill_in 'doorkeeper_application[scopes]', with: 'blahblah'
+
+      click_button 'Submit'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2018-01-03T05:01:42Z

spec/requests/applications/applications_request_spec.rb

+      fill_in 'doorkeeper_application[name]', with: 'My Application'
+      fill_in 'doorkeeper_application[redirect_uri]',
+              with: 'https://example.com'
+      fill_in 'doorkeeper_application[scopes]', with: 'blahblah'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

talklittle · 2018-02-15T11:21:46Z

@nbulaj Rebased to fix conflicts with master.

not yet merged into Doorkeeper as of 4.3.0, so temporarily pointing to a git branch. see doorkeeper-gem/doorkeeper#1010

not yet merged into Doorkeeper as of 4.3.1, so temporarily pointing to a git branch. see doorkeeper-gem/doorkeeper#1010

nbulaj · 2018-03-12T11:50:20Z

lib/doorkeeper/orm/active_record/application.rb

@@ -12,6 +12,14 @@ class Application < ActiveRecord::Base
    validates :uid, uniqueness: true
    validates :redirect_uri, redirect_uri: true

+    validates_each :scopes, if: :enforce_scopes? do |record, attr, value|


Let's move it it from the validates_each to validate with custom method or just validate with a block (because validates_each is responsible for processing bunch of attributes, but we process only one - scopes).

nbulaj · 2018-03-12T11:50:57Z

lib/doorkeeper/orm/active_record/application.rb

@@ -12,6 +12,14 @@ class Application < ActiveRecord::Base
    validates :uid, uniqueness: true
    validates :redirect_uri, redirect_uri: true

+    validates_each :scopes, if: :enforce_scopes? do |record, attr, value|
+      valstr = value.to_s


No need to do to_s because ActiveModel / ActiveRecord already done type-casting for us.

nbulaj · 2018-03-12T11:51:52Z

lib/doorkeeper/orm/active_record/application.rb

@@ -12,6 +12,14 @@ class Application < ActiveRecord::Base
    validates :uid, uniqueness: true
    validates :redirect_uri, redirect_uri: true

+    validates_each :scopes, if: :enforce_scopes? do |record, attr, value|
+      valstr = value.to_s
+      unless valstr.blank? ||


Lets rewrite it to if - unless with more than one condition is hard to read and maintain.

nbulaj · 2018-03-12T11:52:39Z

spec/requests/applications/applications_request_spec.rb

+    end
+
+    scenario "adding app validating scope, blank scope is accepted" do
+      config_is_set("enforce_configured_scopes", true)


Let's add a test with multiple scopes to be sure everything is OK

nbulaj · 2018-03-12T11:53:36Z

Hi @talklittle . Could you please take an attention to my comments below? Also could you please add an entry to NEWS.md file and squash all the commits to a single one? Thanks!

houndci-bot · 2018-03-14T07:23:26Z

spec/requests/applications/applications_request_spec.rb

+
+    scenario "adding app validating scope, bad scope with multiple scopes configured" do
+      config_is_set("enforce_configured_scopes", true)
+      config_is_set("optional_scopes", Doorkeeper::OAuth::Scopes.from_array(["read", "write", "admin"]))


Use %w or %W for an array of words.

houndci-bot · 2018-03-14T07:23:26Z

spec/requests/applications/applications_request_spec.rb

+
+    scenario "adding app validating scope, multiple scopes configured" do
+      config_is_set("enforce_configured_scopes", true)
+      config_is_set("optional_scopes", Doorkeeper::OAuth::Scopes.from_array(["read", "write", "admin"]))


Use %w or %W for an array of words.

houndci-bot · 2018-03-14T07:23:26Z

lib/doorkeeper/orm/active_record/application.rb

@@ -41,5 +43,15 @@ def generate_uid
    def generate_secret
      self.secret = UniqueToken.generate if secret.blank?
    end
+
+    def validate_configured_scopes
+      if !scopes.blank? && !ScopeChecker.valid?(scopes.to_s, Doorkeeper.configuration.scopes)


Line is too long. [93/80]

houndci-bot · 2018-03-14T07:27:21Z

lib/doorkeeper/orm/active_record/application.rb

+
+    def validate_configured_scopes
+      if !scopes.blank? &&
+          !ScopeChecker.valid?(scopes.to_s, Doorkeeper.configuration.scopes)


Align the operands of a condition in an if statement spanning multiple lines.

houndci-bot · 2018-03-14T07:27:21Z

lib/doorkeeper/orm/active_record/application.rb

+
+    def validate_configured_scopes
+      if !scopes.blank? &&
+          !ScopeChecker.valid?(scopes.to_s, Doorkeeper.configuration.scopes)


Align the operands of a condition in an if statement spanning multiple lines.

talklittle · 2018-03-14T07:59:26Z

@nbulaj Thanks for review. Applied your feedback.

nbulaj · 2018-03-16T07:44:00Z

Thank you @talklittle 👍

not yet merged into Doorkeeper as of 4.3.1, so temporarily pointing to a git branch. see doorkeeper-gem/doorkeeper#1010

houndci-bot reviewed Jan 3, 2018

View reviewed changes

talklittle force-pushed the validate-application-scopes-configured branch from 12d0977 to b5b20c8 Compare January 3, 2018 05:01

houndci-bot reviewed Jan 3, 2018

View reviewed changes

talklittle force-pushed the validate-application-scopes-configured branch 2 times, most recently from a1592d0 to 750f361 Compare January 3, 2018 05:06

nbulaj force-pushed the master branch from d87618a to 342f887 Compare January 31, 2018 14:57

nbulaj added the bug? label Feb 4, 2018

nbulaj force-pushed the master branch from e122698 to 11b99da Compare February 7, 2018 09:19

talklittle force-pushed the validate-application-scopes-configured branch from 750f361 to 08ae385 Compare February 15, 2018 05:06

talklittle force-pushed the validate-application-scopes-configured branch from 08ae385 to 558f167 Compare February 26, 2018 21:32

talklittle added a commit to talklittle/lobsters that referenced this pull request Feb 26, 2018

oauth: require predefined scopes for applications

d981b06

not yet merged into Doorkeeper as of 4.3.0, so temporarily pointing to a git branch. see doorkeeper-gem/doorkeeper#1010

nbulaj force-pushed the master branch 3 times, most recently from 14903a4 to 54e8345 Compare March 6, 2018 09:10

talklittle force-pushed the validate-application-scopes-configured branch from 558f167 to d4782e8 Compare March 8, 2018 03:07

talklittle added a commit to talklittle/lobsters that referenced this pull request Mar 8, 2018

oauth: require predefined scopes for applications

7e1e589

not yet merged into Doorkeeper as of 4.3.1, so temporarily pointing to a git branch. see doorkeeper-gem/doorkeeper#1010

nbulaj self-assigned this Mar 12, 2018

nbulaj added this to the 5.0 milestone Mar 12, 2018

nbulaj reviewed Mar 12, 2018

View reviewed changes

nbulaj added enhancement and removed bug? labels Mar 12, 2018

talklittle force-pushed the validate-application-scopes-configured branch from d4782e8 to 588890c Compare March 14, 2018 07:23

houndci-bot reviewed Mar 14, 2018

View reviewed changes

talklittle force-pushed the validate-application-scopes-configured branch from 588890c to 69c5a33 Compare March 14, 2018 07:27

houndci-bot reviewed Mar 14, 2018

View reviewed changes

talklittle force-pushed the validate-application-scopes-configured branch from 69c5a33 to 80b8e06 Compare March 14, 2018 07:28

Configuration to enforce application scopes

bcb29d3

talklittle force-pushed the validate-application-scopes-configured branch from 80b8e06 to bcb29d3 Compare March 14, 2018 20:08

nbulaj merged commit 829772b into doorkeeper-gem:master Mar 16, 2018

talklittle added a commit to talklittle/lobsters that referenced this pull request Mar 28, 2018

oauth: require predefined scopes for applications

dc2d508

not yet merged into Doorkeeper as of 4.3.1, so temporarily pointing to a git branch. see doorkeeper-gem/doorkeeper#1010

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Configuration to enforce application scopes #1010

Configuration to enforce application scopes #1010

talklittle commented Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

houndci-bot Jan 3, 2018

talklittle commented Feb 15, 2018

nbulaj Mar 12, 2018

nbulaj Mar 12, 2018

nbulaj Mar 12, 2018

nbulaj Mar 12, 2018

nbulaj commented Mar 12, 2018

houndci-bot Mar 14, 2018

houndci-bot Mar 14, 2018

houndci-bot Mar 14, 2018

houndci-bot Mar 14, 2018

houndci-bot Mar 14, 2018

talklittle commented Mar 14, 2018

nbulaj commented Mar 16, 2018

		@@ -1,94 +1,132 @@
		require 'spec_helper_integration'
		require "spec_helper_integration"

Configuration to enforce application scopes #1010

Configuration to enforce application scopes #1010

Conversation

talklittle commented Jan 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

talklittle commented Feb 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nbulaj commented Mar 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

talklittle commented Mar 14, 2018

nbulaj commented Mar 16, 2018