From 56475ae9bced62923e6ed5255244bad99a1e3813 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 26 Feb 2015 02:53:37 +0000 Subject: Merge branch 'fix-another-migration' into 'master' Prevent another migration from failing due to indirect use of model columns not yet created See https://github.com/gitlabhq/gitlabhq/issues/8817#issuecomment-76007077 cc @marin See merge request !1597 --- db/migrate/20141006143943_move_slack_service_to_webhook.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20141006143943_move_slack_service_to_webhook.rb b/db/migrate/20141006143943_move_slack_service_to_webhook.rb index a8e07033a5d..5836cd6b8db 100644 --- a/db/migrate/20141006143943_move_slack_service_to_webhook.rb +++ b/db/migrate/20141006143943_move_slack_service_to_webhook.rb @@ -10,7 +10,7 @@ class MoveSlackServiceToWebhook < ActiveRecord::Migration slack_service.properties.delete('subdomain') # Room is configured on the Slack side slack_service.properties.delete('room') - slack_service.save + slack_service.save(validate: false) end end end -- cgit v1.2.3 From d59d4d41455d5f27aaa8dcd458dc4c746506a6cc Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 19 Feb 2015 20:48:49 +0000 Subject: Merge branch 'projects-limit-default' into 'master' Correctly set default projects limit for new users. See #2014. Note that the projects limit still isn't retroactively applied to existing users (probably as intended) See merge request !1547 --- app/models/user.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index b203b57060e..08ad619a90c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -56,14 +56,13 @@ class User < ActiveRecord::Base include Gitlab::ConfigHelper include TokenAuthenticatable extend Gitlab::ConfigHelper - extend Gitlab::CurrentSettings + include Gitlab::CurrentSettings default_value_for :admin, false default_value_for :can_create_group, gitlab_config.default_can_create_group default_value_for :can_create_team, false default_value_for :hide_no_ssh_key, false default_value_for :hide_no_password, false - default_value_for :projects_limit, current_application_settings.default_projects_limit default_value_for :theme_id, gitlab_config.default_theme devise :database_authenticatable, :lockable, :async, @@ -142,6 +141,7 @@ class User < ActiveRecord::Base before_save :ensure_authentication_token after_save :ensure_namespace_correct + after_initialize :set_projects_limit after_create :post_create_hook after_destroy :post_destroy_hook @@ -468,6 +468,13 @@ class User < ActiveRecord::Base end end + def set_projects_limit + connection_default_value_defined = new_record? && !projects_limit_changed? + return unless self.projects_limit.nil? || connection_default_value_defined + + self.projects_limit = current_application_settings.default_projects_limit + end + def requires_ldap_check? if !Gitlab.config.ldap.enabled false -- cgit v1.2.3 From c0f8b95655a7397925626904b1dd8915ef752fc5 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 26 Feb 2015 16:57:26 +0000 Subject: Merge branch 'fix_gitlab_importer' into 'master' Fix GitLab && Gitorious importers. Hide already imported projects Fixes #2060 See merge request !1594 --- app/controllers/import/gitlab_controller.rb | 2 +- app/controllers/import/gitorious_controller.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/import/gitlab_controller.rb b/app/controllers/import/gitlab_controller.rb index a51ea36aff8..c18178abf76 100644 --- a/app/controllers/import/gitlab_controller.rb +++ b/app/controllers/import/gitlab_controller.rb @@ -16,7 +16,7 @@ class Import::GitlabController < Import::BaseController @already_added_projects = current_user.created_projects.where(import_type: "gitlab") already_added_projects_names = @already_added_projects.pluck(:import_source) - @repos.to_a.reject!{ |repo| already_added_projects_names.include? repo["path_with_namespace"] } + @repos = @repos.to_a.reject{ |repo| already_added_projects_names.include? repo["path_with_namespace"] } end def jobs diff --git a/app/controllers/import/gitorious_controller.rb b/app/controllers/import/gitorious_controller.rb index 627b4a171b8..6067a87ee04 100644 --- a/app/controllers/import/gitorious_controller.rb +++ b/app/controllers/import/gitorious_controller.rb @@ -15,7 +15,7 @@ class Import::GitoriousController < Import::BaseController @already_added_projects = current_user.created_projects.where(import_type: "gitorious") already_added_projects_names = @already_added_projects.pluck(:import_source) - @repos.to_a.reject! { |repo| already_added_projects_names.include? repo.full_name } + @repos.reject! { |repo| already_added_projects_names.include? repo.full_name } end def jobs -- cgit v1.2.3 From 650a7c73c4372799fb4d2c42003cece504b28e46 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Mar 2015 20:05:12 +0000 Subject: Merge branch 'project-existence-leak' into 'master' Don't leak information about private project existence via Git-over-SSH/HTTP. Fixes #2040 and https://gitlab.com/gitlab-org/gitlab-ce/issues/343. Both `Grack::Auth` (used by Git-over-HTTP) and `Api::Internal /allowed` (used by gitlab-shell/Git-over-SSH) now return a generic "Not Found" error when the project exists but the user doesn't have access to it. See merge request !1578 --- lib/api/internal.rb | 39 ++++---- lib/gitlab/backend/grack_auth.rb | 52 +++++----- spec/lib/gitlab/backend/grack_auth_spec.rb | 146 +++++++++++++++++++++++++++++ 3 files changed, 196 insertions(+), 41 deletions(-) create mode 100644 spec/lib/gitlab/backend/grack_auth_spec.rb diff --git a/lib/api/internal.rb b/lib/api/internal.rb index ba3fe619b92..753d0fcbd98 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -16,6 +16,17 @@ module API # post "/allowed" do status 200 + + actor = if params[:key_id] + Key.find_by(id: params[:key_id]) + elsif params[:user_id] + User.find_by(id: params[:user_id]) + end + + unless actor + return Gitlab::GitAccessStatus.new(false, 'No such user or key') + end + project_path = params[:project] # Check for *.wiki repositories. @@ -32,26 +43,20 @@ module API project = Project.find_with_namespace(project_path) - unless project - return Gitlab::GitAccessStatus.new(false, 'No such project') + if project + status = access.check( + actor, + params[:action], + project, + params[:changes] + ) end - actor = if params[:key_id] - Key.find_by(id: params[:key_id]) - elsif params[:user_id] - User.find_by(id: params[:user_id]) - end - - unless actor - return Gitlab::GitAccessStatus.new(false, 'No such user or key') + if project && status && status.allowed? + status + else + Gitlab::GitAccessStatus.new(false, 'No such project') end - - access.check( - actor, - params[:action], - project, - params[:changes] - ) end # diff --git a/lib/gitlab/backend/grack_auth.rb b/lib/gitlab/backend/grack_auth.rb index 3f207c56631..2f73a21b2ef 100644 --- a/lib/gitlab/backend/grack_auth.rb +++ b/lib/gitlab/backend/grack_auth.rb @@ -10,8 +10,9 @@ module Grack @request = Rack::Request.new(env) @auth = Request.new(env) - # Need this patch due to the rails mount + @gitlab_ci = false + # Need this patch due to the rails mount # Need this if under RELATIVE_URL_ROOT unless Gitlab.config.gitlab.relative_url_root.empty? # If website is mounted using relative_url_root need to remove it first @@ -22,8 +23,12 @@ module Grack @env['SCRIPT_NAME'] = "" - if project - auth! + auth! + + if project && authorized_request? + @app.call(env) + elsif @user.nil? && !@gitlab_ci + unauthorized else render_not_found end @@ -32,35 +37,30 @@ module Grack private def auth! - if @auth.provided? - return bad_request unless @auth.basic? - - # Authentication with username and password - login, password = @auth.credentials + return unless @auth.provided? - # Allow authentication for GitLab CI service - # if valid token passed - if gitlab_ci_request?(login, password) - return @app.call(env) - end + return bad_request unless @auth.basic? - @user = authenticate_user(login, password) + # Authentication with username and password + login, password = @auth.credentials - if @user - Gitlab::ShellEnv.set_env(@user) - @env['REMOTE_USER'] = @auth.username - end + # Allow authentication for GitLab CI service + # if valid token passed + if gitlab_ci_request?(login, password) + @gitlab_ci = true + return end - if authorized_request? - @app.call(env) - else - unauthorized + @user = authenticate_user(login, password) + + if @user + Gitlab::ShellEnv.set_env(@user) + @env['REMOTE_USER'] = @auth.username end end def gitlab_ci_request?(login, password) - if login == "gitlab-ci-token" && project.gitlab_ci? + if login == "gitlab-ci-token" && project && project.gitlab_ci? token = project.gitlab_ci_service.token if token.present? && token == password && git_cmd == 'git-upload-pack' @@ -107,6 +107,8 @@ module Grack end def authorized_request? + return true if @gitlab_ci + case git_cmd when *Gitlab::GitAccess::DOWNLOAD_COMMANDS if user @@ -141,7 +143,9 @@ module Grack end def project - @project ||= project_by_path(@request.path_info) + return @project if defined?(@project) + + @project = project_by_path(@request.path_info) end def project_by_path(path) diff --git a/spec/lib/gitlab/backend/grack_auth_spec.rb b/spec/lib/gitlab/backend/grack_auth_spec.rb new file mode 100644 index 00000000000..768312f0028 --- /dev/null +++ b/spec/lib/gitlab/backend/grack_auth_spec.rb @@ -0,0 +1,146 @@ +require "spec_helper" + +describe Grack::Auth do + let(:user) { create(:user) } + let(:project) { create(:project) } + + let(:app) { lambda { |env| [200, {}, "Success!"] } } + let!(:auth) { Grack::Auth.new(app) } + let(:env) { + { + "rack.input" => "", + "REQUEST_METHOD" => "GET", + "QUERY_STRING" => "service=git-upload-pack" + } + } + let(:status) { auth.call(env).first } + + describe "#call" do + context "when the project doesn't exist" do + before do + env["PATH_INFO"] = "doesnt/exist.git" + end + + context "when no authentication is provided" do + it "responds with status 401" do + expect(status).to eq(401) + end + end + + context "when username and password are provided" do + context "when authentication fails" do + before do + env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, "nope") + end + + it "responds with status 401" do + expect(status).to eq(401) + end + end + + context "when authentication succeeds" do + before do + env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password) + end + + it "responds with status 404" do + expect(status).to eq(404) + end + end + end + end + + context "when the project exists" do + before do + env["PATH_INFO"] = project.path_with_namespace + ".git" + end + + context "when the project is public" do + before do + project.update_attribute(:visibility_level, Project::PUBLIC) + end + + it "responds with status 200" do + expect(status).to eq(200) + end + end + + context "when the project is private" do + before do + project.update_attribute(:visibility_level, Project::PRIVATE) + end + + context "when no authentication is provided" do + it "responds with status 401" do + expect(status).to eq(401) + end + end + + context "when username and password are provided" do + context "when authentication fails" do + before do + env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, "nope") + end + + it "responds with status 401" do + expect(status).to eq(401) + end + end + + context "when authentication succeeds" do + before do + env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password) + end + + context "when the user has access to the project" do + before do + project.team << [user, :master] + end + + context "when the user is blocked" do + before do + user.block + project.team << [user, :master] + end + + it "responds with status 404" do + expect(status).to eq(404) + end + end + + context "when the user isn't blocked" do + it "responds with status 200" do + expect(status).to eq(200) + end + end + end + + context "when the user doesn't have access to the project" do + it "responds with status 404" do + expect(status).to eq(404) + end + end + end + end + + context "when a gitlab ci token is provided" do + let(:token) { "123" } + + before do + gitlab_ci_service = project.build_gitlab_ci_service + gitlab_ci_service.active = true + gitlab_ci_service.token = token + gitlab_ci_service.project_url = "http://google.com" + gitlab_ci_service.save + + env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials("gitlab-ci-token", token) + end + + it "responds with status 200" do + expect(status).to eq(200) + end + end + end + end + end +end -- cgit v1.2.3 From 1be453d66b97fd5434e2c8c71b1b1ef586f1a572 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 3 Mar 2015 16:15:25 -0800 Subject: User correct name for reject_blocked --- app/controllers/uploads_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 73b124bb34c..243e954ebb3 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -1,5 +1,5 @@ class UploadsController < ApplicationController - skip_before_filter :authenticate_user!, :reject_blocked + skip_before_filter :authenticate_user!, :reject_blocked! before_filter :authorize_access def show @@ -20,7 +20,7 @@ class UploadsController < ApplicationController def authorize_access unless params[:mounted_as] == 'avatar' - authenticate_user! && reject_blocked + authenticate_user! && reject_blocked! end end end -- cgit v1.2.3 From 63e74ce9d648603d12544ae989b3c56afe8a0e9a Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Wed, 4 Mar 2015 09:51:30 -0800 Subject: Update the Changelog. --- CHANGELOG | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 6e9d2dafad4..eb43ea89589 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,18 @@ -v 7.8.0 (unreleased) +v 7.8.2 + - Fix service migration issue when upgrading from versions prior to 7.3 + - Fix setting of the default use project limit via admin UI + - Fix showing of already imported projects for GitLab and Gitorious importers + - Fix response of push to repository to return "Not found" if user doesn't have access + - Fix check if user is allowed to view the file attachment + +v 7.8.1 + - Fix run of custom post receive hooks + - Fix migration that caused issues when upgrading to version 7.8 from versions prior to 7.3 + - Fix the warning for LDAP users about need to set password + - Fix avatars which were not shown for non logged in users + - Fix urls for the issues when relative url was enabled + +v 7.8.0 - Fix access control and protection against XSS for note attachments and other uploads. - Replace highlight.js with rouge-fork rugments (Stefan Tatschner) - Make project search case insensitive (Hannes Rosenögger) @@ -28,7 +42,7 @@ v 7.8.0 (unreleased) - Allow configuring protection of the default branch upon first push (Marco Wessel) - Add gitlab.com importer - Add an ability to login with gitlab.com - - Add a commit calendar to the user profile (Hannes Rosenögger) + - Add a commit calendar to the user profile (Hannes Rosenögger) - Submit comment on command-enter - Notify all members of a group when that group is mentioned in a comment, for example: `@gitlab-org` or `@sales`. - Extend issue clossing pattern to include "Resolve", "Resolves", "Resolved", "Resolving" and "Close" @@ -43,7 +57,7 @@ v 7.8.0 (unreleased) - API: Access groups with their path (Julien Bianchi) - Added link to milestone and keeping resource context on smaller viewports for issues and merge requests (Jason Blanchard) - Allow notification email to be set separately from primary email. - - API: Add support for editing an existing project (Mika Mäenpää and Hannes Rosenögger) + - API: Add support for editing an existing project (Mika Mäenpää and Hannes Rosenögger) - Don't have Markdown preview fail for long comments/wiki pages. - When test web hook - show error message instead of 500 error page if connection to hook url was reset - Added support for firing system hooks on group create/destroy and adding/removing users to group (Boyan Tabakov) -- cgit v1.2.3 From 520a49d32592bbb8d1bd91e01d53ae94fa1a1850 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Mar 2015 18:57:49 +0000 Subject: Merge branch 'case-sensetivity-import' into 'master' Fix import check for case sensetive namespaces If you already have namespace `ABc` and you try to import project with namespace `abC` - import will fail with 422 error. cc @valery See merge request !1618 --- app/controllers/import/base_controller.rb | 2 +- app/models/namespace.rb | 5 +++++ spec/models/namespace_spec.rb | 10 ++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/controllers/import/base_controller.rb b/app/controllers/import/base_controller.rb index 4df171dbcfe..7dc0cac8d4c 100644 --- a/app/controllers/import/base_controller.rb +++ b/app/controllers/import/base_controller.rb @@ -3,7 +3,7 @@ class Import::BaseController < ApplicationController private def get_or_create_namespace - existing_namespace = Namespace.find_by("path = ? OR name = ?", @target_namespace, @target_namespace) + existing_namespace = Namespace.find_by_path_or_name(@target_namespace) if existing_namespace if existing_namespace.owner == current_user diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 2c7ed376265..35280889a86 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -48,6 +48,11 @@ class Namespace < ActiveRecord::Base where('lower(path) = :value', value: path.downcase).first end + # Case insensetive search for namespace by path or name + def self.find_by_path_or_name(path) + find_by("lower(path) = :path OR lower(name) = :path", path: path.downcase) + end + def self.search(query) where("name LIKE :query OR path LIKE :query", query: "%#{query}%") end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 4e268f8d8fa..ed6845c82cc 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -75,4 +75,14 @@ describe Namespace do expect(namespace.rm_dir).to be_truthy end end + + describe :find_by_path_or_name do + before do + @namespace = create(:namespace, name: 'WoW', path: 'woW') + end + + it { expect(Namespace.find_by_path_or_name('wow')).to eq(@namespace) } + it { expect(Namespace.find_by_path_or_name('WOW')).to eq(@namespace) } + it { expect(Namespace.find_by_path_or_name('unknown')).to eq(nil) } + end end -- cgit v1.2.3 From 56f75bde488b260cab909c9d93b8c2bab735d939 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Wed, 4 Mar 2015 10:16:13 -0800 Subject: Update changelog. --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index eb43ea89589..e6844bf7af5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 7.8.2 - Fix showing of already imported projects for GitLab and Gitorious importers - Fix response of push to repository to return "Not found" if user doesn't have access - Fix check if user is allowed to view the file attachment + - Fix import check for case sensetive namespaces v 7.8.1 - Fix run of custom post receive hooks -- cgit v1.2.3