diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb index 00d2cc01192..6fc336714b6 100644 --- a/app/controllers/admin/applications_controller.rb +++ b/app/controllers/admin/applications_controller.rb @@ -6,11 +6,9 @@ class Admin::ApplicationsController < Admin::ApplicationController before_action :set_application, only: [:show, :edit, :update, :destroy] before_action :load_scopes, only: [:new, :create, :edit, :update] - # rubocop: disable CodeReuse/ActiveRecord def index - @applications = Doorkeeper::Application.where("owner_id IS NULL") + @applications = ApplicationsFinder.new.execute end - # rubocop: enable CodeReuse/ActiveRecord def show end @@ -49,11 +47,9 @@ class Admin::ApplicationsController < Admin::ApplicationController private - # rubocop: disable CodeReuse/ActiveRecord def set_application - @application = Doorkeeper::Application.where("owner_id IS NULL").find(params[:id]) + @application = ApplicationsFinder.new(id: params[:id]).execute end - # rubocop: enable CodeReuse/ActiveRecord # Only allow a trusted parameter "white list" through. def application_params diff --git a/app/finders/applications_finder.rb b/app/finders/applications_finder.rb new file mode 100644 index 00000000000..3ded90f3fd5 --- /dev/null +++ b/app/finders/applications_finder.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class ApplicationsFinder + attr_reader :params + + def initialize(params = {}) + @params = params + end + + def execute + applications = Doorkeeper::Application.where(owner_id: nil) # rubocop: disable CodeReuse/ActiveRecord + by_id(applications) + end + + private + + def by_id(applications) + return applications unless params[:id] + + Doorkeeper::Application.find_by(id: params[:id]) # rubocop: disable CodeReuse/ActiveRecord + end +end diff --git a/changelogs/unreleased/52559-applications-api-get-delete.yml b/changelogs/unreleased/52559-applications-api-get-delete.yml new file mode 100644 index 00000000000..19f98a2bb56 --- /dev/null +++ b/changelogs/unreleased/52559-applications-api-get-delete.yml @@ -0,0 +1,5 @@ +--- +title: Add Applications API endpoints for listing and deleting entries. +merge_request: 22296 +author: Jean-Baptiste Vasseur +type: added diff --git a/doc/api/applications.md b/doc/api/applications.md index 6d244594b71..d74a3cdf5c1 100644 --- a/doc/api/applications.md +++ b/doc/api/applications.md @@ -4,12 +4,12 @@ [ce-8160]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8160 +Only admin user can use the Applications API. + ## Create a application Create a application by posting a JSON payload. -User must be admin to do that. - Returns `200` if the request succeeds. ``` @@ -30,8 +30,55 @@ Example response: ```json { + "id":1, "application_id": "5832fc6e14300a0d962240a8144466eef4ee93ef0d218477e55f11cf12fc3737", + "application_name": "MyApplication", "secret": "ee1dd64b6adc89cf7e2c23099301ccc2c61b441064e9324d963c46902a85ec34", "callback_url": "http://redirect.uri" } ``` + +## List all applications + +List all registered applications. + +``` +GET /applications +``` + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/applications +``` + +Example response: + +```json +[ + { + "id":1, + "application_id": "5832fc6e14300a0d962240a8144466eef4ee93ef0d218477e55f11cf12fc3737", + "application_name": "MyApplication", + "callback_url": "http://redirect.uri" + } +] +``` + +> Note: the `secret` value will not be exposed by this API. + +## Delete an application + +Delete a specific application. + +Returns `204` if the request succeeds. + +``` +DELETE /applications/:id +``` + +Parameters: + +- `id` (required) - The id of the application (not the application_id) + +```bash +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/applications/:id +``` diff --git a/lib/api/applications.rb b/lib/api/applications.rb index f29cd7fc003..92717e04543 100644 --- a/lib/api/applications.rb +++ b/lib/api/applications.rb @@ -24,6 +24,22 @@ module API render_validation_error! application end end + + desc 'Get applications' do + success Entities::Application + end + get do + applications = ApplicationsFinder.new.execute + present applications, with: Entities::Application + end + + desc 'Delete an application' + delete ':id' do + application = ApplicationsFinder.new(params).execute + application.destroy + + status 204 + end end end end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 65855e8aac4..18c30723d73 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1412,7 +1412,9 @@ module API end class Application < Grape::Entity + expose :id expose :uid, as: :application_id + expose :name, as: :application_name expose :redirect_uri, as: :callback_url end diff --git a/spec/finders/applications_finder_spec.rb b/spec/finders/applications_finder_spec.rb new file mode 100644 index 00000000000..14d6b35cc27 --- /dev/null +++ b/spec/finders/applications_finder_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ApplicationsFinder do + let(:application1) { create(:application, name: 'some_application', owner: nil, redirect_uri: 'http://some_application.url', scopes: '') } + let(:application2) { create(:application, name: 'another_application', owner: nil, redirect_uri: 'http://other_application.url', scopes: '') } + + describe '#execute' do + it 'returns an array of applications' do + found = described_class.new.execute + + expect(found).to match_array([application1, application2]) + end + it 'returns the application by id' do + params = { id: application1.id } + found = described_class.new(params).execute + + expect(found).to match(application1) + end + end +end diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb index f56bc932f40..270e12bf201 100644 --- a/spec/requests/api/applications_spec.rb +++ b/spec/requests/api/applications_spec.rb @@ -5,6 +5,7 @@ describe API::Applications, :api do let(:admin_user) { create(:user, admin: true) } let(:user) { create(:user, admin: false) } + let!(:application) { create(:application, name: 'another_application', redirect_uri: 'http://other_application.url', scopes: '') } describe 'POST /applications' do context 'authenticated and authorized user' do @@ -15,7 +16,7 @@ describe API::Applications, :api do application = Doorkeeper::Application.find_by(name: 'application_name', redirect_uri: 'http://application.url') - expect(response).to have_http_status 201 + expect(response).to have_gitlab_http_status(201) expect(json_response).to be_a Hash expect(json_response['application_id']).to eq application.uid expect(json_response['secret']).to eq application.secret @@ -27,7 +28,7 @@ describe API::Applications, :api do post api('/applications', admin_user), name: 'application_name', redirect_uri: 'wrong_url_format', scopes: '' end.not_to change { Doorkeeper::Application.count } - expect(response).to have_http_status 400 + expect(response).to have_gitlab_http_status(400) expect(json_response).to be_a Hash expect(json_response['message']['redirect_uri'][0]).to eq('must be an absolute URI.') end @@ -37,7 +38,7 @@ describe API::Applications, :api do post api('/applications', admin_user), redirect_uri: 'http://application.url', scopes: '' end.not_to change { Doorkeeper::Application.count } - expect(response).to have_http_status 400 + expect(response).to have_gitlab_http_status(400) expect(json_response).to be_a Hash expect(json_response['error']).to eq('name is missing') end @@ -47,7 +48,7 @@ describe API::Applications, :api do post api('/applications', admin_user), name: 'application_name', scopes: '' end.not_to change { Doorkeeper::Application.count } - expect(response).to have_http_status 400 + expect(response).to have_gitlab_http_status(400) expect(json_response).to be_a Hash expect(json_response['error']).to eq('redirect_uri is missing') end @@ -57,7 +58,7 @@ describe API::Applications, :api do post api('/applications', admin_user), name: 'application_name', redirect_uri: 'http://application.url' end.not_to change { Doorkeeper::Application.count } - expect(response).to have_http_status 400 + expect(response).to have_gitlab_http_status(400) expect(json_response).to be_a Hash expect(json_response['error']).to eq('scopes is missing') end @@ -69,7 +70,7 @@ describe API::Applications, :api do post api('/applications', user), name: 'application_name', redirect_uri: 'http://application.url', scopes: '' end.not_to change { Doorkeeper::Application.count } - expect(response).to have_http_status 403 + expect(response).to have_gitlab_http_status(403) end end @@ -79,7 +80,62 @@ describe API::Applications, :api do post api('/applications'), name: 'application_name', redirect_uri: 'http://application.url' end.not_to change { Doorkeeper::Application.count } - expect(response).to have_http_status 401 + expect(response).to have_gitlab_http_status(401) + end + end + end + + describe 'GET /applications' do + context 'authenticated and authorized user' do + it 'can list application' do + get api('/applications', admin_user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to be_a(Array) + end + end + + context 'authorized user without authorization' do + it 'cannot list application' do + get api('/applications', user) + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'non-authenticated user' do + it 'cannot list application' do + get api('/applications') + + expect(response).to have_gitlab_http_status(401) + end + end + end + + describe 'DELETE /applications/:id' do + context 'authenticated and authorized user' do + it 'can delete an application' do + expect do + delete api("/applications/#{application.id}", admin_user) + end.to change { Doorkeeper::Application.count }.by(-1) + + expect(response).to have_gitlab_http_status(204) + end + end + + context 'authorized user without authorization' do + it 'cannot delete an application' do + delete api("/applications/#{application.id}", user) + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'non-authenticated user' do + it 'cannot delete an application' do + delete api("/applications/#{application.id}") + + expect(response).to have_gitlab_http_status(401) end end end