diff options
author | Robert Schilling <rschilling@student.tugraz.at> | 2015-05-03 15:00:58 +0200 |
---|---|---|
committer | Robert Schilling <rschilling@student.tugraz.at> | 2015-05-03 15:00:58 +0200 |
commit | 78da7b1c486cb5c07a51e727a497e882bea4bc55 (patch) | |
tree | 5ae1364af111f97504f254a8469f04553a1c9169 | |
parent | 5f3eef6e56ac4908c3fc67ca238340a3caf5a9b8 (diff) | |
parent | 747232eeda7c79ea65a5c208399a6c72872ff4bc (diff) | |
download | gitlab-ce-78da7b1c486cb5c07a51e727a497e882bea4bc55.tar.gz |
Merge branch 'master' of github.com:gitlabhq/gitlabhq
36 files changed, 353 insertions, 108 deletions
diff --git a/CHANGELOG b/CHANGELOG index be0c005fa06..87b3954ef4f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,8 +2,10 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.11.0 (unreleased) - Make Reply-To config apply to change e-mail confirmation and other Devise notifications (Stan Hu) + - Add application setting to restrict user signups to e-mail domains (Stan Hu) - Don't allow a merge request to be merged when its title starts with "WIP". - Add a page title to every page. + - Allow primary email to be set to an email that you've already added. - Get Gitorious importer to work again. - Fix clone URL field and X11 Primary selection (Dmitry Medvinsky) - Ignore invalid lines in .gitmodules @@ -23,6 +25,8 @@ v 7.11.0 (unreleased) - Don't crash when an MR from a fork has a cross-reference comment from the target project on of its commits. - Include commit comments in MR from a forked project. - Fix adding new group members from admin area + - Group milestones by title in the dashboard and all other issue views. + - Query issues, merge requests and milestones with their IID through API (Julien Bianchi) - Add default project and snippet visibility settings to the admin web UI. - Show incompatible projects in Google Code import status (Stan Hu) - Fix bug where commit data would not appear in some subdirectories (Stan Hu) @@ -30,6 +34,7 @@ v 7.11.0 (unreleased) - Move snippets UI to fluid layout - Improve UI for sidebar. Increase separation between navigation and content - Improve new project command options (Ben Bodenmiller) + - Add common method to force UTF-8 and use it to properly handle non-ascii OAuth user properties (Onur Küçük) - Prevent sending empty messages to HipChat (Chulki Lee) - Improve UI for mobile phones on dashboard and project pages - Add room notification and message color option for HipChat diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 8f6a766635a..3975e30835e 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -41,7 +41,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :max_attachment_size, :default_project_visibility, :default_snippet_visibility, - restricted_visibility_levels: [] + :restricted_signup_domains_raw, + restricted_visibility_levels: [], ) end end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index adb83996f8b..d36e359934c 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -102,8 +102,7 @@ class Admin::UsersController < Admin::ApplicationController email = user.emails.find(params[:email_id]) email.destroy - user.set_notification_email - user.save if user.notification_email_changed? + user.update_secondary_emails! respond_to do |format| format.html { redirect_to :back, notice: "Successfully removed email." } diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c9b34eac4b0..eee10d6c22a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -287,40 +287,15 @@ class ApplicationController < ActionController::Base @filter_params end - def set_filter_values(collection) - assignee_id = @filter_params[:assignee_id] - author_id = @filter_params[:author_id] - milestone_id = @filter_params[:milestone_id] - - @sort = @filter_params[:sort] - @assignees = User.where(id: collection.pluck(:assignee_id)) - @authors = User.where(id: collection.pluck(:author_id)) - @milestones = Milestone.where(id: collection.pluck(:milestone_id)) - - if assignee_id.present? && !assignee_id.to_i.zero? - @assignee = @assignees.find_by(id: assignee_id) - end - - if author_id.present? && !author_id.to_i.zero? - @author = @authors.find_by(id: author_id) - end - - if milestone_id.present? && !milestone_id.to_i.zero? - @milestone = @milestones.find_by(id: milestone_id) - end - end - def get_issues_collection set_filters_params issues = IssuesFinder.new.execute(current_user, @filter_params) - set_filter_values(issues) issues end def get_merge_requests_collection set_filters_params merge_requests = MergeRequestsFinder.new.execute(current_user, @filter_params) - set_filter_values(merge_requests) merge_requests end diff --git a/app/controllers/profiles/emails_controller.rb b/app/controllers/profiles/emails_controller.rb index 3e904700de5..0ede9b8e21b 100644 --- a/app/controllers/profiles/emails_controller.rb +++ b/app/controllers/profiles/emails_controller.rb @@ -1,14 +1,17 @@ class Profiles::EmailsController < Profiles::ApplicationController def index @primary = current_user.email - @public_email = current_user.public_email @emails = current_user.emails end def create @email = current_user.emails.new(email_params) - flash[:alert] = @email.errors.full_messages.first unless @email.save + if @email.save + NotificationService.new.new_email(@email) + else + flash[:alert] = @email.errors.full_messages.first + end redirect_to profile_emails_url end @@ -17,9 +20,7 @@ class Profiles::EmailsController < Profiles::ApplicationController @email = current_user.emails.find(params[:id]) @email.destroy - current_user.set_notification_email - current_user.set_public_email - current_user.save if current_user.notification_email_changed? or current_user.public_email_changed? + current_user.update_secondary_emails! respond_to do |format| format.html { redirect_to profile_emails_url } diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 2c0702073d4..b8f367c6339 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -113,8 +113,9 @@ class IssuableFinder end def by_milestone(items) - if params[:milestone_id].present? - items = items.where(milestone_id: (params[:milestone_id] == NONE ? nil : params[:milestone_id])) + if params[:milestone_title].present? + milestone_ids = (params[:milestone_title] == NONE ? nil : Milestone.where(title: params[:milestone_title]).pluck(:id)) + items = items.where(milestone_id: milestone_ids) end items diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb index 282bdf744d2..93e33ebefd8 100644 --- a/app/helpers/milestones_helper.rb +++ b/app/helpers/milestones_helper.rb @@ -28,6 +28,7 @@ module MilestonesHelper Milestone.where(project_id: @projects) end.active - options_from_collection_for_select(milestones, 'id', 'title', params[:milestone_id]) + grouped_milestones = Milestones::GroupService.new(milestones).execute + options_from_collection_for_select(grouped_milestones, 'title', 'title', params[:milestone_title]) end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 9406fb91939..f2cebde9705 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -18,11 +18,13 @@ # restricted_visibility_levels :text # max_attachment_size :integer default(10) # default_project_visibility :integer -# default_snippet_visibility :integer +# restricted_signup_domains :text # class ApplicationSetting < ActiveRecord::Base serialize :restricted_visibility_levels + serialize :restricted_signup_domains, Array + attr_accessor :restricted_signup_domains_raw validates :home_page_url, allow_blank: true, @@ -55,11 +57,29 @@ class ApplicationSetting < ActiveRecord::Base restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], max_attachment_size: Settings.gitlab['max_attachment_size'], default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], - default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'] + default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], + restricted_signup_domains: Settings.gitlab['restricted_signup_domains'] ) end def home_page_url_column_exist ActiveRecord::Base.connection.column_exists?(:application_settings, :home_page_url) end + + def restricted_signup_domains_raw + self.restricted_signup_domains.join("\n") unless self.restricted_signup_domains.nil? + end + + def restricted_signup_domains_raw=(values) + self.restricted_signup_domains = [] + self.restricted_signup_domains = values.split( + /\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace + | # or + \s # any whitespace character + | # or + [\r\n] # any number of newline characters + /x) + self.restricted_signup_domains.reject! { |d| d.empty? } + end + end diff --git a/app/models/email.rb b/app/models/email.rb index 556b0e9586e..935705e2ed4 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -18,7 +18,6 @@ class Email < ActiveRecord::Base validates :email, presence: true, email: { strict_mode: true }, uniqueness: true validate :unique_email, if: ->(email) { email.email_changed? } - after_create :notify before_validation :cleanup_email def cleanup_email @@ -28,8 +27,4 @@ class Email < ActiveRecord::Base def unique_email self.errors.add(:email, 'has already been taken') if User.exists?(email: self.email) end - - def notify - NotificationService.new.new_email(self) - end end diff --git a/app/models/user.rb b/app/models/user.rb index d6b93afe739..9f198368129 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -139,13 +139,16 @@ class User < ActiveRecord::Base validate :avatar_type, if: ->(user) { user.avatar_changed? } validate :unique_email, if: ->(user) { user.email_changed? } validate :owns_notification_email, if: ->(user) { user.notification_email_changed? } + validate :owns_public_email, if: ->(user) { user.public_email_changed? } validates :avatar, file_size: { maximum: 200.kilobytes.to_i } before_validation :generate_password, on: :create + before_validation :restricted_signup_domains, on: :create before_validation :sanitize_attrs before_validation :set_notification_email, if: ->(user) { user.email_changed? } before_validation :set_public_email, if: ->(user) { user.public_email_changed? } + after_update :update_emails_with_primary_email, if: ->(user) { user.email_changed? } before_save :ensure_authentication_token after_save :ensure_namespace_correct after_initialize :set_projects_limit @@ -276,13 +279,29 @@ class User < ActiveRecord::Base end def unique_email - self.errors.add(:email, 'has already been taken') if Email.exists?(email: self.email) + if !self.emails.exists?(email: self.email) && Email.exists?(email: self.email) + self.errors.add(:email, 'has already been taken') + end end def owns_notification_email self.errors.add(:notification_email, "is not an email you own") unless self.all_emails.include?(self.notification_email) end + def owns_public_email + self.errors.add(:public_email, "is not an email you own") unless self.all_emails.include?(self.public_email) + end + + def update_emails_with_primary_email + primary_email_record = self.emails.find_by(email: self.email) + if primary_email_record + primary_email_record.destroy + self.emails.create(email: self.email_was) + + self.update_secondary_emails! + end + end + # Groups user has access to def authorized_groups @authorized_groups ||= begin @@ -448,10 +467,16 @@ class User < ActiveRecord::Base def set_public_email if self.public_email.blank? || !self.all_emails.include?(self.public_email) - self.public_email = '' + self.public_email = nil end end + def update_secondary_emails! + self.set_notification_email + self.set_public_email + self.save if self.notification_email_changed? || self.public_email_changed? + end + def set_projects_limit connection_default_value_defined = new_record? && !projects_limit_changed? return unless self.projects_limit.nil? || connection_default_value_defined @@ -611,4 +636,27 @@ class User < ActiveRecord::Base select(:project_id). uniq.map(&:project_id) end + + def restricted_signup_domains + email_domains = current_application_settings.restricted_signup_domains + + unless email_domains.blank? + match_found = email_domains.any? do |domain| + escaped = Regexp.escape(domain).gsub('\*','.*?') + regexp = Regexp.new "^#{escaped}$", Regexp::IGNORECASE + email_domain = Mail::Address.new(self.email).domain + email_domain =~ regexp + end + + unless match_found + self.errors.add :email, + 'is not whitelisted. ' + + 'Email domains valid for registration are: ' + + email_domains.join(', ') + return false + end + end + + true + end end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 87e7c9634e9..f6eb00ea0bd 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -72,6 +72,11 @@ = f.label :max_attachment_size, 'Maximum attachment size (MB)', class: 'control-label col-sm-2' .col-sm-10 = f.number_field :max_attachment_size, class: 'form-control' + .form-group + = f.label :restricted_signup_domains, 'Restricted domains for sign-ups', class: 'control-label col-sm-2' + .col-sm-10 + = f.text_area :restricted_signup_domains_raw, placeholder: 'domain.com', class: 'form-control' + .help-block Ex: domain.com, *.domain.com. Wildcards allowed. Use separate lines for multiple entries. .form-actions = f.submit 'Save', class: 'btn btn-primary' diff --git a/app/views/profiles/emails/index.html.haml b/app/views/profiles/emails/index.html.haml index c17e01425d8..2c0d0e10a4c 100644 --- a/app/views/profiles/emails/index.html.haml +++ b/app/views/profiles/emails/index.html.haml @@ -5,11 +5,15 @@ Your %b Primary Email will be used for avatar detection and web based operations, such as edits and merges. - %br +%p.light Your %b Notification Email will be used for account notifications. - %br +%p.light + Your + %b Public Email + will be displayed on your public profile. +%p.light All email addresses will be used to identify your commits. %hr @@ -21,13 +25,17 @@ %li %strong= @primary %span.label.label-success Primary Email - - if @primary === @public_email + - if @primary === current_user.public_email %span.label.label-info Public Email + - if @primary === current_user.notification_email + %span.label.label-info Notification Email - @emails.each do |email| %li %strong= email.email - - if email.email === @public_email + - if email.email === current_user.public_email %span.label.label-info Public Email + - if email.email === current_user.notification_email + %span.label.label-info Notification Email %span.cgray added #{time_ago_with_tooltip(email.created_at)} = link_to 'Remove', profile_email_path(email), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-remove pull-right' diff --git a/app/views/shared/_issuable_filter.html.haml b/app/views/shared/_issuable_filter.html.haml index f9eb2dcfa28..fa8b4eae314 100644 --- a/app/views/shared/_issuable_filter.html.haml +++ b/app/views/shared/_issuable_filter.html.haml @@ -15,7 +15,7 @@ #{state_filters_text_for(:all, @project)} .issues-details-filters - = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_id, :label_name]), method: :get, class: 'filter-form' do + = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name]), method: :get, class: 'filter-form' do - if controller.controller_name == 'issues' .check-all-holder = check_box_tag "check_all_issues", nil, false, @@ -31,7 +31,7 @@ placeholder: 'Author', class: 'trigger-submit', any_user: true, first_user: true) .filter-item.inline.milestone-filter - = select_tag('milestone_id', projects_milestones_options, class: "select2 trigger-submit", prompt: 'Milestone') + = select_tag('milestone_title', projects_milestones_options, class: "select2 trigger-submit", prompt: 'Milestone') - if @project .filter-item.inline.labels-filter diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 0abd34fc3e0..e5ac66a2323 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -132,6 +132,7 @@ Settings.gitlab.default_projects_features['wiki'] = true if Settings.g Settings.gitlab.default_projects_features['snippets'] = false if Settings.gitlab.default_projects_features['snippets'].nil? Settings.gitlab.default_projects_features['visibility_level'] = Settings.send(:verify_constant, Gitlab::VisibilityLevel, Settings.gitlab.default_projects_features['visibility_level'], Gitlab::VisibilityLevel::PRIVATE) Settings.gitlab['repository_downloads_path'] = File.absolute_path(Settings.gitlab['repository_downloads_path'] || 'tmp/repositories', Rails.root) +Settings.gitlab['restricted_signup_domains'] ||= [] # # Gravatar diff --git a/db/migrate/20150502064022_add_restricted_signup_domains_to_application_settings.rb b/db/migrate/20150502064022_add_restricted_signup_domains_to_application_settings.rb new file mode 100644 index 00000000000..184e2653610 --- /dev/null +++ b/db/migrate/20150502064022_add_restricted_signup_domains_to_application_settings.rb @@ -0,0 +1,5 @@ +class AddRestrictedSignupDomainsToApplicationSettings < ActiveRecord::Migration + def change + add_column :application_settings, :restricted_signup_domains, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 02882186fc8..f0cccb8b749 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150429002313) do +ActiveRecord::Schema.define(version: 20150502064022) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -31,6 +31,7 @@ ActiveRecord::Schema.define(version: 20150429002313) do t.integer "max_attachment_size", default: 10, null: false t.integer "default_project_visibility" t.integer "default_snippet_visibility" + t.text "restricted_signup_domains" end create_table "broadcast_messages", force: true do |t| diff --git a/doc/api/groups.md b/doc/api/groups.md index b5a4b05ccaf..c903a850fdd 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -35,7 +35,7 @@ Parameters: ## New group -Creates a new project group. Available only for admin. +Creates a new project group. Available only for users who can create groups. ``` POST /groups diff --git a/doc/api/issues.md b/doc/api/issues.md index a7dd8b74c35..d407bc35d79 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -99,11 +99,13 @@ GET /projects/:id/issues?labels=foo,bar GET /projects/:id/issues?labels=foo,bar&state=opened GET /projects/:id/issues?milestone=1.0.0 GET /projects/:id/issues?milestone=1.0.0&state=opened +GET /projects/:id/issues?iid=42 ``` Parameters: - `id` (required) - The ID of a project +- `iid` (optional) - Return the issue having the given `iid` - `state` (optional) - Return `all` issues or just those that are `opened` or `closed` - `labels` (optional) - Comma-separated list of label names - `milestone` (optional) - Milestone title diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 6a272539e45..c1d82ad9576 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -10,11 +10,13 @@ The pagination parameters `page` and `per_page` can be used to restrict the list GET /projects/:id/merge_requests GET /projects/:id/merge_requests?state=opened GET /projects/:id/merge_requests?state=all +GET /projects/:id/merge_requests?iid=42 ``` Parameters: - `id` (required) - The ID of a project +- `iid` (optional) - Return the request having the given `iid` - `state` (optional) - Return `all` requests or just those that are `merged`, `opened` or `closed` - `order_by` (optional) - Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` @@ -388,6 +390,6 @@ Parameters: ] ``` -## Comments on issues +## Comments on merge requets Comments are done via the notes resource. diff --git a/doc/api/milestones.md b/doc/api/milestones.md index d48b3bcce8a..a6828728264 100644 --- a/doc/api/milestones.md +++ b/doc/api/milestones.md @@ -6,6 +6,7 @@ Returns a list of project milestones. ``` GET /projects/:id/milestones +GET /projects/:id/milestones?iid=42 ``` ```json @@ -27,6 +28,7 @@ GET /projects/:id/milestones Parameters: - `id` (required) - The ID of a project +- `iid` (optional) - Return the milestone having the given `iid` ## Get single milestone diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 8cb9f920975..f768c750402 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -20,7 +20,7 @@ module API present @groups, with: Entities::Group end - # Create group. Available only for admin + # Create group. Available only for users who can create groups. # # Parameters: # name (required) - The name of the group @@ -28,7 +28,7 @@ module API # Example Request: # POST /groups post do - authenticated_as_admin! + authorize! :create_group, current_user required_attributes! [:name, :path] attrs = attributes_for_keys [:name, :path, :description] diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index be133a2920b..85e9081680d 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -173,6 +173,10 @@ module API end end + def filter_by_iid(items, iid) + items.where(iid: iid) + end + # error helpers def forbidden!(reason = nil) diff --git a/lib/api/issues.rb b/lib/api/issues.rb index ff062be6040..c8db93eb778 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -51,6 +51,7 @@ module API # # Parameters: # id (required) - The ID of a project + # iid (optional) - Return the project issue having the given `iid` # state (optional) - Return "opened" or "closed" issues # labels (optional) - Comma-separated list of label names # milestone (optional) - Milestone title @@ -66,10 +67,12 @@ module API # GET /projects/:id/issues?labels=foo,bar&state=opened # GET /projects/:id/issues?milestone=1.0.0 # GET /projects/:id/issues?milestone=1.0.0&state=closed + # GET /issues?iid=42 get ":id/issues" do issues = user_project.issues issues = filter_issues_state(issues, params[:state]) unless params[:state].nil? issues = filter_issues_labels(issues, params[:labels]) unless params[:labels].nil? + issues = filter_by_iid(issues, params[:iid]) unless params[:iid].nil? unless params[:milestone].nil? issues = filter_issues_milestone(issues, params[:milestone]) diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index b252c57faed..2216a12a87a 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -24,6 +24,7 @@ module API # # Parameters: # id (required) - The ID of a project + # iid (optional) - Return the project MR having the given `iid` # state (optional) - Return requests "merged", "opened" or "closed" # order_by (optional) - Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at` # sort (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` @@ -36,11 +37,16 @@ module API # GET /projects/:id/merge_requests?order_by=updated_at # GET /projects/:id/merge_requests?sort=desc # GET /projects/:id/merge_requests?sort=asc + # GET /projects/:id/merge_requests?iid=42 # get ":id/merge_requests" do authorize! :read_merge_request, user_project merge_requests = user_project.merge_requests + unless params[:iid].nil? + merge_requests = filter_by_iid(merge_requests, params[:iid]) + end + merge_requests = case params["state"] when "opened" then merge_requests.opened @@ -169,8 +175,8 @@ module API # Merge MR # # Parameters: - # id (required) - The ID of a project - # merge_request_id (required) - ID of MR + # id (required) - The ID of a project + # merge_request_id (required) - ID of MR # merge_commit_message (optional) - Custom merge commit message # Example: # PUT /projects/:id/merge_request/:merge_request_id/merge @@ -209,7 +215,7 @@ module API # Get a merge request's comments # # Parameters: - # id (required) - The ID of a project + # id (required) - The ID of a project # merge_request_id (required) - ID of MR # Examples: # GET /projects/:id/merge_request/:merge_request_id/comments @@ -225,9 +231,9 @@ module API # Post comment to merge request # # Parameters: - # id (required) - The ID of a project + # id (required) - The ID of a project # merge_request_id (required) - ID of MR - # note (required) - Text of comment + # note (required) - Text of comment # Examples: # POST /projects/:id/merge_request/:merge_request_id/comments # diff --git a/lib/gitlab/o_auth/auth_hash.rb b/lib/gitlab/o_auth/auth_hash.rb index ce52beec78e..0f16c925900 100644 --- a/lib/gitlab/o_auth/auth_hash.rb +++ b/lib/gitlab/o_auth/auth_hash.rb @@ -9,11 +9,11 @@ module Gitlab end def uid - auth_hash.uid.to_s + Gitlab::Utils.force_utf8(auth_hash.uid.to_s) end def provider - auth_hash.provider + Gitlab::Utils.force_utf8(auth_hash.provider.to_s) end def info @@ -21,23 +21,28 @@ module Gitlab end def name - (info.try(:name) || full_name).to_s.force_encoding('utf-8') + Gitlab::Utils.force_utf8((info.try(:name) || full_name).to_s) end def full_name - "#{info.first_name} #{info.last_name}" + Gitlab::Utils.force_utf8("#{info.first_name} #{info.last_name}") end def username - (info.try(:nickname) || generate_username).to_s.force_encoding('utf-8') + Gitlab::Utils.force_utf8( + (info.try(:nickname) || generate_username).to_s + ) end def email - (info.try(:email) || generate_temporarily_email).downcase + Gitlab::Utils.force_utf8( + (info.try(:email) || generate_temporarily_email).downcase + ) end def password - @password ||= Devise.friendly_token[0, 8].downcase + devise_friendly_token = Devise.friendly_token[0, 8].downcase + @password ||= Gitlab::Utils.force_utf8(devise_friendly_token) end # Get the first part of the email address (before @) diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index bd184c27187..d13fe0ef8a9 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -9,5 +9,9 @@ module Gitlab def system_silent(cmd) Popen::popen(cmd).last.zero? end + + def force_utf8(str) + str.force_encoding(Encoding::UTF_8) + end end end diff --git a/spec/factories.rb b/spec/factories.rb index a5c335c82bc..19f2935f30e 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -22,6 +22,7 @@ FactoryGirl.define do password "12345678" confirmed_at { Time.now } confirmation_token { nil } + can_create_group true trait :admin do admin true diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index e217f0739d2..66d73b2505c 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -95,7 +95,7 @@ describe 'Issues', feature: true do let(:issue) { @issue } it 'should allow filtering by issues with no specified milestone' do - visit namespace_project_issues_path(project.namespace, project, milestone_id: IssuableFinder::NONE) + visit namespace_project_issues_path(project.namespace, project, milestone_title: IssuableFinder::NONE) expect(page).not_to have_content 'foobar' expect(page).to have_content 'barbaz' @@ -103,7 +103,7 @@ describe 'Issues', feature: true do end it 'should allow filtering by a specified milestone' do - visit namespace_project_issues_path(project.namespace, project, milestone_id: issue.milestone.id) + visit namespace_project_issues_path(project.namespace, project, milestone_title: issue.milestone.title) expect(page).to have_content 'foobar' expect(page).not_to have_content 'barbaz' diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 479fa950387..69bac387d20 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -43,7 +43,7 @@ describe IssuesFinder do end it 'should filter by milestone id' do - params = { scope: "all", milestone_id: milestone.id, state: 'opened' } + params = { scope: "all", milestone_title: milestone.title, state: 'opened' } issues = IssuesFinder.new.execute(user, params) expect(issues).to eq([issue1]) end diff --git a/spec/lib/gitlab/o_auth/auth_hash_spec.rb b/spec/lib/gitlab/o_auth/auth_hash_spec.rb index 5eb77b492b2..678086ffa14 100644 --- a/spec/lib/gitlab/o_auth/auth_hash_spec.rb +++ b/spec/lib/gitlab/o_auth/auth_hash_spec.rb @@ -2,54 +2,109 @@ require 'spec_helper' describe Gitlab::OAuth::AuthHash do let(:auth_hash) do - Gitlab::OAuth::AuthHash.new(double({ - provider: 'twitter', - uid: uid, - info: double(info_hash) - })) + Gitlab::OAuth::AuthHash.new( + double({ + provider: provider_ascii, + uid: uid_ascii, + info: double(info_hash) + }) + ) end - let(:uid) { 'my-uid' } - let(:email) { 'my-email@example.com' } - let(:nickname) { 'my-nickname' } + + let(:uid_raw) { + "CN=Onur K\xC3\xBC\xC3\xA7\xC3\xBCk,OU=Test,DC=example,DC=net" + } + let(:email_raw) { "onur.k\xC3\xBC\xC3\xA7\xC3\xBCk@example.net" } + let(:nickname_raw) { "ok\xC3\xBC\xC3\xA7\xC3\xBCk" } + let(:first_name_raw) { 'Onur' } + let(:last_name_raw) { "K\xC3\xBC\xC3\xA7\xC3\xBCk" } + let(:name_raw) { "Onur K\xC3\xBC\xC3\xA7\xC3\xBCk" } + + let(:provider_ascii) { 'ldap'.force_encoding(Encoding::ASCII_8BIT) } + let(:uid_ascii) { uid_raw.force_encoding(Encoding::ASCII_8BIT) } + let(:email_ascii) { email_raw.force_encoding(Encoding::ASCII_8BIT) } + let(:nickname_ascii) { nickname_raw.force_encoding(Encoding::ASCII_8BIT) } + let(:first_name_ascii) { first_name_raw.force_encoding(Encoding::ASCII_8BIT) } + let(:last_name_ascii) { last_name_raw.force_encoding(Encoding::ASCII_8BIT) } + let(:name_ascii) { name_raw.force_encoding(Encoding::ASCII_8BIT) } + + let(:provider_utf8) { provider_ascii.force_encoding(Encoding::UTF_8) } + let(:uid_utf8) { uid_ascii.force_encoding(Encoding::UTF_8) } + let(:email_utf8) { email_ascii.force_encoding(Encoding::UTF_8) } + let(:nickname_utf8) { nickname_ascii.force_encoding(Encoding::UTF_8) } + let(:name_utf8) { name_ascii.force_encoding(Encoding::UTF_8) } + let(:info_hash) { { - email: email, - nickname: nickname, - name: 'John', - first_name: "John", - last_name: "Who" + email: email_ascii, + first_name: first_name_ascii, + last_name: last_name_ascii, + name: name_ascii, + nickname: nickname_ascii, + uid: uid_ascii } } - context "defaults" do - it { expect(auth_hash.provider).to eql 'twitter' } - it { expect(auth_hash.uid).to eql uid } - it { expect(auth_hash.email).to eql email } - it { expect(auth_hash.username).to eql nickname } - it { expect(auth_hash.name).to eql "John" } + context 'defaults' do + it { expect(auth_hash.provider).to eql provider_utf8 } + it { expect(auth_hash.uid).to eql uid_utf8 } + it { expect(auth_hash.email).to eql email_utf8 } + it { expect(auth_hash.username).to eql nickname_utf8 } + it { expect(auth_hash.name).to eql name_utf8 } it { expect(auth_hash.password).to_not be_empty } end - context "email not provided" do + context 'email not provided' do before { info_hash.delete(:email) } - it "generates a temp email" do + + it 'generates a temp email' do expect( auth_hash.email).to start_with('temp-email-for-oauth') end end - context "username not provided" do + context 'username not provided' do before { info_hash.delete(:nickname) } - it "takes the first part of the email as username" do - expect( auth_hash.username ).to eql "my-email" + it 'takes the first part of the email as username' do + expect(auth_hash.username).to eql 'onur-kucuk' end end - context "name not provided" do + context 'name not provided' do before { info_hash.delete(:name) } - it "concats first and lastname as the name" do - expect( auth_hash.name ).to eql "John Who" + it 'concats first and lastname as the name' do + expect(auth_hash.name).to eql name_utf8 + end + end + + context 'auth_hash constructed with ASCII-8BIT encoding' do + it 'forces utf8 encoding on uid' do + auth_hash.uid.encoding.should eql Encoding::UTF_8 + end + + it 'forces utf8 encoding on provider' do + auth_hash.provider.encoding.should eql Encoding::UTF_8 + end + + it 'forces utf8 encoding on name' do + auth_hash.name.encoding.should eql Encoding::UTF_8 + end + + it 'forces utf8 encoding on full_name' do + auth_hash.full_name.encoding.should eql Encoding::UTF_8 + end + + it 'forces utf8 encoding on username' do + auth_hash.username.encoding.should eql Encoding::UTF_8 + end + + it 'forces utf8 encoding on email' do + auth_hash.email.encoding.should eql Encoding::UTF_8 + end + + it 'forces utf8 encoding on password' do + auth_hash.password.encoding.should eql Encoding::UTF_8 end end -end
\ No newline at end of file +end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index b4f0b2c201a..876502d03c2 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -21,4 +21,28 @@ require 'spec_helper' describe ApplicationSetting, models: true do it { expect(ApplicationSetting.create_from_defaults).to be_valid } + + context 'restricted signup domains' do + let(:setting) { ApplicationSetting.create_from_defaults } + + it 'set single domain' do + setting.restricted_signup_domains_raw = 'example.com' + expect(setting.restricted_signup_domains).to eq(['example.com']) + end + + it 'set multiple domains with spaces' do + setting.restricted_signup_domains_raw = 'example.com *.example.com' + expect(setting.restricted_signup_domains).to eq(['example.com', '*.example.com']) + end + + it 'set multiple domains with newlines and a space' do + setting.restricted_signup_domains_raw = "example.com\n *.example.com" + expect(setting.restricted_signup_domains).to eq(['example.com', '*.example.com']) + end + + it 'set multiple domains with commas' do + setting.restricted_signup_domains_raw = "example.com, *.example.com" + expect(setting.restricted_signup_domains).to eq(['example.com', '*.example.com']) + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 24384e8bf22..441aa793133 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -54,6 +54,8 @@ require 'spec_helper' describe User do + include Gitlab::CurrentSettings + describe "Associations" do it { is_expected.to have_one(:namespace) } it { is_expected.to have_many(:snippets).class_name('Snippet').dependent(:destroy) } @@ -112,6 +114,51 @@ describe User do user = build(:user, email: "lol!'+=?><#$%^&*()@gmail.com") expect(user).to be_invalid end + + context 'when no signup domains listed' do + before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return([]) } + it 'accepts any email' do + user = build(:user, email: "info@example.com") + expect(user).to be_valid + end + end + + context 'when a signup domain is listed and subdomains are allowed' do + before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return(['example.com', '*.example.com']) } + it 'accepts info@example.com' do + user = build(:user, email: "info@example.com") + expect(user).to be_valid + end + + it 'accepts info@test.example.com' do + user = build(:user, email: "info@test.example.com") + expect(user).to be_valid + end + + it 'rejects example@test.com' do + user = build(:user, email: "example@test.com") + expect(user).to be_invalid + end + end + + context 'when a signup domain is listed and subdomains are not allowed' do + before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return(['example.com']) } + + it 'accepts info@example.com' do + user = build(:user, email: "info@example.com") + expect(user).to be_valid + end + + it 'rejects info@test.example.com' do + user = build(:user, email: "info@test.example.com") + expect(user).to be_invalid + end + + it 'rejects example@test.com' do + user = build(:user, email: "example@test.com") + expect(user).to be_invalid + end + end end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index d963dbac9f1..62b42d63fc2 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' describe API::API, api: true do include ApiHelpers - let(:user1) { create(:user) } + let(:user1) { create(:user, can_create_group: false) } let(:user2) { create(:user) } + let(:user3) { create(:user) } let(:admin) { create(:admin) } let!(:group1) { create(:group) } let!(:group2) { create(:group) } @@ -94,32 +95,32 @@ describe API::API, api: true do end describe "POST /groups" do - context "when authenticated as user" do + context "when authenticated as user without group permissions" do it "should not create group" do post api("/groups", user1), attributes_for(:group) expect(response.status).to eq(403) end end - context "when authenticated as admin" do + context "when authenticated as user with group permissions" do it "should create group" do - post api("/groups", admin), attributes_for(:group) + post api("/groups", user3), attributes_for(:group) expect(response.status).to eq(201) end it "should not create group, duplicate" do - post api("/groups", admin), {name: "Duplicate Test", path: group2.path} + post api("/groups", user3), {name: 'Duplicate Test', path: group2.path} expect(response.status).to eq(400) expect(response.message).to eq("Bad Request") end it "should return 400 bad request error if name not given" do - post api("/groups", admin), {path: group2.path} + post api("/groups", user3), {path: group2.path} expect(response.status).to eq(400) end it "should return 400 bad request error if path not given" do - post api("/groups", admin), { name: 'test' } + post api("/groups", user3), {name: 'test'} expect(response.status).to eq(400) end end @@ -133,8 +134,8 @@ describe API::API, api: true do end it "should not remove a group if not an owner" do - user3 = create(:user) - group1.add_user(user3, Gitlab::Access::MASTER) + user4 = create(:user) + group1.add_user(user4, Gitlab::Access::MASTER) delete api("/groups/#{group1.id}", user3) expect(response.status).to eq(403) end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index b6b0427debf..8770786f49a 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -194,6 +194,14 @@ describe API::API, api: true do expect(json_response['iid']).to eq(issue.iid) end + it 'should return a project issue by iid' do + get api("/projects/#{project.id}/issues?iid=#{issue.iid}", user) + response.status.should == 200 + json_response.first['title'].should == issue.title + json_response.first['id'].should == issue.id + json_response.first['iid'].should == issue.iid + end + it "should return 404 if issue id not found" do get api("/projects/#{project.id}/issues/54321", user) expect(response.status).to eq(404) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 5fca831f9be..dcd50f73326 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -115,6 +115,14 @@ describe API::API, api: true do expect(json_response['iid']).to eq(merge_request.iid) end + it 'should return merge_request by iid' do + url = "/projects/#{project.id}/merge_requests?iid=#{merge_request.iid}" + get api(url, user) + response.status.should == 200 + json_response.first['title'].should == merge_request.title + json_response.first['id'].should == merge_request.id + end + it "should return a 404 error if merge_request_id not found" do get api("/projects/#{project.id}/merge_request/999", user) expect(response.status).to eq(404) diff --git a/spec/requests/api/milestones_spec.rb b/spec/requests/api/milestones_spec.rb index effb0723476..6890dd1f3a7 100644 --- a/spec/requests/api/milestones_spec.rb +++ b/spec/requests/api/milestones_spec.rb @@ -30,6 +30,13 @@ describe API::API, api: true do expect(json_response['iid']).to eq(milestone.iid) end + it 'should return a project milestone by iid' do + get api("/projects/#{project.id}/milestones?iid=#{milestone.iid}", user) + response.status.should == 200 + json_response.first['title'].should == milestone.title + json_response.first['id'].should == milestone.id + end + it 'should return 401 error if user not authenticated' do get api("/projects/#{project.id}/milestones/#{milestone.id}") expect(response.status).to eq(401) |