diff options
151 files changed, 2756 insertions, 1655 deletions
diff --git a/CHANGELOG b/CHANGELOG index 17e50289537..ffe5169bfd2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,7 +1,10 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.11.0 (unreleased) + - Don't show duplicate deploy keys + - Fix commit time being displayed in the wrong timezone in some cases (Hannes Rosenögger) - Make the first branch pushed to an empty repository the default HEAD (Stan Hu) + - Fix broken view when using a tag to display a tree that contains git submodules (Stan Hu) - 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". @@ -22,6 +25,9 @@ v 7.11.0 (unreleased) - Fix bug causing `@whatever` inside an issue's first code block to be picked up as a user mention. - Fix bug causing `@whatever` inside an inline code snippet (backtick-style) to be picked up as a user mention. - When use change branches link at MR form - save source branch selection instead of target one + - Improve handling of large diffs + - Added GitLab Event header for project hooks + - Add Two-factor authentication (2FA) for GitLab logins - Show Atom feed buttons everywhere where applicable. - Add project activity atom feed. - Don't crash when an MR from a fork has a cross-reference comment from the target project on one of its commits. @@ -33,6 +39,7 @@ v 7.11.0 (unreleased) - Show incompatible projects in Google Code import status (Stan Hu) - Fix bug where commit data would not appear in some subdirectories (Stan Hu) - Unescape branch names in compare commit (Stan Hu) + - Task lists are now usable in comments, and will show up in Markdown previews. - Fix bug where avatar filenames were not actually deleted from the database during removal (Stan Hu) - Fix bug where Slack service channel was not saved in admin template settings. (Stan Hu) - Move snippets UI to fluid layout @@ -45,6 +52,10 @@ v 7.11.0 (unreleased) - Allow to use non-ASCII letters and dashes in project and namespace name. (Jakub Jirutka) - Add footnotes support to Markdown (Guillaume Delbergue) - Add current_sign_in_at to UserFull REST api. + - Make Sidekiq MemoryKiller shutdown signal configurable + - Add "Create Merge Request" buttons to commits and branches pages and push event. + - Show user roles by comments. + - Fix automatic blocking of auto-created users from Active Directory. v 7.10.2 - Fix CI links on MR page diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3165b7379d3..895202b58e2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -63,7 +63,7 @@ Merge requests can be filed either at [gitlab.com](https://gitlab.com/gitlab-org If you are new to GitLab development (or web development in general), search for the label `easyfix` ([gitlab.com](https://gitlab.com/gitlab-org/gitlab-ce/issues?label_name=easyfix), [github](https://github.com/gitlabhq/gitlabhq/labels/easyfix)). Those are issues easy to fix, marked by the GitLab core-team. If you are unsure how to proceed but want to help, mention one of the core-team members to give you a hint. -To start with GitLab download the [GitLab Development Kit](https://gitlab.com/gitlab-org/gitlab-development-kit) and see [Development section](doc/development/README.md) in the help file. +To start with GitLab download the [GitLab Development Kit](https://gitlab.com/gitlab-org/gitlab-development-kit) and see [Development section](doc/development/README.md) in the help file. ### Merge request guidelines @@ -161,6 +161,7 @@ If you add a dependency in GitLab (such as an operating system package) please c 1. [Shell commands](doc/development/shell_commands.md) created by GitLab contributors to enhance security 1. [Markdown](http://www.cirosantilli.com/markdown-styleguide) 1. Interface text should be written subjectively instead of objectively. It should be the gitlab core team addressing a person. It should be written in present time and never use past tense (has been/was). For example instead of "prohibited this user from being saved due to the following errors:" the text should be "sorry, we could not create your account because:". Also these [excellent writing guidelines](https://github.com/NARKOZ/guides#writing). +1. [Migrations](doc/development/migration_style_guide.md) This is also the style used by linting tools such as [RuboCop](https://github.com/bbatsov/rubocop), [PullReview](https://www.pullreview.com/) and [Hound CI](https://houndci.com). @@ -28,18 +28,23 @@ gem 'omniauth-google-oauth2' gem 'omniauth-twitter' gem 'omniauth-github' gem 'omniauth-shibboleth' -gem 'omniauth-kerberos' +gem 'omniauth-kerberos', group: :kerberos gem 'omniauth-gitlab' gem 'omniauth-bitbucket' gem 'doorkeeper', '2.1.3' gem "rack-oauth2", "~> 1.0.5" +# Two-factor authentication +gem 'devise-two-factor' +gem 'rqrcode-rails3' +gem 'attr_encrypted', '1.3.4' + # Browser detection gem "browser" # Extracting information from a git repository # Provide access to Gitlab::Git library -gem "gitlab_git", '~> 7.1.10' +gem "gitlab_git", '~> 7.1.11' # Ruby/Rack Git Smart-HTTP Server Handler gem 'gitlab-grack', '~> 2.0.2', require: 'grack' @@ -87,20 +92,17 @@ gem "six" # Seed data gem "seed-fu" -# Markup pipeline for GitLab +# Markdown and HTML processing gem 'html-pipeline', '~> 1.11.0' - -# Markdown to HTML -gem "github-markup" - -# Required markup gems by github-markdown -gem 'redcarpet', '~> 3.2.3' +gem 'task_list', '~> 1.0.0', require: 'task_list/railtie' +gem 'github-markup' +gem 'redcarpet', '~> 3.2.3' gem 'RedCloth' -gem 'rdoc', '~>3.6' -gem 'org-ruby', '= 0.9.12' -gem 'creole', '~>0.3.6' -gem 'wikicloth', '=0.8.1' -gem 'asciidoctor', '= 0.1.4' +gem 'rdoc', '~>3.6' +gem 'org-ruby', '= 0.9.12' +gem 'creole', '~>0.3.6' +gem 'wikicloth', '=0.8.1' +gem 'asciidoctor', '= 0.1.4' # Diffs gem 'diffy', '~> 3.0.3' @@ -251,7 +253,6 @@ group :development, :test do # PhantomJS driver for Capybara gem 'poltergeist', '~> 1.5.1' - gem 'jasmine', '~> 2.2.0' gem 'jasmine-rails' gem "spring", '~> 1.3.1' diff --git a/Gemfile.lock b/Gemfile.lock index 676a5197900..9940ab15242 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -46,6 +46,8 @@ GEM ast (2.0.0) astrolabe (1.3.0) parser (>= 2.2.0.pre.3, < 3.0) + attr_encrypted (1.3.4) + encryptor (>= 1.3.0) attr_required (1.0.0) autoprefixer-rails (5.1.11) execjs @@ -136,6 +138,13 @@ GEM warden (~> 1.2.3) devise-async (0.9.0) devise (~> 3.2) + devise-two-factor (1.0.1) + activemodel + activesupport + attr_encrypted (~> 1.3.2) + devise (~> 3.2.4) + rails + rotp (~> 1.6.1) diff-lcs (1.2.5) diffy (3.0.3) docile (1.1.5) @@ -147,6 +156,7 @@ GEM email_spec (1.5.0) launchy (~> 2.1) mail (~> 2.2) + encryptor (1.3.0) enumerize (0.7.0) activesupport (>= 3.2) equalizer (0.0.8) @@ -215,7 +225,7 @@ GEM mime-types (~> 1.19) gitlab_emoji (0.1.0) gemojione (~> 2.0) - gitlab_git (7.1.10) + gitlab_git (7.1.11) activesupport (~> 4.0) charlock_holmes (~> 0.6) gitlab-linguist (~> 3.0) @@ -290,11 +300,6 @@ GEM i18n (0.7.0) ice_cube (0.11.1) ice_nine (0.10.0) - jasmine (2.2.0) - jasmine-core (~> 2.2) - phantomjs - rack (>= 1.2.1) - rake jasmine-core (2.2.0) jasmine-rails (0.10.8) jasmine-core (>= 1.3, < 3.0) @@ -487,7 +492,11 @@ GEM rest-client (1.6.7) mime-types (>= 1.16) rinku (1.7.3) + rotp (1.6.1) rouge (1.7.7) + rqrcode (0.4.2) + rqrcode-rails3 (0.1.7) + rqrcode (>= 0.4.2) rspec (2.99.0) rspec-core (~> 2.99.0) rspec-expectations (~> 2.99.0) @@ -597,6 +606,8 @@ GEM stamp (0.5.0) state_machine (1.2.0) stringex (2.5.2) + task_list (1.0.2) + html-pipeline temple (0.6.7) term-ansicolor (1.2.2) tins (~> 0.8) @@ -673,6 +684,7 @@ DEPENDENCIES annotate (~> 2.6.0.beta2) asana (~> 0.0.6) asciidoctor (= 0.1.4) + attr_encrypted (= 1.3.4) awesome_print better_errors binding_of_caller @@ -694,6 +706,7 @@ DEPENDENCIES default_value_for (~> 3.0.0) devise (= 3.2.4) devise-async (= 0.9.0) + devise-two-factor diffy (~> 3.0.3) doorkeeper (= 2.1.3) dropzonejs-rails @@ -710,7 +723,7 @@ DEPENDENCIES gitlab-grack (~> 2.0.2) gitlab-linguist (~> 3.0.1) gitlab_emoji (~> 0.1) - gitlab_git (~> 7.1.10) + gitlab_git (~> 7.1.11) gitlab_meta (= 7.0) gitlab_omniauth-ldap (= 1.2.1) gollum-lib (~> 4.0.2) @@ -724,7 +737,6 @@ DEPENDENCIES hipchat (~> 1.5.0) html-pipeline (~> 1.11.0) httparty - jasmine (~> 2.2.0) jasmine-rails jquery-atwho-rails (~> 1.0.0) jquery-rails @@ -766,6 +778,7 @@ DEPENDENCIES redis-rails request_store rerun (~> 0.10.0) + rqrcode-rails3 rspec-rails (= 2.99) rubocop (= 0.28.0) rugments @@ -789,6 +802,7 @@ DEPENDENCIES spring-commands-spinach (= 1.0.0) stamp state_machine + task_list (~> 1.0.0) test_after_commit thin tinder (~> 1.9.2) diff --git a/app/assets/javascripts/behaviors/taskable.js.coffee b/app/assets/javascripts/behaviors/taskable.js.coffee deleted file mode 100644 index ddce71c1886..00000000000 --- a/app/assets/javascripts/behaviors/taskable.js.coffee +++ /dev/null @@ -1,21 +0,0 @@ -window.updateTaskState = (taskableType) -> - objType = taskableType.data - isChecked = $(this).prop("checked") - if $(this).is(":checked") - stateEvent = "task_check" - else - stateEvent = "task_uncheck" - - taskableUrl = $("form.edit-" + objType).first().attr("action") - taskableNum = taskableUrl.match(/\d+$/) - taskNum = 0 - $("li.task-list-item input:checkbox").each( (index, e) => - if e == this - taskNum = index + 1 - ) - - $.ajax - type: "PATCH" - url: taskableUrl - data: objType + "[state_event]=" + stateEvent + - "&" + objType + "[task_num]=" + taskNum diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index 06787ddf874..2baaf430d98 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -112,6 +112,13 @@ class Dispatcher new NamespaceSelect() when 'dashboard' shortcut_handler = new ShortcutsDashboardNavigation() + switch path[1] + when 'issues', 'merge_requests' + new UsersSelect() + when 'groups' + switch path[1] + when 'issues', 'merge_requests' + new UsersSelect() when 'profiles' new Profile() when 'projects' diff --git a/app/assets/javascripts/issue.js.coffee b/app/assets/javascripts/issue.js.coffee index 4e2e6550eb2..86ad3d03bac 100644 --- a/app/assets/javascripts/issue.js.coffee +++ b/app/assets/javascripts/issue.js.coffee @@ -1,3 +1,7 @@ +#= require jquery +#= require jquery.waitforimages +#= require task_list + class @Issue constructor: -> $('.edit-issue.inline-update input[type="submit"]').hide() @@ -6,11 +10,11 @@ class @Issue $(".context .inline-update").on "change", "#issue_assignee_id", -> $(this).submit() - if $("a.btn-close").length - $("li.task-list-item input:checkbox").prop("disabled", false) + # Prevent duplicate event bindings + @disableTaskList() - $('.task-list-item input:checkbox').off('change') - $('.task-list-item input:checkbox').change('issue', updateTaskState) + if $("a.btn-close").length + @initTaskList() $('.issue-details').waitForImages -> $('.issuable-affix').affix offset: @@ -22,3 +26,22 @@ class @Issue $(@).width($(@).outerWidth()) .on 'affixed-top.bs.affix affixed-bottom.bs.affix', -> $(@).width('') + + initTaskList: -> + $('.issue-details .js-task-list-container').taskList('enable') + $(document).on 'tasklist:changed', '.issue-details .js-task-list-container', @updateTaskList + + disableTaskList: -> + $('.issue-details .js-task-list-container').taskList('disable') + $(document).off 'tasklist:changed', '.issue-details .js-task-list-container' + + # TODO (rspeicher): Make the issue description inline-editable like a note so + # that we can re-use its form here + updateTaskList: -> + patchData = {} + patchData['issue'] = {'description': $('.js-task-list-field', this).val()} + + $.ajax + type: 'PATCH' + url: $('form.js-issue-update').attr('action') + data: patchData diff --git a/app/assets/javascripts/merge_request.js.coffee b/app/assets/javascripts/merge_request.js.coffee index ae5d088d593..7c1e2b822d7 100644 --- a/app/assets/javascripts/merge_request.js.coffee +++ b/app/assets/javascripts/merge_request.js.coffee @@ -1,3 +1,7 @@ +#= require jquery +#= require bootstrap +#= require task_list + class @MergeRequest constructor: (@opts) -> @initContextWidget() @@ -17,8 +21,11 @@ class @MergeRequest disableButtonIfEmptyField '#commit_message', '.accept_merge_request' + # Prevent duplicate event bindings + @disableTaskList() + if $("a.btn-close").length - $("li.task-list-item input:checkbox").prop("disabled", false) + @initTaskList() $('.merge-request-details').waitForImages -> $('.issuable-affix').affix offset: @@ -77,9 +84,6 @@ class @MergeRequest this.$('.remove_source_branch_in_progress').hide() this.$('.remove_source_branch_widget.failed').show() - $('.task-list-item input:checkbox').off('change') - $('.task-list-item input:checkbox').change('merge_request', updateTaskState) - activateTab: (action) -> this.$('.merge-request-tabs li').removeClass 'active' this.$('.tab-content').hide() @@ -156,3 +160,22 @@ class @MergeRequest else setTimeout(merge_request.mergeInProgress, 3000) dataType: 'json' + + initTaskList: -> + $('.merge-request-details .js-task-list-container').taskList('enable') + $(document).on 'tasklist:changed', '.merge-request-details .js-task-list-container', @updateTaskList + + disableTaskList: -> + $('.merge-request-details .js-task-list-container').taskList('disable') + $(document).off 'tasklist:changed', '.merge-request-details .js-task-list-container' + + # TODO (rspeicher): Make the merge request description inline-editable like a + # note so that we can re-use its form here + updateTaskList: -> + patchData = {} + patchData['merge_request'] = {'description': $('.js-task-list-field', this).val()} + + $.ajax + type: 'PATCH' + url: $('form.js-merge-request-update').attr('action') + data: patchData diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 6dfe10f0006..c25b1ddb066 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -1,3 +1,12 @@ +#= require jquery +#= require autosave +#= require bootstrap +#= require dropzone +#= require dropzone_input +#= require gfm_auto_complete +#= require jquery.atwho +#= require task_list + class @Notes @interval: null @@ -11,6 +20,7 @@ class @Notes @setupMainTargetNoteForm() @cleanBinding() @addBinding() + @initTaskList() addBinding: -> # add note to UI after creation @@ -81,6 +91,9 @@ class @Notes $(document).off "click", ".js-note-target-reopen" $(document).off "click", ".js-note-target-close" + $('.note .js-task-list-container').taskList('disable') + $(document).off 'tasklist:changed', '.note .js-task-list-container' + initRefresh: -> clearInterval(Notes.interval) Notes.interval = setInterval => @@ -114,6 +127,7 @@ class @Notes if @isNewNote(note) @note_ids.push(note.id) $('ul.main-notes-list').append(note.html) + @initTaskList() ### Check if note does not exists on page @@ -268,6 +282,8 @@ class @Notes note_li.replaceWith(note.html) note_li.find('.note-edit-form').hide() note_li.find('.note-body > .note-text').show() + note_li.find('js-task-list-container').taskList('enable') + @enableTaskList() ### Called in response to clicking the edit note link @@ -479,3 +495,13 @@ class @Notes else form.find('.js-note-target-reopen').text('Reopen') form.find('.js-note-target-close').text('Close') + + initTaskList: -> + @enableTaskList() + $(document).on 'tasklist:changed', '.note .js-task-list-container', @updateTaskList + + enableTaskList: -> + $('.note .js-task-list-container').taskList('enable') + + updateTaskList: -> + $('form', this).submit() diff --git a/app/assets/stylesheets/base/mixins.scss b/app/assets/stylesheets/base/mixins.scss index b8b163a42b2..a0794e7825a 100644 --- a/app/assets/stylesheets/base/mixins.scss +++ b/app/assets/stylesheets/base/mixins.scss @@ -21,6 +21,10 @@ @include border-radius($radius 0 0 $radius) } +@mixin border-radius-right($radius) { + @include border-radius(0 0 $radius $radius) +} + @mixin linear-gradient($from, $to) { background-image: -webkit-gradient(linear, 0 0, 0 100%, from($from), to($to)); background-image: -webkit-linear-gradient($from, $to); diff --git a/app/assets/stylesheets/generic/typography.scss b/app/assets/stylesheets/generic/typography.scss index 36a9a540747..e5590897947 100644 --- a/app/assets/stylesheets/generic/typography.scss +++ b/app/assets/stylesheets/generic/typography.scss @@ -37,7 +37,9 @@ pre { position: relative; a.anchor { - display: none; + // Setting `display: none` would prevent the anchor being scrolled to, so + // instead we set the height to 0 and it gets updated on hover. + height: 0; } &:hover > a.anchor { diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss index 84361e15481..359f4073e87 100644 --- a/app/assets/stylesheets/pages/commits.scss +++ b/app/assets/stylesheets/pages/commits.scss @@ -29,10 +29,6 @@ .commits-feed-holder { float: right; - - .btn { - padding: 4px 12px; - } } li.commit { diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 97b19deb3ed..61b907e39be 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -44,6 +44,14 @@ ul.notes { } .author-username { } + + .note-role { + float: right; + margin-top: 2px; + border: 1px solid #bbb; + background-color: transparent; + color: #999; + } } .discussion { @@ -62,6 +70,16 @@ ul.notes { word-wrap: break-word; @include md-typography; + // Reduce left padding of first ul element + ul.task-list:first-child { + padding-left: 10px; + + // sub-lists should be padded normally + ul { + padding-left: 20px; + } + } + hr { margin: 10px 0; } @@ -126,6 +144,7 @@ ul.notes { .note { &.note:hover { .note-actions { display: block; } + .note-actions + .note-role { display: none; } } .discussion-header:hover { .discussion-actions { display: block; } @@ -143,6 +162,8 @@ ul.notes { } a { + margin-left: 5px; + @extend .cgray; &:hover { diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index e6a4bc62c58..224aea2db59 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -102,6 +102,15 @@ .input-group-addon { background: #FAFAFA; + + &.git-protocols { + padding: 0; + border: none; + + .input-group-btn:last-child > .btn { + @include border-radius-right(0); + } + } } } @@ -240,7 +249,8 @@ ul.nav.nav-projects-tabs { } .breadcrumb.repo-breadcrumb { - padding: 2px 0; + padding: 0; + line-height: 34px; background: white; border: none; font-size: 16px; diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss index 57f63b52aa1..34ee4d7b31e 100644 --- a/app/assets/stylesheets/pages/tree.scss +++ b/app/assets/stylesheets/pages/tree.scss @@ -106,17 +106,9 @@ } } -.tree-download-holder .btn { - padding: 4px 12px; -} - .tree-ref-holder { float: left; margin-right: 15px; - - .select2-container .select2-choice, .select2-container.select2-drop-above .select2-choice { - padding: 4px 12px; - } } .readme-holder { diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb index 0a463239d74..690096bdbcf 100644 --- a/app/controllers/admin/hooks_controller.rb +++ b/app/controllers/admin/hooks_controller.rb @@ -33,7 +33,7 @@ class Admin::HooksController < Admin::ApplicationController owner_name: "Someone", owner_email: "example@gitlabhq.com" } - @hook.execute(data) + @hook.execute(data, 'system_hooks') redirect_to :back end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index eee10d6c22a..8ce881c7414 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -252,7 +252,7 @@ class ApplicationController < ActionController::Base end def configure_permitted_parameters - devise_parameter_sanitizer.sanitize(:sign_in) { |u| u.permit(:username, :email, :password, :login, :remember_me) } + devise_parameter_sanitizer.for(:sign_in) { |u| u.permit(:username, :email, :password, :login, :remember_me, :otp_attempt) } end def hexdigest(string) diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index dcbbe5baa4b..88459d4080a 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -15,4 +15,25 @@ class PasswordsController < Devise::PasswordsController respond_with(resource) end end + + # After a user resets their password, prompt for 2FA code if enabled instead + # of signing in automatically + # + # See http://git.io/vURrI + def update + super do |resource| + # TODO (rspeicher): In Devise master (> 3.4.1), we can set + # `Devise.sign_in_after_reset_password = false` and avoid this mess. + if resource.errors.empty? && resource.try(:otp_required_for_login?) + resource.unlock_access! if unlockable?(resource) + + # Since we are not signing this user in, we use the :updated_not_active + # message which only contains "Your password was changed successfully." + set_flash_message(:notice, :updated_not_active) if is_flashing_format? + + # Redirect to sign in so they can enter 2FA code + respond_with(resource, location: new_session_path(resource)) and return + end + end + end end diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb new file mode 100644 index 00000000000..30ee6891733 --- /dev/null +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -0,0 +1,49 @@ +class Profiles::TwoFactorAuthsController < Profiles::ApplicationController + def new + unless current_user.otp_secret + current_user.otp_secret = User.generate_otp_secret + current_user.save! + end + + @qr_code = build_qr_code + end + + def create + if current_user.valid_otp?(params[:pin_code]) + current_user.otp_required_for_login = true + @codes = current_user.generate_otp_backup_codes! + current_user.save! + + render 'create' + else + @error = 'Invalid pin code' + @qr_code = build_qr_code + render 'new' + end + end + + def codes + @codes = current_user.generate_otp_backup_codes! + current_user.save! + end + + def destroy + current_user.update_attributes({ + otp_required_for_login: false, + encrypted_otp_secret: nil, + encrypted_otp_secret_iv: nil, + encrypted_otp_secret_salt: nil, + otp_backup_codes: nil + }) + + redirect_to profile_account_path + end + + private + + def build_qr_code + issuer = "GitLab | #{current_user.email}" + uri = current_user.otp_provisioning_uri(current_user.email, issuer: issuer) + RQRCode::render_qrcode(uri, :svg, level: :m, unit: 3) + end +end diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 7c20b81c0b1..c5f085c236f 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -6,11 +6,12 @@ class Projects::CompareController < Projects::ApplicationController before_action :authorize_download_code! def index + @ref = Addressable::URI.unescape(params[:to]) end def show base_ref = Addressable::URI.unescape(params[:from]) - head_ref = Addressable::URI.unescape(params[:to]) + @ref = head_ref = Addressable::URI.unescape(params[:to]) compare_result = CompareService.new.execute( current_user, diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 3f11d7afe6f..d4ff0d97561 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,4 +1,12 @@ class SessionsController < Devise::SessionsController + prepend_before_action :authenticate_with_two_factor, only: [:create] + + # This action comes from DeviseController, but because we call `sign_in` + # manually inside `authenticate_with_two_factor`, not skipping this action + # would cause a "You are already signed in." error message to be shown upon + # successful login. + skip_before_action :require_no_authentication, only: [:create] + def new redirect_path = if request.referer.present? && (params['redirect_to_referer'] == 'yes') @@ -14,7 +22,7 @@ class SessionsController < Devise::SessionsController # Prevent a 'you are already signed in' message directly after signing: # we should never redirect to '/users/sign_in' after signing in successfully. - unless redirect_path == '/users/sign_in' + unless redirect_path == new_user_session_path store_location_for(:redirect, redirect_path) end @@ -27,11 +35,54 @@ class SessionsController < Devise::SessionsController def create super do |resource| - # User has successfully signed in, so clear any unused reset tokens + # User has successfully signed in, so clear any unused reset token if resource.reset_password_token.present? resource.update_attributes(reset_password_token: nil, reset_password_sent_at: nil) end end end + + private + + def user_params + params.require(:user).permit(:login, :password, :remember_me, :otp_attempt) + end + + def find_user + if user_params[:login] + User.by_login(user_params[:login]) + elsif user_params[:otp_attempt] && session[:otp_user_id] + User.find(session[:otp_user_id]) + end + end + + def authenticate_with_two_factor + user = self.resource = find_user + + return unless user && user.otp_required_for_login + + if user_params[:otp_attempt].present? && session[:otp_user_id] + if valid_otp_attempt?(user) + # Remove any lingering user data from login + session.delete(:otp_user_id) + + sign_in(user) and return + else + flash.now[:alert] = 'Invalid two-factor code.' + render :two_factor and return + end + else + if user && user.valid_password?(user_params[:password]) + # Save the user's ID to session so we can ask for a one-time password + session[:otp_user_id] = user.id + render :two_factor and return + end + end + end + + def valid_otp_attempt?(user) + user.valid_otp?(user_params[:otp_attempt]) || + user.invalidate_otp_backup_code!(user_params[:otp_attempt]) + end end diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 17edff68be2..28536e359e5 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -52,13 +52,13 @@ class UploadsController < ApplicationController def upload_model upload_models = { - user: User, - project: Project, - note: Note, - group: Group + "user" => User, + "project" => Project, + "note" => Note, + "group" => Group } - upload_models[params[:model].to_sym] + upload_models[params[:model]] end def upload_mount diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 6e86400a4f6..672be54e66f 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -214,7 +214,7 @@ module ApplicationHelper def time_ago_with_tooltip(date, placement = 'top', html_class = 'time_ago') capture_haml do haml_tag :time, date.to_s, - class: html_class, datetime: date.getutc.iso8601, title: date.stamp('Aug 21, 2011 9:23pm'), + class: html_class, datetime: date.getutc.iso8601, title: date.in_time_zone.stamp('Aug 21, 2011 9:23pm'), data: { toggle: 'tooltip', placement: placement } haml_tag :script, "$('." + html_class + "').timeago().tooltip()" diff --git a/app/helpers/compare_helper.rb b/app/helpers/compare_helper.rb index 01847c6b807..f1dc906cab4 100644 --- a/app/helpers/compare_helper.rb +++ b/app/helpers/compare_helper.rb @@ -1,21 +1,20 @@ module CompareHelper - def compare_to_mr_button? - @project.merge_requests_enabled && - params[:from].present? && - params[:to].present? && - @repository.branch_names.include?(params[:from]) && - @repository.branch_names.include?(params[:to]) && - params[:from] != params[:to] && - !@refs_are_same + def create_mr_button?(from = params[:from], to = params[:to], project = @project) + from.present? && + to.present? && + from != to && + project.merge_requests_enabled && + project.repository.branch_names.include?(from) && + project.repository.branch_names.include?(to) end - def compare_mr_path + def create_mr_path(from = params[:from], to = params[:to], project = @project) new_namespace_project_merge_request_path( - @project.namespace, - @project, + project.namespace, + project, merge_request: { - source_branch: params[:to], - target_branch: params[:from] + source_branch: to, + target_branch: from } ) end diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 162778ade58..1b10795bb7b 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -7,14 +7,23 @@ module DiffHelper end end - def safe_diff_files(diffs) - diffs.first(allowed_diff_size).map do |diff| - Gitlab::Diff::File.new(diff) + def allowed_diff_lines + if diff_hard_limit_enabled? + Commit::DIFF_HARD_LIMIT_LINES + else + Commit::DIFF_SAFE_LINES end end - def show_diff_size_warning?(diffs) - diffs.size > allowed_diff_size + def safe_diff_files(diffs) + lines = 0 + safe_files = [] + diffs.first(allowed_diff_size).each do |diff| + lines += diff.diff.lines.count + break if lines > allowed_diff_lines + safe_files << Gitlab::Diff::File.new(diff) + end + safe_files end def diff_hard_limit_enabled? diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index c9fd0f0362b..18c75a8726b 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -25,12 +25,16 @@ module EventsHelper def event_filter_link(key, tooltip) key = key.to_s - active = if @event_filter.active? key - 'active' - end + active = 'active' if @event_filter.active?(key) + link_opts = { + class: 'event_filter_link', + id: "#{key}_event_filter", + title: "Filter by #{tooltip.downcase}", + data: { toggle: 'tooltip', placement: 'top' } + } content_tag :li, class: "filter_icon #{active}" do - link_to request.path, class: 'has_tooltip event_filter_link', id: "#{key}_event_filter", 'data-original-title' => 'Filter by ' + tooltip.downcase do + link_to request.path, link_opts do icon(icon_for_event[key]) + content_tag(:span, ' ' + tooltip) end end diff --git a/app/helpers/icons_helper.rb b/app/helpers/icons_helper.rb index a9030729b48..a730684f8f3 100644 --- a/app/helpers/icons_helper.rb +++ b/app/helpers/icons_helper.rb @@ -26,15 +26,15 @@ module IconsHelper end def public_icon - icon('globe') + icon('globe fw') end def internal_icon - icon('shield') + icon('shield fw') end def private_icon - icon('lock') + icon('lock fw') end def file_type_icon_class(type, mode, name) diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index ab44fa6ee43..271b53aa2b6 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -9,6 +9,10 @@ module NotesHelper hidden_field_tag(:target_id, note.noteable.id) end + def note_editable?(note) + note.editable? && can?(current_user, :admin_note, note) + end + def link_to_commit_diff_line_note(note) if note.for_commit_diff_line? link_to( diff --git a/app/mailers/devise_mailer.rb b/app/mailers/devise_mailer.rb index e4b65c19b3f..b616add283a 100644 --- a/app/mailers/devise_mailer.rb +++ b/app/mailers/devise_mailer.rb @@ -1,4 +1,4 @@ class DeviseMailer < Devise::Mailer - default from: "#{Gitlab.config.outgoing_emails.display_name} <#{Gitlab.config.outgoing_emails.from}>" - default reply_to: Gitlab.config.outgoing_emails.reply_to + default from: "#{Gitlab.config.gitlab.email_display_name} <#{Gitlab.config.gitlab.email_from}>" + default reply_to: Gitlab.config.gitlab.email_reply_to end diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index c6bc716930d..9cb7077e59d 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -129,7 +129,7 @@ module Emails if send_from_committer_email && can_send_from_user_email?(@author) @author.email else - Gitlab.config.outgoing_emails.reply_to + Gitlab.config.gitlab.email_reply_to end mail(from: sender(author_id, send_from_committer_email), diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index a9800f7f103..79fb48b00d3 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -17,7 +17,7 @@ class Notify < ActionMailer::Base helper_method :current_user, :can? default from: Proc.new { default_sender_address.format } - default reply_to: Gitlab.config.outgoing_emails.reply_to + default reply_to: Gitlab.config.gitlab.email_reply_to # Just send email with 2 seconds delay def self.delay @@ -50,9 +50,9 @@ class Notify < ActionMailer::Base # The default email address to send emails from def default_sender_address - Mail::Address.new(Gitlab.config.outgoing_emails.from).tap do |address| - address.display_name = Gitlab.config.outgoing_emails.display_name - end + address = Mail::Address.new(Gitlab.config.gitlab.email_from) + address.display_name = Gitlab.config.gitlab.email_display_name + address end def can_send_from_user_email?(sender) diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index 3ef3e8b67d8..a5957391bb7 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -39,7 +39,7 @@ module Mentionable # Determine whether or not a cross-reference Note has already been created between this Mentionable and # the specified target. def has_mentioned?(target) - Note.cross_reference_exists?(target, local_reference) + SystemNoteService.cross_reference_exists?(target, local_reference) end def mentioned_users(current_user = nil, p = project) diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb index bbb3b301a9f..33b4814d7ec 100644 --- a/app/models/concerns/taskable.rb +++ b/app/models/concerns/taskable.rb @@ -1,51 +1,36 @@ +require 'task_list' + # Contains functionality for objects that can have task lists in their # descriptions. Task list items can be added with Markdown like "* [x] Fix # bugs". # # Used by MergeRequest and Issue module Taskable - TASK_PATTERN_MD = /^(?<bullet> *[*-] *)\[(?<checked>[ xX])\]/.freeze - TASK_PATTERN_HTML = /^<li>(?<p_tag>\s*<p>)?\[(?<checked>[ xX])\]/.freeze - - # Change the state of a task list item for this Taskable. Edit the object's - # description by finding the nth task item and changing its checkbox - # placeholder to "[x]" if +checked+ is true, or "[ ]" if it's false. - # Note: task numbering starts with 1 - def update_nth_task(n, checked) - index = 0 - check_char = checked ? 'x' : ' ' + # Called by `TaskList::Summary` + def task_list_items + return [] if description.blank? - # Do this instead of using #gsub! so that ActiveRecord detects that a field - # has changed. - self.description = self.description.gsub(TASK_PATTERN_MD) do |match| - index += 1 - case index - when n then "#{$LAST_MATCH_INFO[:bullet]}[#{check_char}]" - else match - end + @task_list_items ||= description.scan(TaskList::Filter::ItemPattern).collect do |item| + # ItemPattern strips out the hyphen, but Item requires it. Rabble rabble. + TaskList::Item.new("- #{item}") end + end - save + def tasks + @tasks ||= TaskList.new(self) end # Return true if this object's description has any task list items. def tasks? - description && description.match(TASK_PATTERN_MD) + tasks.summary.items? end # Return a string that describes the current state of this Taskable's task - # list items, e.g. "20 tasks (12 done, 8 unfinished)" + # list items, e.g. "20 tasks (12 completed, 8 remaining)" def task_status - return nil unless description - - num_tasks = 0 - num_done = 0 - - description.scan(TASK_PATTERN_MD) do - num_tasks += 1 - num_done += 1 unless $LAST_MATCH_INFO[:checked] == ' ' - end + return '' if description.blank? - "#{num_tasks} tasks (#{num_done} done, #{num_tasks - num_done} unfinished)" + sum = tasks.summary + "#{sum.item_count} tasks (#{sum.complete_count} completed, #{sum.incomplete_count} remaining)" end end diff --git a/app/models/hooks/service_hook.rb b/app/models/hooks/service_hook.rb index 2e11239c40b..5b38ade2e6b 100644 --- a/app/models/hooks/service_hook.rb +++ b/app/models/hooks/service_hook.rb @@ -17,4 +17,8 @@ class ServiceHook < WebHook belongs_to :service + + def execute(data) + super(data, 'service_hook') + end end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 315d96af1b9..e9fd441352d 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -30,12 +30,15 @@ class WebHook < ActiveRecord::Base validates :url, presence: true, format: { with: /\A#{URI.regexp(%w(http https))}\z/, message: "should be a valid url" } - def execute(data) + def execute(data, hook_name) parsed_url = URI.parse(url) if parsed_url.userinfo.blank? WebHook.post(url, body: data.to_json, - headers: { "Content-Type" => "application/json" }, + headers: { + "Content-Type" => "application/json", + "X-Gitlab-Event" => hook_name.singularize.titleize + }, verify: false) else post_url = url.gsub("#{parsed_url.userinfo}@", "") @@ -45,7 +48,10 @@ class WebHook < ActiveRecord::Base } WebHook.post(post_url, body: data.to_json, - headers: { "Content-Type" => "application/json" }, + headers: { + "Content-Type" => "application/json", + "X-Gitlab-Event" => hook_name.singularize.titleize + }, verify: false, basic_auth: auth) end @@ -54,7 +60,7 @@ class WebHook < ActiveRecord::Base false end - def async_execute(data) - Sidekiq::Client.enqueue(ProjectWebHookWorker, id, data) + def async_execute(data, hook_name) + Sidekiq::Client.enqueue(ProjectWebHookWorker, id, data, hook_name) end end diff --git a/app/models/note.rb b/app/models/note.rb index cbce6786683..6939a7e73a0 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -63,143 +63,9 @@ class Note < ActiveRecord::Base after_update :set_references class << self - def create_status_change_note(noteable, project, author, status, source) - body = "Status changed to #{status}#{' by ' + source.gfm_reference if source}" - - create( - noteable: noteable, - project: project, - author: author, - note: body, - system: true - ) - end - - # +noteable+ was referenced from +mentioner+, by including GFM in either - # +mentioner+'s description or an associated Note. - # Create a system Note associated with +noteable+ with a GFM back-reference - # to +mentioner+. - def create_cross_reference_note(noteable, mentioner, author) - gfm_reference = mentioner_gfm_ref(noteable, mentioner) - - note_options = { - project: noteable.project, - author: author, - note: cross_reference_note_content(gfm_reference), - system: true - } - - if noteable.kind_of?(Commit) - note_options.merge!(noteable_type: 'Commit', commit_id: noteable.id) - else - note_options.merge!(noteable: noteable) - end - - create(note_options) unless cross_reference_disallowed?(noteable, mentioner) - end - - def create_milestone_change_note(noteable, project, author, milestone) - body = if milestone.nil? - 'Milestone removed' - else - "Milestone changed to #{milestone.title}" - end - - create( - noteable: noteable, - project: project, - author: author, - note: body, - system: true - ) - end - - def create_assignee_change_note(noteable, project, author, assignee) - body = assignee.nil? ? 'Assignee removed' : "Reassigned to @#{assignee.username}" - - create({ - noteable: noteable, - project: project, - author: author, - note: body, - system: true - }) - end - - def create_labels_change_note(noteable, project, author, added_labels, removed_labels) - labels_count = added_labels.count + removed_labels.count - added_labels = added_labels.map{ |label| "~#{label.id}" }.join(' ') - removed_labels = removed_labels.map{ |label| "~#{label.id}" }.join(' ') - message = '' - - if added_labels.present? - message << "added #{added_labels}" - end - - if added_labels.present? && removed_labels.present? - message << ' and ' - end - - if removed_labels.present? - message << "removed #{removed_labels}" - end - - message << ' ' << 'label'.pluralize(labels_count) - body = "#{message.capitalize}" - - create( - noteable: noteable, - project: project, - author: author, - note: body, - system: true - ) - end - - def create_new_commits_note(merge_request, project, author, new_commits, existing_commits = [], oldrev = nil) - total_count = new_commits.length + existing_commits.length - commits_text = ActionController::Base.helpers.pluralize(total_count, 'commit') - body = "Added #{commits_text}:\n\n" - - if existing_commits.length > 0 - commit_ids = - if existing_commits.length == 1 - existing_commits.first.short_id - else - if oldrev - "#{Commit.truncate_sha(oldrev)}...#{existing_commits.last.short_id}" - else - "#{existing_commits.first.short_id}..#{existing_commits.last.short_id}" - end - end - - commits_text = ActionController::Base.helpers.pluralize(existing_commits.length, 'commit') - - branch = - if merge_request.for_fork? - "#{merge_request.target_project_namespace}:#{merge_request.target_branch}" - else - merge_request.target_branch - end - - message = "* #{commit_ids} - #{commits_text} from branch `#{branch}`" - body << message - body << "\n" - end - - new_commits.each do |commit| - message = "* #{commit.short_id} - #{commit.title}" - body << message - body << "\n" - end - - create( - noteable: merge_request, - project: project, - author: author, - note: body, - system: true - ) + # TODO (rspeicher): Update usages + def create_cross_reference_note(*args) + SystemNoteService.cross_reference(*args) end def discussions_from_notes(notes) @@ -227,88 +93,19 @@ class Note < ActiveRecord::Base [:discussion, type.try(:underscore), id, line_code].join("-").to_sym end - # Determine if cross reference note should be created. - # eg. mentioning a commit in MR comments which exists inside a MR - # should not create "mentioned in" note. - def cross_reference_disallowed?(noteable, mentioner) - if mentioner.kind_of?(MergeRequest) - mentioner.commits.map(&:id).include? noteable.id - end - end - - # Determine whether or not a cross-reference note already exists. - def cross_reference_exists?(noteable, mentioner) - gfm_reference = mentioner_gfm_ref(noteable, mentioner, true) - notes = if noteable.is_a?(Commit) - where(commit_id: noteable.id, noteable_type: 'Commit') - else - where(noteable_id: noteable.id, noteable_type: noteable.class) - end - - notes.where('note like ?', cross_reference_note_pattern(gfm_reference)). - system.any? - end - def search(query) where("note like :query", query: "%#{query}%") end + end - def cross_reference_note_prefix - 'mentioned in ' - end - - private - - def cross_reference_note_content(gfm_reference) - cross_reference_note_prefix + "#{gfm_reference}" - end - - def cross_reference_note_pattern(gfm_reference) - # Older cross reference notes contained underscores for emphasis - "%" + cross_reference_note_content(gfm_reference) + "%" - end - - # Prepend the mentioner's namespaced project path to the GFM reference for - # cross-project references. For same-project references, return the - # unmodified GFM reference. - def mentioner_gfm_ref(noteable, mentioner, cross_reference = false) - if mentioner.is_a?(Commit) && cross_reference - return mentioner.gfm_reference.sub('commit ', 'commit %') - end - - full_gfm_reference(mentioner.project, noteable.project, mentioner) - end - - # Return the +mentioner+ GFM reference. If the mentioner and noteable - # projects are not the same, add the mentioning project's path to the - # returned value. - def full_gfm_reference(mentioning_project, noteable_project, mentioner) - if mentioning_project == noteable_project - mentioner.gfm_reference - else - if mentioner.is_a?(Commit) - mentioner.gfm_reference.sub( - /(commit )/, - "\\1#{mentioning_project.path_with_namespace}@" - ) - else - mentioner.gfm_reference.sub( - /(issue |merge request )/, - "\\1#{mentioning_project.path_with_namespace}" - ) - end - end - end + def cross_reference? + system && SystemNoteService.cross_reference?(note) end def max_attachment_size current_application_settings.max_attachment_size.megabytes.to_i end - def cross_reference? - note.start_with?(self.class.cross_reference_note_prefix) - end - def find_diff return nil unless noteable && noteable.diffs.present? @@ -449,16 +246,6 @@ class Note < ActiveRecord::Base @discussion_id ||= Note.build_discussion_id(noteable_type, noteable_id || commit_id, line_code) end - # Returns true if this is a downvote note, - # otherwise false is returned - def downvote? - votable? && (note.start_with?('-1') || - note.start_with?(':-1:') || - note.start_with?(':thumbsdown:') || - note.start_with?(':thumbs_down_sign:') - ) - end - def for_commit? noteable_type == "Commit" end @@ -500,14 +287,18 @@ class Note < ActiveRecord::Base nil end - # Returns true if this is an upvote note, - # otherwise false is returned + DOWNVOTES = %w(-1 :-1: :thumbsdown: :thumbs_down_sign:) + + # Check if the note is a downvote + def downvote? + votable? && note.start_with?(*DOWNVOTES) + end + + UPVOTES = %w(+1 :+1: :thumbsup: :thumbs_up_sign:) + + # Check if the note is an upvote def upvote? - votable? && (note.start_with?('+1') || - note.start_with?(':+1:') || - note.start_with?(':thumbsup:') || - note.start_with?(':thumbs_up_sign:') - ) + votable? && note.start_with?(*UPVOTES) end def superceded?(notes) diff --git a/app/models/project.rb b/app/models/project.rb index 6a2d7895446..09d3ffd22fe 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -483,7 +483,7 @@ class Project < ActiveRecord::Base def execute_hooks(data, hooks_scope = :push_hooks) hooks.send(hooks_scope).each do |hook| - hook.async_execute(data) + hook.async_execute(data, hooks_scope.to_s) end end diff --git a/app/models/user.rb b/app/models/user.rb index 703a7d32705..4dd37e73564 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -50,6 +50,11 @@ # bitbucket_access_token :string(255) # bitbucket_access_token_secret :string(255) # location :string(255) +# encrypted_otp_secret :string(255) +# encrypted_otp_secret_iv :string(255) +# encrypted_otp_secret_salt :string(255) +# otp_required_for_login :boolean +# otp_backup_codes :text # public_email :string(255) default(""), not null # @@ -70,8 +75,14 @@ class User < ActiveRecord::Base default_value_for :hide_no_password, false default_value_for :theme_id, gitlab_config.default_theme - devise :database_authenticatable, :lockable, :async, - :recoverable, :rememberable, :trackable, :validatable, :omniauthable, :confirmable, :registerable + devise :two_factor_authenticatable, + otp_secret_encryption_key: File.read(Rails.root.join('.secret')).chomp + + devise :two_factor_backupable, otp_number_of_backup_codes: 10 + serialize :otp_backup_codes, JSON + + devise :lockable, :async, :recoverable, :rememberable, :trackable, + :validatable, :omniauthable, :confirmable, :registerable attr_accessor :force_random_password @@ -438,7 +449,7 @@ class User < ActiveRecord::Base end def project_deploy_keys - DeployKey.in_projects(self.authorized_projects.pluck(:id)) + DeployKey.unscoped.in_projects(self.authorized_projects.pluck(:id)).distinct(:id) end def accessible_deploy_keys diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 5e1906ad2ae..8960235b093 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -2,17 +2,17 @@ class IssuableBaseService < BaseService private def create_assignee_note(issuable) - Note.create_assignee_change_note( + SystemNoteService.change_assignee( issuable, issuable.project, current_user, issuable.assignee) end def create_milestone_note(issuable) - Note.create_milestone_change_note( + SystemNoteService.change_milestone( issuable, issuable.project, current_user, issuable.milestone) end def create_labels_note(issuable, added_labels, removed_labels) - Note.create_labels_change_note( + SystemNoteService.change_label( issuable, issuable.project, current_user, added_labels, removed_labels) end end diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index f670019cc63..138465859ce 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -14,7 +14,7 @@ module Issues private def create_note(issue, current_commit) - Note.create_status_change_note(issue, issue.project, current_user, issue.state, current_commit) + SystemNoteService.change_status(issue, issue.project, current_user, issue.state, current_commit) end end end diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb index 1e5c398516d..e48ca359f4f 100644 --- a/app/services/issues/reopen_service.rb +++ b/app/services/issues/reopen_service.rb @@ -14,7 +14,7 @@ module Issues private def create_note(issue) - Note.create_status_change_note(issue, issue.project, current_user, issue.state, nil) + SystemNoteService.change_status(issue, issue.project, current_user, issue.state, nil) end end end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index f6e1ae6f283..e455fe95791 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -2,7 +2,7 @@ module MergeRequests class BaseService < ::IssuableBaseService def create_note(merge_request) - Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil) + SystemNoteService.change_status(merge_request, merge_request.target_project, current_user, merge_request.state, nil) end def hook_data(merge_request, action) diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index e9b526d1fb7..66610a08a44 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -82,8 +82,9 @@ module MergeRequests mr_commit_ids.include?(commit.id) end - Note.create_new_commits_note(merge_request, merge_request.project, - @current_user, new_commits, existing_commits, @oldrev) + SystemNoteService.add_commits(merge_request, merge_request.project, + @current_user, new_commits, + existing_commits, @oldrev) end end diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index c5d0b08845b..60235b6be2a 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -7,12 +7,12 @@ class SystemHooksService def execute_hooks(data) SystemHook.all.each do |sh| - async_execute_hook sh, data + async_execute_hook(sh, data, 'system_hooks') end end - def async_execute_hook(hook, data) - Sidekiq::Client.enqueue(SystemHookWorker, hook.id, data) + def async_execute_hook(hook, data, hook_name) + Sidekiq::Client.enqueue(SystemHookWorker, hook.id, data, hook_name) end def build_event_data(model, event) diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb new file mode 100644 index 00000000000..0614f8689a4 --- /dev/null +++ b/app/services/system_note_service.rb @@ -0,0 +1,303 @@ +# SystemNoteService +# +# Used for creating system notes (e.g., when a user references a merge request +# from an issue, an issue's assignee changes, an issue is closed, etc.) +class SystemNoteService + # Called when commits are added to a Merge Request + # + # noteable - Noteable object + # project - Project owning noteable + # author - User performing the change + # new_commits - Array of Commits added since last push + # existing_commits - Array of Commits added in a previous push + # oldrev - TODO (rspeicher): I have no idea what this actually does + # + # See new_commit_summary and existing_commit_summary. + # + # Returns the created Note object + def self.add_commits(noteable, project, author, new_commits, existing_commits = [], oldrev = nil) + total_count = new_commits.length + existing_commits.length + commits_text = "#{total_count} commit".pluralize(total_count) + + body = "Added #{commits_text}:\n\n" + body << existing_commit_summary(noteable, existing_commits, oldrev) + body << new_commit_summary(new_commits).join("\n") + + create_note(noteable: noteable, project: project, author: author, note: body) + end + + # Called when the assignee of a Noteable is changed or removed + # + # noteable - Noteable object + # project - Project owning noteable + # author - User performing the change + # assignee - User being assigned, or nil + # + # Example Note text: + # + # "Assignee removed" + # + # "Reassigned to @rspeicher" + # + # Returns the created Note object + def self.change_assignee(noteable, project, author, assignee) + body = assignee.nil? ? 'Assignee removed' : "Reassigned to @#{assignee.username}" + + create_note(noteable: noteable, project: project, author: author, note: body) + end + + # Called when one or more labels on a Noteable are added and/or removed + # + # noteable - Noteable object + # project - Project owning noteable + # author - User performing the change + # added_labels - Array of Labels added + # removed_labels - Array of Labels removed + # + # Example Note text: + # + # "Added ~1 and removed ~2 ~3 labels" + # + # "Added ~4 label" + # + # "Removed ~5 label" + # + # Returns the created Note object + def self.change_label(noteable, project, author, added_labels, removed_labels) + labels_count = added_labels.count + removed_labels.count + + references = ->(label) { "~#{label.id}" } + added_labels = added_labels.map(&references).join(' ') + removed_labels = removed_labels.map(&references).join(' ') + + body = '' + + if added_labels.present? + body << "added #{added_labels}" + body << ' and ' if removed_labels.present? + end + + if removed_labels.present? + body << "removed #{removed_labels}" + end + + body << ' ' << 'label'.pluralize(labels_count) + body = "#{body.capitalize}" + + create_note(noteable: noteable, project: project, author: author, note: body) + end + + # Called when the milestone of a Noteable is changed + # + # noteable - Noteable object + # project - Project owning noteable + # author - User performing the change + # milestone - Milestone being assigned, or nil + # + # Example Note text: + # + # "Milestone removed" + # + # "Miletone changed to 7.11" + # + # Returns the created Note object + def self.change_milestone(noteable, project, author, milestone) + body = 'Milestone ' + body += milestone.nil? ? 'removed' : "changed to #{milestone.title}" + + create_note(noteable: noteable, project: project, author: author, note: body) + end + + # Called when the status of a Noteable is changed + # + # noteable - Noteable object + # project - Project owning noteable + # author - User performing the change + # status - String status + # source - Mentionable performing the change, or nil + # + # Example Note text: + # + # "Status changed to merged" + # + # "Status changed to closed by bc17db76" + # + # Returns the created Note object + def self.change_status(noteable, project, author, status, source) + body = "Status changed to #{status}" + body += " by #{source.gfm_reference}" if source + + create_note(noteable: noteable, project: project, author: author, note: body) + end + + # Called when a Mentionable references a Noteable + # + # noteable - Noteable object being referenced + # mentioner - Mentionable object + # author - User performing the reference + # + # Example Note text: + # + # "Mentioned in #1" + # + # "Mentioned in !2" + # + # "Mentioned in 54f7727c" + # + # See cross_reference_note_content. + # + # Returns the created Note object + def self.cross_reference(noteable, mentioner, author) + return if cross_reference_disallowed?(noteable, mentioner) + + gfm_reference = mentioner_gfm_ref(noteable, mentioner) + + note_options = { + project: noteable.project, + author: author, + note: cross_reference_note_content(gfm_reference) + } + + if noteable.kind_of?(Commit) + note_options.merge!(noteable_type: 'Commit', commit_id: noteable.id) + else + note_options.merge!(noteable: noteable) + end + + create_note(note_options) + end + + def self.cross_reference?(note_text) + note_text.start_with?(cross_reference_note_prefix) + end + + # Check if a cross-reference is disallowed + # + # This method prevents adding a "mentioned in !1" note on every single commit + # in a merge request. + # + # noteable - Noteable object being referenced + # mentioner - Mentionable object + # + # Returns Boolean + def self.cross_reference_disallowed?(noteable, mentioner) + return false unless MergeRequest === mentioner + return false unless Commit === noteable + + mentioner.commits.include?(noteable) + end + + def self.cross_reference_exists?(noteable, mentioner) + # Initial scope should be system notes of this noteable type + notes = Note.system.where(noteable_type: noteable.class) + + if noteable.is_a?(Commit) + # Commits have non-integer IDs, so they're stored in `commit_id` + notes = notes.where(commit_id: noteable.id) + else + notes = notes.where(noteable_id: noteable.id) + end + + gfm_reference = mentioner_gfm_ref(noteable, mentioner, true) + notes = notes.where(note: cross_reference_note_content(gfm_reference)) + + notes.count > 0 + end + + private + + def self.create_note(args = {}) + Note.create(args.merge(system: true)) + end + + # Prepend the mentioner's namespaced project path to the GFM reference for + # cross-project references. For same-project references, return the + # unmodified GFM reference. + def self.mentioner_gfm_ref(noteable, mentioner, cross_reference = false) + # FIXME (rspeicher): This was breaking things. + # if mentioner.is_a?(Commit) && cross_reference + # return mentioner.gfm_reference.sub('commit ', 'commit %') + # end + + full_gfm_reference(mentioner.project, noteable.project, mentioner) + end + + # Return the +mentioner+ GFM reference. If the mentioner and noteable + # projects are not the same, add the mentioning project's path to the + # returned value. + def self.full_gfm_reference(mentioning_project, noteable_project, mentioner) + if mentioning_project == noteable_project + mentioner.gfm_reference + else + if mentioner.is_a?(Commit) + mentioner.gfm_reference.sub( + /(commit )/, + "\\1#{mentioning_project.path_with_namespace}@" + ) + else + mentioner.gfm_reference.sub( + /(issue |merge request )/, + "\\1#{mentioning_project.path_with_namespace}" + ) + end + end + end + + def self.cross_reference_note_prefix + 'mentioned in ' + end + + def self.cross_reference_note_content(gfm_reference) + "#{cross_reference_note_prefix}#{gfm_reference}" + end + + # Build an Array of lines detailing each commit added in a merge request + # + # new_commits - Array of new Commit objects + # + # Returns an Array of Strings + def self.new_commit_summary(new_commits) + new_commits.collect do |commit| + "* #{commit.short_id} - #{commit.title}" + end + end + + # Build a single line summarizing existing commits being added in a merge + # request + # + # noteable - MergeRequest object + # existing_commits - Array of existing Commit objects + # oldrev - Optional String SHA of ... TODO (rspeicher): I have no idea what this actually does. + # + # Examples: + # + # "* ea0f8418...2f4426b7 - 24 commits from branch `master`" + # + # "* ea0f8418..4188f0ea - 15 commits from branch `fork:master`" + # + # "* ea0f8418 - 1 commit from branch `feature`" + # + # Returns a newline-terminated String + def self.existing_commit_summary(noteable, existing_commits, oldrev = nil) + return '' if existing_commits.empty? + + count = existing_commits.size + + commit_ids = if count == 1 + existing_commits.first.short_id + else + if oldrev + "#{Commit.truncate_sha(oldrev)}...#{existing_commits.last.short_id}" + else + "#{existing_commits.first.short_id}..#{existing_commits.last.short_id}" + end + end + + commits_text = "#{count} commit".pluralize(count) + + branch = noteable.target_branch + branch = "#{noteable.target_project_namespace}:#{branch}" if noteable.for_fork? + + "* #{commit_ids} - #{commits_text} from branch `#{branch}`\n" + end +end diff --git a/app/services/test_hook_service.rb b/app/services/test_hook_service.rb index 21ec2c01cb8..e85e58751e7 100644 --- a/app/services/test_hook_service.rb +++ b/app/services/test_hook_service.rb @@ -1,6 +1,6 @@ class TestHookService def execute(hook, current_user) data = Gitlab::PushDataBuilder.build_sample(hook.project, current_user) - hook.execute(data) + hook.execute(data, 'push_hooks') end end diff --git a/app/views/devise/sessions/two_factor.html.haml b/app/views/devise/sessions/two_factor.html.haml new file mode 100644 index 00000000000..22b2c1a186b --- /dev/null +++ b/app/views/devise/sessions/two_factor.html.haml @@ -0,0 +1,10 @@ +%div + .login-box + .login-heading + %h3 Two-factor Authentication + .login-body + = form_for(resource, as: resource_name, url: session_path(resource_name), method: :post) do |f| + = f.text_field :otp_attempt, class: 'form-control', placeholder: 'Two-factor authentication code', required: true, autofocus: true + %p.help-block.hint If you've lost your phone, you may enter one of your recovery codes. + .prepend-top-20 + = f.submit "Verify code", class: "btn btn-save" diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index 60d7978b13f..1da702be384 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -17,15 +17,27 @@ - few_commits.each do |commit| = render "events/commit", commit: commit, project: project + - create_mr = current_user == event.author && event.new_ref? && create_mr_button?(event.project.default_branch, event.ref_name, event.project) - if event.commits_count > 1 %li.commits-stat - if event.commits_count > 2 %span ... and #{event.commits_count - 2} more commits. + - if event.md_ref? - from = event.commit_from - from_label = truncate_sha(from) - else - from = event.project.default_branch - from_label = from + = link_to namespace_project_compare_path(event.project.namespace, event.project, from: from, to: event.commit_to) do - %strong Compare → #{from_label}...#{truncate_sha(event.commit_to)} + Compare #{from_label}...#{truncate_sha(event.commit_to)} + + - if create_mr + or + = link_to create_mr_path(event.project.default_branch, event.ref_name, event.project) do + create a merge request + - elsif create_mr + %li.commits-stat + = link_to create_mr_path(event.project.default_branch, event.ref_name, event.project) do + Create Merge Request diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index 31d8ed3ed86..ac37fd4c1c1 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -4,7 +4,7 @@ = icon('user fw') %span Profile - = nav_link(controller: :accounts) do + = nav_link(controller: [:accounts, :two_factor_auths]) do = link_to profile_account_path, title: 'Account', data: {placement: 'right'} do = icon('gear fw') %span diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 1c3a3d68aca..6ac60b01f85 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -26,6 +26,33 @@ %span You don`t have one yet. Click generate to fix it. = f.submit 'Generate', class: "btn success btn-build-token" + - unless current_user.ldap_user? + %fieldset + - if current_user.otp_required_for_login + %legend.text-success + = icon('check') + Two-factor Authentication enabled + %div + .pull-right + = link_to 'Disable Two-factor Authentication', profile_two_factor_auth_path, method: :delete, class: 'btn btn-close btn-sm', + data: { confirm: 'Are you sure?' } + %p + If you lose your recovery codes you can + %strong + = succeed ',' do + = link_to 'generate new ones', codes_profile_two_factor_auth_path, method: :post, data: { confirm: 'Are you sure?' } + invalidating all previous codes. + + - else + %legend Two-factor Authentication + %div + %p + Increase your account's security by enabling two-factor authentication (2FA). + %p + Each time you log in you’ll be required to provide your username and + password as usual, plus a randomly-generated code from your phone. + %div + = link_to 'Enable Two-factor Authentication', new_profile_two_factor_auth_path, class: 'btn btn-success' - if show_profile_social_tab? %fieldset @@ -38,7 +65,7 @@ class: "btn btn-lg #{'active' if oauth_active?(provider)}" - if oauth_active?(provider) = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'btn btn-lg' do - %i.fa.fa-close + = icon('close') - if show_profile_username_tab? %fieldset.update-username @@ -52,7 +79,7 @@ .loading-gif.hide %p - %i.fa.fa-spinner.fa-spin + = icon('spinner spin') Saving new username %p.light = user_url(@user) diff --git a/app/views/profiles/two_factor_auths/_codes.html.haml b/app/views/profiles/two_factor_auths/_codes.html.haml new file mode 100644 index 00000000000..1b1395eaa17 --- /dev/null +++ b/app/views/profiles/two_factor_auths/_codes.html.haml @@ -0,0 +1,11 @@ +%p.slead + Should you ever lose your phone, each of these recovery codes can be used one + time each to regain access to your account. Please save them in a safe place. + +.codes.well + %ul + - @codes.each do |code| + %li + %span.monospace= code + += link_to 'Proceed', profile_account_path, class: 'btn btn-success' diff --git a/app/views/profiles/two_factor_auths/codes.html.haml b/app/views/profiles/two_factor_auths/codes.html.haml new file mode 100644 index 00000000000..addf356697a --- /dev/null +++ b/app/views/profiles/two_factor_auths/codes.html.haml @@ -0,0 +1,5 @@ +- page_title 'Recovery Codes', 'Two-factor Authentication' + +%h3.page-title Two-factor Authentication Recovery codes +%hr += render 'codes' diff --git a/app/views/profiles/two_factor_auths/create.html.haml b/app/views/profiles/two_factor_auths/create.html.haml new file mode 100644 index 00000000000..e330aadac13 --- /dev/null +++ b/app/views/profiles/two_factor_auths/create.html.haml @@ -0,0 +1,6 @@ +- page_title 'Two-factor Authentication', 'Account' + +.alert.alert-success + Congratulations! You have enabled Two-factor Authentication! + += render 'codes' diff --git a/app/views/profiles/two_factor_auths/new.html.haml b/app/views/profiles/two_factor_auths/new.html.haml new file mode 100644 index 00000000000..fe03a259a12 --- /dev/null +++ b/app/views/profiles/two_factor_auths/new.html.haml @@ -0,0 +1,23 @@ +- page_title 'Two-factor Authentication', 'Account' + +%h2.page-title Two-Factor Authentication (2FA) +%p + Download the Google Authenticator application from App Store for iOS or + Google Play for Android and scan this code. + +%hr + += form_tag profile_two_factor_auth_path, method: :post, class: 'form-horizontal' do |f| + - if @error + .alert.alert-danger + = @error + .form-group + .col-sm-2 + .col-sm-10 + = raw @qr_code + .form-group + = label_tag :pin_code, nil, class: "control-label" + .col-sm-10 + = text_field_tag :pin_code, nil, class: "form-control", required: true, autofocus: true + .form-actions + = submit_tag 'Submit', class: 'btn btn-success' diff --git a/app/views/projects/_aside.html.haml b/app/views/projects/_aside.html.haml index 1865b5be8c6..c2f56996ba8 100644 --- a/app/views/projects/_aside.html.haml +++ b/app/views/projects/_aside.html.haml @@ -1,83 +1,102 @@ .clearfix - .append-bottom-20 - = render "shared/clone_panel" - - unless @project.empty_repo? .well - %h4 Repository - %ul.nav.nav-pills - %li= link_to pluralize(number_with_delimiter(@repository.commit_count), 'commit'), namespace_project_commits_path(@project.namespace, @project, @ref || @repository.root_ref) - %li= link_to pluralize(number_with_delimiter(@repository.branch_names.count), 'branch'), namespace_project_branches_path(@project.namespace, @project) - %li= link_to pluralize(number_with_delimiter(@repository.tag_names.count), 'tag'), namespace_project_tags_path(@project.namespace, @project) + %h4.visibility-level-label + = visibility_level_icon(@project.visibility_level) + = "#{visibility_level_label(@project.visibility_level).capitalize} project" - .actions - = link_to namespace_project_compare_index_path(@project.namespace, @project, from: @repository.root_ref, to: @ref || @repository.root_ref), class: 'btn btn-sm' do - %i.fa.fa-exchange - Compare code + - if @repository.changelog || @repository.license || @repository.contribution_guide + %ul.nav.nav-pills + - if @repository.changelog + %li.hidden-xs + = link_to changelog_url(@project) do + = icon("list-alt fw") + Changelog + - if @repository.license + %li + = link_to license_url(@project) do + = icon("check-circle-o fw") + License + - if @repository.contribution_guide + %li + = link_to contribution_guide_url(@project) do + = icon("info-circle fw") + Contribution guide - - if can?(current_user, :download_code, @project) - - = render 'projects/repositories/download_archive', split_button: true, btn_class: 'btn-group-sm' - - - unless @project.empty_repo? - .well - %h4 Contribute - %ul.nav.nav-pills - - if @repository.changelog - %li.hidden-xs - = link_to changelog_url(@project) do - Changelog - - if @repository.contribution_guide - %li.hidden-xs - = link_to contribution_guide_url(@project) do - Contribution guide - - if @repository.license - %li - = link_to license_url(@project) do - License .actions - = link_to url_for_new_issue(@project, only_path: true), title: "New Issue", class: 'btn btn-sm' do - %i.fa.fa-fw.fa-exclamation-circle - New issue + - if can? current_user, :write_issue, @project + = link_to url_for_new_issue(@project, only_path: true), title: "New Issue", class: 'btn btn-sm append-right-10' do + = icon("exclamation-circle fw") + New Issue + - if can? current_user, :write_merge_request, @project - = link_to new_namespace_project_merge_request_path(@project.namespace, @project), class: "btn btn-sm", title: "New Merge Request" do - %i.fa.fa-plus + = icon("plus fw") New Merge Request - - - - if @project.archived? - .alert.alert-warning - %h4 - %i.fa.fa-exclamation-triangle - Archived project! - %p Repository is read-only - - - if @project.forked_from_project + - if forked_from_project = @project.forked_from_project .well %h4 + = icon("code-fork fw") Forked from .pull-right - = link_to @project.forked_from_project.namespace.try(:name), project_path(@project.forked_from_project) + = link_to forked_from_project.namespace.try(:name), project_path(forked_from_project) + - if version = @repository.version + .well + %h4 + = icon("clock-o fw") + Version + .pull-right + = link_to version_url(@project) do + = @repository.blob_by_oid(version.id).data -- if version = @repository.version - .well - %h4 - Version - .pull-right - = link_to version_url(@project) do - = @repository.blob_by_oid(version.id).data + - @project.ci_services.each do |ci_service| + - if ci_service.active? && ci_service.respond_to?(:builds_path) + .well + %h4 + = icon("check fw") + = ci_service.title + .pull-right + - if ci_service.respond_to?(:status_img_path) + = link_to ci_service.builds_path, :'data-no-turbolink' => 'data-no-turbolink' do + = image_tag ci_service.status_img_path, alt: "build status" + - else + = link_to 'view builds', ci_service.builds_path, :'data-no-turbolink' => 'data-no-turbolink' -- @project.ci_services.each do |ci_service| - - if ci_service.active? && ci_service.respond_to?(:builds_path) + - unless @project.empty_repo? .well %h4 - = ci_service.title - .pull-right - - if ci_service.respond_to?(:status_img_path) - = link_to ci_service.builds_path, :'data-no-turbolink' => 'data-no-turbolink' do - = image_tag ci_service.status_img_path, alt: "build status" - - else - = link_to 'view builds', ci_service.builds_path, :'data-no-turbolink' => 'data-no-turbolink' + = icon("archive fw") + Repository + + %ul.nav.nav-pills + %li + = link_to namespace_project_commits_path(@project.namespace, @project, @ref || @repository.root_ref) do + = icon("history fw") + = pluralize(number_with_delimiter(@repository.commit_count), 'commit') + %li + = link_to namespace_project_branches_path(@project.namespace, @project) do + = icon("code-fork fw") + = pluralize(number_with_delimiter(@repository.branch_names.count), 'branch') + %li + = link_to namespace_project_tags_path(@project.namespace, @project) do + = icon("tags fw") + = pluralize(number_with_delimiter(@repository.tag_names.count), 'tag') + + .actions + = link_to namespace_project_compare_index_path(@project.namespace, @project, from: @repository.root_ref, to: @ref || @repository.root_ref), class: 'btn btn-sm append-right-10' do + %i.fa.fa-exchange + Compare code + + - if can?(current_user, :download_code, @project) + = render 'projects/repositories/download_archive', split_button: true, btn_class: 'btn-group-sm' + + = render "shared/clone_panel" + + - if @project.archived? + .alert.alert-warning + %h4 + = icon("exclamation-triangle fw") + Archived project! + %p Repository is read-only diff --git a/app/views/projects/_section.html.haml b/app/views/projects/_section.html.haml index 0b7f4cb780a..f4f876f3809 100644 --- a/app/views/projects/_section.html.haml +++ b/app/views/projects/_section.html.haml @@ -1,10 +1,12 @@ %ul.nav.nav-tabs %li.active = link_to '#tab-activity', 'data-toggle' => 'tab' do + = icon("tachometer") Activity - if @repository.readme %li = link_to '#tab-readme', 'data-toggle' => 'tab' do + = icon("file-text-o") Readme .tab-content .tab-pane.active#tab-activity diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 4e7415be4aa..43412624da6 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -10,16 +10,19 @@ %i.fa.fa-lock protected .pull-right - - if can?(current_user, :download_code, @project) - = render 'projects/repositories/download_archive', ref: branch.name, btn_class: 'btn-grouped btn-group-xs' + - if create_mr_button?(@repository.root_ref, branch.name) + = link_to create_mr_path(@repository.root_ref, branch.name), class: 'btn btn-grouped btn-xs' do + = icon('plus') + Merge Request + - if branch.name != @repository.root_ref = link_to namespace_project_compare_index_path(@project.namespace, @project, from: @repository.root_ref, to: branch.name), class: 'btn btn-grouped btn-xs', method: :post, title: "Compare" do - %i.fa.fa-files-o + = icon("exchange") Compare - if can_remove_branch?(@project, branch.name) = link_to namespace_project_branch_path(@project.namespace, @project, branch.name), class: 'btn btn-grouped btn-xs btn-remove remove-row', method: :delete, data: { confirm: 'Removed branch cannot be restored. Are you sure?'}, remote: true do - %i.fa.fa-trash-o + = icon("trash-o") - if commit %ul.list-unstyled diff --git a/app/views/projects/commits/_head.html.haml b/app/views/projects/commits/_head.html.haml index a714f5f79e0..66261c7336d 100644 --- a/app/views/projects/commits/_head.html.haml +++ b/app/views/projects/commits/_head.html.haml @@ -1,17 +1,22 @@ %ul.nav.nav-tabs = nav_link(controller: [:commit, :commits]) do - = link_to namespace_project_commits_path(@project.namespace, @project, @repository.root_ref) do + = link_to namespace_project_commits_path(@project.namespace, @project, @ref || @repository.root_ref) do + = icon("history") Commits %span.badge= number_with_precision(@repository.commit_count, precision: 0, delimiter: ',') = nav_link(controller: :compare) do - = link_to 'Compare', namespace_project_compare_index_path(@project.namespace, @project, from: @repository.root_ref, to: @ref || @repository.root_ref) + = link_to namespace_project_compare_index_path(@project.namespace, @project, from: @repository.root_ref, to: @ref || @repository.root_ref) do + = icon("exchange") + Compare = nav_link(html_options: {class: branches_tab_class}) do = link_to namespace_project_branches_path(@project.namespace, @project) do + = icon("code-fork") Branches %span.badge.js-totalbranch-count= @repository.branches.size = nav_link(controller: :tags) do = link_to namespace_project_tags_path(@project.namespace, @project) do + = icon("tags") Tags %span.badge.js-totaltags-count= @repository.tags.length diff --git a/app/views/projects/commits/show.html.haml b/app/views/projects/commits/show.html.haml index c8531b090a6..9682100a54c 100644 --- a/app/views/projects/commits/show.html.haml +++ b/app/views/projects/commits/show.html.haml @@ -8,11 +8,17 @@ .tree-ref-holder = render 'shared/ref_switcher', destination: 'commits' -- if current_user && current_user.private_token - .commits-feed-holder.hidden-xs.hidden-sm - = link_to namespace_project_commits_path(@project.namespace, @project, @ref, {format: :atom, private_token: current_user.private_token}), title: "Feed", class: 'btn' do - %i.fa.fa-rss - Commits feed +.commits-feed-holder.hidden-xs.hidden-sm + - if create_mr_button?(@repository.root_ref, @ref) + = link_to create_mr_path(@repository.root_ref, @ref), class: 'btn btn-success' do + = icon('plus') + Create Merge Request + + - if current_user && current_user.private_token + = link_to namespace_project_commits_path(@project.namespace, @project, @ref, {format: :atom, private_token: current_user.private_token}), title: "Feed", class: 'prepend-left-10 btn' do + = icon("rss") + Commits Feed + %ul.breadcrumb.repo-breadcrumb = commits_breadcrumbs diff --git a/app/views/projects/compare/_form.html.haml b/app/views/projects/compare/_form.html.haml index dfb1dded9ea..a0e904cfd8b 100644 --- a/app/views/projects/compare/_form.html.haml +++ b/app/views/projects/compare/_form.html.haml @@ -13,9 +13,10 @@ = text_field_tag :to, params[:to], class: "form-control" = button_tag "Compare", class: "btn btn-create commits-compare-btn" - - if compare_to_mr_button? - = link_to compare_mr_path, class: 'prepend-left-10 btn' do - %strong Make a merge request + - if create_mr_button? + = link_to create_mr_path, class: 'prepend-left-10 btn' do + = icon("plus") + Create Merge Request :javascript diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index b49aee504fe..ec8974c5475 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -5,11 +5,13 @@ = parallel_diff_btn = render 'projects/diffs/stats', diffs: diffs -- if show_diff_size_warning?(diffs) - = render 'projects/diffs/warning', diffs: diffs +- diff_files = safe_diff_files(diffs) + +- if diff_files.count < diffs.size + = render 'projects/diffs/warning', diffs: diffs, shown_files_count: diff_files.count .files - - safe_diff_files(diffs).each_with_index do |diff_file, index| + - diff_files.each_with_index do |diff_file, index| = render 'projects/diffs/file', diff_file: diff_file, i: index, project: project - if @diff_timeout diff --git a/app/views/projects/diffs/_warning.html.haml b/app/views/projects/diffs/_warning.html.haml index 47abbba2eb2..bd0b7376ba7 100644 --- a/app/views/projects/diffs/_warning.html.haml +++ b/app/views/projects/diffs/_warning.html.haml @@ -14,6 +14,6 @@ = link_to "Email patch", merge_request_path(@merge_request, format: :patch), class: "btn btn-warning btn-sm" %p To preserve performance only - %strong #{allowed_diff_size} of #{diffs.size} + %strong #{shown_files_count} of #{diffs.size} files are displayed. diff --git a/app/views/projects/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml index ceca2653016..2016f5c709c 100644 --- a/app/views/projects/issues/_discussion.html.haml +++ b/app/views/projects/issues/_discussion.html.haml @@ -1,9 +1,9 @@ - content_for :note_actions do - if can?(current_user, :modify_issue, @issue) - if @issue.closed? - = link_to 'Reopen Issue', issue_path(@issue, issue: {state_event: :reopen }, status_only: true), method: :put, class: "btn btn-grouped btn-reopen js-note-target-reopen", title: 'Reopen Issue' + = link_to 'Reopen Issue', issue_path(@issue, issue: {state_event: :reopen}, status_only: true), method: :put, class: 'btn btn-grouped btn-reopen js-note-target-reopen', title: 'Reopen Issue' - else - = link_to 'Close Issue', issue_path(@issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn btn-grouped btn-close js-note-target-close", title: "Close Issue" + = link_to 'Close Issue', issue_path(@issue, issue: {state_event: :close}, status_only: true), method: :put, class: 'btn btn-grouped btn-close js-note-target-close', title: 'Close Issue' = render 'shared/show_aside' @@ -15,11 +15,11 @@ %span= pluralize(@issue.participants(current_user).count, 'participant') - @issue.participants(current_user).each do |participant| = link_to_member(@project, participant, name: false, size: 24) - .voting_notes#notes= render "projects/notes/notes_with_form" + .voting_notes#notes= render 'projects/notes/notes_with_form' %aside.col-md-3 .issuable-affix .clearfix - %span.slead.has_tooltip{:"data-original-title" => 'Cross-project reference'} + %span.slead.has_tooltip{title: 'Cross-project reference'} = cross_project_reference(@project, @issue) %hr .context diff --git a/app/views/projects/issues/_issue_context.html.haml b/app/views/projects/issues/_issue_context.html.haml index 7cadb9983fa..323f5c84a85 100644 --- a/app/views/projects/issues/_issue_context.html.haml +++ b/app/views/projects/issues/_issue_context.html.haml @@ -1,4 +1,4 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @issue], remote: true, html: {class: 'edit-issue inline-update'} do |f| += form_for [@project.namespace.becomes(Namespace), @project, @issue], remote: true, html: {class: 'edit-issue inline-update js-issue-update'} do |f| %div.prepend-top-20 .issuable-context-title %label diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 81478dfe568..ee1b2a08bc4 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -13,17 +13,17 @@ .pull-right - if can?(current_user, :write_issue, @project) - = link_to new_namespace_project_issue_path(@project.namespace, @project), class: "btn btn-grouped new-issue-link", title: "New Issue", id: "new_issue_link" do - %i.fa.fa-plus + = link_to new_namespace_project_issue_path(@project.namespace, @project), class: 'btn btn-grouped new-issue-link', title: 'New Issue', id: 'new_issue_link' do + = icon('plus') New Issue - if can?(current_user, :modify_issue, @issue) - if @issue.closed? - = link_to 'Reopen', issue_path(@issue, issue: {state_event: :reopen }, status_only: true), method: :put, class: "btn btn-grouped btn-reopen" + = link_to 'Reopen', issue_path(@issue, issue: {state_event: :reopen}, status_only: true), method: :put, class: 'btn btn-grouped btn-reopen' - else - = link_to 'Close', issue_path(@issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn btn-grouped btn-close", title: "Close Issue" + = link_to 'Close', issue_path(@issue, issue: {state_event: :close}, status_only: true), method: :put, class: 'btn btn-grouped btn-close', title: 'Close Issue' - = link_to edit_namespace_project_issue_path(@project.namespace, @project, @issue), class: "btn btn-grouped issuable-edit" do - %i.fa.fa-pencil-square-o + = link_to edit_namespace_project_issue_path(@project.namespace, @project, @issue), class: 'btn btn-grouped issuable-edit' do + = icon('pencil-square-o') Edit %hr @@ -31,11 +31,13 @@ = gfm escape_once(@issue.title) %div - if @issue.description.present? - .description + .description{class: can?(current_user, :modify_issue, @issue) ? 'js-task-list-container' : ''} .wiki = preserve do - = markdown(@issue.description, parse_tasks: true) + = markdown(@issue.description) + %textarea.hidden.js-task-list-field + = @issue.description %hr .issue-discussion - = render "projects/issues/discussion" + = render 'projects/issues/discussion' diff --git a/app/views/projects/merge_requests/show/_context.html.haml b/app/views/projects/merge_requests/show/_context.html.haml index eb80391ebcd..a5a821c1847 100644 --- a/app/views/projects/merge_requests/show/_context.html.haml +++ b/app/views/projects/merge_requests/show/_context.html.haml @@ -1,4 +1,4 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @merge_request], remote: true, html: {class: 'edit-merge_request inline-update'} do |f| += form_for [@project.namespace.becomes(Namespace), @project, @merge_request], remote: true, html: {class: 'edit-merge_request inline-update js-merge-request-update'} do |f| %div.prepend-top-20 .issuable-context-title %label @@ -19,13 +19,13 @@ %span.back-to-milestone = link_to namespace_project_milestone_path(@project.namespace, @project, @merge_request.milestone) do %strong - %i.fa.fa-clock-o + = icon('clock-o') = @merge_request.milestone.title - else none .issuable-context-selectbox - if can?(current_user, :modify_merge_request, @merge_request) - = f.select(:milestone_id, milestone_options(@merge_request), { include_blank: "Select milestone" }, {class: 'select2 select2-compact js-select2 js-milestone'}) + = f.select(:milestone_id, milestone_options(@merge_request), { include_blank: 'Select milestone' }, {class: 'select2 select2-compact js-select2 js-milestone'}) = hidden_field_tag :merge_request_context = f.submit class: 'btn' @@ -35,13 +35,13 @@ %label Subscription: %button.btn.btn-block.subscribe-button{:type => 'button'} - %i.fa.fa-eye - %span= @merge_request.subscribed?(current_user) ? "Unsubscribe" : "Subscribe" - - subscribtion_status = @merge_request.subscribed?(current_user) ? "subscribed" : "unsubscribed" - .subscription-status{"data-status" => subscribtion_status} - .description-block.unsubscribed{class: ( "hidden" if @merge_request.subscribed?(current_user) )} + = icon('eye') + %span= @merge_request.subscribed?(current_user) ? 'Unsubscribe' : 'Subscribe' + - subscribtion_status = @merge_request.subscribed?(current_user) ? 'subscribed' : 'unsubscribed' + .subscription-status{data: {status: subscribtion_status}} + .description-block.unsubscribed{class: ( 'hidden' if @merge_request.subscribed?(current_user) )} You're not receiving notifications from this thread. - .description-block.subscribed{class: ( "hidden" unless @merge_request.subscribed?(current_user) )} + .description-block.subscribed{class: ( 'hidden' unless @merge_request.subscribed?(current_user) )} You're receiving notifications because you're subscribed to this thread. :coffeescript diff --git a/app/views/projects/merge_requests/show/_mr_box.html.haml b/app/views/projects/merge_requests/show/_mr_box.html.haml index ada9ae58b8f..b3470ba37d6 100644 --- a/app/views/projects/merge_requests/show/_mr_box.html.haml +++ b/app/views/projects/merge_requests/show/_mr_box.html.haml @@ -3,7 +3,9 @@ %div - if @merge_request.description.present? - .description + .description{class: can?(current_user, :modify_merge_request, @merge_request) ? 'js-task-list-container' : ''} .wiki = preserve do - = markdown(@merge_request.description, parse_tasks: true) + = markdown(@merge_request.description) + %textarea.hidden.js-task-list-field + = @merge_request.description diff --git a/app/views/projects/notes/_edit_form.html.haml b/app/views/projects/notes/_edit_form.html.haml index acb3991d294..a663950f031 100644 --- a/app/views/projects/notes/_edit_form.html.haml +++ b/app/views/projects/notes/_edit_form.html.haml @@ -1,15 +1,14 @@ .note-edit-form = form_for note, url: namespace_project_note_path(@project.namespace, @project, note), method: :put, remote: true, authenticity_token: true do |f| = note_target_fields(note) - = render layout: 'projects/md_preview', locals: { preview_class: "note-text" } do - = render 'projects/zen', f: f, attr: :note, - classes: 'note_text js-note-text' + = render layout: 'projects/md_preview', locals: { preview_class: 'note-text' } do + = render 'projects/zen', f: f, attr: :note, classes: 'note_text js-note-text js-task-list-field' .comment-hints.clearfix - .pull-left Comments are parsed with #{link_to "GitLab Flavored Markdown", help_page_path("markdown", "markdown"),{ target: '_blank', tabindex: -1 }} - .pull-right Attach files by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector', tabindex: -1 }. + .pull-left Comments are parsed with #{link_to 'GitLab Flavored Markdown', help_page_path('markdown', 'markdown'),{ target: '_blank', tabindex: -1 }} + .pull-right Attach files by dragging & dropping or #{link_to 'selecting them', '#', class: 'markdown-selector', tabindex: -1 }. .note-form-actions .buttons - = f.submit 'Save Comment', class: "btn btn-primary btn-save btn-grouped js-comment-button" - = link_to 'Cancel', "#", class: "btn btn-cancel note-edit-cancel"
\ No newline at end of file + = f.submit 'Save Comment', class: 'btn btn-primary btn-save btn-grouped js-comment-button' + = link_to 'Cancel', '#', class: 'btn btn-cancel note-edit-cancel' diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 0728f8fa42b..2f3c407d6e9 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -2,29 +2,37 @@ .timeline-entry-inner .timeline-icon - if note.system - %span.fa.fa-circle + %span= icon('circle') - else = link_to user_path(note.author) do - = image_tag avatar_icon(note.author_email), class: "avatar s40", alt: '' + = image_tag avatar_icon(note.author_email), class: 'avatar s40', alt: '' .timeline-content .note-header .note-actions - = link_to "##{dom_id(note)}", name: dom_id(note) do - %i.fa.fa-link + = link_to "##{dom_id(note)}", name: dom_id(note), title: "Link here" do + = icon('link fw') Link here - - - if can?(current_user, :admin_note, note) && note.editable? - = link_to "#", title: "Edit comment", class: "js-note-edit" do - %i.fa.fa-pencil-square-o + + - if note_editable?(note) + = link_to '#', title: 'Edit comment', class: 'js-note-edit' do + = icon('pencil-square-o fw') Edit - - = link_to namespace_project_note_path(@project.namespace, @project, note), title: "Remove comment", method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: "danger js-note-delete" do - %i.fa.fa-trash-o.cred + + = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'danger js-note-delete' do + = icon('trash-o fw', class: 'cred') Remove + + - unless note.system + - member = note.project.team.find_member(note.author.id) + - if member + %span.note-role.label + = member.human_access + - if note.system = link_to user_path(note.author) do - = image_tag avatar_icon(note.author_email), class: "avatar s16", alt: '' - = link_to_member(@project, note.author, avatar: false) + = image_tag avatar_icon(note.author_email), class: 'avatar s16', alt: '' + + = link_to_member(note.project, note.author, avatar: false) %span.author-username = '@' + note.author.username %span.note-last-update @@ -33,24 +41,24 @@ - if note.superceded?(@notes) - if note.upvote? %span.vote.upvote.label.label-gray.strikethrough - %i.fa.fa-thumbs-up + = icon('thumbs-up') \+1 - if note.downvote? %span.vote.downvote.label.label-gray.strikethrough - %i.fa.fa-thumbs-down + = icon('thumbs-down') \-1 - else - if note.upvote? %span.vote.upvote.label.label-success - %i.fa.fa-thumbs-up + = icon('thumbs-up') \+1 - if note.downvote? %span.vote.downvote.label.label-danger - %i.fa.fa-thumbs-down + = icon('thumbs-down') \-1 - .note-body + .note-body{class: note_editable?(note) ? 'js-task-list-container' : ''} .note-text = preserve do = markdown(note.note, {no_header_anchors: true}) @@ -62,10 +70,10 @@ = link_to note.attachment.url, target: '_blank' do = image_tag note.attachment.url, class: 'note-image-attach' .attachment - = link_to note.attachment.url, target: "_blank" do - %i.fa.fa-paperclip + = link_to note.attachment.url, target: '_blank' do + = icon('paperclip') = note.attachment_identifier - = link_to delete_attachment_namespace_project_note_path(@project.namespace, @project, note), - title: "Delete this attachment", method: :delete, remote: true, data: { confirm: 'Are you sure you want to remove the attachment?' }, class: "danger js-note-attachment-delete" do - %i.fa.fa-trash-o.cred + = link_to delete_attachment_namespace_project_note_path(note.project.namespace, note.project, note), + title: 'Delete this attachment', method: :delete, remote: true, data: { confirm: 'Are you sure you want to remove the attachment?' }, class: 'danger js-note-attachment-delete' do + = icon('trash-o', class: 'cred') .clear diff --git a/app/views/projects/tree/show.html.haml b/app/views/projects/tree/show.html.haml index 72916cad182..04590f65b27 100644 --- a/app/views/projects/tree/show.html.haml +++ b/app/views/projects/tree/show.html.haml @@ -8,7 +8,7 @@ - if can? current_user, :download_code, @project .tree-download-holder - = render 'projects/repositories/download_archive', ref: @ref, btn_class: 'btn-group-sm pull-right hidden-xs hidden-sm', split_button: true + = render 'projects/repositories/download_archive', ref: @ref, btn_class: 'btn-group pull-right hidden-xs hidden-sm', split_button: true #tree-holder.tree-holder.clearfix = render "tree", tree: @tree diff --git a/app/views/shared/_clone_panel.html.haml b/app/views/shared/_clone_panel.html.haml index aaab0d35f59..3f489a04e71 100644 --- a/app/views/shared/_clone_panel.html.haml +++ b/app/views/shared/_clone_panel.html.haml @@ -1,24 +1,24 @@ - project = project || @project .git-clone-holder.input-group - .input-group-btn - %button{ | - :type => 'button', | - class: "btn btn-sm #{ 'active' if default_clone_protocol == 'ssh' }#{ ' has_tooltip' if current_user && current_user.require_ssh_key? }", | - :"data-clone" => project.ssh_url_to_repo, | - :"data-title" => "Add an SSH key to your profile<br> to pull or push via SSH", - :"data-html" => "true", - :"data-container" => "body"} - SSH - %button{ | - :type => 'button', | - class: "btn btn-sm #{ 'active' if default_clone_protocol == 'http' }#{ ' has_tooltip' if current_user && current_user.require_password? }", | - :"data-clone" => project.http_url_to_repo, | - :"data-title" => "Set a password on your account<br> to pull or push via #{gitlab_config.protocol.upcase}", - :"data-html" => "true", - :"data-container" => "body"} - = gitlab_config.protocol.upcase + .input-group-addon.git-protocols + .input-group-btn + %button{ | + class: "btn btn-sm #{ 'active' if default_clone_protocol == 'ssh' }#{ ' has_tooltip' if current_user && current_user.require_ssh_key? }", | + :"data-clone" => project.ssh_url_to_repo, | + :"data-title" => "Add an SSH key to your profile<br> to pull or push via SSH", + :"data-html" => "true", + :"data-container" => "body"} + SSH + .input-group-btn + %button{ | + class: "btn btn-sm #{ 'active' if default_clone_protocol == 'http' }#{ ' has_tooltip' if current_user && current_user.require_password? }", | + :"data-clone" => project.http_url_to_repo, | + :"data-title" => "Set a password on your account<br> to pull or push via #{gitlab_config.protocol.upcase}", + :"data-html" => "true", + :"data-container" => "body"} + = gitlab_config.protocol.upcase = text_field_tag :project_clone, default_url_to_repo(project), class: "js-select-on-focus form-control input-sm", readonly: true - - if project.kind_of?(Project) + - if project.kind_of?(Project) && project.empty_repo? .input-group-addon .visibility-level-label.has_tooltip{'data-title' => "#{visibility_level_label(project.visibility_level)} project" } = visibility_level_icon(project.visibility_level) diff --git a/app/views/shared/_issuable_filter.html.haml b/app/views/shared/_issuable_filter.html.haml index fa8b4eae314..4ab9421f013 100644 --- a/app/views/shared/_issuable_filter.html.haml +++ b/app/views/shared/_issuable_filter.html.haml @@ -51,8 +51,6 @@ = button_tag "Update issues", class: "btn update_selected_issues btn-save" :coffeescript - new UsersSelect() - $('form.filter-form').on 'submit', (event) -> event.preventDefault() Turbolinks.visit @.action + '&' + $(@).serialize() diff --git a/app/workers/project_web_hook_worker.rb b/app/workers/project_web_hook_worker.rb index 73085c046bd..fb878965288 100644 --- a/app/workers/project_web_hook_worker.rb +++ b/app/workers/project_web_hook_worker.rb @@ -3,8 +3,8 @@ class ProjectWebHookWorker sidekiq_options queue: :project_web_hook - def perform(hook_id, data) + def perform(hook_id, data, hook_name) data = data.with_indifferent_access - WebHook.find(hook_id).execute(data) + WebHook.find(hook_id).execute(data, hook_name) end end diff --git a/app/workers/system_hook_worker.rb b/app/workers/system_hook_worker.rb index 3ebc62b7e7a..a122c274763 100644 --- a/app/workers/system_hook_worker.rb +++ b/app/workers/system_hook_worker.rb @@ -3,7 +3,7 @@ class SystemHookWorker sidekiq_options queue: :system_hook - def perform(hook_id, data) - SystemHook.find(hook_id).execute data + def perform(hook_id, data, hook_name) + SystemHook.find(hook_id).execute(data, hook_name) end end diff --git a/config/application.rb b/config/application.rb index fa399533e52..7e899cc3b5b 100644 --- a/config/application.rb +++ b/config/application.rb @@ -31,7 +31,7 @@ module Gitlab config.encoding = "utf-8" # Configure sensitive parameters which will be filtered from the log file. - config.filter_parameters.push(:password, :password_confirmation, :private_token) + config.filter_parameters.push(:password, :password_confirmation, :private_token, :otp_attempt) # Enable escaping HTML in JSON. config.active_support.escape_html_entities_in_json = true diff --git a/config/gitlab.teatro.yml b/config/gitlab.teatro.yml index 07133503843..f0656400beb 100644 --- a/config/gitlab.teatro.yml +++ b/config/gitlab.teatro.yml @@ -7,6 +7,8 @@ production: &base user: root + email_from: example@example.com + support_email: support@example.com default_projects_features: @@ -17,9 +19,6 @@ production: &base snippets: false visibility_level: "private" # can be "private" | "internal" | "public" - outgoing_emails: - from: example@example.com - issues_tracker: gravatar: @@ -76,8 +75,6 @@ test: gitlab: host: localhost port: 80 - outgoing_emails: - delivery_method: :test issues_tracker: redmine: title: "Redmine" diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index f0b65e1e905..bd2081688d1 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -10,7 +10,7 @@ # How to use: # 1. Copy file as gitlab.yml # 2. Update gitlab -> host with your fully qualified domain name -# 3. Update outgoing_emails -> from +# 3. Update gitlab -> email_from # 4. If you installed Git from source, change git -> bin_path to /usr/local/bin/git # IMPORTANT: If Git was installed in a different location use that instead. # You can check with `which git`. If a wrong path of Git is specified, it will @@ -46,6 +46,16 @@ production: &base # To see all available zones, run `bundle exec rake time:zones:all RAILS_ENV=production` # time_zone: 'UTC' + ## Email settings + # Uncomment and set to false if you need to disable email sending from GitLab (default: true) + # email_enabled: true + # Email address used in the "From" field in mails sent by GitLab + email_from: example@example.com + email_display_name: GitLab + email_reply_to: noreply@example.com + + # Email server smtp settings are in config/initializers/smtp_settings.rb.sample + # default_can_create_group: false # default: true # username_changing_enabled: false # default: true - User can change her username/namespace ## Default theme @@ -79,57 +89,6 @@ production: &base # The default is 'tmp/repositories' relative to the root of the Rails app. # repository_downloads_path: tmp/repositories - ## Settings for sending emails - outgoing_emails: - - # Set to false if you need to disable email sending from GitLab (default: true). - enabled: true - - # Email address and name used in the "From" field in mails sent by GitLab. - from: example@example.com - display_name: GitLab - - # Email address used in "Reply-To" field in mails sent by GitLab. - reply_to: noreply@example.com - - # Defines a delivery method. Possible values are: - # :sendmail - send emails using sendmail command (default). - # :smtp - send emails using SMTP server. - # :letter_opener - open sent emails in browser. - # :test - save emails to ActionMailer::Base.deliveries array. - delivery_method: :sendmail - - # Settings for the :sendmail delivery method. - sendmail_settings: - # The location of the sendmail executable (default: /usr/sbin/sendmail). - location: /usr/sbin/sendmail - - # The command line arguments to be passed to sendmail (default: -i -t). - arguments: '-i -t' - - # Settings for the :smtp delivery method. - smtp_settings: - # Hostname of the SMTP server used to send emails (default: localhost). - address: localhost - - # Port of the SMTP server used to send emails (default: 25). - port: 25 - - # If your mail server requires authentication, you need to specify the - # authentication type here. Possible values are: - # :plain - send the password in the clear text. - # :login - send password Base64 encoded. - # :cram_md5 - combines a Challenge/Response mechanism to exchange information and - # a cryptographic Message Digest 5 algorithm to hash important information. - # authentication: :plain - # user_name: 'gitlab' - # password: '123456' - - # Detects if STARTTLS is enabled in your SMTP server and starts to use it. - # Set this to false if there is a problem with your server certificate - # that you cannot resolve. It should be true for Gmail SMTP. - # enable_starttls_auto: true - ## Gravatar ## For Libravatar see: http://doc.gitlab.com/ce/customization/libravatar.html gravatar: @@ -336,8 +295,6 @@ production: &base development: <<: *base - outgoing_emails: - delivery_method: :letter_opener test: <<: *base @@ -346,8 +303,6 @@ test: gitlab: host: localhost port: 80 - outgoing_emails: - delivery_method: :test # When you run tests we clone and setup gitlab-shell # In order to setup it correctly you need to specify diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index ce0023e0795..e5ac66a2323 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -105,6 +105,10 @@ Settings.gitlab['https'] = false if Settings.gitlab['https'].nil? Settings.gitlab['port'] ||= Settings.gitlab.https ? 443 : 80 Settings.gitlab['relative_url_root'] ||= ENV['RAILS_RELATIVE_URL_ROOT'] || '' Settings.gitlab['protocol'] ||= Settings.gitlab.https ? "https" : "http" +Settings.gitlab['email_enabled'] ||= true if Settings.gitlab['email_enabled'].nil? +Settings.gitlab['email_from'] ||= "gitlab@#{Settings.gitlab.host}" +Settings.gitlab['email_display_name'] ||= "GitLab" +Settings.gitlab['email_reply_to'] ||= "noreply@#{Settings.gitlab.host}" Settings.gitlab['url'] ||= Settings.send(:build_gitlab_url) Settings.gitlab['user'] ||= 'git' Settings.gitlab['user_home'] ||= begin @@ -131,26 +135,6 @@ Settings.gitlab['repository_downloads_path'] = File.absolute_path(Settings.gitla Settings.gitlab['restricted_signup_domains'] ||= [] # -# Outgoing emails -# -Settings['outgoing_emails'] ||= Settingslogic.new({}) -Settings['outgoing_emails'].tap do |opts| - # For backward compatibility. TODO remove in next major release. - opts['enabled'] ||= Settings.gitlab['email_enabled'] - opts['from'] ||= Settings.gitlab['email_from'] - opts['display_name'] ||= Settings.gitlab['display_name'] - opts['reply_to'] ||= Settings.gitlab['email_reply_to'] - - opts['enabled'] ||= opts['enabled'].nil? - opts['display_name'] ||= "GitLab" - opts['from'] ||= "gitlab@#{Settings.gitlab.host}" - opts['reply_to'] ||= "noreply@#{Settings.gitlab.host}" - opts['delivery_method'] ||= :sendmail - opts['sendmail_settings'] ||= {} - opts['smtp_settings'] ||= {} -end - -# # Gravatar # Settings['gravatar'] ||= Settingslogic.new({}) diff --git a/config/initializers/6_rack_profiler.rb b/config/initializers/6_rack_profiler.rb index bdad6202b11..38a5fa98dc2 100644 --- a/config/initializers/6_rack_profiler.rb +++ b/config/initializers/6_rack_profiler.rb @@ -5,4 +5,5 @@ if Rails.env.development? Rack::MiniProfilerRails.initialize!(Rails.application) Rack::MiniProfiler.config.position = 'right' Rack::MiniProfiler.config.start_hidden = true + Rack::MiniProfiler.config.skip_paths << '/specs' end diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 8f8c4169740..091548348b1 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -1,6 +1,11 @@ # Use this hook to configure devise mailer, warden hooks and so forth. The first # four configuration values can also be set straight in your models. Devise.setup do |config| + config.warden do |manager| + manager.default_strategies(scope: :user).unshift :two_factor_authenticatable + manager.default_strategies(scope: :user).unshift :two_factor_backupable + end + # ==> Mailer Configuration # Configure the class responsible to send e-mails. config.mailer = "DeviseMailer" diff --git a/config/initializers/disable_email_interceptor.rb b/config/initializers/disable_email_interceptor.rb index 122a5829916..c76a6b8b19f 100644 --- a/config/initializers/disable_email_interceptor.rb +++ b/config/initializers/disable_email_interceptor.rb @@ -1,4 +1,2 @@ # Interceptor in lib/disable_email_interceptor.rb -unless Gitlab.config.outgoing_emails.enabled - ActionMailer::Base.register_interceptor(DisableEmailInterceptor) -end +ActionMailer::Base.register_interceptor(DisableEmailInterceptor) unless Gitlab.config.gitlab.email_enabled diff --git a/config/initializers/email_settings.rb b/config/initializers/email_settings.rb deleted file mode 100644 index 99cb09ebfd0..00000000000 --- a/config/initializers/email_settings.rb +++ /dev/null @@ -1,5 +0,0 @@ -Gitlab.config.outgoing_emails.tap do |c| - Gitlab::Application.config.action_mailer.delivery_method = c.delivery_method - ActionMailer::Base.smtp_settings = c.smtp_settings.symbolize_keys - ActionMailer::Base.sendmail_settings = c.sendmail_settings.symbolize_keys -end diff --git a/config/initializers/smtp_settings.rb.sample b/config/initializers/smtp_settings.rb.sample new file mode 100644 index 00000000000..f0fe2fdfa43 --- /dev/null +++ b/config/initializers/smtp_settings.rb.sample @@ -0,0 +1,22 @@ +# To enable smtp email delivery for your GitLab instance do the following: +# 1. Rename this file to smtp_settings.rb +# 2. Edit settings inside this file +# 3. Restart GitLab instance +# +# For full list of options and their values see http://api.rubyonrails.org/classes/ActionMailer/Base.html +# + +if Rails.env.production? + Gitlab::Application.config.action_mailer.delivery_method = :smtp + + ActionMailer::Base.smtp_settings = { + address: "email.server.com", + port: 456, + user_name: "smtp", + password: "123456", + domain: "gitlab.company.com", + authentication: :login, + enable_starttls_auto: true, + openssl_verify_mode: 'peer' # See ActionMailer documentation for other possible options + } +end diff --git a/config/routes.rb b/config/routes.rb index 4b38dede7b4..bf2cb6421c5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -226,6 +226,11 @@ Gitlab::Application.routes.draw do resources :keys resources :emails, only: [:index, :create, :destroy] resource :avatar, only: [:destroy] + resource :two_factor_auth, only: [:new, :create, :destroy] do + member do + post :codes + end + end end end diff --git a/db/migrate/20150327223628_add_devise_two_factor_to_users.rb b/db/migrate/20150327223628_add_devise_two_factor_to_users.rb new file mode 100644 index 00000000000..11b026ee8f3 --- /dev/null +++ b/db/migrate/20150327223628_add_devise_two_factor_to_users.rb @@ -0,0 +1,8 @@ +class AddDeviseTwoFactorToUsers < ActiveRecord::Migration + def change + add_column :users, :encrypted_otp_secret, :string + add_column :users, :encrypted_otp_secret_iv, :string + add_column :users, :encrypted_otp_secret_salt, :string + add_column :users, :otp_required_for_login, :boolean + end +end diff --git a/db/migrate/20150331183602_add_devise_two_factor_backupable_to_users.rb b/db/migrate/20150331183602_add_devise_two_factor_backupable_to_users.rb new file mode 100644 index 00000000000..913958db7c5 --- /dev/null +++ b/db/migrate/20150331183602_add_devise_two_factor_backupable_to_users.rb @@ -0,0 +1,5 @@ +class AddDeviseTwoFactorBackupableToUsers < ActiveRecord::Migration + def change + add_column :users, :otp_backup_codes, :text + end +end diff --git a/db/migrate/20150423033240_add_default_project_visibililty_to_application_settings.rb b/db/migrate/20150423033240_add_default_project_visibililty_to_application_settings.rb index 9b0f13f3fa7..50a9b2439e0 100644 --- a/db/migrate/20150423033240_add_default_project_visibililty_to_application_settings.rb +++ b/db/migrate/20150423033240_add_default_project_visibililty_to_application_settings.rb @@ -1,7 +1,11 @@ class AddDefaultProjectVisibililtyToApplicationSettings < ActiveRecord::Migration - def change + def up add_column :application_settings, :default_project_visibility, :integer visibility = Settings.gitlab.default_projects_features['visibility_level'] execute("update application_settings set default_project_visibility = #{visibility}") end + + def down + remove_column :application_settings, :default_project_visibility + end end diff --git a/db/migrate/20150425164646_gitlab_change_collation_for_tag_names.acts_as_taggable_on_engine.rb b/db/migrate/20150425164646_gitlab_change_collation_for_tag_names.acts_as_taggable_on_engine.rb new file mode 100644 index 00000000000..281c88d2a7d --- /dev/null +++ b/db/migrate/20150425164646_gitlab_change_collation_for_tag_names.acts_as_taggable_on_engine.rb @@ -0,0 +1,10 @@ +# This migration is a duplicate of 20150425164651_change_collation_for_tag_names.acts_as_taggable_on_engine.rb +# It shold be applied before the index additions to ensure that `name` is case sensitive. + +class GitlabChangeCollationForTagNames < ActiveRecord::Migration + def up + if ActsAsTaggableOn::Utils.using_mysql? + execute("ALTER TABLE tags MODIFY name varchar(255) CHARACTER SET utf8 COLLATE utf8_bin;") + end + end +end diff --git a/db/migrate/20150425164647_remove_duplicate_tags.rb b/db/migrate/20150425164647_remove_duplicate_tags.rb new file mode 100644 index 00000000000..13e5038db9c --- /dev/null +++ b/db/migrate/20150425164647_remove_duplicate_tags.rb @@ -0,0 +1,17 @@ +class RemoveDuplicateTags < ActiveRecord::Migration + def up + select_all("SELECT name, COUNT(id) as cnt FROM tags GROUP BY name HAVING COUNT(id) > 1").each do |tag| + tag_name = quote_string(tag["name"]) + duplicate_ids = select_all("SELECT id FROM tags WHERE name = '#{tag_name}'").map{|tag| tag["id"]} + origin_tag_id = duplicate_ids.first + duplicate_ids.delete origin_tag_id + + execute("UPDATE taggings SET tag_id = #{origin_tag_id} WHERE tag_id IN(#{duplicate_ids.join(",")})") + execute("DELETE FROM tags WHERE id IN(#{duplicate_ids.join(",")})") + end + end + + def down + + end +end diff --git a/db/migrate/20150425164648_add_missing_unique_indices.acts_as_taggable_on_engine.rb b/db/migrate/20150425164648_add_missing_unique_indices.acts_as_taggable_on_engine.rb index 4ca676f6c72..c1b78681519 100644 --- a/db/migrate/20150425164648_add_missing_unique_indices.acts_as_taggable_on_engine.rb +++ b/db/migrate/20150425164648_add_missing_unique_indices.acts_as_taggable_on_engine.rb @@ -3,8 +3,15 @@ class AddMissingUniqueIndices < ActiveRecord::Migration def self.up add_index :tags, :name, unique: true - remove_index :taggings, :tag_id - remove_index :taggings, [:taggable_id, :taggable_type, :context] + # pre-GitLab v6.7.0 may not have these indices since there were no + # migrations for them + if index_exists?(:taggings, :tag_id) + remove_index :taggings, :tag_id + end + + if index_exists?(:taggings, [:taggable_id, :taggable_type, :context]) + remove_index :taggings, [:taggable_id, :taggable_type, :context] + end add_index :taggings, [:tag_id, :taggable_id, :taggable_type, :context, :tagger_id, :tagger_type], unique: true, name: 'taggings_idx' diff --git a/db/migrate/20150429002313_remove_abandoned_group_members_records.rb b/db/migrate/20150429002313_remove_abandoned_group_members_records.rb index 6013605bb35..244637e1c4a 100644 --- a/db/migrate/20150429002313_remove_abandoned_group_members_records.rb +++ b/db/migrate/20150429002313_remove_abandoned_group_members_records.rb @@ -1,6 +1,9 @@ class RemoveAbandonedGroupMembersRecords < ActiveRecord::Migration - def change + def up execute("DELETE FROM members WHERE type = 'GroupMember' AND source_id NOT IN(\ SELECT id FROM namespaces WHERE type='Group')") end + + def down + end end diff --git a/db/migrate/20150509180749_convert_legacy_reference_notes.rb b/db/migrate/20150509180749_convert_legacy_reference_notes.rb new file mode 100644 index 00000000000..b02605489be --- /dev/null +++ b/db/migrate/20150509180749_convert_legacy_reference_notes.rb @@ -0,0 +1,16 @@ +# Convert legacy Markdown-emphasized notes to the current, non-emphasized format +# +# _mentioned in 54f7727c850972f0401c1312a7c4a6a380de5666_ +# +# becomes +# +# mentioned in 54f7727c850972f0401c1312a7c4a6a380de5666 +class ConvertLegacyReferenceNotes < ActiveRecord::Migration + def up + execute %q{UPDATE notes SET note = trim(both '_' from note) WHERE system = true AND note LIKE '\_%\_'} + end + + def down + # noop + end +end diff --git a/db/schema.rb b/db/schema.rb index 04abf9bb9a6..c89d22d241c 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: 20150502064022) do +ActiveRecord::Schema.define(version: 20150509180749) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -493,6 +493,11 @@ ActiveRecord::Schema.define(version: 20150502064022) do t.string "bitbucket_access_token_secret" t.string "location" t.string "public_email", default: "", null: false + t.string "encrypted_otp_secret" + t.string "encrypted_otp_secret_iv" + t.string "encrypted_otp_secret_salt" + t.boolean "otp_required_for_login" + t.text "otp_backup_codes" end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree diff --git a/doc/development/README.md b/doc/development/README.md index d5d264be19d..16df0b40c47 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -1,4 +1,4 @@ -# Development +# Development - [Architecture](architecture.md) of GitLab - [Shell commands](shell_commands.md) in the GitLab codebase @@ -6,3 +6,4 @@ - [CI setup](ci_setup.md) for testing GitLab - [Sidekiq debugging](sidekiq_debugging.md) - [UI guide](ui_guide.md) for building GitLab with existing css styles and elements +- [Migration Style Guide](migration_style_guide.md) for creating safe migrations diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md new file mode 100644 index 00000000000..4fa1961fde9 --- /dev/null +++ b/doc/development/migration_style_guide.md @@ -0,0 +1,88 @@ +# Migration Style Guide + +When writing migrations for GitLab, you have to take into account that +these will be ran by hundreds of thousands of organizations of all sizes, some with +many years of data in their database. + +In addition, having to take a server offline for a an upgrade small or big is +a big burden for most organizations. For this reason it is important that your +migrations are written carefully, can be applied online and adhere to the style guide below. + +When writing your migrations, also consider that databases might have stale data +or inconsistencies and guard for that. Try to make as little assumptions as possible +about the state of the database. + +Please don't depend on GitLab specific code since it can change in future versions. +If needed copy-paste GitLab code into the migration to make make it forward compatible. + +## Comments in the migration + +Each migration you write needs to have the two following pieces of information +as comments. + +### Online, Offline, errors? + +First, you need to provide information on whether the migration can be applied: + +1. online without errors (works on previous version and new one) +2. online with errors on old instances after migrating +3. online with errors on new instances while migrating +4. offline (needs to happen without app servers to prevent db corruption) + +It is always preferable to have a migration run online. If you expect the migration +to take particularly long (for instance, if it loops through all notes), +this is valuable information to add. + +### Reversibility + +Your migration should be reversible. This is very important, as it should +be possible to downgrade in case of a vulnerability or bugs. + +In your migration, add a comment describing how the reversibility of the +migration was tested. + + +## Removing indices + +If you need to remove index, please add a condition like in following example: + +``` +remove_index :namespaces, column: :name if index_exists?(:namespaces, :name) +``` + +## Adding indices + +If you need to add an unique index please keep in mind there is possibility of existing duplicates. If it is possible write a separate migration for handling this situation. It can be just removing or removing with overwriting all references to these duplicates depend on situation. + +## Testing + +Make sure that your migration works with MySQL and PostgreSQL with data. An empty database does not guarantee that your migration is correct. + +Make sure your migration can be reversed. + +## Data migration + +Please prefer Arel and plain SQL over usual ActiveRecord syntax. In case of using plain SQL you need to quote all input manually with `quote_string` helper. + +Example with Arel: + +``` +users = Arel::Table.new(:users) +users.group(users[:user_id]).having(users[:id].count.gt(5)) + +#updtae other tables with this results +``` + +Example with plain SQL and `quote_string` helper: + +``` +select_all("SELECT name, COUNT(id) as cnt FROM tags GROUP BY name HAVING COUNT(id) > 1").each do |tag| + tag_name = quote_string(tag["name"]) + duplicate_ids = select_all("SELECT id FROM tags WHERE name = '#{tag_name}'").map{|tag| tag["id"]} + origin_tag_id = duplicate_ids.first + duplicate_ids.delete origin_tag_id + + execute("UPDATE taggings SET tag_id = #{origin_tag_id} WHERE tag_id IN(#{duplicate_ids.join(",")})") + execute("DELETE FROM tags WHERE id IN(#{duplicate_ids.join(",")})") +end +``` diff --git a/doc/install/installation.md b/doc/install/installation.md index ca25eaea799..e777f6bbb47 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -62,7 +62,13 @@ up-to-date and install it. Install the required packages (needed to compile Ruby and native extensions to Ruby gems): - sudo apt-get install -y build-essential zlib1g-dev libyaml-dev libssl-dev libgdbm-dev libreadline-dev libncurses5-dev libffi-dev curl openssh-server redis-server checkinstall libxml2-dev libxslt-dev libcurl4-openssl-dev libicu-dev logrotate python-docutils pkg-config cmake libkrb5-dev nodejs + sudo apt-get install -y build-essential zlib1g-dev libyaml-dev libssl-dev libgdbm-dev libreadline-dev libncurses5-dev libffi-dev curl openssh-server redis-server checkinstall libxml2-dev libxslt-dev libcurl4-openssl-dev libicu-dev logrotate python-docutils pkg-config cmake nodejs + +If you want to use Kerberos for user authentication, then install libkrb5-dev: + + sudo apt-get install libkrb5-dev + +**Note:** If you don't know what Kerberos is, then you certainly don't need it. Make sure you have the right version of Git installed @@ -276,10 +282,12 @@ We recommend using a PostgreSQL database. For MySQL check [MySQL setup guide](da **Note:** As of bundler 1.5.2, you can invoke `bundle install -jN` (where `N` the number of your processor cores) and enjoy the parallel gems installation with measurable difference in completion time (~60% faster). Check the number of your cores with `nproc`. For more information check this [post](http://robots.thoughtbot.com/parallel-gem-installing-using-bundler). First make sure you have bundler >= 1.5.2 (run `bundle -v`) as it addresses some [issues](https://devcenter.heroku.com/changelog-items/411) that were [fixed](https://github.com/bundler/bundler/pull/2817) in 1.5.2. # For PostgreSQL (note, the option says "without ... mysql") - sudo -u git -H bundle install --deployment --without development test mysql aws + sudo -u git -H bundle install --deployment --without development test mysql aws kerberos # Or if you use MySQL (note, the option says "without ... postgres") - sudo -u git -H bundle install --deployment --without development test postgres aws + sudo -u git -H bundle install --deployment --without development test postgres aws kerberos + +**Note:** If you want to use Kerberos for user authentication, then omit `kerberos` in the `--without` option above. ### Install GitLab Shell diff --git a/doc/markdown/markdown.md b/doc/markdown/markdown.md index 8ec5a20035f..30c29084e34 100644 --- a/doc/markdown/markdown.md +++ b/doc/markdown/markdown.md @@ -172,7 +172,7 @@ GFM will turn that reference into a link so you can navigate between them easily GFM will recognize the following: | input | references | -|-----------------------:|:---------------------------| +|:-----------------------|:---------------------------| | `@user_name` | specific user | | `@group_name` | specific group | | `@all` | entire team | @@ -189,7 +189,7 @@ GFM will recognize the following: GFM also recognizes certain cross-project references: | input | references | -|----------------------------------------:|:------------------------| +|:----------------------------------------|:------------------------| | `namespace/project#123` | issue | | `namespace/project!123` | merge request | | `namespace/project$123` | snippet | @@ -198,15 +198,23 @@ GFM also recognizes certain cross-project references: ## Task Lists -You can add task lists to merge request and issue descriptions to keep track of to-do items. To create a task, add an unordered list to the description in an issue or merge request, formatted like so: +You can add task lists to issues, merge requests and comments. To create a task list, add a specially-formatted Markdown list, like so: ```no-highlight -* [x] Completed task -* [ ] Unfinished task - * [x] Nested task +- [x] Completed task +- [ ] Incomplete task + - [ ] Sub-task 1 + - [x] Sub-task 2 + - [ ] Sub-task 3 ``` -Task lists can only be created in descriptions, not in titles or comments. Task item state can be managed by editing the description's Markdown or by clicking the rendered checkboxes. +- [x] Completed task +- [ ] Incomplete task + - [ ] Sub-task 1 + - [x] Sub-task 2 + - [ ] Sub-task 3 + +Task lists can only be created in descriptions, not in titles. Task item state can be managed by editing the description's Markdown or by toggling the rendered check boxes. # Standard Markdown @@ -246,51 +254,38 @@ Alt-H2 ### Header IDs and links -All markdown rendered headers automatically get IDs, except for comments. +All Markdown-rendered headers automatically get IDs, except in comments. On hover a link to those IDs becomes visible to make it easier to copy the link to the header to give it to someone else. The IDs are generated from the content of the header according to the following rules: -1. remove the heading hashes `#` and process the rest of the line as it would be processed if it were not a header -2. from the result, remove all HTML tags, but keep their inner content -3. convert all characters to lowercase -4. convert all characters except `[a-z0-9_-]` into hyphens `-` -5. transform multiple adjacent hyphens into a single hyphen -6. remove trailing and heading hyphens +1. All text is converted to lowercase +1. All non-word text (e.g., punctuation, HTML) is removed +1. All spaces are converted to hyphens +1. Two or more hyphens in a row are converted to one +1. If a header with the same ID has already been generated, a unique + incrementing number is appended. For example: ``` -###### ..Ab_c-d. e [anchor](URL) .. +# This header has spaces in it +## This header has a :thumbsup: in it +# This header has Unicode in it: 한글 +## This header has spaces in it +### This header has spaces in it ``` -which renders as: - -###### ..Ab_c-d. e [anchor](URL) .. - -will first be converted by step 1) into a string like: +Would generate the following link IDs: -``` -..Ab_c-d. e <a href="URL">anchor</a> <img src="URL" alt="alt text"/>.. -``` +1. `this-header-has-spaces-in-it` +1. `this-header-has-a-in-it` +1. `this-header-has-unicode-in-it-한글` +1. `this-header-has-spaces-in-it-1` +1. `this-header-has-spaces-in-it-2` -After removing the tags in step 2) we get: - -``` -..Ab_c-d. e anchor .. -``` - -And applying all the other steps gives the id: - -``` -ab_c-d-e-anchor -``` - -Note in particular how: - -- for markdown anchors `[text](URL)`, only the `text` is used -- markdown images `` are completely ignored +Note that the Emoji processing happens before the header IDs are generated, so the Emoji is converted to an image which then gets removed from the ID. ## Emphasis @@ -322,8 +317,6 @@ Strikethrough uses two tildes. ~~Scratch this.~~ 1. Ordered sub-list 4. And another item. - Some text that should be aligned with the above item. - * Unordered list can use asterisks - Or minuses + Or pluses @@ -336,8 +329,6 @@ Strikethrough uses two tildes. ~~Scratch this.~~ 1. Ordered sub-list 4. And another item. - Some text that should be aligned with the above item. - * Unordered list can use asterisks - Or minuses + Or pluses @@ -432,7 +423,7 @@ Quote break. You can also use raw HTML in your Markdown, and it'll mostly work pretty well. -See the documentation for HTML::Pipeline's [SanitizationFilter](http://www.rubydoc.info/gems/html-pipeline/HTML/Pipeline/SanitizationFilter#WHITELIST-constant) class for the list of allowed HTML tags and attributes. In addition to the default `SanitizationFilter` whitelist, GitLab allows the `class`, `id`, and `style` attributes. +See the documentation for HTML::Pipeline's [SanitizationFilter](http://www.rubydoc.info/gems/html-pipeline/HTML/Pipeline/SanitizationFilter#WHITELIST-constant) class for the list of allowed HTML tags and attributes. In addition to the default `SanitizationFilter` whitelist, GitLab allows `span` elements. ```no-highlight <dl> @@ -536,6 +527,20 @@ Code above produces next output: The row of dashes between the table header and body must have at least three dashes in each column. +By including colons in the header row, you can align the text within that column: + +``` +| Left Aligned | Centered | Right Aligned | Left Aligned | Centered | Right Aligned | +| :----------- | :------: | ------------: | :----------- | :------: | ------------: | +| Cell 1 | Cell 2 | Cell 3 | Cell 4 | Cell 5 | Cell 6 | +| Cell 7 | Cell 8 | Cell 9 | Cell 10 | Cell 11 | Cell 12 | +``` + +| Left Aligned | Centered | Right Aligned | Left Aligned | Centered | Right Aligned | +| :----------- | :------: | ------------: | :----------- | :------: | ------------: | +| Cell 1 | Cell 2 | Cell 3 | Cell 4 | Cell 5 | Cell 6 | +| Cell 7 | Cell 8 | Cell 9 | Cell 10 | Cell 11 | Cell 12 | + ## References - This document leveraged heavily from the [Markdown-Cheatsheet](https://github.com/adam-p/markdown-here/wiki/Markdown-Cheatsheet). diff --git a/doc/operations/sidekiq_memory_killer.md b/doc/operations/sidekiq_memory_killer.md index 867b01b0d5a..811c2192a19 100644 --- a/doc/operations/sidekiq_memory_killer.md +++ b/doc/operations/sidekiq_memory_killer.md @@ -36,3 +36,5 @@ The MemoryKiller is controlled using environment variables. Existing jobs get 30 seconds to finish. After that, the MemoryKiller tells Sidekiq to shut down, and an external supervision mechanism (e.g. Runit) must restart Sidekiq. +- `SIDEKIQ_MEMORY_KILLER_SHUTDOWN_SIGNAL`: defaults to 'SIGTERM'. The name of + the final signal sent to the Sidekiq process when we want it to shut down. diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index bfef975024f..bca4fcfb404 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -9,6 +9,8 @@ This archive will be saved in backup_path (see `config/gitlab.yml`). The filename will be `[TIMESTAMP]_gitlab_backup.tar`. This timestamp can be used to restore an specific backup. You can only restore a backup to exactly the same version of GitLab that you created it on, for example 7.2.1. +If you are interested in GitLab CI backup please follow to the [CI backup documentation](https://gitlab.com/gitlab-org/gitlab-ci/blob/master/doc/raketasks/backup_restore.md)* + ``` # use this command if you've installed GitLab with the Omnibus package sudo gitlab-rake gitlab:backup:create @@ -150,11 +152,9 @@ If you have an installation from source, please consider backing up your `gitlab You can only restore a backup to exactly the same version of GitLab that you created it on, for example 7.2.1. -``` -# Omnibus package installation -sudo gitlab-rake gitlab:backup:restore +### Installation from source -# installation from source +``` bundle exec rake gitlab:backup:restore RAILS_ENV=production ``` @@ -196,11 +196,45 @@ Restoring repositories: Deleting tmp directories...[DONE] ``` -## Configure cron to make daily backups +### Omnibus installations + +We will assume that you have installed GitLab from an omnibus package and run +`sudo gitlab-ctl reconfigure` at least once. + +First make sure your backup tar file is in `/var/opt/gitlab/backups`. + +```shell +sudo cp 1393513186_gitlab_backup.tar /var/opt/gitlab/backups/ +``` -For Omnibus package installations, see https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/README.md#scheduling-a-backup . +Next, restore the backup by running the restore command. You need to specify the +timestamp of the backup you are restoring. -For installation from source: +```shell +# Stop processes that are connected to the database +sudo gitlab-ctl stop unicorn +sudo gitlab-ctl stop sidekiq + +# This command will overwrite the contents of your GitLab database! +sudo gitlab-rake gitlab:backup:restore BACKUP=1393513186 + +# Start GitLab +sudo gitlab-ctl start + +# Create satellites +sudo gitlab-rake gitlab:satellites:create + +# Check GitLab +sudo gitlab-rake gitlab:check SANITIZE=true +``` + +If there is a GitLab version mismatch between your backup tar file and the installed +version of GitLab, the restore command will abort with an error. Install a package for +the [required version](https://www.gitlab.com/downloads/archives/) and try again. + +## Configure cron to make daily backups + +### For installation from source: ``` cd /home/git/gitlab sudo -u git -H editor config/gitlab.yml # Enable keep_time in the backup section to automatically delete old backups @@ -217,6 +251,32 @@ Add the following lines at the bottom: The `CRON=1` environment setting tells the backup script to suppress all progress output if there are no errors. This is recommended to reduce cron spam. +### For omnibus installations + +To schedule a cron job that backs up your repositories and GitLab metadata, use the root user: + +``` +sudo su - +crontab -e +``` + +There, add the following line to schedule the backup for everyday at 2 AM: + +``` +0 2 * * * /opt/gitlab/bin/gitlab-rake gitlab:backup:create CRON=1 +``` + +You may also want to set a limited lifetime for backups to prevent regular +backups using all your disk space. To do this add the following lines to +`/etc/gitlab/gitlab.rb` and reconfigure: + +``` +# limit backup lifetime to 7 days - 604800 seconds +gitlab_rails['backup_keep_time'] = 604800 +``` + +NOTE: This cron job does not [backup your omnibus-gitlab configuration](#backup-and-restore-omnibus-gitlab-configuration) or [SSH host keys](https://superuser.com/questions/532040/copy-ssh-keys-from-one-server-to-another-server/532079#532079). + ## Alternative backup strategies If your GitLab server contains a lot of Git repository data you may find the GitLab backup script to be too slow. diff --git a/doc/release/master.md b/doc/release/master.md index 19070b46a0d..9163e652003 100644 --- a/doc/release/master.md +++ b/doc/release/master.md @@ -31,3 +31,32 @@ git remote add gl git@gitlab.com:gitlab-org/gitlab-ce.git gpa ``` +# Yanking packages from packages.gitlab.com + +In case something went wrong with the release and there is a need to remove the packages you can yank the packages by following the +procedure described in [package cloud documentation](https://packagecloud.io/docs#yank_pkg). + +You need to have: + +1. `package_cloud` gem installed (sudo gem install package_cloud) +1. Email and password for packages.gitlab.com +1. Make sure that you are supplying the url to packages.gitlab.com (default is packagecloud.io) + +Example of yanking a package: + +```bash +package_cloud yank --url https://packages.gitlab.com gitlab/gitlab-ce/el/6 gitlab-ce-7.10.2~omnibus-1.x86_64.rpm +``` + +If you are attempting this for the first time the output will look something like: + +```bash +Looking for repository at gitlab/gitlab-ce... No config file exists at /Users/marin/.packagecloud. Login to create one. +Email: +marin@gitlab.com +Password: + +Got your token. Writing a config file to /Users/marin/.packagecloud... success! +success! +Attempting to yank package at gitlab/gitlab-ce/el/6/gitlab-ce-7.10.2~omnibus-1.x86_64.rpm...done! +``` diff --git a/doc/release/patch.md b/doc/release/patch.md index 4c7b471785f..a569bb3da8d 100644 --- a/doc/release/patch.md +++ b/doc/release/patch.md @@ -44,7 +44,7 @@ Create release tag and push to remotes: bundle exec rake release["x.x.x"] ``` -### Release +## Release 1. [Build new packages with the latest version](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/doc/release.md) 1. Apply the patch to GitLab.com and the private GitLab development server @@ -53,3 +53,4 @@ bundle exec rake release["x.x.x"] 1. Send tweets about the release from `@gitlab`, tweet should include the most important feature that the release is addressing and link to the blog post 1. Note in the 'GitLab X.X regressions' issue that the patch was published (CE only) 1. [Create new AMIs](https://dev.gitlab.org/gitlab/AMI/blob/master/README.md) +1. Create a new patch release issue for the next potential release
\ No newline at end of file diff --git a/doc/security/README.md b/doc/security/README.md index 49dfa6eec76..473f3632dcd 100644 --- a/doc/security/README.md +++ b/doc/security/README.md @@ -4,3 +4,4 @@ - [Rack attack](rack_attack.md) - [Web Hooks and insecure internal web services](webhooks.md) - [Information exclusivity](information_exclusivity.md) +- [Reset your root password](reset_root_password.md)
\ No newline at end of file diff --git a/doc/security/reset_root_password.md b/doc/security/reset_root_password.md new file mode 100644 index 00000000000..3c13f262677 --- /dev/null +++ b/doc/security/reset_root_password.md @@ -0,0 +1,40 @@ +# How to reset your root password + +Log into your server with root privileges. Then start a Ruby on Rails console. + +Start the console with this command: + +```bash +gitlab-rails console production +``` + +Wait until the console has loaded. + +There are multiple ways to find your user. You can search for email or username. + +```bash +user = User.where(id: 1).first +``` + +or + +```bash +user = User.find_by(email: 'admin@local.host') +``` + +Now you can change your password: + +```bash +user.password = 'secret_pass' +user.password_confirmation = 'secret_pass' +``` + +It's important that you change both password and password_confirmation to make it work. + +Don't forget to save the changes. + +```bash +user.save! +``` + +Exit the console and try to login with your new password.
\ No newline at end of file diff --git a/doc/system_hooks/system_hooks.md b/doc/system_hooks/system_hooks.md index f9b6d37d840..b0e4613cdef 100644 --- a/doc/system_hooks/system_hooks.md +++ b/doc/system_hooks/system_hooks.md @@ -6,6 +6,12 @@ System hooks can be used, e.g. for logging or changing information in a LDAP ser ## Hooks request example +**Request header**: + +``` +X-Gitlab-Event: System Hook +``` + **Project created:** ```json diff --git a/doc/update/mysql_to_postgresql.md b/doc/update/mysql_to_postgresql.md index 50941db25f6..2c43cf59c1f 100644 --- a/doc/update/mysql_to_postgresql.md +++ b/doc/update/mysql_to_postgresql.md @@ -1,7 +1,7 @@ # Migrating GitLab from MySQL to Postgres *Make sure you view this [guide from the `master` branch](../../../master/doc/update/mysql_to_postgresql.md) for the most up to date instructions.* -If you are replacing MySQL with Postgres while keeping GitLab on the same server all you need to do is to export from MySQL, import into Postgres and rebuild the indexes as described below. If you are also moving GitLab to another server, or if you are switching to omnibus-gitlab, you may want to use a GitLab backup file. The second part of this documents explains the procedure to do this. +If you are replacing MySQL with Postgres while keeping GitLab on the same server all you need to do is to export from MySQL, convert the resulting SQL file, and import it into Postgres. If you are also moving GitLab to another server, or if you are switching to omnibus-gitlab, you may want to use a GitLab backup file. The second part of this documents explains the procedure to do this. ## Export from MySQL and import into Postgres @@ -14,13 +14,11 @@ sudo service gitlab stop git clone https://github.com/gitlabhq/mysql-postgresql-converter.git -b gitlab cd mysql-postgresql-converter -mysqldump --compatible=postgresql --default-character-set=utf8 -r databasename.mysql -u root gitlabhq_production -p -python db_converter.py databasename.mysql databasename.psql +mysqldump --compatible=postgresql --default-character-set=utf8 -r gitlabhq_production.mysql -u root gitlabhq_production -p +python db_converter.py gitlabhq_production.mysql gitlabhq_production.psql # Import the database dump as the application database user -sudo -u git psql -f databasename.psql -d gitlabhq_production - -# Rebuild indexes (see below) +sudo -u git psql -f gitlabhq_production.psql -d gitlabhq_production # Install gems for PostgreSQL (note: the line below states '--without ... mysql') sudo -u git -H bundle install --without development test mysql --deployment @@ -28,51 +26,6 @@ sudo -u git -H bundle install --without development test mysql --deployment sudo service gitlab start ``` -## Rebuild indexes - -The lanyrd database converter script does not preserve all indexes, so we have to recreate them ourselves after migrating from MySQL. It is not necessary to shut down GitLab for this process. - -### For non-omnibus installations - -On non-omnibus installations (distributed using Git) we retrieve the index declarations from version control using `git stash`. - -``` -# Clone the database converter on your Postgres-backed GitLab server -cd /tmp -git clone https://github.com/gitlabhq/mysql-postgresql-converter.git -b gitlab - -cd /home/git/gitlab - -# Stash changes to db/schema.rb to make sure we can find the right index statements -sudo -u git -H git stash - -# Generate add_index.rb -ruby /tmp/mysql-postgresql-converter/add_index_statements.rb db/schema.rb > /tmp/mysql-postgresql-converter/add_index.rb - -# Create the indexes -sudo -u git -H bundle exec rails runner -e production 'eval $stdin.read' < /tmp/mysql-postgresql-converter/add_index.rb -``` - -### For omnibus-gitlab installations - -On omnibus-gitlab we need to get the index declarations from a file called `schema.rb.bundled`. For versions older than 6.9, we need to download the file. - -``` -# Clone the database converter on your Postgres-backed GitLab server -cd /tmp -/opt/gitlab/embedded/bin/git clone https://github.com/gitlabhq/mysql-postgresql-converter.git -b gitlab -cd /tmp/mysql-postgresql-converter - -# Download schema.rb.bundled if necessary -test -e /opt/gitlab/embedded/service/gitlab-rails/db/schema.rb.bundled || sudo /opt/gitlab/embedded/bin/curl -o /opt/gitlab/embedded/service/gitlab-rails/db/schema.rb.bundled https://gitlab.com/gitlab-org/gitlab-ce/raw/v6.9.1/db/schema.rb - -# Generate add_index.rb -/opt/gitlab/embedded/bin/ruby add_index_statements.rb /opt/gitlab/embedded/service/gitlab-rails/db/schema.rb.bundled > add_index.rb - -# Create the indexes -/opt/gitlab/bin/gitlab-rails runner 'eval $stdin.read' < add_index.rb -``` - ## Converting a GitLab backup file from MySQL to Postgres **Note:** Please make sure to have Python 2.7.x (or higher) installed. diff --git a/doc/web_hooks/web_hooks.md b/doc/web_hooks/web_hooks.md index 851f50f5e9a..01082555192 100644 --- a/doc/web_hooks/web_hooks.md +++ b/doc/web_hooks/web_hooks.md @@ -12,6 +12,12 @@ If you send a web hook to an SSL endpoint [the certificate will not be verified] Triggered when you push to the repository except when pushing tags. +**Request header**: + +``` +X-Gitlab-Event: Push Hook +``` + **Request body:** ```json @@ -63,6 +69,13 @@ Triggered when you push to the repository except when pushing tags. Triggered when you create (or delete) tags to the repository. +**Request header**: + +``` +X-Gitlab-Event: Tag Push Hook +``` + + **Request body:** ```json @@ -92,6 +105,12 @@ Triggered when you create (or delete) tags to the repository. Triggered when a new issue is created or an existing issue was updated/closed/reopened. +**Request header**: + +``` +X-Gitlab-Event: Issue Hook +``` + **Request body:** ```json @@ -126,6 +145,12 @@ Triggered when a new issue is created or an existing issue was updated/closed/re Triggered when a new merge request is created or an existing merge request was updated/merged/closed. +**Request header**: + +``` +X-Gitlab-Event: Merge Request Hook +``` + **Request body:** ```json diff --git a/features/project/deploy_keys.feature b/features/project/deploy_keys.feature index a71f6124d9c..47cf774094f 100644 --- a/features/project/deploy_keys.feature +++ b/features/project/deploy_keys.feature @@ -9,9 +9,10 @@ Feature: Project Deploy Keys Then I should see project deploy key Scenario: I should see project deploy keys - Given other project has deploy key + Given other projects have deploy keys When I visit project deploy keys page - Then I should see other project deploy key + Then I should see other project deploy key + And I should only see the same deploy key once Scenario: I should see public deploy keys Given public deploy key exists @@ -26,7 +27,7 @@ Feature: Project Deploy Keys And I should see newly created deploy key Scenario: I attach other project deploy key to project - Given other project has deploy key + Given other projects have deploy keys And I visit project deploy keys page When I click attach deploy key Then I should be on deploy keys page diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature index eb813884d1e..bf84e2f8e87 100644 --- a/features/project/issues/issues.feature +++ b/features/project/issues/issues.feature @@ -134,48 +134,15 @@ Feature: Project Issues And I should see "Release 0.4" in issues And I should not see "Tweet control" in issues - Scenario: Issue description should render task checkboxes - Given project "Shop" has "Tasks-open" open issue with task markdown - When I visit issue page "Tasks-open" - Then I should see task checkboxes in the description - - @javascript - Scenario: Issue notes should not render task checkboxes - Given project "Shop" has "Tasks-open" open issue with task markdown - When I visit issue page "Tasks-open" - And I leave a comment with task markdown - Then I should not see task checkboxes in the comment - @javascript Scenario: Issue notes should be editable with +1 - Given project "Shop" has "Tasks-open" open issue with task markdown - When I visit issue page "Tasks-open" + Given project "Shop" have "Release 0.4" open issue + When I visit issue page "Release 0.4" And I leave a comment with a header containing "Comment with a header" Then The comment with the header should not have an ID And I edit the last comment with a +1 Then I should see +1 in the description - # Task status in issues list - - Scenario: Issues list should display task status - Given project "Shop" has "Tasks-open" open issue with task markdown - When I visit project "Shop" issues page - Then I should see the task status for the Taskable - - # Toggling task items - - @javascript - Scenario: Task checkboxes should be enabled for an open issue - Given project "Shop" has "Tasks-open" open issue with task markdown - When I visit issue page "Tasks-open" - Then Task checkboxes should be enabled - - @javascript - Scenario: Task checkboxes should be disabled for a closed issue - Given project "Shop" has "Tasks-closed" closed issue with task markdown - When I visit issue page "Tasks-closed" - Then Task checkboxes should be disabled - # Issue description preview @javascript @@ -212,8 +179,8 @@ Feature: Project Issues @javascript Scenario: I can unsubscribe from issue - Given project "Shop" has "Tasks-open" open issue with task markdown - When I visit issue page "Tasks-open" + Given project "Shop" have "Release 0.4" open issue + When I visit issue page "Release 0.4" Then I should see that I am subscribed When I click button "Unsubscribe" Then I should see that I am unsubscribed diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index cbb5c8eb39b..60caf783fe4 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -96,16 +96,6 @@ Feature: Project Merge Requests And I leave a comment with a header containing "Comment with a header" Then The comment with the header should not have an ID - Scenario: Merge request description should render task checkboxes - Given project "Shop" has "MR-task-open" open MR with task markdown - When I visit merge request page "MR-task-open" - Then I should see task checkboxes in the description - - Scenario: Merge request notes should not render task checkboxes - Given project "Shop" has "MR-task-open" open MR with task markdown - When I visit merge request page "MR-task-open" - Then I should not see task checkboxes in the comment - # Toggling inline comments @javascript @@ -173,28 +163,6 @@ Feature: Project Merge Requests And I click on the Changes tab via Javascript Then I should see the proper Inline and Side-by-side links - # Task status in issues list - - Scenario: Merge requests list should display task status - Given project "Shop" has "MR-task-open" open MR with task markdown - When I visit project "Shop" merge requests page - Then I should see the task status for the Taskable - - # Toggling task items - - @javascript - Scenario: Task checkboxes should be enabled for an open merge request - Given project "Shop" has "MR-task-open" open MR with task markdown - When I visit merge request page "MR-task-open" - Then Task checkboxes should be enabled - - @javascript - Scenario: Task checkboxes should be disabled for a closed merge request - Given project "Shop" has "MR-task-open" open MR with task markdown - And I visit merge request page "MR-task-open" - And I click link "Close" - Then Task checkboxes should be disabled - # Description preview @javascript diff --git a/features/steps/project/commits/commits.rb b/features/steps/project/commits/commits.rb index 30b1934b363..c888e82e207 100644 --- a/features/steps/project/commits/commits.rb +++ b/features/steps/project/commits/commits.rb @@ -12,7 +12,7 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps end step 'I click atom feed link' do - click_link "Feed" + click_link "Commits Feed" end step 'I see commits atom feed' do diff --git a/features/steps/project/deploy_keys.rb b/features/steps/project/deploy_keys.rb index 50e14513a7a..81d1182cd1b 100644 --- a/features/steps/project/deploy_keys.rb +++ b/features/steps/project/deploy_keys.rb @@ -45,10 +45,20 @@ class Spinach::Features::ProjectDeployKeys < Spinach::FeatureSteps end end - step 'other project has deploy key' do - @second_project = create :project, namespace: create(:group) + step 'other projects have deploy keys' do + @second_project = create(:project, namespace: create(:group)) @second_project.team << [current_user, :master] create(:deploy_keys_project, project: @second_project) + + @third_project = create(:project, namespace: create(:group)) + @third_project.team << [current_user, :master] + create(:deploy_keys_project, project: @third_project, deploy_key: @second_project.deploy_keys.first) + end + + step 'I should only see the same deploy key once' do + within '.available-keys' do + page.should have_selector('ul li', count: 1) + end end step 'public deploy key exists' do diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index b8e282b2029..504f0cff724 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -179,14 +179,6 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps author: project.users.first) end - step 'project "Shop" has "Tasks-open" open issue with task markdown' do - create_taskable(:issue, 'Tasks-open') - end - - step 'project "Shop" has "Tasks-closed" closed issue with task markdown' do - create_taskable(:closed_issue, 'Tasks-closed') - end - step 'empty project "Empty Project"' do create :empty_project, name: 'Empty Project', namespace: @user.namespace end diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index 12d4d81159e..f67e6e3d8ca 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -108,10 +108,6 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps author: project.users.first) end - step 'project "Shop" has "MR-task-open" open MR with task markdown' do - create_taskable(:merge_request, 'MR-task-open') - end - step 'I switch to the diff tab' do visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) end diff --git a/features/steps/shared/markdown.rb b/features/steps/shared/markdown.rb index a7231c47d14..943640007a9 100644 --- a/features/steps/shared/markdown.rb +++ b/features/steps/shared/markdown.rb @@ -10,55 +10,10 @@ module SharedMarkdown find(:xpath, "#{node.path}/..").text.should == text end - def create_taskable(type, title) - desc_text = <<EOT.gsub(/^ {6}/, '') - * [ ] Task 1 - * [x] Task 2 -EOT - - case type - when :issue, :closed_issue - options = { project: project } - when :merge_request - options = { source_project: project, target_project: project } - end - - create( - type, - options.merge(title: title, - author: project.users.first, - description: desc_text) - ) - end - step 'Header "Description header" should have correct id and link' do header_should_have_correct_id_and_link(1, 'Description header', 'description-header') end - step 'I should see task checkboxes in the description' do - expect(page).to have_selector( - 'div.description li.task-list-item input[type="checkbox"]' - ) - end - - step 'I should see the task status for the Taskable' do - expect(find(:css, 'span.task-status').text).to eq( - '2 tasks (1 done, 1 unfinished)' - ) - end - - step 'Task checkboxes should be enabled' do - expect(page).to have_selector( - 'div.description li.task-list-item input[type="checkbox"]:enabled' - ) - end - - step 'Task checkboxes should be disabled' do - expect(page).to have_selector( - 'div.description li.task-list-item input[type="checkbox"]:disabled' - ) - end - step 'I should not see the Markdown preview' do expect(find('.gfm-form .js-md-preview')).not_to be_visible end diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb index 583746d4475..2f66e61b214 100644 --- a/features/steps/shared/note.rb +++ b/features/steps/shared/note.rb @@ -122,20 +122,6 @@ module SharedNote end end - step 'I leave a comment with task markdown' do - within('.js-main-target-form') do - fill_in 'note[note]', with: '* [x] Task item' - click_button 'Add Comment' - sleep 0.05 - end - end - - step 'I should not see task checkboxes in the comment' do - expect(page).not_to have_selector( - 'li.note div.timeline-content input[type="checkbox"]' - ) - end - step 'I edit the last comment with a +1' do find(".note").hover find('.js-note-edit').click diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index e3cf1b92cda..d9401bd540c 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -323,16 +323,6 @@ module SharedPaths visit namespace_project_issue_path(issue.project.namespace, issue.project, issue) end - step 'I visit issue page "Tasks-open"' do - issue = Issue.find_by(title: 'Tasks-open') - visit namespace_project_issue_path(issue.project.namespace, issue.project, issue) - end - - step 'I visit issue page "Tasks-closed"' do - issue = Issue.find_by(title: 'Tasks-closed') - visit namespace_project_issue_path(issue.project.namespace, issue.project, issue) - end - step 'I visit project "Shop" labels page' do project = Project.find_by(name: 'Shop') visit namespace_project_labels_path(project.namespace, project) @@ -363,16 +353,6 @@ module SharedPaths visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr) end - step 'I visit merge request page "MR-task-open"' do - mr = MergeRequest.find_by(title: 'MR-task-open') - visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr) - end - - step 'I visit merge request page "MR-task-closed"' do - mr = MergeRequest.find_by(title: 'MR-task-closed') - visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr) - end - step 'I visit project "Shop" merge requests page' do visit namespace_project_merge_requests_path(project.namespace, project) end diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb index 518964db50d..22b8f90dc5c 100644 --- a/lib/api/system_hooks.rb +++ b/lib/api/system_hooks.rb @@ -47,7 +47,7 @@ module API owner_name: "Someone", owner_email: "example@gitlabhq.com" } - @hook.execute(data) + @hook.execute(data, 'system_hooks') data end diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index 960fb3849b4..16ff03c38d4 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -40,7 +40,7 @@ module Gitlab user.block unless user.blocked? false else - user.activate if user.blocked? + user.activate if user.blocked? && !ldap_config.block_auto_created_users true end else diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 8348e28d0f5..63294aa54c0 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -1,4 +1,5 @@ require 'html/pipeline' +require 'task_list/filter' module Gitlab # Custom parser for GitLab-flavored Markdown @@ -31,9 +32,9 @@ module Gitlab # Public: Parse the provided text with GitLab-Flavored Markdown # # text - the source text - # options - parse_tasks - render tasks - # - xhtml - output XHTML instead of HTML - # - reference_only_path - Use relative path for reference links + # options - A Hash of options used to customize output (default: {}): + # :xhtml - output XHTML instead of HTML + # :reference_only_path - Use relative path for reference links # project - the project # html_options - extra options for the reference links as given to link_to def gfm_with_options(text, options = {}, project = @project, html_options = {}) @@ -45,7 +46,6 @@ module Gitlab text = text.dup.to_str options.reverse_merge!( - parse_tasks: false, xhtml: false, reference_only_path: true ) @@ -76,10 +76,6 @@ module Gitlab text = result[:output].to_html(save_with: save_options) - if options[:parse_tasks] - text = parse_tasks(text) - end - text.html_safe end @@ -106,28 +102,10 @@ module Gitlab Gitlab::Markdown::SnippetReferenceFilter, Gitlab::Markdown::CommitRangeReferenceFilter, Gitlab::Markdown::CommitReferenceFilter, - Gitlab::Markdown::LabelReferenceFilter - ] - end + Gitlab::Markdown::LabelReferenceFilter, - # Turn list items that start with "[ ]" into HTML checkbox inputs. - def parse_tasks(text) - li_tag = '<li class="task-list-item">' - unchecked_box = '<input type="checkbox" value="on" disabled />' - checked_box = unchecked_box.sub(/\/>$/, 'checked="checked" />') - - # Regexp captures don't seem to work when +text+ is an - # ActiveSupport::SafeBuffer, hence the `String.new` - String.new(text).gsub(Taskable::TASK_PATTERN_HTML) do - checked = $LAST_MATCH_INFO[:checked].downcase == 'x' - p_tag = $LAST_MATCH_INFO[:p_tag] - - if checked - "#{li_tag}#{p_tag}#{checked_box}" - else - "#{li_tag}#{p_tag}#{unchecked_box}" - end - end + TaskList::Filter + ] end end end diff --git a/lib/gitlab/markdown/sanitization_filter.rb b/lib/gitlab/markdown/sanitization_filter.rb index 9a154e0b2fe..6f33155badf 100644 --- a/lib/gitlab/markdown/sanitization_filter.rb +++ b/lib/gitlab/markdown/sanitization_filter.rb @@ -10,8 +10,9 @@ module Gitlab def whitelist whitelist = HTML::Pipeline::SanitizationFilter::WHITELIST - # Allow `class` and `id` on all elements - whitelist[:attributes][:all].push('class', 'id') + # Allow code highlighting + whitelist[:attributes]['pre'] = %w(class) + whitelist[:attributes]['span'] = %w(class) # Allow table alignment whitelist[:attributes]['th'] = %w(style) @@ -23,6 +24,9 @@ module Gitlab # Remove `rel` attribute from `a` elements whitelist[:transformers].push(remove_rel) + # Remove `class` attribute from non-highlight spans + whitelist[:transformers].push(clean_spans) + whitelist end @@ -33,6 +37,17 @@ module Gitlab end end end + + def clean_spans + lambda do |env| + return unless env[:node_name] == 'span' + return unless env[:node].has_attribute?('class') + + unless has_ancestor?(env[:node], 'pre') + env[:node].remove_attribute('class') + end + end + end end end end diff --git a/lib/gitlab/markdown/table_of_contents_filter.rb b/lib/gitlab/markdown/table_of_contents_filter.rb index c97612dafb8..38887c9778c 100644 --- a/lib/gitlab/markdown/table_of_contents_filter.rb +++ b/lib/gitlab/markdown/table_of_contents_filter.rb @@ -31,7 +31,7 @@ module Gitlab id = text.downcase id.gsub!(PUNCTUATION_REGEXP, '') # remove punctuation id.gsub!(' ', '-') # replace spaces with dash - id.squeeze!(' -') # replace multiple spaces or dashes with one + id.squeeze!('-') # replace multiple dashes with one uniq = (headers[id] > 0) ? "-#{headers[id]}" : '' headers[id] += 1 diff --git a/lib/gitlab/sidekiq_middleware/memory_killer.rb b/lib/gitlab/sidekiq_middleware/memory_killer.rb index 0f2db50e98c..f33b2dedf4a 100644 --- a/lib/gitlab/sidekiq_middleware/memory_killer.rb +++ b/lib/gitlab/sidekiq_middleware/memory_killer.rb @@ -7,6 +7,7 @@ module Gitlab GRACE_TIME = (ENV['SIDEKIQ_MEMORY_KILLER_GRACE_TIME'] || 15 * 60).to_s.to_i # Wait 30 seconds for running jobs to finish during graceful shutdown SHUTDOWN_WAIT = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_WAIT'] || 30).to_s.to_i + SHUTDOWN_SIGNAL = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_SIGNAL'] || 'SIGTERM').to_s # Create a mutex used to ensure there will be only one thread waiting to # shut Sidekiq down @@ -24,19 +25,19 @@ module Gitlab Sidekiq.logger.warn "current RSS #{current_rss} exceeds maximum RSS "\ "#{MAX_RSS}" - Sidekiq.logger.warn "spawned thread that will shut down PID "\ - "#{Process.pid} in #{GRACE_TIME} seconds" + Sidekiq.logger.warn "this thread will shut down PID #{Process.pid} "\ + "in #{GRACE_TIME} seconds" sleep(GRACE_TIME) Sidekiq.logger.warn "sending SIGUSR1 to PID #{Process.pid}" Process.kill('SIGUSR1', Process.pid) Sidekiq.logger.warn "waiting #{SHUTDOWN_WAIT} seconds before sending "\ - "SIGTERM to PID #{Process.pid}" + "#{SHUTDOWN_SIGNAL} to PID #{Process.pid}" sleep(SHUTDOWN_WAIT) - Sidekiq.logger.warn "sending SIGTERM to PID #{Process.pid}" - Process.kill('SIGTERM', Process.pid) + Sidekiq.logger.warn "sending #{SHUTDOWN_SIGNAL} to PID #{Process.pid}" + Process.kill(SHUTDOWN_SIGNAL, Process.pid) end end diff --git a/lib/tasks/brakeman.rake b/lib/tasks/brakeman.rake index 3a225801ff2..5d4e0740373 100644 --- a/lib/tasks/brakeman.rake +++ b/lib/tasks/brakeman.rake @@ -1,6 +1,8 @@ desc 'Security check via brakeman' task :brakeman do - if system("brakeman --skip-files lib/backup/repository.rb -w3 -z") + # We get 0 warnings at level 'w3' but we would like to reach 'w2'. Merge + # requests are welcome! + if system(*%W(brakeman --skip-files lib/backup/repository.rb -w3 -z)) puts 'Security check succeed' else puts 'Security check failed' diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index f967fddf56d..1a6303b6c82 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -320,7 +320,7 @@ namespace :gitlab do options = { "user.name" => "GitLab", - "user.email" => Gitlab.config.outgoing_emails.from, + "user.email" => Gitlab.config.gitlab.email_from, "core.autocrlf" => "input" } correct_options = options.map do |name, value| diff --git a/lib/tasks/jasmine.rake b/lib/tasks/jasmine.rake new file mode 100644 index 00000000000..9e2cceffa19 --- /dev/null +++ b/lib/tasks/jasmine.rake @@ -0,0 +1,12 @@ +# Since we no longer explicitly require the 'jasmine' gem, we lost the +# `jasmine:ci` task used by GitLab CI jobs. +# +# This provides a simple alias to run the `spec:javascript` task from the +# 'jasmine-rails' gem. +task jasmine: ['jasmine:ci'] + +namespace :jasmine do + task :ci do + Rake::Task['spec:javascript'].invoke + end +end diff --git a/spec/controllers/profiles/two_factor_auths_controller_spec.rb b/spec/controllers/profiles/two_factor_auths_controller_spec.rb new file mode 100644 index 00000000000..f05d1f5fbe1 --- /dev/null +++ b/spec/controllers/profiles/two_factor_auths_controller_spec.rb @@ -0,0 +1,126 @@ +require 'spec_helper' + +describe Profiles::TwoFactorAuthsController do + before do + # `user` should be defined within the action-specific describe blocks + sign_in(user) + + allow(subject).to receive(:current_user).and_return(user) + end + + describe 'GET new' do + let(:user) { create(:user) } + + it 'generates otp_secret' do + expect { get :new }.to change { user.otp_secret } + end + + it 'assigns qr_code' do + code = double('qr code') + expect(subject).to receive(:build_qr_code).and_return(code) + + get :new + expect(assigns[:qr_code]).to eq code + end + end + + describe 'POST create' do + let(:user) { create(:user) } + let(:pin) { 'pin-code' } + + def go + post :create, pin_code: pin + end + + context 'with valid pin' do + before do + expect(user).to receive(:valid_otp?).with(pin).and_return(true) + end + + it 'sets otp_required_for_login' do + go + + user.reload + expect(user.otp_required_for_login).to eq true + end + + it 'presents plaintext codes for the user to save' do + expect(user).to receive(:generate_otp_backup_codes!).and_return(%w(a b c)) + + go + + expect(assigns[:codes]).to match_array %w(a b c) + end + + it 'renders create' do + go + expect(response).to render_template(:create) + end + end + + context 'with invalid pin' do + before do + expect(user).to receive(:valid_otp?).with(pin).and_return(false) + end + + it 'assigns error' do + go + expect(assigns[:error]).to eq 'Invalid pin code' + end + + it 'assigns qr_code' do + code = double('qr code') + expect(subject).to receive(:build_qr_code).and_return(code) + + go + expect(assigns[:qr_code]).to eq code + end + + it 'renders new' do + go + expect(response).to render_template(:new) + end + end + end + + describe 'POST codes' do + let(:user) { create(:user, :two_factor) } + + it 'presents plaintext codes for the user to save' do + expect(user).to receive(:generate_otp_backup_codes!).and_return(%w(a b c)) + + post :codes + expect(assigns[:codes]).to match_array %w(a b c) + end + + it 'persists the generated codes' do + post :codes + + user.reload + expect(user.otp_backup_codes).not_to be_empty + end + end + + describe 'DELETE destroy' do + let(:user) { create(:user, :two_factor) } + let!(:codes) { user.generate_otp_backup_codes! } + + it 'clears all 2FA-related fields' do + expect(user.otp_required_for_login).to eq true + expect(user.otp_backup_codes).not_to be_nil + expect(user.encrypted_otp_secret).not_to be_nil + + delete :destroy + + expect(user.otp_required_for_login).to eq false + expect(user.otp_backup_codes).to be_nil + expect(user.encrypted_otp_secret).to be_nil + end + + it 'redirects to profile_account_path' do + delete :destroy + + expect(response).to redirect_to(profile_account_path) + end + end +end diff --git a/spec/factories.rb b/spec/factories.rb index 19f2935f30e..26e8a795fa4 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -28,6 +28,13 @@ FactoryGirl.define do admin true end + trait :two_factor do + before(:create) do |user| + user.otp_required_for_login = true + user.otp_secret = User.generate_otp_secret + end + end + factory :omniauth_user do ignore do extern_uid '123456' diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index f1c33461b55..e1009d5916e 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -25,15 +25,16 @@ FactoryGirl.define do note "Note" author - factory :note_on_commit, traits: [:on_commit] - factory :note_on_commit_diff, traits: [:on_commit, :on_diff] - factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note] - factory :note_on_merge_request, traits: [:on_merge_request] + factory :note_on_commit, traits: [:on_commit] + factory :note_on_commit_diff, traits: [:on_commit, :on_diff] + factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note] + factory :note_on_merge_request, traits: [:on_merge_request] factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff] - factory :note_on_project_snippet, traits: [:on_project_snippet] + factory :note_on_project_snippet, traits: [:on_project_snippet] + factory :system_note, traits: [:system] trait :on_commit do - project factory: :project + project commit_id RepoHelpers.sample_commit.id noteable_type "Commit" end @@ -43,7 +44,7 @@ FactoryGirl.define do end trait :on_merge_request do - project factory: :project + project noteable_id 1 noteable_type "MergeRequest" end @@ -58,6 +59,10 @@ FactoryGirl.define do noteable_type "Snippet" end + trait :system do + system true + end + trait :with_attachment do attachment { fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "`/png") } end diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb new file mode 100644 index 00000000000..046a9f6191d --- /dev/null +++ b/spec/features/login_spec.rb @@ -0,0 +1,101 @@ +require 'spec_helper' + +feature 'Login' do + describe 'with two-factor authentication' do + context 'with valid username/password' do + let(:user) { create(:user, :two_factor) } + + before do + login_with(user) + expect(page).to have_content('Two-factor Authentication') + end + + def enter_code(code) + fill_in 'Two-factor authentication code', with: code + click_button 'Verify code' + end + + it 'does not show a "You are already signed in." error message' do + enter_code(user.current_otp) + expect(page).not_to have_content('You are already signed in.') + end + + context 'using one-time code' do + it 'allows login with valid code' do + enter_code(user.current_otp) + expect(current_path).to eq root_path + end + + it 'blocks login with invalid code' do + enter_code('foo') + expect(page).to have_content('Invalid two-factor code') + end + + it 'allows login with invalid code, then valid code' do + enter_code('foo') + expect(page).to have_content('Invalid two-factor code') + + enter_code(user.current_otp) + expect(current_path).to eq root_path + end + end + + context 'using backup code' do + let(:codes) { user.generate_otp_backup_codes! } + + before do + expect(codes.size).to eq 10 + + # Ensure the generated codes get saved + user.save + end + + context 'with valid code' do + it 'allows login' do + enter_code(codes.sample) + expect(current_path).to eq root_path + end + + it 'invalidates the used code' do + expect { enter_code(codes.sample) }. + to change { user.reload.otp_backup_codes.size }.by(-1) + end + end + + context 'with invalid code' do + it 'blocks login' do + code = codes.sample + expect(user.invalidate_otp_backup_code!(code)).to eq true + + user.save! + expect(user.reload.otp_backup_codes.size).to eq 9 + + enter_code(code) + expect(page).to have_content('Invalid two-factor code.') + end + end + end + end + end + + describe 'without two-factor authentication' do + let(:user) { create(:user) } + + it 'allows basic login' do + login_with(user) + expect(current_path).to eq root_path + end + + it 'does not show a "You are already signed in." error message' do + login_with(user) + expect(page).not_to have_content('You are already signed in.') + end + + it 'blocks invalid login' do + user = create(:user, password: 'not-the-default') + + login_with(user) + expect(page).to have_content('Invalid email or password.') + end + end +end diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown_spec.rb index 57d01236505..8f3dfc8d5a9 100644 --- a/spec/features/markdown_spec.rb +++ b/spec/features/markdown_spec.rb @@ -24,6 +24,7 @@ require 'erb' # -> Rinku (http, https, ftp) # -> Other schemes # -> References +# -> TaskList # -> `html_safe` # -> Template # @@ -59,8 +60,8 @@ describe 'GitLab Markdown' do @feat.teardown end - # Given a header ID, goes to that element's parent (the header), then to its - # second sibling (the body). + # Given a header ID, goes to that element's parent (the header itself), then + # its next sibling element (the body). def get_section(id) @doc.at_css("##{id}").parent.next_element end @@ -118,18 +119,18 @@ describe 'GitLab Markdown' do describe 'HTML::Pipeline' do describe 'SanitizationFilter' do it 'uses a permissive whitelist' do - expect(@doc).to have_selector('b#manual-b') - expect(@doc).to have_selector('em#manual-em') - expect(@doc).to have_selector("code#manual-code") + expect(@doc).to have_selector('b:contains("b tag")') + expect(@doc).to have_selector('em:contains("em tag")') + expect(@doc).to have_selector('code:contains("code tag")') expect(@doc).to have_selector('kbd:contains("s")') expect(@doc).to have_selector('strike:contains(Emoji)') - expect(@doc).to have_selector('img#manual-img') - expect(@doc).to have_selector('br#manual-br') - expect(@doc).to have_selector('hr#manual-hr') + expect(@doc).to have_selector('img[src*="smile.png"]') + expect(@doc).to have_selector('br') + expect(@doc).to have_selector('hr') end it 'permits span elements' do - expect(@doc).to have_selector('span#span-class-light.light') + expect(@doc).to have_selector('span:contains("span tag")') end it 'permits table alignment' do @@ -143,13 +144,12 @@ describe 'GitLab Markdown' do end it 'removes `rel` attribute from links' do - expect(@doc).to have_selector('a#a-rel-nofollow') - expect(@doc).not_to have_selector('a#a-rel-nofollow[rel]') + body = get_section('sanitizationfilter') + expect(body).not_to have_selector('a[rel]') end it "removes `href` from `a` elements if it's fishy" do - expect(@doc).to have_selector('a#a-href-javascript') - expect(@doc).not_to have_selector('a#a-href-javascript[href]') + expect(@doc).not_to have_selector('a[href*="javascript"]') end end @@ -227,7 +227,8 @@ describe 'GitLab Markdown' do %w(code a kbd).each do |elem| it "ignores links inside '#{elem}' element" do - expect(@doc.at_css("#{elem}#autolink-#{elem}").child).to be_text + body = get_section('autolinkfilter') + expect(body).not_to have_selector("#{elem} a") end end end @@ -279,6 +280,15 @@ describe 'GitLab Markdown' do expect(body).to have_selector('a.gfm.gfm-label', count: 3) end end + + describe 'Task Lists' do + it 'generates task lists' do + body = get_section('task-lists') + expect(body).to have_selector('ul.task-list', count: 2) + expect(body).to have_selector('li.task-list-item', count: 7) + expect(body).to have_selector('input[checked]', count: 3) + end + end end end @@ -289,9 +299,8 @@ end # once. Unfortunately RSpec will not let you access `let`s in a `before(:all)` # block, so we fake it by encapsulating all the shared setup in this class. # -# The class contains the raw Markup used in the test, dynamically substituting -# real objects, created from factories and setup on-demand, when referenced in -# the Markdown. +# The class renders `spec/fixtures/markdown.md.erb` using ERB, allowing for +# reference to the factory-created objects. class MarkdownFeature include FactoryGirl::Syntax::Methods diff --git a/spec/features/password_reset_spec.rb b/spec/features/password_reset_spec.rb new file mode 100644 index 00000000000..a34efce09ef --- /dev/null +++ b/spec/features/password_reset_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +feature 'Password reset' do + def forgot_password + click_on 'Forgot your password?' + fill_in 'Email', with: user.email + click_button 'Reset password' + user.reload + end + + def get_reset_token + mail = ActionMailer::Base.deliveries.last + body = mail.body.encoded + body.scan(/reset_password_token=(.+)\"/).flatten.first + end + + def reset_password(password = 'password') + visit edit_user_password_path(reset_password_token: get_reset_token) + + fill_in 'New password', with: password + fill_in 'Confirm new password', with: password + click_button 'Change your password' + end + + describe 'with two-factor authentication' do + let(:user) { create(:user, :two_factor) } + + it 'requires login after password reset' do + visit root_path + + forgot_password + reset_password + + expect(page).to have_content("Your password was changed successfully.") + expect(page).not_to have_content("You are now signed in.") + expect(current_path).to eq new_user_session_path + end + end + + describe 'without two-factor authentication' do + let(:user) { create(:user) } + + it 'automatically logs in after password reset' do + visit root_path + + forgot_password + reset_password + + expect(current_path).to eq root_path + expect(page).to have_content("Your password was changed successfully. You are now signed in.") + end + end +end diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb new file mode 100644 index 00000000000..2099fc40cca --- /dev/null +++ b/spec/features/task_lists_spec.rb @@ -0,0 +1,151 @@ +require 'spec_helper' + +feature 'Task Lists' do + include Warden::Test::Helpers + + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:user2) { create(:user) } + + let(:markdown) do + <<-MARKDOWN.strip_heredoc + This is a task list: + + - [ ] Incomplete entry 1 + - [x] Complete entry 1 + - [ ] Incomplete entry 2 + - [x] Complete entry 2 + - [ ] Incomplete entry 3 + - [ ] Incomplete entry 4 + MARKDOWN + end + + before do + Warden.test_mode! + + project.team << [user, :master] + project.team << [user2, :guest] + + login_as(user) + end + + def visit_issue(project, issue) + visit namespace_project_issue_path(project.namespace, project, issue) + end + + describe 'for Issues' do + let!(:issue) { create(:issue, description: markdown, author: user, project: project) } + + it 'renders' do + visit_issue(project, issue) + + expect(page).to have_selector('ul.task-list', count: 1) + expect(page).to have_selector('li.task-list-item', count: 6) + expect(page).to have_selector('ul input[checked]', count: 2) + end + + it 'contains the required selectors' do + visit_issue(project, issue) + + container = '.issue-details .description.js-task-list-container' + + expect(page).to have_selector(container) + expect(page).to have_selector("#{container} .wiki .task-list .task-list-item .task-list-item-checkbox") + expect(page).to have_selector("#{container} .js-task-list-field") + expect(page).to have_selector('form.js-issue-update') + expect(page).to have_selector('a.btn-close') + end + + it 'is only editable by author' do + visit_issue(project, issue) + expect(page).to have_selector('.js-task-list-container') + + logout(:user) + + login_as(user2) + visit current_path + expect(page).not_to have_selector('.js-task-list-container') + end + + it 'provides a summary on Issues#index' do + visit namespace_project_issues_path(project.namespace, project) + expect(page).to have_content("6 tasks (2 completed, 4 remaining)") + end + end + + describe 'for Notes' do + let!(:issue) { create(:issue, author: user, project: project) } + let!(:note) { create(:note, note: markdown, noteable: issue, author: user) } + + it 'renders for note body' do + visit_issue(project, issue) + + expect(page).to have_selector('.note ul.task-list', count: 1) + expect(page).to have_selector('.note li.task-list-item', count: 6) + expect(page).to have_selector('.note ul input[checked]', count: 2) + end + + it 'contains the required selectors' do + visit_issue(project, issue) + + expect(page).to have_selector('.note .js-task-list-container') + expect(page).to have_selector('.note .js-task-list-container .task-list .task-list-item .task-list-item-checkbox') + expect(page).to have_selector('.note .js-task-list-container .js-task-list-field') + end + + it 'is only editable by author' do + visit_issue(project, issue) + expect(page).to have_selector('.js-task-list-container') + + logout(:user) + + login_as(user2) + visit current_path + expect(page).not_to have_selector('.js-task-list-container') + end + end + + describe 'for Merge Requests' do + def visit_merge_request(project, merge) + visit namespace_project_merge_request_path(project.namespace, project, merge) + end + + let!(:merge) { create(:merge_request, :simple, description: markdown, author: user, source_project: project) } + + it 'renders for description' do + visit_merge_request(project, merge) + + expect(page).to have_selector('ul.task-list', count: 1) + expect(page).to have_selector('li.task-list-item', count: 6) + expect(page).to have_selector('ul input[checked]', count: 2) + end + + it 'contains the required selectors' do + visit_merge_request(project, merge) + + container = '.merge-request-details .description.js-task-list-container' + + expect(page).to have_selector(container) + expect(page).to have_selector("#{container} .wiki .task-list .task-list-item .task-list-item-checkbox") + expect(page).to have_selector("#{container} .js-task-list-field") + expect(page).to have_selector('form.js-merge-request-update') + expect(page).to have_selector('a.btn-close') + end + + it 'is only editable by author' do + visit_merge_request(project, merge) + expect(page).to have_selector('.js-task-list-container') + + logout(:user) + + login_as(user2) + visit current_path + expect(page).not_to have_selector('.js-task-list-container') + end + + it 'provides a summary on MergeRequests#index' do + visit namespace_project_merge_requests_path(project.namespace, project) + expect(page).to have_content("6 tasks (2 completed, 4 remaining)") + end + end +end diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb index 6b79aad8f86..64817ec6700 100644 --- a/spec/fixtures/markdown.md.erb +++ b/spec/fixtures/markdown.md.erb @@ -54,36 +54,34 @@ After the Markdown has been turned into HTML, it gets passed through... ### SanitizationFilter -GitLab uses <a href="http://git.io/vfW8a" class="sanitize" id="sanitize-link">HTML::Pipeline::SanitizationFilter</a> +GitLab uses <a href="http://git.io/vfW8a">HTML::Pipeline::SanitizationFilter</a> to sanitize the generated HTML, stripping dangerous or unwanted tags. Its default whitelist is pretty permissive. Check it: -<b id="manual-b">This text is bold</b> and <em id="manual-em">this text is emphasized</em>. +<b>b tag</b> and <em>em tag</em>. -<code id="manual-code">echo "Hello, world!"</code> +<code>code tag</code> Press <kbd>s</kbd> to search. -<strike>Emoji</strike> Plain old images! <img -src="http://www.emoji-cheat-sheet.com/graphics/emojis/smile.png" width="20" -height="20" id="manual-img" /> +<strike>Emoji</strike> Plain old images! <img src="http://www.emoji-cheat-sheet.com/graphics/emojis/smile.png" width="20" height="20" /> Here comes a line break: -<br id="manual-br" /> +<br /> And a horizontal rule: -<hr id="manual-hr" /> +<hr /> As permissive as it is, we've allowed even more stuff: -<span class="light" id="span-class-light">Span elements</span> +<span>span tag</span> -<a href="#" rel="nofollow" id="a-rel-nofollow">This is a link with a defined rel attribute, which should be removed</a> +<a href="#" rel="nofollow">This is a link with a defined rel attribute, which should be removed</a> -<a href="javascript:alert('Hi')" id="a-href-javascript">This is a link trying to be sneaky. It gets its link removed entirely.</a> +<a href="javascript:alert('Hi')">This is a link trying to be sneaky. It gets its link removed entirely.</a> ### Escaping @@ -125,9 +123,9 @@ These are all plain text that should get turned into links: But it shouldn't autolink text inside certain tags: -- <code id="autolink-code">http://about.gitlab.com/</code> -- <a id="autolink-a">http://about.gitlab.com/</a> -- <kbd id="autolink-kbd">http://about.gitlab.com/</kbd> +- <code>http://about.gitlab.com/</code> +- <a>http://about.gitlab.com/</a> +- <kbd>http://about.gitlab.com/</kbd> ### Reference Filters (e.g., #<%= issue.iid %>) @@ -184,3 +182,13 @@ References should be parseable even inside _!<%= merge_request.iid %>_ emphasis. - Label by name in quotes: ~"<%= label.name %>" - Ignored in code: `~<%= simple_label.name %>` - Ignored in links: [Link to ~<%= simple_label.id %>](#label-link) + +### Task Lists + +- [ ] Incomplete task 1 +- [x] Complete task 1 +- [ ] Incomplete task 2 + - [ ] Incomplete sub-task 1 + - [ ] Incomplete sub-task 2 + - [x] Complete sub-task 1 +- [X] Complete task 2 diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 95719b4b49f..dd4c1d645e2 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -5,7 +5,8 @@ describe DiffHelper do let(:project) { create(:project) } let(:commit) { project.commit(sample_commit.id) } - let(:diff) { commit.diffs.first } + let(:diffs) { commit.diffs } + let(:diff) { diffs.first } let(:diff_file) { Gitlab::Diff::File.new(diff) } describe 'diff_hard_limit_enabled?' do @@ -30,6 +31,57 @@ describe DiffHelper do end end + describe 'allowed_diff_lines' do + it 'should return hard limit for number of lines in a diff if force diff is true' do + allow(controller).to receive(:params) { { force_show_diff: true } } + expect(allowed_diff_lines).to eq(50000) + end + + it 'should return safe limit for numbers of lines a diff if force diff is false' do + expect(allowed_diff_lines).to eq(5000) + end + end + + describe 'safe_diff_files' do + it 'should return all files from a commit that is smaller than safe limits' do + expect(safe_diff_files(diffs).length).to eq(2) + end + + it 'should return only the first file if the diff line count in the 2nd file takes the total beyond safe limits' do + diffs[1].diff.stub(lines: [""] * 4999) #simulate 4999 lines + expect(safe_diff_files(diffs).length).to eq(1) + end + + it 'should return all files from a commit that is beyond safe limit for numbers of lines if force diff is true' do + allow(controller).to receive(:params) { { force_show_diff: true } } + diffs[1].diff.stub(lines: [""] * 4999) #simulate 4999 lines + expect(safe_diff_files(diffs).length).to eq(2) + end + + it 'should return only the first file if the diff line count in the 2nd file takes the total beyond hard limits' do + allow(controller).to receive(:params) { { force_show_diff: true } } + diffs[1].diff.stub(lines: [""] * 49999) #simulate 49999 lines + expect(safe_diff_files(diffs).length).to eq(1) + end + + it 'should return only a safe number of file diffs if a commit touches more files than the safe limits' do + large_diffs = diffs * 100 #simulate 200 diffs + expect(safe_diff_files(large_diffs).length).to eq(100) + end + + it 'should return all file diffs if a commit touches more files than the safe limits but force diff is true' do + allow(controller).to receive(:params) { { force_show_diff: true } } + large_diffs = diffs * 100 #simulate 200 diffs + expect(safe_diff_files(large_diffs).length).to eq(200) + end + + it 'should return a limited file diffs if a commit touches more files than the hard limits and force diff is true' do + allow(controller).to receive(:params) { { force_show_diff: true } } + very_large_diffs = diffs * 1000 #simulate 2000 diffs + expect(safe_diff_files(very_large_diffs).length).to eq(1000) + end + end + describe 'parallel_diff' do it 'should return an array of arrays containing the parsed diff' do expect(parallel_diff(diff_file, 0)). diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index b6be82e4109..2f67879efdc 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -43,115 +43,6 @@ describe GitlabMarkdownHelper do expect(gfm(actual)).to match(expected) end end - - context 'parse_tasks: true' do - before(:all) do - @source_text_asterisk = <<-EOT.strip_heredoc - * [ ] valid unchecked task - * [x] valid lowercase checked task - * [X] valid uppercase checked task - * [ ] valid unchecked nested task - * [x] valid checked nested task - - [ ] not an unchecked task - no list item - [x] not a checked task - no list item - - * [ ] not an unchecked task - too many spaces - * [x ] not a checked task - too many spaces - * [] not an unchecked task - no spaces - * Not a task [ ] - not at beginning - EOT - - @source_text_dash = <<-EOT.strip_heredoc - - [ ] valid unchecked task - - [x] valid lowercase checked task - - [X] valid uppercase checked task - - [ ] valid unchecked nested task - - [x] valid checked nested task - EOT - end - - it 'should render checkboxes at beginning of asterisk list items' do - rendered_text = markdown(@source_text_asterisk, parse_tasks: true) - - expect(rendered_text).to match(/<input.*checkbox.*valid unchecked task/) - expect(rendered_text).to match( - /<input.*checkbox.*valid lowercase checked task/ - ) - expect(rendered_text).to match( - /<input.*checkbox.*valid uppercase checked task/ - ) - end - - it 'should render checkboxes at beginning of dash list items' do - rendered_text = markdown(@source_text_dash, parse_tasks: true) - - expect(rendered_text).to match(/<input.*checkbox.*valid unchecked task/) - expect(rendered_text).to match( - /<input.*checkbox.*valid lowercase checked task/ - ) - expect(rendered_text).to match( - /<input.*checkbox.*valid uppercase checked task/ - ) - end - - it 'should render checkboxes for nested tasks' do - rendered_text = markdown(@source_text_asterisk, parse_tasks: true) - - expect(rendered_text).to match( - /<input.*checkbox.*valid unchecked nested task/ - ) - expect(rendered_text).to match( - /<input.*checkbox.*valid checked nested task/ - ) - end - - it 'should not be confused by whitespace before bullets' do - rendered_text_asterisk = markdown(@source_text_asterisk, parse_tasks: true) - rendered_text_dash = markdown(@source_text_dash, parse_tasks: true) - - expect(rendered_text_asterisk).to match( - /<input.*checkbox.*valid unchecked nested task/ - ) - expect(rendered_text_asterisk).to match( - /<input.*checkbox.*valid checked nested task/ - ) - expect(rendered_text_dash).to match( - /<input.*checkbox.*valid unchecked nested task/ - ) - expect(rendered_text_dash).to match( - /<input.*checkbox.*valid checked nested task/ - ) - end - - it 'should not render checkboxes outside of list items' do - rendered_text = markdown(@source_text_asterisk, parse_tasks: true) - - expect(rendered_text).not_to match( - /<input.*checkbox.*not an unchecked task - no list item/ - ) - expect(rendered_text).not_to match( - /<input.*checkbox.*not a checked task - no list item/ - ) - end - - it 'should not render checkboxes with invalid formatting' do - rendered_text = markdown(@source_text_asterisk, parse_tasks: true) - - expect(rendered_text).not_to match( - /<input.*checkbox.*not an unchecked task - too many spaces/ - ) - expect(rendered_text).not_to match( - /<input.*checkbox.*not a checked task - too many spaces/ - ) - expect(rendered_text).not_to match( - /<input.*checkbox.*not an unchecked task - no spaces/ - ) - expect(rendered_text).not_to match( - /Not a task.*<input.*checkbox.*not at beginning/ - ) - end - end end describe '#link_to_gfm' do diff --git a/spec/javascripts/helpers/.gitkeep b/spec/javascripts/helpers/.gitkeep deleted file mode 100644 index e69de29bb2d..00000000000 --- a/spec/javascripts/helpers/.gitkeep +++ /dev/null diff --git a/spec/javascripts/issue_spec.js.coffee b/spec/javascripts/issue_spec.js.coffee new file mode 100644 index 00000000000..13b25862f57 --- /dev/null +++ b/spec/javascripts/issue_spec.js.coffee @@ -0,0 +1,36 @@ +#= require jquery +#= require jasmine-fixture +#= require issue + +describe 'Issue', -> + describe 'task lists', -> + selectors = { + container: '.issue-details .description.js-task-list-container' + item: '.wiki ul.task-list li.task-list-item input.task-list-item-checkbox[type=checkbox] {Task List Item}' + textarea: '.wiki textarea.js-task-list-field{- [ ] Task List Item}' + form: 'form.js-issue-update[action="/foo"]' + close: 'a.btn-close' + } + + beforeEach -> + $container = affix(selectors.container) + + # # These two elements are siblings inside the container + $container.find('.js-task-list-container').append(affix(selectors.item)) + $container.find('.js-task-list-container').append(affix(selectors.textarea)) + + # Task lists don't get initialized unless this button exists. Not ideal. + $container.append(affix(selectors.close)) + + # This form is used to get the `update` URL. Not ideal. + $container.append(affix(selectors.form)) + + @issue = new Issue() + + it 'submits an ajax request on tasklist:changed', -> + spyOn($, 'ajax').and.callFake (req) -> + expect(req.type).toBe('PATCH') + expect(req.url).toBe('/foo') + expect(req.data.issue.description).not.toBe(null) + + $('.js-task-list-field').trigger('tasklist:changed') diff --git a/spec/javascripts/merge_request_spec.js.coffee b/spec/javascripts/merge_request_spec.js.coffee new file mode 100644 index 00000000000..3ebc4a4eed5 --- /dev/null +++ b/spec/javascripts/merge_request_spec.js.coffee @@ -0,0 +1,36 @@ +#= require jquery +#= require jasmine-fixture +#= require merge_request + +describe 'MergeRequest', -> + describe 'task lists', -> + selectors = { + container: '.merge-request-details .description.js-task-list-container' + item: '.wiki ul.task-list li.task-list-item input.task-list-item-checkbox[type=checkbox] {Task List Item}' + textarea: '.wiki textarea.js-task-list-field{- [ ] Task List Item}' + form: 'form.js-merge-request-update[action="/foo"]' + close: 'a.btn-close' + } + + beforeEach -> + $container = affix(selectors.container) + + # # These two elements are siblings inside the container + $container.find('.js-task-list-container').append(affix(selectors.item)) + $container.find('.js-task-list-container').append(affix(selectors.textarea)) + + # Task lists don't get initialized unless this button exists. Not ideal. + $container.append(affix(selectors.close)) + + # This form is used to get the `update` URL. Not ideal. + $container.append(affix(selectors.form)) + + @merge = new MergeRequest({}) + + it 'submits an ajax request on tasklist:changed', -> + spyOn($, 'ajax').and.callFake (req) -> + expect(req.type).toBe('PATCH') + expect(req.url).toBe('/foo') + expect(req.data.merge_request.description).not.toBe(null) + + $('.js-task-list-field').trigger('tasklist:changed') diff --git a/spec/javascripts/notes_spec.js.coffee b/spec/javascripts/notes_spec.js.coffee new file mode 100644 index 00000000000..de2e8e7f6c8 --- /dev/null +++ b/spec/javascripts/notes_spec.js.coffee @@ -0,0 +1,30 @@ +#= require jquery +#= require jasmine-fixture +#= require notes + +window.gon = {} +window.disableButtonIfEmptyField = -> null + +describe 'Notes', -> + describe 'task lists', -> + selectors = { + container: 'li.note .js-task-list-container' + item: '.note-text ul.task-list li.task-list-item input.task-list-item-checkbox[type=checkbox] {Task List Item}' + textarea: '.note-edit-form form textarea.js-task-list-field{- [ ] Task List Item}' + } + + beforeEach -> + $container = affix(selectors.container) + + # These two elements are siblings inside the container + $container.find('.js-task-list-container').append(affix(selectors.item)) + $container.find('.js-task-list-container').append(affix(selectors.textarea)) + + @notes = new Notes() + + it 'submits the form on tasklist:changed', -> + submitted = false + $('form').on 'submit', (e) -> submitted = true; e.preventDefault() + + $('.js-task-list-field').trigger('tasklist:changed') + expect(submitted).toBe(true) diff --git a/spec/javascripts/support/jasmine.yml b/spec/javascripts/support/jasmine.yml index f4b01c9f27a..168c9618643 100644 --- a/spec/javascripts/support/jasmine.yml +++ b/spec/javascripts/support/jasmine.yml @@ -9,11 +9,6 @@ # defaults to spec/javascripts spec_dir: spec/javascripts -# list of file expressions to include as helpers into spec runner -# relative path from spec_dir -helpers: - - "helpers/**/*.{js.coffee,js,coffee}" - # list of file expressions to include as specs into spec runner # relative path from spec_dir spec_files: diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 707a0521ab3..2189e313d6a 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -16,7 +16,7 @@ describe Gitlab::LDAP::Access do context 'when the user is found' do before { Gitlab::LDAP::Person.stub(find_by_dn: :ldap_user) } - context 'and the user is diabled via active directory' do + context 'and the user is disabled via active directory' do before { Gitlab::LDAP::Person.stub(disabled_via_active_directory?: true) } it { is_expected.to be_falsey } @@ -36,9 +36,28 @@ describe Gitlab::LDAP::Access do it { is_expected.to be_truthy } - it "should unblock user in GitLab" do - access.allowed? - user.should_not be_blocked + context 'when auto-created users are blocked' do + + before do + Gitlab::LDAP::Config.any_instance.stub(block_auto_created_users: true) + end + + it "does not unblock user in GitLab" do + access.allowed? + user.should be_blocked + end + end + + context "when auto-created users are not blocked" do + + before do + Gitlab::LDAP::Config.any_instance.stub(block_auto_created_users: false) + end + + it "should unblock user in GitLab" do + access.allowed? + user.should_not be_blocked + end end end diff --git a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb index ab909a68635..4a1aa766149 100644 --- a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb +++ b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb @@ -29,17 +29,27 @@ module Gitlab::Markdown exp = act = "<dl>\n<dt>Term</dt>\n<dd>Definition</dd>\n</dl>" expect(filter(act).to_html).to eq exp end + + it 'sanitizes `class` attribute on any element' do + act = %q{<strong class="foo">Strong</strong>} + expect(filter(act).to_html).to eq %q{<strong>Strong</strong>} + end + + it 'sanitizes `id` attribute on any element' do + act = %q{<em id="foo">Emphasis</em>} + expect(filter(act).to_html).to eq %q{<em>Emphasis</em>} + end end describe 'custom whitelist' do - it 'allows `class` attribute on any element' do - exp = act = %q{<strong class="foo">Strong</strong>} + it 'allows syntax highlighting' do + exp = act = %q{<pre class="code highlight white c"><code><span class="k">def</span></code></pre>} expect(filter(act).to_html).to eq exp end - it 'allows `id` attribute on any element' do - exp = act = %q{<em id="foo">Emphasis</em>} - expect(filter(act).to_html).to eq exp + it 'sanitizes `class` attribute from non-highlight spans' do + act = %q{<span class="k">def</span>} + expect(filter(act).to_html).to eq %q{<span>def</span>} end it 'allows `style` attribute on table elements' do diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index f626efe1f76..dbcf7286e45 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -5,9 +5,9 @@ describe Notify do include EmailSpec::Matchers include RepoHelpers - let(:gitlab_sender_display_name) { Gitlab.config.outgoing_emails.display_name } - let(:gitlab_sender) { Gitlab.config.outgoing_emails.from } - let(:gitlab_sender_reply_to) { Gitlab.config.outgoing_emails.reply_to } + let(:gitlab_sender_display_name) { Gitlab.config.gitlab.email_display_name } + let(:gitlab_sender) { Gitlab.config.gitlab.email_from } + let(:gitlab_sender_reply_to) { Gitlab.config.gitlab.email_reply_to } let(:recipient) { create(:user, email: 'recipient@example.com') } let(:project) { create(:project) } diff --git a/spec/models/hooks/service_hook_spec.rb b/spec/models/hooks/service_hook_spec.rb index 96bf74d45da..d9714596f5d 100644 --- a/spec/models/hooks/service_hook_spec.rb +++ b/spec/models/hooks/service_hook_spec.rb @@ -21,4 +21,37 @@ describe ServiceHook do describe "Associations" do it { is_expected.to belong_to :service } end + + describe "execute" do + before(:each) do + @service_hook = create(:service_hook) + @data = { project_id: 1, data: {}} + + WebMock.stub_request(:post, @service_hook.url) + end + + it "POSTs to the web hook URL" do + @service_hook.execute(@data) + expect(WebMock).to have_requested(:post, @service_hook.url).with( + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Service Hook'} + ).once + end + + it "POSTs the data as JSON" do + json = @data.to_json + + @service_hook.execute(@data) + expect(WebMock).to have_requested(:post, @service_hook.url).with( + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Service Hook'} + ).once + end + + it "catches exceptions" do + expect(WebHook).to receive(:post).and_raise("Some HTTP Post error") + + expect { + @service_hook.execute(@data) + }.to raise_error + end + end end diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 810b311a40b..e4b6b886565 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -26,32 +26,47 @@ describe SystemHook do it "project_create hook" do Projects::CreateService.new(create(:user), name: 'empty').execute - expect(WebMock).to have_requested(:post, @system_hook.url).with(body: /project_create/).once + expect(WebMock).to have_requested(:post, @system_hook.url).with( + body: /project_create/, + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'} + ).once end it "project_destroy hook" do user = create(:user) project = create(:empty_project, namespace: user.namespace) Projects::DestroyService.new(project, user, {}).execute - expect(WebMock).to have_requested(:post, @system_hook.url).with(body: /project_destroy/).once + expect(WebMock).to have_requested(:post, @system_hook.url).with( + body: /project_destroy/, + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'} + ).once end it "user_create hook" do create(:user) - expect(WebMock).to have_requested(:post, @system_hook.url).with(body: /user_create/).once + expect(WebMock).to have_requested(:post, @system_hook.url).with( + body: /user_create/, + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'} + ).once end it "user_destroy hook" do user = create(:user) user.destroy - expect(WebMock).to have_requested(:post, @system_hook.url).with(body: /user_destroy/).once + expect(WebMock).to have_requested(:post, @system_hook.url).with( + body: /user_destroy/, + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'} + ).once end it "project_create hook" do user = create(:user) project = create(:project) project.team << [user, :master] - expect(WebMock).to have_requested(:post, @system_hook.url).with(body: /user_add_to_team/).once + expect(WebMock).to have_requested(:post, @system_hook.url).with( + body: /user_add_to_team/, + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'} + ).once end it "project_destroy hook" do @@ -59,13 +74,17 @@ describe SystemHook do project = create(:project) project.team << [user, :master] project.project_members.destroy_all - expect(WebMock).to have_requested(:post, @system_hook.url).with(body: /user_remove_from_team/).once + expect(WebMock).to have_requested(:post, @system_hook.url).with( + body: /user_remove_from_team/, + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'} + ).once end it 'group create hook' do create(:group) expect(WebMock).to have_requested(:post, @system_hook.url).with( - body: /group_create/ + body: /group_create/, + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'} ).once end @@ -73,7 +92,8 @@ describe SystemHook do group = create(:group) group.destroy expect(WebMock).to have_requested(:post, @system_hook.url).with( - body: /group_destroy/ + body: /group_destroy/, + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'} ).once end @@ -82,7 +102,8 @@ describe SystemHook do user = create(:user) group.add_user(user, Gitlab::Access::MASTER) expect(WebMock).to have_requested(:post, @system_hook.url).with( - body: /user_add_to_group/ + body: /user_add_to_group/, + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'} ).once end @@ -92,7 +113,8 @@ describe SystemHook do group.add_user(user, Gitlab::Access::MASTER) group.group_members.destroy_all expect(WebMock).to have_requested(:post, @system_hook.url).with( - body: /user_remove_from_group/ + body: /user_remove_from_group/, + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'} ).once end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 67ec9193ad7..9f5ef3eff70 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -52,22 +52,26 @@ describe ProjectHook do end it "POSTs to the web hook URL" do - @project_hook.execute(@data) - expect(WebMock).to have_requested(:post, @project_hook.url).once + @project_hook.execute(@data, 'push_hooks') + expect(WebMock).to have_requested(:post, @project_hook.url).with( + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Push Hook'} + ).once end it "POSTs the data as JSON" do json = @data.to_json - @project_hook.execute(@data) - expect(WebMock).to have_requested(:post, @project_hook.url).with(body: json).once + @project_hook.execute(@data, 'push_hooks') + expect(WebMock).to have_requested(:post, @project_hook.url).with( + headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Push Hook'} + ).once end it "catches exceptions" do expect(WebHook).to receive(:post).and_raise("Some HTTP Post error") expect { - @project_hook.execute(@data) + @project_hook.execute(@data, 'push_hooks') }.to raise_error end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 4a6bfdb2910..ddacba58261 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -20,68 +20,88 @@ require 'spec_helper' describe Note do - describe "Associations" do + describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:noteable) } it { is_expected.to belong_to(:author).class_name('User') } end - describe "Mass assignment" do - end - - describe "Validation" do + describe 'validation' do it { is_expected.to validate_presence_of(:note) } it { is_expected.to validate_presence_of(:project) } end - describe "Voting score" do - let(:project) { create(:project) } + describe '#votable?' do + it 'is true for issue notes' do + note = build(:note_on_issue) + expect(note).to be_votable + end + + it 'is true for merge request notes' do + note = build(:note_on_merge_request) + expect(note).to be_votable + end + + it 'is false for merge request diff notes' do + note = build(:note_on_merge_request_diff) + expect(note).not_to be_votable + end + + it 'is false for commit notes' do + note = build(:note_on_commit) + expect(note).not_to be_votable + end - it "recognizes a neutral note" do - note = create(:votable_note, note: "This is not a +1 note") + it 'is false for commit diff notes' do + note = build(:note_on_commit_diff) + expect(note).not_to be_votable + end + end + + describe 'voting score' do + it 'recognizes a neutral note' do + note = build(:votable_note, note: 'This is not a +1 note') expect(note).not_to be_upvote expect(note).not_to be_downvote end - it "recognizes a neutral emoji note" do + it 'recognizes a neutral emoji note' do note = build(:votable_note, note: "I would :+1: this, but I don't want to") expect(note).not_to be_upvote expect(note).not_to be_downvote end - it "recognizes a +1 note" do - note = create(:votable_note, note: "+1 for this") + it 'recognizes a +1 note' do + note = build(:votable_note, note: '+1 for this') expect(note).to be_upvote end - it "recognizes a +1 emoji as a vote" do - note = build(:votable_note, note: ":+1: for this") + it 'recognizes a +1 emoji as a vote' do + note = build(:votable_note, note: ':+1: for this') expect(note).to be_upvote end - it "recognizes a thumbsup emoji as a vote" do - note = build(:votable_note, note: ":thumbsup: for this") + it 'recognizes a thumbsup emoji as a vote' do + note = build(:votable_note, note: ':thumbsup: for this') expect(note).to be_upvote end - it "recognizes a -1 note" do - note = create(:votable_note, note: "-1 for this") + it 'recognizes a -1 note' do + note = build(:votable_note, note: '-1 for this') expect(note).to be_downvote end - it "recognizes a -1 emoji as a vote" do - note = build(:votable_note, note: ":-1: for this") + it 'recognizes a -1 emoji as a vote' do + note = build(:votable_note, note: ':-1: for this') expect(note).to be_downvote end - it "recognizes a thumbsdown emoji as a vote" do - note = build(:votable_note, note: ":thumbsdown: for this") + it 'recognizes a thumbsdown emoji as a vote' do + note = build(:votable_note, note: ':thumbsdown: for this') expect(note).to be_downvote end end - let(:project) { create(:project) } - describe "Commit notes" do let!(:note) { create(:note_on_commit, note: "+1 from me") } let!(:commit) { note.noteable } @@ -100,10 +120,6 @@ describe Note do it "should be recognized by #for_commit?" do expect(note).to be_for_commit end - - it "should not be votable" do - expect(note).not_to be_votable - end end describe "Commit diff line notes" do @@ -128,461 +144,7 @@ describe Note do end end - describe "Issue notes" do - let!(:note) { create(:note_on_issue, note: "+1 from me") } - - it "should not be votable" do - expect(note).to be_votable - end - end - - describe "Merge request notes" do - let!(:note) { create(:note_on_merge_request, note: "+1 from me") } - - it "should be votable" do - expect(note).to be_votable - end - end - - describe "Merge request diff line notes" do - let!(:note) { create(:note_on_merge_request_diff, note: "+1 from me") } - - it "should not be votable" do - expect(note).not_to be_votable - end - end - - describe '#create_status_change_note' do - let(:project) { create(:project) } - let(:thing) { create(:issue, project: project) } - let(:author) { create(:user) } - let(:status) { 'new_status' } - - subject { Note.create_status_change_note(thing, project, author, status, nil) } - - it 'creates and saves a Note' do - is_expected.to be_a Note - expect(subject.id).not_to be_nil - end - - describe '#noteable' do - subject { super().noteable } - it { is_expected.to eq(thing) } - end - - describe '#project' do - subject { super().project } - it { is_expected.to eq(thing.project) } - end - - describe '#author' do - subject { super().author } - it { is_expected.to eq(author) } - end - - describe '#note' do - subject { super().note } - it { is_expected.to eq("Status changed to #{status}") } - end - - it 'appends a back-reference if a closing mentionable is supplied' do - commit = double('commit', gfm_reference: 'commit 123456') - n = Note.create_status_change_note(thing, project, author, status, commit) - - expect(n.note).to eq("Status changed to #{status} by commit 123456") - end - end - - describe '#create_assignee_change_note' do - let(:project) { create(:project) } - let(:thing) { create(:issue, project: project) } - let(:author) { create(:user) } - let(:assignee) { create(:user, username: "assigned_user") } - - subject { Note.create_assignee_change_note(thing, project, author, assignee) } - - context 'creates and saves a Note' do - it { is_expected.to be_a Note } - - describe '#id' do - subject { super().id } - it { is_expected.not_to be_nil } - end - end - - describe '#noteable' do - subject { super().noteable } - it { is_expected.to eq(thing) } - end - - describe '#project' do - subject { super().project } - it { is_expected.to eq(thing.project) } - end - - describe '#author' do - subject { super().author } - it { is_expected.to eq(author) } - end - - describe '#note' do - subject { super().note } - it { is_expected.to eq('Reassigned to @assigned_user') } - end - - context 'assignee is removed' do - let(:assignee) { nil } - - describe '#note' do - subject { super().note } - it { is_expected.to eq('Assignee removed') } - end - end - end - - describe '#create_labels_change_note' do - let(:project) { create(:project) } - let(:thing) { create(:issue, project: project) } - let(:author) { create(:user) } - let(:label1) { create(:label) } - let(:label2) { create(:label) } - let(:added_labels) { [label1, label2] } - let(:removed_labels) { [] } - - subject { Note.create_labels_change_note(thing, project, author, added_labels, removed_labels) } - - context 'creates and saves a Note' do - it { is_expected.to be_a Note } - - describe '#id' do - subject { super().id } - it { is_expected.not_to be_nil } - end - end - - describe '#noteable' do - subject { super().noteable } - it { is_expected.to eq(thing) } - end - - describe '#project' do - subject { super().project } - it { is_expected.to eq(thing.project) } - end - - describe '#author' do - subject { super().author } - it { is_expected.to eq(author) } - end - - describe '#note' do - subject { super().note } - it { is_expected.to eq("Added ~#{label1.id} ~#{label2.id} labels") } - end - - context 'label is removed' do - let(:added_labels) { [label1] } - let(:removed_labels) { [label2] } - - describe '#note' do - subject { super().note } - it { is_expected.to eq("Added ~#{label1.id} and removed ~#{label2.id} labels") } - end - end - end - - describe '#create_milestone_change_note' do - let(:project) { create(:project) } - let(:thing) { create(:issue, project: project) } - let(:milestone) { create(:milestone, project: project, title: "first_milestone") } - let(:author) { create(:user) } - - subject { Note.create_milestone_change_note(thing, project, author, milestone) } - - context 'creates and saves a Note' do - it { is_expected.to be_a Note } - - describe '#id' do - subject { super().id } - it { is_expected.not_to be_nil } - end - end - - describe '#project' do - subject { super().project } - it { is_expected.to eq(thing.project) } - end - - describe '#author' do - subject { super().author } - it { is_expected.to eq(author) } - end - - describe '#note' do - subject { super().note } - it { is_expected.to eq("Milestone changed to first_milestone") } - end - end - - describe '#create_cross_reference_note' do - let(:project) { create(:project) } - let(:author) { create(:user) } - let(:issue) { create(:issue, project: project) } - let(:mergereq) { create(:merge_request, :simple, target_project: project, source_project: project) } - let(:commit) { project.commit } - - # Test all of {issue, merge request, commit} in both the referenced and referencing - # roles, to ensure that the correct information can be inferred from any argument. - - context 'issue from a merge request' do - subject { Note.create_cross_reference_note(issue, mergereq, author) } - - it { is_expected.to be_valid } - - describe '#noteable' do - subject { super().noteable } - it { is_expected.to eq(issue) } - end - - describe '#project' do - subject { super().project } - it { is_expected.to eq(issue.project) } - end - - describe '#author' do - subject { super().author } - it { is_expected.to eq(author) } - end - - describe '#note' do - subject { super().note } - it { is_expected.to eq("mentioned in merge request !#{mergereq.iid}") } - end - end - - context 'issue from a commit' do - subject { Note.create_cross_reference_note(issue, commit, author) } - - it { is_expected.to be_valid } - - describe '#noteable' do - subject { super().noteable } - it { is_expected.to eq(issue) } - end - - describe '#note' do - subject { super().note } - it { is_expected.to eq("mentioned in commit #{commit.sha}") } - end - end - - context 'merge request from an issue' do - subject { Note.create_cross_reference_note(mergereq, issue, author) } - - it { is_expected.to be_valid } - - describe '#noteable' do - subject { super().noteable } - it { is_expected.to eq(mergereq) } - end - - describe '#project' do - subject { super().project } - it { is_expected.to eq(mergereq.project) } - end - - describe '#note' do - subject { super().note } - it { is_expected.to eq("mentioned in issue ##{issue.iid}") } - end - end - - context 'commit from a merge request' do - subject { Note.create_cross_reference_note(commit, mergereq, author) } - - it { is_expected.to be_valid } - - describe '#noteable' do - subject { super().noteable } - it { is_expected.to eq(commit) } - end - - describe '#project' do - subject { super().project } - it { is_expected.to eq(project) } - end - - describe '#note' do - subject { super().note } - it { is_expected.to eq("mentioned in merge request !#{mergereq.iid}") } - end - end - - context 'commit contained in a merge request' do - subject { Note.create_cross_reference_note(mergereq.commits.first, mergereq, author) } - - it { is_expected.to be_nil } - end - - context 'commit from issue' do - subject { Note.create_cross_reference_note(commit, issue, author) } - - it { is_expected.to be_valid } - - describe '#noteable_type' do - subject { super().noteable_type } - it { is_expected.to eq("Commit") } - end - - describe '#noteable_id' do - subject { super().noteable_id } - it { is_expected.to be_nil } - end - - describe '#commit_id' do - subject { super().commit_id } - it { is_expected.to eq(commit.id) } - end - - describe '#note' do - subject { super().note } - it { is_expected.to eq("mentioned in issue ##{issue.iid}") } - end - end - - context 'commit from commit' do - let(:parent_commit) { commit.parents.first } - subject { Note.create_cross_reference_note(commit, parent_commit, author) } - - it { is_expected.to be_valid } - - describe '#noteable_type' do - subject { super().noteable_type } - it { is_expected.to eq("Commit") } - end - - describe '#noteable_id' do - subject { super().noteable_id } - it { is_expected.to be_nil } - end - - describe '#commit_id' do - subject { super().commit_id } - it { is_expected.to eq(commit.id) } - end - - describe '#note' do - subject { super().note } - it { is_expected.to eq("mentioned in commit #{parent_commit.id}") } - end - end - end - - describe '#cross_reference_exists?' do - let(:project) { create :project } - let(:author) { create :user } - let(:issue) { create :issue } - let(:commit0) { project.commit } - let(:commit1) { project.commit('HEAD~2') } - - before do - Note.create_cross_reference_note(issue, commit0, author) - end - - it 'detects if a mentionable has already been mentioned' do - expect(Note.cross_reference_exists?(issue, commit0)).to be_truthy - end - - it 'detects if a mentionable has not already been mentioned' do - expect(Note.cross_reference_exists?(issue, commit1)).to be_falsey - end - - context 'commit on commit' do - before do - Note.create_cross_reference_note(commit0, commit1, author) - end - - it { expect(Note.cross_reference_exists?(commit0, commit1)).to be_truthy } - it { expect(Note.cross_reference_exists?(commit1, commit0)).to be_falsey } - end - - context 'legacy note with Markdown emphasis' do - let(:issue2) { create :issue, project: project } - let!(:note) do - create :note, system: true, noteable_id: issue2.id, - noteable_type: "Issue", note: "_mentioned in issue " \ - "#{issue.project.path_with_namespace}##{issue.iid}_" - end - - it 'detects if a mentionable with emphasis has been mentioned' do - expect(Note.cross_reference_exists?(issue2, issue)).to be_truthy - end - end - end - - describe '#cross_references_with_underscores?' do - let(:project) { create :project, path: "first_project" } - let(:second_project) { create :project, path: "second_project" } - - let(:author) { create :user } - let(:issue0) { create :issue, project: project } - let(:issue1) { create :issue, project: second_project } - let!(:note) { Note.create_cross_reference_note(issue0, issue1, author) } - - it 'detects if a mentionable has already been mentioned' do - expect(Note.cross_reference_exists?(issue0, issue1)).to be_truthy - end - - it 'detects if a mentionable has not already been mentioned' do - expect(Note.cross_reference_exists?(issue1, issue0)).to be_falsey - end - - it 'detects that text has underscores' do - expect(note.note).to eq("mentioned in issue #{second_project.path_with_namespace}##{issue1.iid}") - end - end - - describe '#system?' do - let(:project) { create(:project) } - let(:issue) { create(:issue, project: project) } - let(:other) { create(:issue, project: project) } - let(:author) { create(:user) } - let(:assignee) { create(:user) } - let(:label) { create(:label) } - let(:milestone) { create(:milestone) } - - it 'should recognize user-supplied notes as non-system' do - @note = create(:note_on_issue) - expect(@note).not_to be_system - end - - it 'should identify status-change notes as system notes' do - @note = Note.create_status_change_note(issue, project, author, 'closed', nil) - expect(@note).to be_system - end - - it 'should identify cross-reference notes as system notes' do - @note = Note.create_cross_reference_note(issue, other, author) - expect(@note).to be_system - end - - it 'should identify assignee-change notes as system notes' do - @note = Note.create_assignee_change_note(issue, project, author, assignee) - expect(@note).to be_system - end - - it 'should identify label-change notes as system notes' do - @note = Note.create_labels_change_note(issue, project, author, [label], []) - expect(@note).to be_system - end - - it 'should identify milestone-change notes as system notes' do - @note = Note.create_milestone_change_note(issue, project, author, milestone) - expect(@note).to be_system - end - end - - describe :authorization do + describe 'authorization' do before do @p1 = create(:project) @p2 = create(:project) @@ -593,7 +155,7 @@ describe Note do @abilities << Ability end - describe :read do + describe 'read' do before do @p1.project_members.create(user: @u2, access_level: ProjectMember::GUEST) @p2.project_members.create(user: @u3, access_level: ProjectMember::GUEST) @@ -604,7 +166,7 @@ describe Note do it { expect(@abilities.allowed?(@u3, :read_note, @p1)).to be_falsey } end - describe :write do + describe 'write' do before do @p1.project_members.create(user: @u2, access_level: ProjectMember::DEVELOPER) @p2.project_members.create(user: @u3, access_level: ProjectMember::DEVELOPER) @@ -615,7 +177,7 @@ describe Note do it { expect(@abilities.allowed?(@u3, :write_note, @p1)).to be_falsey } end - describe :admin do + describe 'admin' do before do @p1.project_members.create(user: @u1, access_level: ProjectMember::REPORTER) @p1.project_members.create(user: @u2, access_level: ProjectMember::MASTER) @@ -631,6 +193,7 @@ describe Note do it_behaves_like 'an editable mentionable' do subject { create :note, noteable: issue, project: project } + let(:project) { create(:project) } let(:issue) { create :issue, project: project } let(:backref_text) { issue.gfm_reference } let(:set_mentionable_text) { ->(txt) { subject.note = txt } } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 771709c127a..0dddcd5bda2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -50,6 +50,11 @@ # bitbucket_access_token :string(255) # bitbucket_access_token_secret :string(255) # location :string(255) +# encrypted_otp_secret :string(255) +# encrypted_otp_secret_iv :string(255) +# encrypted_otp_secret_salt :string(255) +# otp_required_for_login :boolean +# otp_backup_codes :text # public_email :string(255) default(""), not null # diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb new file mode 100644 index 00000000000..4e4cb6d19ed --- /dev/null +++ b/spec/services/system_note_service_spec.rb @@ -0,0 +1,346 @@ +require 'spec_helper' + +describe SystemNoteService do + let(:project) { create(:project) } + let(:author) { create(:user) } + let(:noteable) { create(:issue, project: project) } + + shared_examples_for 'a system note' do + it 'is valid' do + expect(subject).to be_valid + end + + it 'sets the noteable model' do + expect(subject.noteable).to eq noteable + end + + it 'sets the project' do + expect(subject.project).to eq project + end + + it 'sets the author' do + expect(subject.author).to eq author + end + + it 'is a system note' do + expect(subject).to be_system + end + end + + describe '.add_commits' do + subject { described_class.add_commits(noteable, project, author, new_commits, old_commits, oldrev) } + + let(:noteable) { create(:merge_request, source_project: project) } + let(:new_commits) { noteable.commits } + let(:old_commits) { [] } + let(:oldrev) { nil } + + it_behaves_like 'a system note' + + describe 'note body' do + let(:note_lines) { subject.note.split("\n").reject(&:blank?) } + + context 'without existing commits' do + it 'adds a message header' do + expect(note_lines[0]).to eq "Added #{new_commits.size} commits:" + end + + it 'adds a message line for each commit' do + new_commits.each_with_index do |commit, i| + # Skip the header + expect(note_lines[i + 1]).to eq "* #{commit.short_id} - #{commit.title}" + end + end + end + + describe 'summary line for existing commits' do + let(:summary_line) { note_lines[1] } + + context 'with one existing commit' do + let(:old_commits) { [noteable.commits.last] } + + it 'includes the existing commit' do + expect(summary_line).to eq "* #{old_commits.first.short_id} - 1 commit from branch `feature`" + end + end + + context 'with multiple existing commits' do + let(:old_commits) { noteable.commits[3..-1] } + + context 'with oldrev' do + let(:oldrev) { noteable.commits[2].id } + + it 'includes a commit range' do + expect(summary_line).to start_with "* #{Commit.truncate_sha(oldrev)}...#{old_commits.last.short_id}" + end + + it 'includes a commit count' do + expect(summary_line).to end_with " - 2 commits from branch `feature`" + end + end + + context 'without oldrev' do + it 'includes a commit range' do + expect(summary_line).to start_with "* #{old_commits[0].short_id}..#{old_commits[-1].short_id}" + end + + it 'includes a commit count' do + expect(summary_line).to end_with " - 2 commits from branch `feature`" + end + end + + context 'on a fork' do + before do + expect(noteable).to receive(:for_fork?).and_return(true) + end + + it 'includes the project namespace' do + expect(summary_line).to end_with "`#{noteable.target_project_namespace}:feature`" + end + end + end + end + end + end + + describe '.change_assignee' do + subject { described_class.change_assignee(noteable, project, author, assignee) } + + let(:assignee) { create(:user) } + + it_behaves_like 'a system note' + + context 'when assignee added' do + it 'sets the note text' do + expect(subject.note).to eq "Reassigned to @#{assignee.username}" + end + end + + context 'when assignee removed' do + let(:assignee) { nil } + + it 'sets the note text' do + expect(subject.note).to eq 'Assignee removed' + end + end + end + + describe '.change_label' do + subject { described_class.change_label(noteable, project, author, added, removed) } + + let(:labels) { create_list(:label, 2) } + let(:added) { [] } + let(:removed) { [] } + + it_behaves_like 'a system note' + + context 'with added labels' do + let(:added) { labels } + let(:removed) { [] } + + it 'sets the note text' do + expect(subject.note).to eq "Added ~#{labels[0].id} ~#{labels[1].id} labels" + end + end + + context 'with removed labels' do + let(:added) { [] } + let(:removed) { labels } + + it 'sets the note text' do + expect(subject.note).to eq "Removed ~#{labels[0].id} ~#{labels[1].id} labels" + end + end + + context 'with added and removed labels' do + let(:added) { [labels[0]] } + let(:removed) { [labels[1]] } + + it 'sets the note text' do + expect(subject.note).to eq "Added ~#{labels[0].id} and removed ~#{labels[1].id} labels" + end + end + end + + describe '.change_milestone' do + subject { described_class.change_milestone(noteable, project, author, milestone) } + + let(:milestone) { create(:milestone, project: project) } + + it_behaves_like 'a system note' + + context 'when milestone added' do + it 'sets the note text' do + expect(subject.note).to eq "Milestone changed to #{milestone.title}" + end + end + + context 'when milestone removed' do + let(:milestone) { nil } + + it 'sets the note text' do + expect(subject.note).to eq 'Milestone removed' + end + end + end + + describe '.change_status' do + subject { described_class.change_status(noteable, project, author, status, source) } + + let(:status) { 'new_status' } + let(:source) { nil } + + it_behaves_like 'a system note' + + context 'with a source' do + let(:source) { double('commit', gfm_reference: 'commit 123456') } + + it 'sets the note text' do + expect(subject.note).to eq "Status changed to #{status} by commit 123456" + end + end + + context 'without a source' do + it 'sets the note text' do + expect(subject.note).to eq "Status changed to #{status}" + end + end + end + + describe '.cross_reference' do + subject { described_class.cross_reference(noteable, mentioner, author) } + + let(:mentioner) { create(:issue, project: project) } + + it_behaves_like 'a system note' + + context 'when cross-reference disallowed' do + before do + expect(described_class).to receive(:cross_reference_disallowed?).and_return(true) + end + + it 'returns nil' do + expect(subject).to be_nil + end + end + + context 'when cross-reference allowed' do + before do + expect(described_class).to receive(:cross_reference_disallowed?).and_return(false) + end + + describe 'note_body' do + context 'cross-project' do + let(:project2) { create(:project) } + let(:mentioner) { create(:issue, project: project2) } + + context 'from Commit' do + let(:mentioner) { project2.repository.commit } + + it 'references the mentioning commit' do + expect(subject.note).to eq "mentioned in commit #{project2.path_with_namespace}@#{mentioner.id}" + end + end + + context 'from non-Commit' do + it 'references the mentioning object' do + expect(subject.note).to eq "mentioned in issue #{project2.path_with_namespace}##{mentioner.iid}" + end + end + end + + context 'within the same project' do + context 'from Commit' do + let(:mentioner) { project.repository.commit } + + it 'references the mentioning commit' do + expect(subject.note).to eq "mentioned in commit #{mentioner.id}" + end + end + + context 'from non-Commit' do + it 'references the mentioning object' do + expect(subject.note).to eq "mentioned in issue ##{mentioner.iid}" + end + end + end + end + end + end + + describe '.cross_reference?' do + it 'is truthy when text begins with expected text' do + expect(described_class.cross_reference?('mentioned in issue #1')).to be_truthy + end + + it 'is falsey when text does not begin with expected text' do + expect(described_class.cross_reference?('this is a note')).to be_falsey + end + end + + describe '.cross_reference_disallowed?' do + context 'when mentioner is not a MergeRequest' do + it 'is falsey' do + mentioner = noteable.dup + expect(described_class.cross_reference_disallowed?(noteable, mentioner)). + to be_falsey + end + end + + context 'when mentioner is a MergeRequest' do + let(:mentioner) { create(:merge_request, :simple, source_project: project) } + let(:noteable) { project.commit } + + it 'is truthy when noteable is in commits' do + expect(mentioner).to receive(:commits).and_return([noteable]) + expect(described_class.cross_reference_disallowed?(noteable, mentioner)). + to be_truthy + end + + it 'is falsey when noteable is not in commits' do + expect(mentioner).to receive(:commits).and_return([]) + expect(described_class.cross_reference_disallowed?(noteable, mentioner)). + to be_falsey + end + end + end + + describe '.cross_reference_exists?' do + let(:commit0) { project.commit } + let(:commit1) { project.commit('HEAD~2') } + + context 'issue from commit' do + before do + # Mention issue (noteable) from commit0 + described_class.cross_reference(noteable, commit0, author) + end + + it 'is truthy when already mentioned' do + expect(described_class.cross_reference_exists?(noteable, commit0)). + to be_truthy + end + + it 'is falsey when not already mentioned' do + expect(described_class.cross_reference_exists?(noteable, commit1)). + to be_falsey + end + end + + context 'commit from commit' do + before do + # Mention commit1 from commit0 + described_class.cross_reference(commit0, commit1, author) + end + + it 'is truthy when already mentioned' do + expect(described_class.cross_reference_exists?(commit0, commit1)). + to be_truthy + end + + it 'is falsey when not already mentioned' do + expect(described_class.cross_reference_exists?(commit1, commit0)). + to be_falsey + end + end + end +end diff --git a/spec/support/taskable_shared_examples.rb b/spec/support/taskable_shared_examples.rb index 8e5e3a8aafc..927c72c7409 100644 --- a/spec/support/taskable_shared_examples.rb +++ b/spec/support/taskable_shared_examples.rb @@ -4,39 +4,29 @@ # subject { Issue or MergeRequest } shared_examples 'a Taskable' do before do - subject.description = <<EOT.gsub(/ {6}/, '') + subject.description = <<-EOT.strip_heredoc * [ ] Task 1 * [x] Task 2 * [x] Task 3 * [ ] Task 4 * [ ] Task 5 -EOT - end - - it 'updates the Nth task correctly' do - subject.update_nth_task(1, true) - expect(subject.description).to match(/\[x\] Task 1/) - - subject.update_nth_task(2, true) - expect(subject.description).to match('\[x\] Task 2') - - subject.update_nth_task(3, false) - expect(subject.description).to match('\[ \] Task 3') - - subject.update_nth_task(4, false) - expect(subject.description).to match('\[ \] Task 4') + EOT end it 'returns the correct task status' do expect(subject.task_status).to match('5 tasks') - expect(subject.task_status).to match('2 done') - expect(subject.task_status).to match('3 unfinished') + expect(subject.task_status).to match('2 completed') + expect(subject.task_status).to match('3 remaining') end - it 'knows if it has tasks' do - expect(subject.tasks?).to be_truthy + describe '#tasks?' do + it 'returns true when object has tasks' do + expect(subject.tasks?).to eq true + end - subject.description = 'Now I have no tasks' - expect(subject.tasks?).to be_falsey + it 'returns false when object has no tasks' do + subject.description = 'Now I have no tasks' + expect(subject.tasks?).to eq false + end end end diff --git a/vendor/assets/javascripts/jasmine-fixture.js b/vendor/assets/javascripts/jasmine-fixture.js index 3815731fd2f..9980aec6ddb 100644..100755 --- a/vendor/assets/javascripts/jasmine-fixture.js +++ b/vendor/assets/javascripts/jasmine-fixture.js @@ -1,4 +1,4 @@ -/* jasmine-fixture - 1.2.2 +/* jasmine-fixture - 1.3.1 * Makes injecting HTML snippets into the DOM easy & clean! * https://github.com/searls/jasmine-fixture */ @@ -8,7 +8,7 @@ (function($) { var ewwSideEffects, jasmineFixture, originalAffix, originalJasmineDotFixture, originalJasmineFixture, root, _, _ref; - root = this; + root = (1, eval)('this'); originalJasmineFixture = root.jasmineFixture; originalJasmineDotFixture = (_ref = root.jasmine) != null ? _ref.fixture : void 0; originalAffix = root.affix; @@ -33,7 +33,7 @@ create = function(selectorOptions, attach) { var $top; $top = null; - _(selectorOptions.split(/[ ](?=[^\]]*?(?:\[|$))/)).inject(function($parent, elementSelector) { + _(selectorOptions.split(/[ ](?![^\{]*\})(?=[^\]]*?(?:\[|$))/)).inject(function($parent, elementSelector) { var $el; if (elementSelector === ">") { return $parent; |