From: Andy Allan Date: Sat, 23 Feb 2019 15:06:30 +0000 (+0100) Subject: Move the permissions call out of api_controller X-Git-Tag: live~2650^2~2 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/8383fd0928a3a431dc58c48d42bf1d19bc43ea34 Move the permissions call out of api_controller --- diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 3568817a3..4f0ce1bd7 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -4,7 +4,7 @@ class Ability include CanCan::Ability def initialize(user) - can [:map, :changes, :permissions], :api + can [:map, :changes], :api can [:relation, :relation_history, :way, :way_history, :node, :node_history, :changeset, :note, :new_note, :query], :browse can :show, :capability @@ -18,6 +18,7 @@ class Ability :search_geonames, :search_osm_nominatim_reverse, :search_geonames_reverse], :geocoder can [:index, :create, :comment, :feed, :show, :search, :mine], Note can [:token, :request_token, :access_token, :test_request], :oauth + can :show, :permission can [:index, :show], Redaction can [:search_all, :search_nodes, :search_ways, :search_relations], :search can [:trackpoints], :swf diff --git a/app/controllers/api/permissions_controller.rb b/app/controllers/api/permissions_controller.rb new file mode 100644 index 000000000..b24aca776 --- /dev/null +++ b/app/controllers/api/permissions_controller.rb @@ -0,0 +1,27 @@ +module Api + class PermissionsController < ApplicationController + skip_before_action :verify_authenticity_token + before_action :api_deny_access_handler + + authorize_resource :class => false + + before_action :check_api_readable + before_action :setup_user_auth + around_action :api_call_handle_error, :api_call_timeout + + # External apps that use the api are able to query which permissions + # they have. This currently returns a list of permissions granted to the current user: + # * if authenticated via OAuth, this list will contain all permissions granted by the user to the access_token. + # * if authenticated via basic auth all permissions are granted, so the list will contain all permissions. + # * unauthenticated users have no permissions, so the list will be empty. + def show + @permissions = if current_token.present? + ClientApplication.all_permissions.select { |p| current_token.read_attribute(p) } + elsif current_user + ClientApplication.all_permissions + else + [] + end + end + end +end diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 8f9cb6adc..05a69b20b 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -5,7 +5,6 @@ class ApiController < ApplicationController authorize_resource :class => false before_action :check_api_readable - before_action :setup_user_auth, :only => [:permissions] around_action :api_call_handle_error, :api_call_timeout # This is probably the most common call of all. It is used for getting the @@ -149,19 +148,4 @@ class ApiController < ApplicationController render :plain => "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours", :status => :bad_request end end - - # External apps that use the api are able to query which permissions - # they have. This currently returns a list of permissions granted to the current user: - # * if authenticated via OAuth, this list will contain all permissions granted by the user to the access_token. - # * if authenticated via basic auth all permissions are granted, so the list will contain all permissions. - # * unauthenticated users have no permissions, so the list will be empty. - def permissions - @permissions = if current_token.present? - ClientApplication.all_permissions.select { |p| current_token.read_attribute(p) } - elsif current_user - ClientApplication.all_permissions - else - [] - end - end end diff --git a/app/views/api/permissions.builder b/app/views/api/permissions/show.builder similarity index 100% rename from app/views/api/permissions.builder rename to app/views/api/permissions/show.builder diff --git a/config/routes.rb b/config/routes.rb index d90d928a0..a01f1f14c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -6,7 +6,7 @@ OpenStreetMap::Application.routes.draw do scope "api/0.6" do get "capabilities" => "api/capabilities#show" - get "permissions" => "api#permissions" + get "permissions" => "api/permissions#show" put "changeset/create" => "changesets#create" post "changeset/:id/upload" => "changesets#upload", :id => /\d+/ diff --git a/test/controllers/api/permissions_controller_test.rb b/test/controllers/api/permissions_controller_test.rb new file mode 100644 index 000000000..eb1bfed16 --- /dev/null +++ b/test/controllers/api/permissions_controller_test.rb @@ -0,0 +1,51 @@ +require "test_helper" + +module Api + class PermissionsControllerTest < ActionController::TestCase + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/api/0.6/permissions", :method => :get }, + { :controller => "api/permissions", :action => "show" } + ) + end + + def test_permissions_anonymous + get :show + assert_response :success + assert_select "osm > permissions", :count => 1 do + assert_select "permission", :count => 0 + end + end + + def test_permissions_basic_auth + basic_authorization create(:user).email, "test" + get :show + assert_response :success + assert_select "osm > permissions", :count => 1 do + assert_select "permission", :count => ClientApplication.all_permissions.size + ClientApplication.all_permissions.each do |p| + assert_select "permission[name='#{p}']", :count => 1 + end + end + end + + def test_permissions_oauth + @request.env["oauth.token"] = AccessToken.new do |token| + # Just to test a few + token.allow_read_prefs = true + token.allow_write_api = true + token.allow_read_gpx = false + end + get :show + assert_response :success + assert_select "osm > permissions", :count => 1 do + assert_select "permission", :count => 2 + assert_select "permission[name='allow_read_prefs']", :count => 1 + assert_select "permission[name='allow_write_api']", :count => 1 + assert_select "permission[name='allow_read_gpx']", :count => 0 + end + end + end +end diff --git a/test/controllers/api_controller_test.rb b/test/controllers/api_controller_test.rb index 9fc0c1567..3f4196ffb 100644 --- a/test/controllers/api_controller_test.rb +++ b/test/controllers/api_controller_test.rb @@ -18,10 +18,6 @@ class ApiControllerTest < ActionController::TestCase ## # test all routes which lead to this controller def test_routes - assert_routing( - { :path => "/api/0.6/permissions", :method => :get }, - { :controller => "api", :action => "permissions" } - ) assert_routing( { :path => "/api/0.6/map", :method => :get }, { :controller => "api", :action => "map" } @@ -276,41 +272,4 @@ class ApiControllerTest < ActionController::TestCase get :changes, :params => { :start => "2010-04-03 09:55:00", :end => "2010-04-03 10:55:00" } assert_response :success end - - def test_permissions_anonymous - get :permissions - assert_response :success - assert_select "osm > permissions", :count => 1 do - assert_select "permission", :count => 0 - end - end - - def test_permissions_basic_auth - basic_authorization create(:user).email, "test" - get :permissions - assert_response :success - assert_select "osm > permissions", :count => 1 do - assert_select "permission", :count => ClientApplication.all_permissions.size - ClientApplication.all_permissions.each do |p| - assert_select "permission[name='#{p}']", :count => 1 - end - end - end - - def test_permissions_oauth - @request.env["oauth.token"] = AccessToken.new do |token| - # Just to test a few - token.allow_read_prefs = true - token.allow_write_api = true - token.allow_read_gpx = false - end - get :permissions - assert_response :success - assert_select "osm > permissions", :count => 1 do - assert_select "permission", :count => 2 - assert_select "permission[name='allow_read_prefs']", :count => 1 - assert_select "permission[name='allow_write_api']", :count => 1 - assert_select "permission[name='allow_read_gpx']", :count => 0 - end - end end