diff --git a/CHANGELOG.md b/CHANGELOG.md index f2ce2a42..89c69ed9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ #### Fixes +* [#1016](https://github.com/ruby-grape/grape-swagger/pull/1016): Hide the nested parameters of a Hash parameter marked `documentation: { hidden: true }`, and pass the resolved `token_owner` to parameter-level `hidden:` procs like route-level ones (raising `TokenOwnerNotFound` if the configured method is missing) - [@numbata](https://github.com/numbata). * [#1012](https://github.com/ruby-grape/grape-swagger/pull/1012): Keep the Danger PR check running in CI by pinning `git` below 6 - [@numbata](https://github.com/numbata). * [#1011](https://github.com/ruby-grape/grape-swagger/pull/1011): Preserve string length, array item and object property limits in body parameter definitions, and emit `minLength`/`maxLength` only for strings (`minProperties`/`maxProperties` for hashes) instead of any non-array type - [@numbata](https://github.com/numbata). * [#1009](https://github.com/ruby-grape/grape-swagger/pull/1009): Bump the Danger `danger-comment.yml` workflow from 0.1.0 to 0.1.1 - [@dependabot[bot]](https://github.com/dependabot%5Bbot%5D). diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index fba5fb40..25fcde2f 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -171,8 +171,12 @@ def consumes_object(route, format) end def params_object(route, options, path, consumes) - parameters = build_request_params(route, options).each_with_object([]) do |(param, value), memo| - next if hidden_parameter?(value) + request_params = build_request_params(route, options) + token_owner = lazy_token_owner(options) + hidden_roots = request_params.select { |_, value| hidden_parameter?(value, token_owner) }.keys + + parameters = request_params.each_with_object([]) do |(param, value), memo| + next if hidden_roots.any? { |root| param == root || param.start_with?("#{root}[") } value = { required: false }.merge(value) if value.is_a?(Hash) _, value = default_type([[param, value]]).first if value == '' @@ -434,21 +438,34 @@ def hidden?(route, options) route_hidden = route.settings.try(:[], :swagger).try(:[], :hidden) hidden_val = route.hidden route_hidden = hidden_val unless hidden_val.nil? - return route_hidden unless route_hidden.is_a?(Proc) + evaluate_hidden(route_hidden, lazy_token_owner(options)) + end - return route_hidden.call unless options[:token_owner] + def hidden_parameter?(value, token_owner) + return false if value[:required] - token_owner = GrapeSwagger::TokenOwnerResolver.resolve(self, options[:token_owner]) - GrapeSwagger::TokenOwnerResolver.evaluate_proc(route_hidden, token_owner) + evaluate_hidden(value.dig(:documentation, :hidden), token_owner) end - def hidden_parameter?(value) - return false if value[:required] + def evaluate_hidden(hidden, token_owner) + return hidden unless hidden.is_a?(Proc) + return hidden.call unless token_owner - if value.dig(:documentation, :hidden).is_a?(Proc) - value.dig(:documentation, :hidden).call - else - value.dig(:documentation, :hidden) + GrapeSwagger::TokenOwnerResolver.evaluate_proc(hidden, token_owner.call) + end + + # Resolves the token owner at most once, and only if a hidden proc asks for it. + def lazy_token_owner(options) + return unless options[:token_owner] + + resolved = false + owner = nil + lambda do + unless resolved + owner = GrapeSwagger::TokenOwnerResolver.resolve(self, options[:token_owner]) + resolved = true + end + owner end end diff --git a/spec/issues/1015_hidden_hash_children_spec.rb b/spec/issues/1015_hidden_hash_children_spec.rb new file mode 100644 index 00000000..3be90299 --- /dev/null +++ b/spec/issues/1015_hidden_hash_children_spec.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'hidden parameter with nested children' do + def parameter_names(path, verb = 'get') + get '/swagger_doc' + Array(JSON.parse(last_response.body).dig('paths', path, verb, 'parameters')).map { |p| p['name'] } + end + + context 'with query parameters' do + let(:app) do + Class.new(Grape::API) do + format :json + + params do + optional :query, type: String + optional :filters, type: Hash, documentation: { hidden: true } do + optional :status, type: String + optional :nested, type: Hash do + optional :kind, type: String + end + end + optional :filters_extra, type: String + optional :partial, type: Hash do + optional :shown, type: String + optional :secret, type: String, documentation: { hidden: true } + end + optional :other, type: Hash do + optional :visible, type: String + end + optional :tags, type: Array, documentation: { hidden: true } do + optional :name, type: String + end + optional :proc_hidden, type: Hash, documentation: { hidden: -> { true } } do + optional :child, type: String + end + end + get('/search') { 'ok' } + + add_swagger_documentation + end + end + + it 'hides the nested children together with the parent only' do + expect(parameter_names('/search')).to eq(%w[query filters_extra partial[shown] other[visible]]) + end + end + + context 'with a hidden proc that receives the token owner' do + let(:app) do + current = owner + + Class.new(Grape::API) do + format :json + + helpers do + define_method(:current_user) { current } + end + + params do + optional :query, type: String + optional :admin_hidden, type: Hash, documentation: { hidden: ->(user) { user == :admin } } do + optional :status, type: String + end + optional :guest_hidden, type: Hash, documentation: { hidden: ->(user) { user == :guest } } do + optional :status, type: String + end + end + get('/search') { 'ok' } + + add_swagger_documentation token_owner: 'current_user' + end + end + + context 'when the owner makes the proc truthy' do + let(:owner) { :admin } + + it 'hides the parent and its children' do + expect(parameter_names('/search')).to eq(%w[query guest_hidden[status]]) + end + end + + context 'when the owner makes the proc falsy' do + let(:owner) { :guest } + + it 'keeps the parent and its children' do + expect(parameter_names('/search')).to eq(%w[query admin_hidden[status]]) + end + end + end + + context 'with body parameters' do + let(:app) do + Class.new(Grape::API) do + format :json + + params do + optional :query, type: String + optional :filters, type: Hash, documentation: { hidden: true } do + optional :status, type: String + end + end + post('/search') { 'ok' } + + add_swagger_documentation + end + end + + # regression guard: body params were already handled before nested hiding was added + it 'leaves no hidden children in the body definition' do + get '/swagger_doc' + json = JSON.parse(last_response.body) + definition = json['definitions']['postSearch'] + expect(definition['properties'].keys).to eq(%w[query]) + end + end +end