Implement review comments from @dbalexandre for !12300.
This commit is contained in:
		
							parent
							
								
									1b8223dd51
								
							
						
					
					
						commit
						4dbfa14e16
					
				|  | @ -30,15 +30,13 @@ module API | |||
|       # endpoint class. If this method is called multiple times on the same class, | ||||
|       # the scopes are all aggregated. | ||||
|       def allow_access_with_scope(scopes, options = {}) | ||||
|         @scopes ||= [] | ||||
| 
 | ||||
|         params = Array.wrap(scopes).map { |scope| { name: scope, if: options[:if] } } | ||||
| 
 | ||||
|         @scopes.concat(params) | ||||
|         Array(scopes).each do |scope| | ||||
|           allowed_scopes << { name: scope, if: options[:if] } | ||||
|         end | ||||
|       end | ||||
| 
 | ||||
|       def scopes | ||||
|         @scopes | ||||
|       def allowed_scopes | ||||
|         @scopes ||= [] | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|  |  | |||
|  | @ -416,8 +416,8 @@ module API | |||
|         begin | ||||
|           endpoint_classes = [options[:for].presence, ::API::API].compact | ||||
|           endpoint_classes.reduce([]) do |memo, endpoint| | ||||
|             if endpoint.respond_to?(:scopes) | ||||
|               memo.concat(endpoint.scopes) | ||||
|             if endpoint.respond_to?(:allowed_scopes) | ||||
|               memo.concat(endpoint.allowed_scopes) | ||||
|             else | ||||
|               memo | ||||
|             end | ||||
|  |  | |||
|  | @ -41,24 +41,22 @@ describe AccessTokenValidationService, services: true do | |||
|     end | ||||
| 
 | ||||
|     context "conditions" do | ||||
|       context "if" do | ||||
|         it "ignores any scopes whose `if` condition returns false" do | ||||
|           token = double("token", scopes: [:api, :read_user]) | ||||
|       it "ignores any scopes whose `if` condition returns false" do | ||||
|         token = double("token", scopes: [:api, :read_user]) | ||||
| 
 | ||||
|           expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { false } }])).to be(false) | ||||
|         end | ||||
|         expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { false } }])).to be(false) | ||||
|       end | ||||
| 
 | ||||
|         it "does not ignore scopes whose `if` condition is not set" do | ||||
|           token = double("token", scopes: [:api, :read_user]) | ||||
|       it "does not ignore scopes whose `if` condition is not set" do | ||||
|         token = double("token", scopes: [:api, :read_user]) | ||||
| 
 | ||||
|           expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { false } }, { name: :read_user }])).to be(true) | ||||
|         end | ||||
|         expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { false } }, { name: :read_user }])).to be(true) | ||||
|       end | ||||
| 
 | ||||
|         it "does not ignore scopes whose `if` condition returns true" do | ||||
|           token = double("token", scopes: [:api, :read_user]) | ||||
|       it "does not ignore scopes whose `if` condition returns true" do | ||||
|         token = double("token", scopes: [:api, :read_user]) | ||||
| 
 | ||||
|           expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { true } }, { name: :read_user, if: ->(_) { false } }])).to be(true) | ||||
|         end | ||||
|         expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { true } }, { name: :read_user, if: ->(_) { false } }])).to be(true) | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue