Commit b7a6ad82 authored by Douwe Maan's avatar Douwe Maan Committed by Rémy Coutable

Merge branch '22435-no-api-state-change-via-rails-session' into 'security'

API: disable rails session auth for non-GET/HEAD requests

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/22435

See merge request !1999
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent 50562ab8
...@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
v 8.10.11 v 8.10.11
- Respect the fork_project permission when forking projects - Respect the fork_project permission when forking projects
- Set a restrictive CORS policy on the API for credentialed requests - Set a restrictive CORS policy on the API for credentialed requests
- API: disable rails session auth for non-GET/HEAD requests
v 8.10.10 v 8.10.10
- Allow the Rails cookie to be used for API authentication. - Allow the Rails cookie to be used for API authentication.
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
namespacesPath: "/api/:version/namespaces.json" namespacesPath: "/api/:version/namespaces.json"
groupProjectsPath: "/api/:version/groups/:id/projects.json" groupProjectsPath: "/api/:version/groups/:id/projects.json"
projectsPath: "/api/:version/projects.json?simple=true" projectsPath: "/api/:version/projects.json?simple=true"
labelsPath: "/api/:version/projects/:id/labels" labelsPath: "/:namespace_path/:project_path/labels"
licensePath: "/api/:version/licenses/:key" licensePath: "/api/:version/licenses/:key"
gitignorePath: "/api/:version/gitignores/:key" gitignorePath: "/api/:version/gitignores/:key"
gitlabCiYmlPath: "/api/:version/gitlab_ci_ymls/:key" gitlabCiYmlPath: "/api/:version/gitlab_ci_ymls/:key"
...@@ -60,14 +60,15 @@ ...@@ -60,14 +60,15 @@
).done (projects) -> ).done (projects) ->
callback(projects) callback(projects)
newLabel: (project_id, data, callback) -> newLabel: (namespace_path, project_path, data, callback) ->
url = Api.buildUrl(Api.labelsPath) url = Api.buildUrl(Api.labelsPath)
url = url.replace(':id', project_id) url = url.replace(':namespace_path', namespace_path)
.replace(':project_path', project_path)
$.ajax( $.ajax(
url: url url: url
type: "POST" type: "POST"
data: data data: {'label': data}
dataType: "json" dataType: "json"
).done (label) -> ).done (label) ->
callback(label) callback(label)
......
...@@ -4,7 +4,8 @@ class @LabelsSelect ...@@ -4,7 +4,8 @@ class @LabelsSelect
$('.js-label-select').each (i, dropdown) -> $('.js-label-select').each (i, dropdown) ->
$dropdown = $(dropdown) $dropdown = $(dropdown)
projectId = $dropdown.data('project-id') namespacePath = $dropdown.data('namespace-path')
projectPath = $dropdown.data('project-path')
labelUrl = $dropdown.data('labels') labelUrl = $dropdown.data('labels')
issueUpdateURL = $dropdown.data('issueUpdate') issueUpdateURL = $dropdown.data('issueUpdate')
selectedLabel = $dropdown.data('selected') selectedLabel = $dropdown.data('selected')
...@@ -88,7 +89,7 @@ class @LabelsSelect ...@@ -88,7 +89,7 @@ class @LabelsSelect
saveLabel = -> saveLabel = ->
# Create new label with API # Create new label with API
Api.newLabel projectId, { Api.newLabel namespacePath, projectPath, {
name: newLabelField.val() name: newLabelField.val()
color: newColorField.val() color: newColorField.val()
}, (label) -> }, (label) ->
......
...@@ -30,9 +30,15 @@ class Projects::LabelsController < Projects::ApplicationController ...@@ -30,9 +30,15 @@ class Projects::LabelsController < Projects::ApplicationController
@label = @project.labels.create(label_params) @label = @project.labels.create(label_params)
if @label.valid? if @label.valid?
redirect_to namespace_project_labels_path(@project.namespace, @project) respond_to do |format|
format.html { redirect_to namespace_project_labels_path(@project.namespace, @project) }
format.json { render json: @label }
end
else else
render 'new' respond_to do |format|
format.html { render 'new' }
format.json { render json: { message: @label.errors.messages }, status: 400 }
end
end end
end end
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
- show_footer = local_assigns.fetch(:show_footer, true) - show_footer = local_assigns.fetch(:show_footer, true)
- data_options = local_assigns.fetch(:data_options, {}) - data_options = local_assigns.fetch(:data_options, {})
- classes = local_assigns.fetch(:classes, []) - classes = local_assigns.fetch(:classes, [])
- dropdown_data = {toggle: 'dropdown', field_name: 'label_name[]', show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"} - dropdown_data = {toggle: 'dropdown', field_name: 'label_name[]', show_no: "true", show_any: "true", selected: params[:label_name], namespace_path: @project.try(:namespace).try(:path), project_path: @project.try(:path), labels: labels_filter_path, default_label: "Label"}
- dropdown_data.merge!(data_options) - dropdown_data.merge!(data_options)
- classes << 'js-extra-options' if extra_options - classes << 'js-extra-options' if extra_options
- classes << 'js-filter-submit' if filter_submit - classes << 'js-filter-submit' if filter_submit
......
...@@ -128,7 +128,7 @@ ...@@ -128,7 +128,7 @@
- issuable.labels_array.each do |label| - issuable.labels_array.each do |label|
= hidden_field_tag "#{issuable.to_ability_name}[label_names][]", label.id, id: nil = hidden_field_tag "#{issuable.to_ability_name}[label_names][]", label.id, id: nil
.dropdown .dropdown
%button.dropdown-menu-toggle.js-label-select.js-multiselect{type: "button", data: {toggle: "dropdown", field_name: "#{issuable.to_ability_name}[label_names][]", ability_name: issuable.to_ability_name, show_no: "true", show_any: "true", project_id: (@project.id if @project), issue_update: issuable_json_path(issuable), labels: (namespace_project_labels_path(@project.namespace, @project, :json) if @project)}} %button.dropdown-menu-toggle.js-label-select.js-multiselect{type: "button", data: {toggle: "dropdown", field_name: "#{issuable.to_ability_name}[label_names][]", ability_name: issuable.to_ability_name, show_no: "true", show_any: "true", namespace_path: @project.try(:namespace).try(:path), project_path: @project.try(:path), issue_update: issuable_json_path(issuable), labels: (namespace_project_labels_path(@project.namespace, @project, :json) if @project)}}
%span.dropdown-toggle-text %span.dropdown-toggle-text
Label Label
= icon('chevron-down') = icon('chevron-down')
......
...@@ -25,8 +25,11 @@ module API ...@@ -25,8 +25,11 @@ module API
end end
# Check the Rails session for valid authentication details # Check the Rails session for valid authentication details
#
# Until CSRF protection is added to the API, disallow this method for
# state-changing endpoints
def find_user_from_warden def find_user_from_warden
warden ? warden.authenticate : nil warden.try(:authenticate) if request.get? || request.head?
end end
def find_user_by_private_token def find_user_by_private_token
......
...@@ -9,7 +9,8 @@ describe API::Helpers, api: true do ...@@ -9,7 +9,8 @@ describe API::Helpers, api: true do
let(:key) { create(:key, user: user) } let(:key) { create(:key, user: user) }
let(:params) { {} } let(:params) { {} }
let(:env) { {} } let(:env) { { 'REQUEST_METHOD' => 'GET' } }
let(:request) { Rack::Request.new(env) }
def set_env(token_usr, identifier) def set_env(token_usr, identifier)
clear_env clear_env
...@@ -51,18 +52,44 @@ describe API::Helpers, api: true do ...@@ -51,18 +52,44 @@ describe API::Helpers, api: true do
describe ".current_user" do describe ".current_user" do
subject { current_user } subject { current_user }
describe "when authenticating via Warden" do describe "Warden authentication" do
before { doorkeeper_guard_returns false } before { doorkeeper_guard_returns false }
context "fails" do context "with invalid credentials" do
context "GET request" do
before { env['REQUEST_METHOD'] = 'GET' }
it { is_expected.to be_nil } it { is_expected.to be_nil }
end end
end
context "succeeds" do context "with valid credentials" do
before { warden_authenticate_returns user } before { warden_authenticate_returns user }
context "GET request" do
before { env['REQUEST_METHOD'] = 'GET' }
it { is_expected.to eq(user) }
end
context "HEAD request" do
before { env['REQUEST_METHOD'] = 'HEAD' }
it { is_expected.to eq(user) } it { is_expected.to eq(user) }
end end
context "PUT request" do
before { env['REQUEST_METHOD'] = 'PUT' }
it { is_expected.to be_nil }
end
context "POST request" do
before { env['REQUEST_METHOD'] = 'POST' }
it { is_expected.to be_nil }
end
context "DELETE request" do
before { env['REQUEST_METHOD'] = 'DELETE' }
it { is_expected.to be_nil }
end
end
end end
describe "when authenticating using a user's private token" do describe "when authenticating using a user's private token" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment