diff --git a/config/feature_flags/development/api_redirect_moved_projects.yml b/config/feature_flags/development/api_redirect_moved_projects.yml new file mode 100644 index 0000000000000000000000000000000000000000..56c01d3862b0d3cc4ff87a75616185d03329a4c6 --- /dev/null +++ b/config/feature_flags/development/api_redirect_moved_projects.yml @@ -0,0 +1,8 @@ +--- +name: api_redirect_moved_projects +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/128642 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/421992 +milestone: '16.3' +type: development +group: group::source code +default_enabled: false diff --git a/doc/api/rest/index.md b/doc/api/rest/index.md index 20e260cfbb46b3dfd08c46ac1d1670d0348904e1..f24b3225f69084c522a396e95e43fece05b1b3ba 100644 --- a/doc/api/rest/index.md +++ b/doc/api/rest/index.md @@ -314,6 +314,7 @@ The following table shows the possible return codes for API requests. | `201 Created` | The `POST` request was successful, and the resource is returned as JSON. | | `202 Accepted` | The `GET`, `PUT` or `DELETE` request was successful, and the resource is scheduled for processing. | | `204 No Content` | The server has successfully fulfilled the request, and there is no additional content to send in the response payload body. | +| `301 Moved Permanently` | The resource has been definitively moved to the URL given by the `Location` headers. | | `304 Not Modified` | The resource hasn't been modified since the last request. | | `400 Bad Request` | A required attribute of the API request is missing. For example, the title of an issue is not given. | | `401 Unauthorized` | The user isn't authenticated. A valid [user token](#authentication) is necessary. | @@ -327,6 +328,33 @@ The following table shows the possible return codes for API requests. | `500 Server Error` | While handling the request, something went wrong on the server. | | `503 Service Unavailable` | The server cannot handle the request because the server is temporarily overloaded. | +## Redirects + +> Introduced in GitLab 16.4 [with a flag](../../user/feature_flags.md) named `api_redirect_moved_projects`. Disabled by default. + +FLAG: +On GitLab.com, this feature is not available. +On self-managed GitLab, by default this feature is not available. To make it available, +an administrator can [enable the feature flag](../../user/feature_flags.md) named `api_redirect_moved_projects`. + +REST API can respond with a redirect and users should be able to handle such responses. +The users should follow the redirect and repeat the request to the URI specified in the `Location` header. + +Example of a project moved to a different path: + +```shell +curl --verbose "https://gitlab.example.com/api/v4/projects/gitlab-org%2Fold-path-project" +``` + +The response is: + +```plaintext +... +< Location: http://gitlab.example.com/api/v4/projects/81 +... +This resource has been moved permanently to https://gitlab.example.com/api/v4/projects/81 +``` + ## Pagination GitLab supports the following pagination methods: diff --git a/ee/lib/ee/api/helpers.rb b/ee/lib/ee/api/helpers.rb index c347818f8a68ebaee7d97d8b7c408ea119023ec1..32c10105ef150e41e8f6e5876d921727c2d983b2 100644 --- a/ee/lib/ee/api/helpers.rb +++ b/ee/lib/ee/api/helpers.rb @@ -69,21 +69,12 @@ def authenticated_with_ldap_admin_access! .allow_group_owners_to_manage_ldap end - override :find_project! - def find_project!(id) - project = find_project(id) - - return forbidden! unless authorized_project_scope?(project) - + override :read_project_ability + def read_project_ability # CI job token authentication: # this method grants limited privileged for admin users # admin users can only access project if they are direct member - ability = job_token_authentication? ? :build_read_project : :read_project - - return project if can?(current_user, ability, project) - return unauthorized! if authenticate_non_public? - - not_found!('Project') + job_token_authentication? ? :build_read_project : super end override :find_group! diff --git a/ee/spec/requests/api/analytics/product_analytics_spec.rb b/ee/spec/requests/api/analytics/product_analytics_spec.rb index ba04a02919c78f1e1754978026c162889451e938..78a462bd5fea5213054b5e9ea577f8248dab8b31 100644 --- a/ee/spec/requests/api/analytics/product_analytics_spec.rb +++ b/ee/spec/requests/api/analytics/product_analytics_spec.rb @@ -71,6 +71,18 @@ let(:request) { get api("/projects/#{project.id}/product_analytics/funnels", current_user) } it_behaves_like 'well behaved cube query', { stub_service: false } + + context 'when a project is moved' do + let_it_be(:redirect_route) { 'new/project/location' } + + it 'returns 404 error' do + project.route.create_redirect(redirect_route) + + get api("/projects/#{CGI.escape(redirect_route)}/product_analytics/funnels", current_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end end private diff --git a/lib/api/api.rb b/lib/api/api.rb index 7a384d9cf59fd7490f1dcfe8f7668314933bb756..8ebd7f83acbb0eaa7e91af2da60ab82c4d571c93 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -15,6 +15,18 @@ class API < ::API::Base LOG_FORMATTER = Gitlab::GrapeLogging::Formatters::LogrageWithTimestamp.new LOGGER = Logger.new(LOG_FILENAME) + class MovedPermanentlyError < StandardError + MSG_PREFIX = 'This resource has been moved permanently to' + + attr_reader :location_url + + def initialize(location_url) + @location_url = location_url + + super("#{MSG_PREFIX} #{location_url}") + end + end + insert_before Grape::Middleware::Error, GrapeLogging::Middleware::RequestLogger, logger: LOGGER, @@ -142,6 +154,10 @@ class API < ::API::Base error! e.message, e.status, e.headers end + rescue_from MovedPermanentlyError do |e| + rack_response(e.message, 301, { 'Location' => e.location_url }) + end + rescue_from Gitlab::Auth::TooManyIps do |e| rack_response({ 'message' => '403 Forbidden' }.to_json, 403) end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index b616f1b35b3aa4f4e3efb5bbd80c5c171b380c7e..e1207e7e222f7279f78bab5cfe54e072f986d511 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -147,7 +147,7 @@ def find_project(id) if id.is_a?(Integer) || id =~ INTEGER_ID_REGEX projects.find_by(id: id) elsif id.include?("/") - projects.find_by_full_path(id) + projects.find_by_full_path(id, follow_redirects: Feature.enabled?(:api_redirect_moved_projects)) end end # rubocop: enable CodeReuse/ActiveRecord @@ -157,10 +157,23 @@ def find_project!(id) return forbidden! unless authorized_project_scope?(project) - return project if can?(current_user, :read_project, project) - return unauthorized! if authenticate_non_public? + unless can?(current_user, read_project_ability, project) + return unauthorized! if authenticate_non_public? + + return not_found!('Project') + end + + if project_moved?(id, project) + return not_allowed!('Non GET methods are not allowed for moved projects') unless request.get? + + return redirect!(url_with_project_id(project)) + end + + project + end - not_found!('Project') + def read_project_ability + :read_project end def authorized_project_scope?(project) @@ -438,6 +451,13 @@ def order_options_with_tie_breaker order_options end + # An error is raised to interrupt user's request and redirect them to the right route. + # The error! helper behaves similarly, but it cannot be used because it formats the + # response message. + def redirect!(location_url) + raise ::API::API::MovedPermanentlyError, location_url + end + # error helpers def forbidden!(reason = nil) @@ -837,6 +857,24 @@ def ip_address def sanitize_id_param(id) id.present? ? id.to_i : nil end + + def project_moved?(id, project) + return false unless Feature.enabled?(:api_redirect_moved_projects) + return false unless id.is_a?(String) && id.include?('/') + return false if project.blank? || id == project.full_path + return false unless params[:id] == id + + true + end + + def url_with_project_id(project) + new_params = params.merge(id: project.id.to_s).transform_values { |v| v.is_a?(String) ? CGI.escape(v) : v } + new_path = GrapePathHelpers::DecoratedRoute.new(route).path_segments_with_values(new_params).join('/') + + Rack::Request.new(env).tap do |r| + r.path_info = "/#{new_path}" + end.url + end end end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index ea341703301e020c69a638473e69fb3f16e65dd9..01acb83360c2d51fee1ee026426ac7ab25876f5e 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -342,6 +342,23 @@ def expect_to_send_git_blob(url, params) expect(response).to have_gitlab_http_status(:ok) end + context 'when a project is moved' do + let_it_be(:redirect_route) { 'new/project/location' } + let_it_be(:file_path) { 'files%2Fruby%2Fpopen.rb' } + + it 'redirects to the new project location' do + project.route.create_redirect(redirect_route) + + url = "/projects/#{CGI.escape(redirect_route)}/repository/files/#{file_path}" + get api(url, api_user, **options), params: params + + expect(response).to have_gitlab_http_status(:moved_permanently) + expect(response.headers['Location']).to start_with( + "#{request.base_url}/api/v4/projects/#{project.id}/repository/files/#{file_path}" + ) + end + end + it 'sets inline content disposition by default' do url = route(file_path) + '/raw' diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 34447e12d115342886a4e05342523af8329c2c34..26132215404f7e8f15d983c1af6287e41afd3016 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -2770,6 +2770,45 @@ def failure_message(diff) expect(json_response['name']).to eq(project.name) end + context 'when a project is moved' do + let(:redirect_route) { 'new/project/location' } + let(:perform_request) { get api("/projects/#{CGI.escape(redirect_route)}", user), params: { license: true } } + + before do + project.route.create_redirect(redirect_route) + end + + it 'redirects to the new project location' do + perform_request + + expect(response).to have_gitlab_http_status(:moved_permanently) + + url = response.headers['Location'] + expect(url).to start_with("#{request.base_url}/api/v4/projects/#{project.id}") + expect(CGI.parse(URI(url).query)).to include({ 'license' => ['true'] }) + end + + context 'when a user do not have access' do + let(:user) { create(:user) } + + it 'returns a 404 error' do + perform_request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when api_redirect_moved_projects is disabled' do + it 'returns a 404 error' do + stub_feature_flags(api_redirect_moved_projects: false) + + perform_request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + it 'returns a 404 error if not found' do get api("/projects/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) @@ -4454,6 +4493,29 @@ def failure_message(diff) expect(response).to have_gitlab_http_status(:forbidden) end end + + context 'when a project is moved' do + let_it_be(:redirect_route) { 'new/project/location' } + let_it_be(:path) { "/projects/#{CGI.escape(redirect_route)}/archive" } + + before do + project.route.create_redirect(redirect_route) + end + + it 'returns 405 error' do + post api(path, user) + + expect(response).to have_gitlab_http_status(:method_not_allowed) + end + + context 'when user do not have access to the project' do + it 'returns 404 error' do + post api(path, create(:user)) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end end describe 'POST /projects/:id/unarchive' do