Commit 4f1a1bbc 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 9989f949
......@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
v 8.11.8
- Respect the fork_project permission when forking projects
- Set a restrictive CORS policy on the API for credentialed requests
- API: disable rails session auth for non-GET/HEAD requests
v 8.11.7
- Avoid conflict with admin labels when importing GitHub labels. !6158
......
......@@ -5,7 +5,7 @@
namespacesPath: "/api/:version/namespaces.json",
groupProjectsPath: "/api/:version/groups/:id/projects.json",
projectsPath: "/api/:version/projects.json?simple=true",
labelsPath: "/api/:version/projects/:id/labels",
labelsPath: "/:namespace_path/:project_path/labels",
licensePath: "/api/:version/licenses/:key",
gitignorePath: "/api/:version/gitignores/:key",
gitlabCiYmlPath: "/api/:version/gitlab_ci_ymls/:key",
......@@ -61,13 +61,14 @@
return callback(projects);
});
},
newLabel: function(project_id, data, callback) {
newLabel: function(namespace_path, project_path, data, callback) {
var url = Api.buildUrl(Api.labelsPath)
.replace(':id', project_id);
.replace(':namespace_path', namespace_path)
.replace(':project_path', project_path);
return $.ajax({
url: url,
type: "POST",
data: data,
data: {'label': data},
dataType: "json"
}).done(function(label) {
return callback(label);
......
......@@ -3,8 +3,7 @@ $(() => {
$('.js-new-board-list').each(function () {
const $this = $(this);
new gl.CreateLabelDropdown($this.closest('.dropdown').find('.dropdown-new-label'), $this.data('project-id'));
new gl.CreateLabelDropdown($this.closest('.dropdown').find('.dropdown-new-label'), $this.data('namespace-path'), $this.data('project-path'));
$this.glDropdown({
data(term, callback) {
......
(function (w) {
class CreateLabelDropdown {
constructor ($el, projectId) {
constructor ($el, namespacePath, projectPath) {
this.$el = $el;
this.projectId = projectId;
this.namespacePath = namespacePath;
this.projectPath = projectPath;
this.$dropdownBack = $('.dropdown-menu-back', this.$el.closest('.dropdown'));
this.$cancelButton = $('.js-cancel-label-btn', this.$el);
this.$newLabelField = $('#new_label_name', this.$el);
......@@ -91,8 +92,8 @@
e.preventDefault();
e.stopPropagation();
Api.newLabel(this.projectId, {
name: this.$newLabelField.val(),
Api.newLabel(this.namespacePath, this.projectPath, {
title: this.$newLabelField.val(),
color: this.$newColorField.val()
}, (label) => {
this.$newLabelCreateButton.enable();
......
......@@ -4,9 +4,10 @@
var _this;
_this = this;
$('.js-label-select').each(function(i, dropdown) {
var $block, $colorPreview, $dropdown, $form, $loading, $selectbox, $sidebarCollapsedValue, $value, abilityName, defaultLabel, enableLabelCreateButton, issueURLSplit, issueUpdateURL, labelHTMLTemplate, labelNoneHTMLTemplate, labelUrl, projectId, saveLabelData, selectedLabel, showAny, showNo, $sidebarLabelTooltip;
var $block, $colorPreview, $dropdown, $form, $loading, $selectbox, $sidebarCollapsedValue, $value, abilityName, defaultLabel, enableLabelCreateButton, issueURLSplit, issueUpdateURL, labelHTMLTemplate, labelNoneHTMLTemplate, labelUrl, namespacePath, projectPath, saveLabelData, selectedLabel, showAny, showNo, $sidebarLabelTooltip;
$dropdown = $(dropdown);
projectId = $dropdown.data('project-id');
namespacePath = $dropdown.data('namespace-path');
projectPath = $dropdown.data('project-path');
labelUrl = $dropdown.data('labels');
issueUpdateURL = $dropdown.data('issueUpdate');
selectedLabel = $dropdown.data('selected');
......@@ -35,7 +36,7 @@
$sidebarLabelTooltip.tooltip();
if ($dropdown.closest('.dropdown').find('.dropdown-new-label').length) {
new gl.CreateLabelDropdown($dropdown.closest('.dropdown').find('.dropdown-new-label'), projectId);
new gl.CreateLabelDropdown($dropdown.closest('.dropdown').find('.dropdown-new-label'), namespacePath, projectPath);
}
saveLabelData = function() {
......
......@@ -30,9 +30,15 @@ class Projects::LabelsController < Projects::ApplicationController
@label = @project.labels.create(label_params)
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
render 'new'
respond_to do |format|
format.html { render 'new' }
format.json { render json: { message: @label.errors.messages }, status: 400 }
end
end
end
......
......@@ -32,7 +32,7 @@
%input.pull-left.form-control{ type: "search", placeholder: "Filter by name...", "v-model" => "filters.search", "debounce" => "250" }
- if can?(current_user, :admin_list, @project)
.dropdown.pull-right
%button.btn.btn-create.js-new-board-list{ type: "button", data: { toggle: "dropdown", labels: labels_filter_path, project_id: @project.try(:id) } }
%button.btn.btn-create.js-new-board-list{ type: "button", data: { toggle: "dropdown", labels: labels_filter_path, namespace_path: @project.try(:namespace).try(:path), project_path: @project.try(:path) } }
Create new list
.dropdown-menu.dropdown-menu-paging.dropdown-menu-align-right.dropdown-menu-issues-board-new.dropdown-menu-selectable
= render partial: "shared/issuable/label_page_default", locals: { show_footer: true, show_create: true, show_boards_content: true, title: "Create a new list" }
......
......@@ -4,7 +4,7 @@
- show_footer = local_assigns.fetch(:show_footer, true)
- data_options = local_assigns.fetch(:data_options, {})
- 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)
- classes << 'js-extra-options' if extra_options
- classes << 'js-filter-submit' if filter_submit
......
......@@ -128,7 +128,7 @@
- issuable.labels_array.each do |label|
= hidden_field_tag "#{issuable.to_ability_name}[label_names][]", label.id, id: nil
.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
Label
= icon('chevron-down')
......
......@@ -21,8 +21,11 @@ module API
end
# 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
warden ? warden.authenticate : nil
warden.try(:authenticate) if request.get? || request.head?
end
def find_user_by_private_token
......
......@@ -9,7 +9,8 @@ describe API::Helpers, api: true do
let(:key) { create(:key, user: user) }
let(:params) { {} }
let(:env) { {} }
let(:env) { { 'REQUEST_METHOD' => 'GET' } }
let(:request) { Rack::Request.new(env) }
def set_env(token_usr, identifier)
clear_env
......@@ -51,17 +52,43 @@ describe API::Helpers, api: true do
describe ".current_user" do
subject { current_user }
describe "when authenticating via Warden" do
describe "Warden authentication" do
before { doorkeeper_guard_returns false }
context "fails" do
it { is_expected.to be_nil }
context "with invalid credentials" do
context "GET request" do
before { env['REQUEST_METHOD'] = 'GET' }
it { is_expected.to be_nil }
end
end
context "succeeds" do
context "with valid credentials" do
before { warden_authenticate_returns user }
it { is_expected.to eq(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) }
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
......
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