diff options
73 files changed, 1017 insertions, 407 deletions
diff --git a/.gitignore b/.gitignore index 91ea81bfc4e..1eb785451f4 100644 --- a/.gitignore +++ b/.gitignore @@ -23,6 +23,7 @@ config/gitlab.yml config/gitlab_ci.yml config/initializers/rack_attack.rb config/initializers/smtp_settings.rb +config/initializers/relative_url.rb config/resque.yml config/unicorn.rb config/secrets.yml diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 9f65e1204d3..c477721f9da 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -89,6 +89,7 @@ spec:other: spinach:project:half: stage: test script: + - RAILS_ENV=test bundle exec rake assets:precompile 2>/dev/null - RAILS_ENV=test SIMPLECOV=true bundle exec rake spinach:project:half tags: - ruby @@ -97,6 +98,7 @@ spinach:project:half: spinach:project:rest: stage: test script: + - RAILS_ENV=test bundle exec rake assets:precompile 2>/dev/null - RAILS_ENV=test SIMPLECOV=true bundle exec rake spinach:project:rest tags: - ruby @@ -105,6 +107,7 @@ spinach:project:rest: spinach:other: stage: test script: + - RAILS_ENV=test bundle exec rake assets:precompile 2>/dev/null - RAILS_ENV=test SIMPLECOV=true bundle exec rake spinach:other tags: - ruby @@ -275,6 +278,7 @@ spinach:project:half:ruby22: only: - master script: + - RAILS_ENV=test bundle exec rake assets:precompile 2>/dev/null - RAILS_ENV=test SIMPLECOV=true bundle exec rake spinach:project:half cache: key: "ruby22" @@ -290,6 +294,7 @@ spinach:project:rest:ruby22: only: - master script: + - RAILS_ENV=test bundle exec rake assets:precompile 2>/dev/null - RAILS_ENV=test SIMPLECOV=true bundle exec rake spinach:project:rest cache: key: "ruby22" @@ -305,6 +310,7 @@ spinach:other:ruby22: only: - master script: + - RAILS_ENV=test bundle exec rake assets:precompile 2>/dev/null - RAILS_ENV=test SIMPLECOV=true bundle exec rake spinach:other cache: key: "ruby22" diff --git a/CHANGELOG b/CHANGELOG index c30a7c2ae35..ce27258af79 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,16 +1,23 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.6.0 (unreleased) + - Contributions to forked projects are included in calendar - Improve the formatting for the user page bio (Connor Shea) - Fix issue when pushing to projects ending in .wiki - Fix avatar stretching by providing a cropping feature (Johann Pardanaud) + - Don't load all of GitLab in mail_room - Strip leading and trailing spaces in URL validator (evuez) + - Return empty array instead of 404 when commit has no statuses in commit status API - Update documentation to reflect Guest role not being enforced on internal projects + - Allow search for logged out users v 8.5.2 - Fix sidebar overlapping content when screen width was below 1200px - Fix permissions for deprecated CI build status badge - Fix error 500 when commenting on a commit + - Fix broken icons on installations with relative URL (Artem Sidorenko) + - Fix import from gitlab.com (KazSawada) + - Fix help keyboard shortcut on relative URL setups (Artem Sidorenko) v 8.5.1 - Fix group projects styles @@ -30,6 +37,7 @@ v 8.5.1 - Re-add the newrelic_rpm gem which was removed without any deprecation or warning (Stan Hu) - Update sentry-raven gem to 0.15.6 - Add build coverage in project's builds page (Steffen Köhler) + - Changed # to ! for merge requests in activity view v 8.5.0 - Fix duplicate "me" in tooltip of the "thumbsup" awards Emoji (Stan Hu) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b13f36af214..714701e7345 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,24 +3,27 @@ **Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* - [Contribute to GitLab](#contribute-to-gitlab) - - [Contributor license agreement](#contributor-license-agreement) - - [Security vulnerability disclosure](#security-vulnerability-disclosure) - - [Closing policy for issues and merge requests](#closing-policy-for-issues-and-merge-requests) - - [Helping others](#helping-others) - - [I want to contribute!](#i-want-to-contribute) - - [Issue tracker](#issue-tracker) - - [Feature proposals](#feature-proposals) - - [Issue tracker guidelines](#issue-tracker-guidelines) - - [Issue weight](#issue-weight) - - [Regression issues](#regression-issues) - - [Merge requests](#merge-requests) - - [Merge request guidelines](#merge-request-guidelines) - - [Merge request description format](#merge-request-description-format) - - [Contribution acceptance criteria](#contribution-acceptance-criteria) - - [Changes for Stable Releases](#changes-for-stable-releases) - - [Definition of done](#definition-of-done) - - [Style guides](#style-guides) - - [Code of conduct](#code-of-conduct) + - [Contributor license agreement](#contributor-license-agreement) + - [Security vulnerability disclosure](#security-vulnerability-disclosure) + - [Closing policy for issues and merge requests](#closing-policy-for-issues-and-merge-requests) + - [Helping others](#helping-others) + - [I want to contribute!](#i-want-to-contribute) + - [Implement design & UI elements](#implement-design-ui-elements) + - [Design reference](#design-reference) + - [UI development kit](#ui-development-kit) + - [Issue tracker](#issue-tracker) + - [Feature proposals](#feature-proposals) + - [Issue tracker guidelines](#issue-tracker-guidelines) + - [Issue weight](#issue-weight) + - [Regression issues](#regression-issues) + - [Merge requests](#merge-requests) + - [Merge request guidelines](#merge-request-guidelines) + - [Merge request description format](#merge-request-description-format) + - [Contribution acceptance criteria](#contribution-acceptance-criteria) + - [Changes for Stable Releases](#changes-for-stable-releases) + - [Definition of done](#definition-of-done) + - [Style guides](#style-guides) + - [Code of conduct](#code-of-conduct) <!-- END doctoc generated TOC please keep comment here to allow auto update --> @@ -83,6 +86,22 @@ GitLab. This was inspired by [an article by Kent C. Dodds][medium-up-for-grabs]. +## Implement design & UI elements + +### Design reference + +The GitLab design reference can be found in the [gitlab-design] project. +The designs are made using Antetype (`.atype` files). You can use the +[free Antetype viewer (Mac OSX only)] or grab an exported PNG from the design +(the PNG is 1:1). + +The current designs can be found in the [`gitlab1.atype` file]. + +### UI development kit + +Implemented UI elements can also be found at https://gitlab.com/help/ui. Please +note that this page isn't comprehensive at this time. + ## Issue tracker To get support for your particular problem please use the @@ -299,13 +318,14 @@ to us than having a minimal commit log. The smaller an MR is the more likely it is it will be merged (quickly). After that you can send more MRs to enhance it. For examples of feedback on merge requests please look at already -[closed merge requests][closed-merge-requests]. If you would like quick feedback on your merge -request feel free to mention one of the Merge Marshalls of the [core team][core-team]. +[closed merge requests][closed-merge-requests]. If you would like quick feedback +on your merge request feel free to mention one of the Merge Marshalls in the +[core team][core-team] or one of the +[Merge request coaches](https://about.gitlab.com/team/). Please ensure that your merge request meets the contribution acceptance criteria. When having your code reviewed and when reviewing merge requests please take the -[thoughtbot code review guidelines](https://github.com/thoughtbot/guides/tree/master/code-review) -into account. +[Thoughtbot code review guide] into account. ### Merge request description format @@ -473,3 +493,7 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor [rss-source]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#source-code-layout [rss-naming]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#naming [doc-styleguide]: doc/development/doc_styleguide.md "Documentation styleguide" +[gitlab-design]: https://gitlab.com/gitlab-org/gitlab-design +[free Antetype viewer (Mac OSX only)]: https://itunes.apple.com/us/app/antetype-viewer/id824152298?mt=12 +[`gitlab1.atype` file]: https://gitlab.com/gitlab-org/gitlab-design/tree/master/gitlab1.atype/ +[Thoughtbot code review guide]: https://github.com/thoughtbot/guides/tree/master/code-review @@ -1,14 +1,14 @@ source "https://rubygems.org" -gem 'rails', '4.2.5.1' +gem 'rails', '4.2.5.2' gem 'rails-deprecated_sanitizer', '~> 1.0.3' # Responders respond_to and respond_with gem 'responders', '~> 2.0' -# Specify a sprockets version due to security issue -# See https://groups.google.com/forum/#!topic/rubyonrails-security/doAVp0YaTqY -gem 'sprockets', '~> 2.12.3' +# Specify a sprockets version due to increased performance +# See https://gitlab.com/gitlab-org/gitlab-ce/issues/6069 +gem 'sprockets', '~> 3.3.5' # Default values for AR models gem "default_value_for", "~> 3.0.0" diff --git a/Gemfile.lock b/Gemfile.lock index 1ba062dd0d6..e048e2f5a56 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -4,41 +4,41 @@ GEM CFPropertyList (2.3.2) RedCloth (4.2.9) ace-rails-ap (2.0.1) - actionmailer (4.2.5.1) - actionpack (= 4.2.5.1) - actionview (= 4.2.5.1) - activejob (= 4.2.5.1) + actionmailer (4.2.5.2) + actionpack (= 4.2.5.2) + actionview (= 4.2.5.2) + activejob (= 4.2.5.2) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 1.0, >= 1.0.5) - actionpack (4.2.5.1) - actionview (= 4.2.5.1) - activesupport (= 4.2.5.1) + actionpack (4.2.5.2) + actionview (= 4.2.5.2) + activesupport (= 4.2.5.2) rack (~> 1.6) rack-test (~> 0.6.2) rails-dom-testing (~> 1.0, >= 1.0.5) rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (4.2.5.1) - activesupport (= 4.2.5.1) + actionview (4.2.5.2) + activesupport (= 4.2.5.2) builder (~> 3.1) erubis (~> 2.7.0) rails-dom-testing (~> 1.0, >= 1.0.5) rails-html-sanitizer (~> 1.0, >= 1.0.2) - activejob (4.2.5.1) - activesupport (= 4.2.5.1) + activejob (4.2.5.2) + activesupport (= 4.2.5.2) globalid (>= 0.3.0) - activemodel (4.2.5.1) - activesupport (= 4.2.5.1) + activemodel (4.2.5.2) + activesupport (= 4.2.5.2) builder (~> 3.1) - activerecord (4.2.5.1) - activemodel (= 4.2.5.1) - activesupport (= 4.2.5.1) + activerecord (4.2.5.2) + activemodel (= 4.2.5.2) + activesupport (= 4.2.5.2) arel (~> 6.0) activerecord-deprecated_finders (1.0.4) activerecord-session_store (0.1.2) actionpack (>= 4.0.0, < 5) activerecord (>= 4.0.0, < 5) railties (>= 4.0.0, < 5) - activesupport (4.2.5.1) + activesupport (4.2.5.2) i18n (~> 0.7) json (~> 1.7, >= 1.7.7) minitest (~> 5.1) @@ -407,7 +407,6 @@ GEM railties (>= 4.0.1) hashie (3.4.3) highline (1.7.8) - hike (1.2.3) hipchat (1.5.2) httparty mimemagic @@ -586,16 +585,16 @@ GEM rack rack-test (0.6.3) rack (>= 1.0) - rails (4.2.5.1) - actionmailer (= 4.2.5.1) - actionpack (= 4.2.5.1) - actionview (= 4.2.5.1) - activejob (= 4.2.5.1) - activemodel (= 4.2.5.1) - activerecord (= 4.2.5.1) - activesupport (= 4.2.5.1) + rails (4.2.5.2) + actionmailer (= 4.2.5.2) + actionpack (= 4.2.5.2) + actionview (= 4.2.5.2) + activejob (= 4.2.5.2) + activemodel (= 4.2.5.2) + activerecord (= 4.2.5.2) + activesupport (= 4.2.5.2) bundler (>= 1.3.0, < 2.0) - railties (= 4.2.5.1) + railties (= 4.2.5.2) sprockets-rails rails-deprecated_sanitizer (1.0.3) activesupport (>= 4.2.0.alpha) @@ -605,9 +604,9 @@ GEM rails-deprecated_sanitizer (>= 1.0.1) rails-html-sanitizer (1.0.3) loofah (~> 2.0) - railties (4.2.5.1) - actionpack (= 4.2.5.1) - activesupport (= 4.2.5.1) + railties (4.2.5.2) + actionpack (= 4.2.5.2) + activesupport (= 4.2.5.2) rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) rainbow (2.0.0) @@ -771,11 +770,8 @@ GEM spring (>= 0.9.1) spring-commands-teaspoon (0.0.2) spring (>= 0.9.1) - sprockets (2.12.4) - hike (~> 1.2) - multi_json (~> 1.0) - rack (~> 1.0) - tilt (~> 1.1, != 1.3.0) + sprockets (3.3.5) + rack (> 1, < 3) sprockets-rails (2.3.3) actionpack (>= 3.0) activesupport (>= 3.0) @@ -807,7 +803,7 @@ GEM rack (~> 1.0) thor (0.19.1) thread_safe (0.3.5) - tilt (1.4.1) + tilt (2.0.2) timfel-krb5-auth (0.8.3) tinder (1.10.1) eventmachine (~> 1.0) @@ -987,7 +983,7 @@ DEPENDENCIES rack-attack (~> 4.3.1) rack-cors (~> 0.4.0) rack-oauth2 (~> 1.2.1) - rails (= 4.2.5.1) + rails (= 4.2.5.2) rails-deprecated_sanitizer (~> 1.0.3) raphael-rails (~> 2.1.2) rblineprof @@ -1024,7 +1020,7 @@ DEPENDENCIES spring-commands-rspec (~> 1.0.4) spring-commands-spinach (~> 1.0.0) spring-commands-teaspoon (~> 0.0.2) - sprockets (~> 2.12.3) + sprockets (~> 3.3.5) state_machines-activerecord (~> 0.3.0) task_list (~> 1.0.2) teaspoon (~> 1.0.0) diff --git a/PROCESS.md b/PROCESS.md index 5f4d67bc10e..cad45d23df9 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -2,23 +2,39 @@ ## Purpose of describing the contributing process -Below we describe the contributing process to GitLab for two reasons. So that contributors know what to expect from maintainers (possible responses, friendly treatment, etc.). And so that maintainers know what to expect from contributors (use the latest version, ensure that the issue is addressed, friendly treatment, etc.). +Below we describe the contributing process to GitLab for two reasons. So that +contributors know what to expect from maintainers (possible responses, friendly +treatment, etc.). And so that maintainers know what to expect from contributors +(use the latest version, ensure that the issue is addressed, friendly treatment, +etc.). ## Common actions ### Issue team -- Looks for issues without [workflow labels](#how-we-handle-issues) and triages issue -- Closes invalid issues with a comment (duplicates, [fixed in newer version](#issue-fixed-in-newer-version), [issue report for old version](#issue-report-for-old-version), not a problem in GitLab, etc.) -- Asks for feedback from issue reporter ([invalid issue reports](#improperly-formatted-issue), [format code](#code-format), etc.) -- Monitors all issues for feedback (but especially ones commented on since automatically watching them) + +- Looks for issues without [workflow labels](#how-we-handle-issues) and triages + issue +- Closes invalid issues with a comment (duplicates, + [fixed in newer version](#issue-fixed-in-newer-version), + [issue report for old version](#issue-report-for-old-version), not a problem + in GitLab, etc.) +- Asks for feedback from issue reporter + ([invalid issue reports](#improperly-formatted-issue), + [format code](#code-format), etc.) +- Monitors all issues for feedback (but especially ones commented on since + automatically watching them) - Closes issues with no feedback from the reporter for two weeks -### Merge marshal +### Merge marshall & merge request coach -- Responds to merge requests the issue team mentions them in and monitors for new merge requests -- Provides feedback to the merge request submitter to improve the merge request (style, tests, etc.) -- Mark merge requests 'ready-for-merge' when they meet the contribution guidelines -- Mention developer(s) based on the [list of members and their specialities](https://about.gitlab.com/core-team/) +- Responds to merge requests the issue team mentions them in and monitors for + new merge requests +- Provides feedback to the merge request submitter to improve the merge request + (style, tests, etc.) +- Mark merge requests `Ready for Merge` when they meet the + [contribution acceptance criteria] +- Mention developer(s) based on the + [list of members and their specialities][team] - Closes merge requests with no feedback from the reporter for two weeks ## Priorities of the issue team @@ -30,29 +46,40 @@ Below we describe the contributing process to GitLab for two reasons. So that co ## Mentioning people -The most important thing is making sure valid issues receive feedback from the development team. Therefore the priority is mentioning developers that can help on those issue. Please select someone with relevant experience from [GitLab core team](https://about.gitlab.com/core-team/). If there is nobody mentioned with that expertise look in the commit history for the affected files to find someone. Avoid mentioning the lead developer, this is the person that is least likely to give a timely response. If the involvement of the lead developer is needed the other core team members will mention this person. +The most important thing is making sure valid issues receive feedback from the +development team. Therefore the priority is mentioning developers that can help +on those issue. Please select someone with relevant experience from +[GitLab core team][core-team]. If there is nobody mentioned with that expertise +look in the commit history for the affected files to find someone. Avoid +mentioning the lead developer, this is the person that is least likely to give a +timely response. If the involvement of the lead developer is needed the other +core team members will mention this person. ## Workflow labels -Workflow labels are purposely not very detailed since that would be hard to keep updated as you would need to re-evaluate them after every comment. We optionally use functional labels on demand when want to group related issues to get an overview (for example all issues related to RVM, to tackle them in one go) and to add details to the issue. - -- *Awaiting feedback*: Feedback pending from the reporter -- *Awaiting confirmation of fix*: The issue should already be solved in **master** (generally you can avoid this workflow item and just close the issue right away) -- *Attached MR*: There is a MR attached and the discussion should happen there - - We need to let issues stay in sync with the MR's. We can do this with a "Closing #XXXX" or "Fixes #XXXX" comment in the MR. We can't close the issue when there is a merge request because sometimes a MR is not good and we just close the MR, then the issue must stay. -- *Developer*: needs help from a developer -- *UX* needs needs help from a UX designer -- *Frontend* needs help from a Front-end engineer -- *Graphics* needs help from a Graphics designer -- *up-for-grabs* is an issue suitable for first-time contributors, of reasonable difficulty and size. Not exclusive with other labels. -- *feature proposal* is a proposal for a new feature for GitLab. People are encouraged to vote +Workflow labels are purposely not very detailed since that would be hard to keep +updated as you would need to re-evaluate them after every comment. We optionally +use functional labels on demand when want to group related issues to get an +overview (for example all issues related to RVM, to tackle them in one go) and +to add details to the issue. + +- ~"Awaiting Feedback" Feedback pending from the reporter +- ~UX needs help from a UX designer +- ~Frontend needs help from a Front-end engineer. Please follow the + ["Implement design & UI elements" guidelines]. +- ~up-for-grabs is an issue suitable for first-time contributors, of reasonable difficulty and size. Not exclusive with other labels. +- ~"feature proposal" is a proposal for a new feature for GitLab. People are encouraged to vote in support or comment for further detail. Do not use `feature request`. - +- ~bug is an issue reporting undesirable or incorrect behavior. +- ~customer is an issue reported by enterprise subscribers. This label should +be accompanied by *bug* or *feature proposal* labels. Example workflow: when a UX designer provided a design but it needs frontend work they remove the UX label and add the frontend label. ## Functional labels -These labels describe what development specialities are involved such as: PostgreSQL, UX, LDAP. +These labels describe what development specialities are involved such as: `CI`, +`Core`, `Documentation`, `Frontend`, `Issues`, `Merge Requests`, `Omnibus`, +`Release`, `Repository`, `UX`. ## Assigning issues @@ -60,21 +87,29 @@ If an issue is complex and needs the attention of a specific person, assignment ## Label colors -- Light orange `#fef2c0`: workflow labels for issue team members (awaiting feedback, awaiting confirmation of fix) -- Bright orange `#eb6420`: workflow labels for core team members (attached MR, awaiting developer action/feedback) -- Light blue `#82C5FF`: functional labels -- Green labels `#009800`: issues that can generally be ignored. For example, issues given the following labels normally can be closed immediately: - - Support (see copy & paste response: [Support requests and configuration questions](#support-requests-and-configuration-questions) +- Light orange `#fef2c0`: workflow labels for issue team members (awaiting + feedback, awaiting confirmation of fix) +- Bright orange `#eb6420`: workflow labels for core team members (attached MR, + awaiting developer action/feedback) +- Light blue `#82C5FF`: functional labels +- Green labels `#009800`: issues that can generally be ignored. For example, + issues given the following labels normally can be closed immediately: + - Support (see copy & paste response: + [Support requests and configuration questions](#support-requests-and-configuration-questions) ## Be kind -Be kind to people trying to contribute. Be aware that people may be a non-native English speaker, they might not understand things or they might be very sensitive as to how you word things. Use Emoji to express your feelings (heart, star, smile, etc.). Some good tips about giving feedback to merge requests is in the [Thoughtbot code review guide](https://github.com/thoughtbot/guides/tree/master/code-review). +Be kind to people trying to contribute. Be aware that people may be a non-native +English speaker, they might not understand things or they might be very +sensitive as to how you word things. Use Emoji to express your feelings (heart, +star, smile, etc.). Some good tips about giving feedback to merge requests is in +the [Thoughtbot code review guide]. ## Copy & paste responses ### Improperly formatted issue -Thanks for the issue report. Please reformat your issue to conform to the issue tracker guidelines found in our \[contributing guidelines\]\(https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#issue-tracker-guidelines). +Thanks for the issue report. Please reformat your issue to conform to the \[contributing guidelines\]\(https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#issue-tracker-guidelines). ### Issue report for old version @@ -110,11 +145,11 @@ This merge request has been closed because a request for more information has no ### Accepting merge requests -Is there an issue on the [issue tracker](https://gitlab.com/gitlab-org/gitlab-ce/issues) -that is similar to this? -Could you please link it here? +Is there an issue on the +\[issue tracker\]\(https://gitlab.com/gitlab-org/gitlab-ce/issues) that is +similar to this? Could you please link it here? Please be aware that new functionality that is not marked -[accepting merge requests](https://gitlab.com/gitlab-org/gitlab-ce/issues?milestone_id=&scope=all&sort=created_desc&state=opened&utf8=%E2%9C%93&assignee_id=&author_id=&milestone_title=&label_name=Accepting+Merge+Requests) +\[accepting merge requests\]\(https://gitlab.com/gitlab-org/gitlab-ce/issues?milestone_id=&scope=all&sort=created_desc&state=opened&utf8=%E2%9C%93&assignee_id=&author_id=&milestone_title=&label_name=Accepting+Merge+Requests) might not make it into GitLab. ### Only accepting merge requests with green tests @@ -129,4 +164,10 @@ rebase with master to see if that solves the issue. We are currently in the process of closing down the issue tracker on GitHub, to prevent duplication with the GitLab.com issue tracker. Since this is an older issue I'll be closing this for now. If you think this is -still an issue I encourage you to open it on the \[GitLab.com issue tracker\](https://gitlab.com/gitlab-org/gitlab-ce/issues). +still an issue I encourage you to open it on the \[GitLab.com issue tracker\]\(https://gitlab.com/gitlab-org/gitlab-ce/issues). + +[core-team]: https://about.gitlab.com/core-team/ +[team]: https://about.gitlab.com/team/ +[contribution acceptance criteria]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#contribution-acceptance-criteria +["Implement design & UI elements" guidelines]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#implement-design-ui-elements +[Thoughtbot code review guide]: https://github.com/thoughtbot/guides/tree/master/code-review @@ -4,4 +4,7 @@ require File.expand_path('../config/application', __FILE__) +relative_url_conf = File.expand_path('../config/initializers/relative_url', __FILE__) +require relative_url_conf if File.exist?("#{relative_url_conf}.rb") + Gitlab::Application.load_tasks diff --git a/app/assets/javascripts/shortcuts.js.coffee b/app/assets/javascripts/shortcuts.js.coffee index f141fb69c3e..9c7c2474aa4 100644 --- a/app/assets/javascripts/shortcuts.js.coffee +++ b/app/assets/javascripts/shortcuts.js.coffee @@ -13,8 +13,10 @@ class @Shortcuts if $('#modal-shortcuts').length > 0 $('#modal-shortcuts').modal('show') else + url = '/help/shortcuts' + url = gon.relative_url_root + url if gon.relative_url_root? $.ajax( - url: '/help/shortcuts', + url: url, dataType: 'script', success: (e) -> if location and location.length > 0 diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 9ecb547b64f..9e8506b2a81 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -305,7 +305,7 @@ table { } .btn-sign-in { - margin-top: 8px; + margin-top: 10px; text-shadow: none; } diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index e0ccd6f100f..de947c89c19 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -13,6 +13,19 @@ transition-duration: .3s; } + .gitlab-text-container-link { + z-index: 1; + position: absolute; + left: 0px; + } + + #logo { + z-index: 2; + position: absolute; + width: 58px; + cursor: pointer; + } + &.right-sidebar-expanded { padding-right: $gutter_width; } @@ -74,7 +87,7 @@ width: 158px; float: left; margin: 0; - margin-left: 14px; + margin-left: 50px; font-size: 19px; line-height: 41px; font-weight: normal; diff --git a/app/assets/stylesheets/pages/login.scss b/app/assets/stylesheets/pages/login.scss index f9c6f1b39f9..61bec02f6c5 100644 --- a/app/assets/stylesheets/pages/login.scss +++ b/app/assets/stylesheets/pages/login.scss @@ -8,6 +8,10 @@ max-width: none; } + .flash-container { + margin-bottom: $gl-padding; + } + .brand-holder { font-size: 18px; line-height: 1.5; diff --git a/app/assets/stylesheets/pages/milestone.scss b/app/assets/stylesheets/pages/milestone.scss index 9144a83647d..d24adbf67e6 100644 --- a/app/assets/stylesheets/pages/milestone.scss +++ b/app/assets/stylesheets/pages/milestone.scss @@ -39,7 +39,7 @@ li.milestone { margin-right: 10px; } - .time-elapsed { + .remaining-days { color: $orange-light; } } diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index 3aaa96da609..84234b15c65 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -1,8 +1,12 @@ .search-results { .search-result-row { - border-bottom: 1px solid #DDD; - padding-bottom: 15px; - margin-bottom: 15px; + border-bottom: 1px solid $border-color; + padding-bottom: $gl-padding; + margin-bottom: $gl-padding; + + &:last-child { + border-bottom: none; + } } } diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index f3bfede4354..8f83fdd02bc 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -12,11 +12,13 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController current_user.save! if current_user.changed? - if two_factor_grace_period_expired? - flash.now[:alert] = 'You must enable Two-factor Authentication for your account.' - else - grace_period_deadline = current_user.otp_grace_period_started_at + two_factor_grace_period.hours - flash.now[:alert] = "You must enable Two-factor Authentication for your account before #{l(grace_period_deadline)}." + if two_factor_authentication_required? + if two_factor_grace_period_expired? + flash.now[:alert] = 'You must enable Two-factor Authentication for your account.' + else + grace_period_deadline = current_user.otp_grace_period_started_at + two_factor_grace_period.hours + flash.now[:alert] = "You must enable Two-factor Authentication for your account before #{l(grace_period_deadline)}." + end end @qr_code = build_qr_code diff --git a/app/controllers/projects/avatars_controller.rb b/app/controllers/projects/avatars_controller.rb index f7e6bb34443..b64dbbd89ce 100644 --- a/app/controllers/projects/avatars_controller.rb +++ b/app/controllers/projects/avatars_controller.rb @@ -1,4 +1,6 @@ class Projects::AvatarsController < Projects::ApplicationController + include BlobHelper + before_action :project def show @@ -7,7 +9,7 @@ class Projects::AvatarsController < Projects::ApplicationController headers['X-Content-Type-Options'] = 'nosniff' headers.store(*Gitlab::Workhorse.send_git_blob(@repository, @blob)) headers['Content-Disposition'] = 'inline' - headers['Content-Type'] = @blob.content_type + headers['Content-Type'] = safe_content_type(@blob) head :ok # 'render nothing: true' messes up the Content-Type else render_404 diff --git a/app/controllers/projects/raw_controller.rb b/app/controllers/projects/raw_controller.rb index 87b4d08da0e..d9723acb1d9 100644 --- a/app/controllers/projects/raw_controller.rb +++ b/app/controllers/projects/raw_controller.rb @@ -1,6 +1,7 @@ # Controller for viewing a file's raw class Projects::RawController < Projects::ApplicationController include ExtractsPath + include BlobHelper before_action :require_non_empty_project before_action :assign_ref_vars @@ -17,7 +18,7 @@ class Projects::RawController < Projects::ApplicationController else headers.store(*Gitlab::Workhorse.send_git_blob(@repository, @blob)) headers['Content-Disposition'] = 'inline' - headers['Content-Type'] = get_blob_type + headers['Content-Type'] = safe_content_type(@blob) head :ok # 'render nothing: true' messes up the Content-Type end else @@ -27,16 +28,6 @@ class Projects::RawController < Projects::ApplicationController private - def get_blob_type - if @blob.text? - 'text/plain; charset=utf-8' - elsif @blob.image? - @blob.content_type - else - 'application/octet-stream' - end - end - def send_lfs_object lfs_object = find_lfs_object diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 9bb42ec86b3..e42d2d73947 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -1,4 +1,6 @@ class SearchController < ApplicationController + skip_before_action :authenticate_user!, :reject_blocked + include SearchHelper layout 'search' diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 6055b606086..626213c6728 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -57,7 +57,7 @@ class UsersController < ApplicationController def contributions_calendar @contributions_calendar ||= Gitlab::ContributionsCalendar. - new(contributed_projects.reject(&:forked?), @user) + new(contributed_projects, @user) end def load_events diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 7143a744869..7f63a2e2cb4 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -134,4 +134,22 @@ module BlobHelper blob.data = Loofah.scrub_fragment(blob.data, :strip).to_xml blob end + + # If we blindly set the 'real' content type when serving a Git blob we + # are enabling XSS attacks. An attacker could upload e.g. a Javascript + # file to a Git repository, trick the browser of a victim into + # downloading the blob, and then the 'application/javascript' content + # type would tell the browser to execute the attacker's Javascript. By + # overriding the content type and setting it to 'text/plain' (in the + # example of Javascript) we tell the browser of the victim not to + # execute untrusted data. + def safe_content_type(blob) + if blob.text? + 'text/plain; charset=utf-8' + elsif blob.image? + blob.content_type + else + 'application/octet-stream' + end + end end diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index 31bf45baeb7..e5fcaab9551 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -168,11 +168,11 @@ module EventsHelper link_to(namespace_project_snippet_path(event.project.namespace, event.project, event.note_target)) do - "#{event.note_target_type} ##{truncate event.note_target_id}" + "#{event.note_target_type} #{truncate event.note_target.to_reference}" end else link_to event_note_target_path(event) do - "#{event.note_target_type} ##{truncate event.note_target_iid}" + "#{event.note_target_type} #{truncate event.note_target.to_reference}" end end else diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb index a42cbcff182..7de81d8dfdb 100644 --- a/app/helpers/milestones_helper.rb +++ b/app/helpers/milestones_helper.rb @@ -36,4 +36,14 @@ module MilestonesHelper options_from_collection_for_select(grouped_milestones, 'name', 'title', params[:milestone_title]) end + + def milestone_remaining_days(milestone) + if milestone.expired? + content_tag(:strong, 'expired') + elsif milestone.due_date + days = milestone.remaining_days + content = content_tag(:strong, days) + content << " #{'day'.pluralize(days)} remaining" + end + end end diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 8f99e3bef9b..7dc2f909b2f 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -110,17 +110,10 @@ class Milestone < ActiveRecord::Base 0 end - # Returns the elapsed time (in percent) since the Milestone creation date until today. - # If the Milestone doesn't have a due_date then returns 0 since we can't calculate the elapsed time. - # If the Milestone is overdue then it returns 100%. - def percent_time_used - return 0 unless due_date - return 100 if expired? + def remaining_days + return 0 if !due_date || expired? - duration = ((created_at - due_date.to_datetime) / 1.day) - days_elapsed = ((created_at - Time.now) / 1.day) - - ((days_elapsed.to_f / duration) * 100).floor + (due_date - Date.today).to_i end def expires_at diff --git a/app/models/repository.rb b/app/models/repository.rb index a214a69d749..c135ab61f6a 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -654,30 +654,38 @@ class Repository end end - def revert(user, commit, base_branch, target_branch = nil) - source_sha = find_branch(base_branch).target - target_branch ||= base_branch - args = [commit.id, source_sha] - args << { mainline: 1 } if commit.merge_commit? + def revert(user, commit, base_branch, revert_tree_id = nil) + source_sha = find_branch(base_branch).target + revert_tree_id ||= check_revert_content(commit, base_branch) - revert_index = rugged.revert_commit(*args) - return false if revert_index.conflicts? - - tree_id = revert_index.write_tree(rugged) - return false unless diff_exists?(source_sha, tree_id) + return false unless revert_tree_id - commit_with_hooks(user, target_branch) do |ref| + commit_with_hooks(user, base_branch) do |ref| committer = user_to_committer(user) source_sha = Rugged::Commit.create(rugged, message: commit.revert_message, author: committer, committer: committer, - tree: tree_id, + tree: revert_tree_id, parents: [rugged.lookup(source_sha)], update_ref: ref) end end + def check_revert_content(commit, base_branch) + source_sha = find_branch(base_branch).target + args = [commit.id, source_sha] + args << { mainline: 1 } if commit.merge_commit? + + revert_index = rugged.revert_commit(*args) + return false if revert_index.conflicts? + + tree_id = revert_index.write_tree(rugged) + return false unless diff_exists?(source_sha, tree_id) + + tree_id + end + def diff_exists?(sha1, sha2) rugged.diff(sha1, sha2).size > 0 end diff --git a/app/models/user.rb b/app/models/user.rb index 6baf2468ade..3098d49d58a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -169,7 +169,7 @@ class User < ActiveRecord::Base validates :avatar_crop_x, :avatar_crop_y, :avatar_crop_size, numericality: { only_integer: true }, presence: true, - if: ->(user) { user.avatar? } + if: ->(user) { user.avatar? && user.avatar_changed? } before_validation :generate_password, on: :create before_validation :restricted_signup_domains, on: :create @@ -362,17 +362,19 @@ class User < ActiveRecord::Base def disable_two_factor! update_attributes( - two_factor_enabled: false, - encrypted_otp_secret: nil, - encrypted_otp_secret_iv: nil, - encrypted_otp_secret_salt: nil, - otp_backup_codes: nil + two_factor_enabled: false, + encrypted_otp_secret: nil, + encrypted_otp_secret_iv: nil, + encrypted_otp_secret_salt: nil, + otp_grace_period_started_at: nil, + otp_backup_codes: nil ) end def namespace_uniq # Return early if username already failed the first uniqueness validation - return if self.errors[:username].include?('has already been taken') + return if self.errors.key?(:username) && + self.errors[:username].include?('has already been taken') namespace_name = self.username existing_namespace = Namespace.by_path(namespace_name) diff --git a/app/services/commits/revert_service.rb b/app/services/commits/revert_service.rb index 43d1c766e35..9cb918d7a2e 100644 --- a/app/services/commits/revert_service.rb +++ b/app/services/commits/revert_service.rb @@ -17,28 +17,28 @@ module Commits def commit revert_into = @create_merge_request ? @commit.revert_branch_name : @target_branch + revert_tree_id = repository.check_revert_content(@commit, @target_branch) - if @create_merge_request - # Temporary branch exists and contains the revert commit - return success if repository.find_branch(revert_into) + if revert_tree_id + create_target_branch(revert_into) if @create_merge_request - create_target_branch - end - - unless repository.revert(current_user, @commit, revert_into) + repository.revert(current_user, @commit, revert_into, revert_tree_id) + success + else error_msg = "Sorry, we cannot revert this #{params[:revert_type_title]} automatically. It may have already been reverted, or a more recent commit may have updated some of its content." raise ReversionError, error_msg end - - success end private - def create_target_branch + def create_target_branch(new_branch) + # Temporary branch exists and contains the revert commit + return success if repository.find_branch(new_branch) + result = CreateBranchService.new(@project, current_user) - .execute(@commit.revert_branch_name, @target_branch, source_project: @source_project) + .execute(new_branch, @target_branch, source_project: @source_project) if result[:status] == :error raise ReversionError, "There was an error creating the source branch: #{result[:message]}" diff --git a/app/views/events/event/_common.html.haml b/app/views/events/event/_common.html.haml index 4ecf1c33d2a..e9e16a7646f 100644 --- a/app/views/events/event/_common.html.haml +++ b/app/views/events/event/_common.html.haml @@ -4,7 +4,7 @@ = event_action_name(event) - if event.target - %strong= link_to "##{event.target_iid}", [event.project.namespace.becomes(Namespace), event.project, event.target] + %strong= link_to event.target.to_reference, [event.project.namespace.becomes(Namespace), event.project, event.target] = event_preposition(event) diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index e53d5b07801..c799e9c588d 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -4,7 +4,7 @@ .header-logo %a#logo = brand_header_logo - = link_to root_path, class: 'home', title: 'Dashboard', id: 'js-shortcuts-home' do + = link_to root_path, class: 'gitlab-text-container-link', title: 'Dashboard', id: 'js-shortcuts-home' do .gitlab-text-container %h3 GitLab diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index 678ed3c2c1f..babfb032236 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -5,11 +5,7 @@ -# Ideally this would be inside the head, but turbolinks only evaluates page-specific JS in the body. = yield :scripts_body_top - - if current_user - = render "layouts/header/default", title: header_title - - else - = render "layouts/header/public", title: header_title - + = render "layouts/header/default", title: header_title = render 'layouts/page', sidebar: sidebar = yield :scripts_body diff --git a/app/views/layouts/ci/_page.html.haml b/app/views/layouts/ci/_page.html.haml index 3cfd36720f0..a13241bebee 100644 --- a/app/views/layouts/ci/_page.html.haml +++ b/app/views/layouts/ci/_page.html.haml @@ -4,7 +4,7 @@ .header-logo %a#logo = brand_header_logo - = link_to root_path, class: 'home', title: 'Dashboard', id: 'js-shortcuts-home' do + = link_to root_path, class: 'gitlab-text-container-link', title: 'Dashboard', id: 'js-shortcuts-home' do .gitlab-text-container %h3 GitLab diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 4781ff23507..77d01a7736c 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -13,30 +13,35 @@ %li.visible-sm.visible-xs = link_to search_path, title: 'Search', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do = icon('search') - - if session[:impersonator_id] - %li.impersonation - = link_to stop_impersonation_admin_users_path, method: :delete, title: 'Stop Impersonation', data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do - = icon('user-secret fw') - - if current_user.is_admin? + - if current_user + - if session[:impersonator_id] + %li.impersonation + = link_to stop_impersonation_admin_users_path, method: :delete, title: 'Stop Impersonation', data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do + = icon('user-secret fw') + - if current_user.is_admin? + %li + = link_to admin_root_path, title: 'Admin Area', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do + = icon('wrench fw') %li - = link_to admin_root_path, title: 'Admin Area', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do - = icon('wrench fw') - %li - = link_to dashboard_todos_path, title: 'Todos', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do - %span.badge.todos-pending-count - = todos_pending_count - - if current_user.can_create_project? + = link_to dashboard_todos_path, title: 'Todos', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do + %span.badge.todos-pending-count + = todos_pending_count + - if current_user.can_create_project? + %li + = link_to new_project_path, title: 'New project', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do + = icon('plus fw') + - if Gitlab::Sherlock.enabled? + %li + = link_to sherlock_transactions_path, title: 'Sherlock Transactions', + data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do + = icon('tachometer fw') %li - = link_to new_project_path, title: 'New project', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do - = icon('plus fw') - - if Gitlab::Sherlock.enabled? - %li - = link_to sherlock_transactions_path, title: 'Sherlock Transactions', - data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do - = icon('tachometer fw') - %li - = link_to destroy_user_session_path, class: 'logout', method: :delete, title: 'Sign out', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do - = icon('sign-out') + = link_to destroy_user_session_path, class: 'logout', method: :delete, title: 'Sign out', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do + = icon('sign-out') + - else + .pull-right + = link_to "Sign in", new_session_path(:user, redirect_to_referer: 'yes'), class: 'btn btn-sign-in btn-success' + %h1.title= title diff --git a/app/views/layouts/header/_public.html.haml b/app/views/layouts/header/_public.html.haml deleted file mode 100644 index a6a26518a0e..00000000000 --- a/app/views/layouts/header/_public.html.haml +++ /dev/null @@ -1,10 +0,0 @@ -%header.navbar.navbar-fixed-top.navbar-gitlab{ class: nav_header_class } - %div{ class: fluid_layout ? "container-fluid" : "container-fluid" } - .header-content - - unless current_controller?('sessions') - .pull-right - = link_to "Sign in", new_session_path(:user, redirect_to_referer: 'yes'), class: 'btn btn-sign-in btn-success' - - %h1.title= title - -= render 'shared/outdated_browser' diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index b9d5982a56f..18cf3f14f0b 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -48,7 +48,7 @@ = note_count .merge-request-info - \##{merge_request.iid} · + #{merge_request.to_reference} · opened #{time_ago_with_tooltip(merge_request.created_at, placement: 'bottom')} by #{link_to_member(@project, merge_request.author, avatar: false)} - if merge_request.target_project.default_branch != merge_request.target_branch diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 648512e5379..d7bc26e24b9 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -1,4 +1,4 @@ -- page_title "#{@merge_request.title} (##{@merge_request.iid})", "Merge Requests" +- page_title "#{@merge_request.title} (#{@merge_request.to_reference})", "Merge Requests" - page_description @merge_request.description - page_card_attributes @merge_request.card_attributes diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml index 14ea7b17786..b634a4af8d2 100644 --- a/app/views/projects/merge_requests/show/_mr_title.html.haml +++ b/app/views/projects/merge_requests/show/_mr_title.html.haml @@ -2,7 +2,7 @@ .status-box{ class: status_box_class(@merge_request) } = @merge_request.state_human_name %span.identifier - Merge Request ##{@merge_request.iid} + Merge Request #{@merge_request.to_reference} %span.creator · by #{link_to_member(@project, @merge_request.author, size: 24)} diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml index 631bc8c3e9d..2cae1ac4e2c 100644 --- a/app/views/projects/milestones/show.html.haml +++ b/app/views/projects/milestones/show.html.haml @@ -60,9 +60,7 @@ %strong== #{@milestone.percent_complete}% complete %span.milestone-stat - %span.time-elapsed - %strong== #{@milestone.percent_time_used}% - time elapsed + %span.remaining-days= milestone_remaining_days(@milestone) %span.pull-right.tab-issues-buttons - if can?(current_user, :create_issue, @project) = link_to new_namespace_project_issue_path(@project.namespace, @project, issue: { milestone_id: @milestone.id }), class: "btn btn-grouped", title: "New Issue" do diff --git a/app/views/search/_form.html.haml b/app/views/search/_form.html.haml index 17b0981f073..a9dbc84da29 100644 --- a/app/views/search/_form.html.haml +++ b/app/views/search/_form.html.haml @@ -11,4 +11,4 @@ = button_tag 'Search', class: "btn btn-primary" - unless params[:snippets].eql? 'true' %br - = render 'filter' + = render 'filter' if current_user diff --git a/app/views/search/_results.html.haml b/app/views/search/_results.html.haml index d0e64537621..eef7b7b5efd 100644 --- a/app/views/search/_results.html.haml +++ b/app/views/search/_results.html.haml @@ -18,7 +18,7 @@ = render 'shared/projects/list', projects: @objects - else = render partial: "search/results/#{@scope.singularize}", collection: @objects - = paginate @objects, theme: 'gitlab' + = paginate @objects, theme: 'gitlab' :javascript $(".search-results .term").highlight("#{escape_javascript(params[:search])}"); diff --git a/config/application.rb b/config/application.rb index b905f1a3e90..28684a3e578 100644 --- a/config/application.rb +++ b/config/application.rb @@ -54,14 +54,6 @@ module Gitlab config.action_view.sanitized_allowed_protocols = %w(smb) - # Relative URL support - # WARNING: We recommend using an FQDN to host GitLab in a root path instead - # of using a relative URL. - # Documentation: http://doc.gitlab.com/ce/install/relative_url.html - # Uncomment and customize the following line to run in a non-root path - # - # config.relative_url_root = "/gitlab" - config.middleware.use Rack::Attack # Allow access to GitLab API from other domains diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 713204b1c51..626268d7648 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -207,11 +207,7 @@ Settings.gitlab_ci['builds_path'] = File.expand_path(Settings.gitlab_c # Reply by email # Settings['incoming_email'] ||= Settingslogic.new({}) -Settings.incoming_email['enabled'] = false if Settings.incoming_email['enabled'].nil? -Settings.incoming_email['port'] = 143 if Settings.incoming_email['port'].nil? -Settings.incoming_email['ssl'] = false if Settings.incoming_email['ssl'].nil? -Settings.incoming_email['start_tls'] = false if Settings.incoming_email['start_tls'].nil? -Settings.incoming_email['mailbox'] = "inbox" if Settings.incoming_email['mailbox'].nil? +Settings.incoming_email['enabled'] = false if Settings.incoming_email['enabled'].nil? # # Build Artifacts diff --git a/config/initializers/relative_url.rb.sample b/config/initializers/relative_url.rb.sample new file mode 100644 index 00000000000..125297d5385 --- /dev/null +++ b/config/initializers/relative_url.rb.sample @@ -0,0 +1,10 @@ +# Relative URL support +# WARNING: We recommend using an FQDN to host GitLab in a root path instead +# of using a relative URL. +# Documentation: http://doc.gitlab.com/ce/install/relative_url.html +# Copy this file to relative_url.rb and customize it to run in a non-root path +# + +Rails.application.configure do + config.relative_url_root = "/gitlab" +end diff --git a/config/mail_room.yml b/config/mail_room.yml index 42f6f74c465..f266a70ee0d 100644 --- a/config/mail_room.yml +++ b/config/mail_room.yml @@ -1,39 +1,52 @@ :mailboxes: <% -require_relative 'config/environment.rb' - -if Gitlab::IncomingEmail.enabled? - config = Gitlab::IncomingEmail.config - - redis_config_file = "config/resque.yml" - redis_url = - if File.exists?(redis_config_file) - YAML.load_file(redis_config_file)[Rails.env] - else - "redis://localhost:6379" - end - %> - - - :host: <%= config.host.to_json %> - :port: <%= config.port.to_json %> - :ssl: <%= config.ssl.to_json %> - :start_tls: <%= config.start_tls.to_json %> - :email: <%= config.user.to_json %> - :password: <%= config.password.to_json %> - - :name: <%= config.mailbox.to_json %> - - :delete_after_delivery: true - - :delivery_method: sidekiq - :delivery_options: - :redis_url: <%= redis_url.to_json %> - :namespace: resque:gitlab - :queue: incoming_email - :worker: EmailReceiverWorker - - :arbitration_method: redis - :arbitration_options: - :redis_url: <%= redis_url.to_json %> - :namespace: mail_room:gitlab +require "yaml" +require "json" + +rails_env = ENV["RAILS_ENV"] || ENV["RACK_ENV"] || "development" + +config_file = ENV["MAIL_ROOM_GITLAB_CONFIG_FILE"] || "config/gitlab.yml" +if File.exists?(config_file) + all_config = YAML.load_file(config_file)[rails_env] + + config = all_config["incoming_email"] || {} + config['enabled'] = false if config['enabled'].nil? + config['port'] = 143 if config['port'].nil? + config['ssl'] = false if config['ssl'].nil? + config['start_tls'] = false if config['start_tls'].nil? + config['mailbox'] = "inbox" if config['mailbox'].nil? + + if config['enabled'] && config['address'] && config['address'].include?('%{key}') + redis_config_file = "config/resque.yml" + redis_url = + if File.exists?(redis_config_file) + YAML.load_file(redis_config_file)[rails_env] + else + "redis://localhost:6379" + end + %> + - + :host: <%= config['host'].to_json %> + :port: <%= config['port'].to_json %> + :ssl: <%= config['ssl'].to_json %> + :start_tls: <%= config['start_tls'].to_json %> + :email: <%= config['user'].to_json %> + :password: <%= config['password'].to_json %> + + :name: <%= config['mailbox'].to_json %> + + :delete_after_delivery: true + + :delivery_method: sidekiq + :delivery_options: + :redis_url: <%= redis_url.to_json %> + :namespace: resque:gitlab + :queue: incoming_email + :worker: EmailReceiverWorker + + :arbitration_method: redis + :arbitration_options: + :redis_url: <%= redis_url.to_json %> + :namespace: mail_room:gitlab + <% end %> <% end %> diff --git a/doc/development/doc_styleguide.md b/doc/development/doc_styleguide.md index 96d1dffbc52..187ec9e7b75 100644 --- a/doc/development/doc_styleguide.md +++ b/doc/development/doc_styleguide.md @@ -85,9 +85,19 @@ Inside the document: ## Notes -- Notes should be in italics with the word `Note:` being bold. Use this form: - `_**Note:** This is something to note._`. If the note spans across multiple - lines it's OK to split the line. +- Notes should be quoted with the word `Note:` being bold. Use this form: + + ``` + >**Note:** + This is something to note. + ``` + + which renders to: + + >**Note:** + This is something to note. + + If the note spans across multiple lines it's OK to split the line. ## New features diff --git a/doc/install/relative_url.md b/doc/install/relative_url.md index b341b6311c6..0245febfcd8 100644 --- a/doc/install/relative_url.md +++ b/doc/install/relative_url.md @@ -25,7 +25,7 @@ that points to your GitLab instance. The TL;DR list of configuration files that you need to change in order to serve GitLab under a relative URL is: -- `/home/git/gitlab/config/application.rb` +- `/home/git/gitlab/config/initializers/relative_url.rb` - `/home/git/gitlab/config/gitlab.yml` - `/home/git/gitlab/config/unicorn.rb` - `/home/git/gitlab-shell/config.yml` @@ -66,8 +66,14 @@ Make sure to follow all steps below: sudo service gitlab stop ``` -1. Edit `/home/git/gitlab/config/application.rb` and uncomment/change the - following line: +1. Create `/home/git/gitlab/config/initializers/relative_url.rb` + + ```shell + cp /home/git/gitlab/config/initializers/relative_url.rb.sample \ + /home/git/gitlab/config/initializers/relative_url.rb + ``` + + and change the following line: ```ruby config.relative_url_root = "/gitlab" @@ -119,8 +125,12 @@ Make sure to follow all steps below: ### Disable relative URL in GitLab -To disable the relative URL, follow the same steps as above and set up the -GitLab URL to one that doesn't contain a relative path. +To disable the relative URL: + +1. Remove `/home/git/gitlab/config/initializers/relative_url.rb` + +1. Follow the same as above starting from 2. and set up the + GitLab URL to one that doesn't contain a relative path. [omnibus-rel]: http://doc.gitlab.com/omnibus/settings/configuration.html#configuring-a-relative-url-for-gitlab "How to setup relative URL in Omnibus GitLab" [restart gitlab]: ../administration/restart_gitlab.md#installations-from-source "How to restart GitLab" diff --git a/doc/integration/omniauth.md b/doc/integration/omniauth.md index 8e6627b2be5..ba47cb16265 100644 --- a/doc/integration/omniauth.md +++ b/doc/integration/omniauth.md @@ -1,8 +1,11 @@ # OmniAuth -GitLab leverages OmniAuth to allow users to sign in using Twitter, GitHub, and other popular services. +GitLab leverages OmniAuth to allow users to sign in using Twitter, GitHub, and +other popular services. -Configuring OmniAuth does not prevent standard GitLab authentication or LDAP (if configured) from continuing to work. Users can choose to sign in using any of the configured mechanisms. +Configuring OmniAuth does not prevent standard GitLab authentication or LDAP +(if configured) from continuing to work. Users can choose to sign in using any +of the configured mechanisms. - [Initial OmniAuth Configuration](#initial-omniauth-configuration) - [Supported Providers](#supported-providers) @@ -28,17 +31,25 @@ contains some settings that are common for all providers. ## Initial OmniAuth Configuration -Before configuring individual OmniAuth providers there are a few global settings that are in common for all providers that we need to consider. +Before configuring individual OmniAuth providers there are a few global settings +that are in common for all providers that we need to consider. - Omniauth needs to be enabled, see details below for example. -- `allow_single_sign_on` defaults to `false`. If `false` users must be created manually or they will not be able to -sign in via OmniAuth. -- `block_auto_created_users` defaults to `true`. If `true` auto created users will be blocked by default and will -have to be unblocked by an administrator before they are able to sign in. -- **Note:** If you set `allow_single_sign_on` to `true` and `block_auto_created_users` to `false` please be aware -that any user on the Internet will be able to successfully sign in to your GitLab without administrative approval. - -If you want to change these settings: +- `allow_single_sign_on` allows you to specify the providers you want to allow to + automatically create an account. It defaults to `false`. If `false` users must + be created manually or they will not be able to sign in via OmniAuth. +- `block_auto_created_users` defaults to `true`. If `true` auto created users will + be blocked by default and will have to be unblocked by an administrator before + they are able to sign in. + +>**Note:** +If you set `block_auto_created_users` to `false`, make sure to only +define providers under `allow_single_sign_on` that you are able to control, like +SAML, Shibboleth, Crowd or Google, or set it to `false` otherwise any user on +the Internet will be able to successfully sign in to your GitLab without +administrative approval. + +To change these settings: * **For omnibus package** @@ -48,11 +59,16 @@ If you want to change these settings: sudo editor /etc/gitlab/gitlab.rb ``` - and change + and change: - ``` + ```ruby gitlab_rails['omniauth_enabled'] = true - gitlab_rails['omniauth_allow_single_sign_on'] = false + + # CAUTION! + # This allows users to login without having a user account first. Define the allowed providers + # using an array, e.g. ["saml", "twitter"], or as true/false to allow all providers or none. + # User accounts will be created automatically when authentication was successful. + gitlab_rails['omniauth_allow_single_sign_on'] = ['saml', 'twitter'] gitlab_rails['omniauth_block_auto_created_users'] = true ``` @@ -66,43 +82,57 @@ If you want to change these settings: sudo -u git -H editor config/gitlab.yml ``` - and change the following section + and change the following section: - ``` + ```yaml ## OmniAuth settings omniauth: # Allow login via Twitter, Google, etc. using OmniAuth providers enabled: true # CAUTION! - # This allows users to login without having a user account first (default: false). + # This allows users to login without having a user account first. Define the allowed providers + # using an array, e.g. ["saml", "twitter"], or as true/false to allow all providers or none. # User accounts will be created automatically when authentication was successful. - allow_single_sign_on: false + allow_single_sign_on: ["saml", "twitter"] + # Locks down those users until they have been cleared by the admin (default: true). block_auto_created_users: true ``` -Now we can choose one or more of the Supported Providers below to continue configuration. +Now we can choose one or more of the Supported Providers listed above to continue +the configuration process. ## Enable OmniAuth for an Existing User -Existing users can enable OmniAuth for specific providers after the account is created. For example, if the user originally signed in with LDAP an OmniAuth provider such as Twitter can be enabled. Follow the steps below to enable an OmniAuth provider for an existing user. +Existing users can enable OmniAuth for specific providers after the account is +created. For example, if the user originally signed in with LDAP, an OmniAuth +provider such as Twitter can be enabled. Follow the steps below to enable an +OmniAuth provider for an existing user. 1. Sign in normally - whether standard sign in, LDAP, or another OmniAuth provider. 1. Go to profile settings (the silhouette icon in the top right corner). 1. Select the "Account" tab. 1. Under "Connected Accounts" select the desired OmniAuth provider, such as Twitter. -1. The user will be redirected to the provider. Once the user authorized GitLab they will be redirected back to GitLab. +1. The user will be redirected to the provider. Once the user authorized GitLab + they will be redirected back to GitLab. The chosen OmniAuth provider is now active and can be used to sign in to GitLab from then on. ## Using Custom Omniauth Providers -GitLab uses [Omniauth](http://www.omniauth.org/) for authentication and already ships with a few providers preinstalled (e.g. LDAP, GitHub, Twitter). But sometimes that is not enough and you need to integrate with other authentication solutions. For these cases you can use the Omniauth provider. +>**Note:** +The following information only applies for installations from source. + +GitLab uses [Omniauth](http://www.omniauth.org/) for authentication and already ships +with a few providers pre-installed (e.g. LDAP, GitHub, Twitter). But sometimes that +is not enough and you need to integrate with other authentication solutions. For +these cases you can use the Omniauth provider. ### Steps -These steps are fairly general and you will need to figure out the exact details from the Omniauth provider's documentation. +These steps are fairly general and you will need to figure out the exact details +from the Omniauth provider's documentation. - Stop GitLab: @@ -128,8 +158,12 @@ These steps are fairly general and you will need to figure out the exact details ### Examples -If you have successfully set up a provider that is not shipped with GitLab itself, please let us know. +If you have successfully set up a provider that is not shipped with GitLab itself, +please let us know. -You can help others by reporting successful configurations and probably share a few insights or provide warnings for common errors or pitfalls by sharing your experience [in the public Wiki](https://github.com/gitlabhq/gitlab-public-wiki/wiki/Custom-omniauth-provider-configurations). +You can help others by reporting successful configurations and probably share a +few insights or provide warnings for common errors or pitfalls by sharing your +experience [in the public Wiki](https://github.com/gitlabhq/gitlab-public-wiki/wiki/Custom-omniauth-provider-configurations). -While we can't officially support every possible authentication mechanism out there, we'd like to at least help those with specific needs. +While we can't officially support every possible authentication mechanism out there, +we'd like to at least help those with specific needs. diff --git a/doc/integration/saml.md b/doc/integration/saml.md index 8841dbdb7c6..c84113556cd 100644 --- a/doc/integration/saml.md +++ b/doc/integration/saml.md @@ -1,10 +1,14 @@ # SAML OmniAuth Provider -GitLab can be configured to act as a SAML 2.0 Service Provider (SP). This allows GitLab to consume assertions from a SAML 2.0 Identity Provider (IdP) such as Microsoft ADFS to authenticate users. +GitLab can be configured to act as a SAML 2.0 Service Provider (SP). This allows +GitLab to consume assertions from a SAML 2.0 Identity Provider (IdP) such as +Microsoft ADFS to authenticate users. -First configure SAML 2.0 support in GitLab, then register the GitLab application in your SAML IdP: +First configure SAML 2.0 support in GitLab, then register the GitLab application +in your SAML IdP: -1. Make sure GitLab is configured with HTTPS. See [Using HTTPS](../install/installation.md#using-https) for instructions. +1. Make sure GitLab is configured with HTTPS. + See [Using HTTPS](../install/installation.md#using-https) for instructions. 1. On your GitLab server, open the configuration file. @@ -22,7 +26,40 @@ First configure SAML 2.0 support in GitLab, then register the GitLab application sudo -u git -H editor config/gitlab.yml ``` -1. See [Initial OmniAuth Configuration](omniauth.md#initial-omniauth-configuration) for initial settings. +1. See [Initial OmniAuth Configuration](omniauth.md#initial-omniauth-configuration) + for initial settings. + +1. To allow your users to use SAML to sign up without having to manually create + an account first, don't forget to add the following values to your configuration: + + For omnibus package: + + ```ruby + gitlab_rails['omniauth_allow_single_sign_on'] = ['saml'] + gitlab_rails['omniauth_block_auto_created_users'] = false + ``` + + For installations from source: + + ```yaml + allow_single_sign_on: ["saml"] + block_auto_created_users: false + ``` + +1. You can also automatically link SAML users with existing GitLab users if their + email addresses match by adding the following setting: + + For omnibus package: + + ```ruby + gitlab_rails['omniauth_auto_link_saml_user'] = true + ``` + + For installations from source: + + ```yaml + auto_link_saml_user: true + ``` 1. Add the provider configuration: @@ -31,15 +68,15 @@ First configure SAML 2.0 support in GitLab, then register the GitLab application ```ruby gitlab_rails['omniauth_providers'] = [ { - "name" => "saml", - args: { + name: 'saml', + args: { assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', idp_sso_target_url: 'https://login.example.com/idp', issuer: 'https://gitlab.example.com', name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' }, - "label" => "Company Login" # optional label for SAML login button, defaults to "Saml" + label: 'Company Login' # optional label for SAML login button, defaults to "Saml" } ] ``` @@ -47,34 +84,52 @@ First configure SAML 2.0 support in GitLab, then register the GitLab application For installations from source: ```yaml - - { name: 'saml', - args: { - assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', - idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', - idp_sso_target_url: 'https://login.example.com/idp', - issuer: 'https://gitlab.example.com', - name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' - } } + - { + name: 'saml', + args: { + assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', + idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', + idp_sso_target_url: 'https://login.example.com/idp', + issuer: 'https://gitlab.example.com', + name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' + }, + label: 'Company Login' # optional label for SAML login button, defaults to "Saml" + } ``` -1. Change the value for 'assertion_consumer_service_url' to match the HTTPS endpoint of GitLab (append 'users/auth/saml/callback' to the HTTPS URL of your GitLab installation to generate the correct value). +1. Change the value for `assertion_consumer_service_url` to match the HTTPS endpoint + of GitLab (append `users/auth/saml/callback` to the HTTPS URL of your GitLab + installation to generate the correct value). -1. Change the values of 'idp_cert_fingerprint', 'idp_sso_target_url', 'name_identifier_format' to match your IdP. Check [the omniauth-saml documentation](https://github.com/PracticallyGreen/omniauth-saml) for details on these options. +1. Change the values of `idp_cert_fingerprint`, `idp_sso_target_url`, + `name_identifier_format` to match your IdP. Check + [the omniauth-saml documentation](https://github.com/omniauth/omniauth-saml) + for details on these options. -1. Change the value of 'issuer' to a unique name, which will identify the application to the IdP. +1. Change the value of `issuer` to a unique name, which will identify the application + to the IdP. 1. Restart GitLab for the changes to take effect. -1. Register the GitLab SP in your SAML 2.0 IdP, using the application name specified in 'issuer'. +1. Register the GitLab SP in your SAML 2.0 IdP, using the application name specified + in `issuer`. -To ease configuration, most IdP accept a metadata URL for the application to provide configuration information to the IdP. To build the metadata URL for GitLab, append 'users/auth/saml/metadata' to the HTTPS URL of your GitLab installation, for instance: +To ease configuration, most IdP accept a metadata URL for the application to provide +configuration information to the IdP. To build the metadata URL for GitLab, append +`users/auth/saml/metadata` to the HTTPS URL of your GitLab installation, for instance: ``` https://gitlab.example.com/users/auth/saml/metadata ``` -At a minimum the IdP *must* provide a claim containing the user's email address, using claim name 'email' or 'mail'. The email will be used to automatically generate the GitLab username. GitLab will also use claims with name 'name', 'first_name', 'last_name' (see [the omniauth-saml gem](https://github.com/PracticallyGreen/omniauth-saml/blob/master/lib/omniauth/strategies/saml.rb) for supported claims). +At a minimum the IdP *must* provide a claim containing the user's email address, using +claim name `email` or `mail`. The email will be used to automatically generate the GitLab +username. GitLab will also use claims with name `name`, `first_name`, `last_name` +(see [the omniauth-saml gem](https://github.com/omniauth/omniauth-saml/blob/master/lib/omniauth/strategies/saml.rb) +for supported claims). -On the sign in page there should now be a SAML button below the regular sign in form. Click the icon to begin the authentication process. If everything goes well the user will be returned to GitLab and will be signed in. +On the sign in page there should now be a SAML button below the regular sign in form. +Click the icon to begin the authentication process. If everything goes well the user +will be returned to GitLab and will be signed in. ## Troubleshooting @@ -82,7 +137,7 @@ If you see a "500 error" in GitLab when you are redirected back from the SAML si this likely indicates that GitLab could not get the email address for the SAML user. Make sure the IdP provides a claim containing the user's email address, using claim name -'email' or 'mail'. The email will be used to automatically generate the GitLab username. +`email` or `mail`. If after signing in into your SAML server you are redirected back to the sign in page and no error is displayed, check your `production.log` file. It will most likely contain the diff --git a/features/search.feature b/features/search.feature index a9234c1a611..3cd52810e59 100644 --- a/features/search.feature +++ b/features/search.feature @@ -65,3 +65,25 @@ Feature: Search And I search for "Wiki content" And I click "Wiki" link Then I should see "test_wiki" link in the search results + + Scenario: I logout and should see project I am looking for + Given project "Shop" is public + And I logout + And I search for "Sho" + Then I should see "Shop" project link + + Scenario: I logout and should see issues I am looking for + Given project "Shop" is public + And I logout + And project has issues + When I search for "Foo" + And I click "Issues" link + Then I should see "Foo" link in the search results + And I should not see "Bar" link in the search results + + Scenario: I logout and should see project code I am looking for + Given project "Shop" is public + And I logout + When I visit project "Shop" page + And I search for "rspec" on project page + Then I should see code results for project "Shop" diff --git a/features/steps/search.rb b/features/steps/search.rb index 79273cbad9a..48ea3fa3876 100644 --- a/features/steps/search.rb +++ b/features/steps/search.rb @@ -18,6 +18,11 @@ class Spinach::Features::Search < Spinach::FeatureSteps click_button "Search" end + step 'I search for "rspec" on project page' do + fill_in "search", with: "rspec" + click_button "Go" + end + step 'I search for "Wiki content"' do fill_in "dashboard_search", with: "content" click_button "Search" @@ -103,4 +108,8 @@ class Spinach::Features::Search < Spinach::FeatureSteps @wiki = ::ProjectWiki.new(project, current_user) @wiki.create_page("test_wiki", "Some Wiki content", :markdown, "first commit") end + + step 'project "Shop" is public' do + project.update_attributes(visibility_level: Project::PUBLIC) + end end diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index 9422d438d21..8e74e177ea0 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -18,10 +18,12 @@ module API # Examples: # GET /projects/:id/repository/commits/:sha/statuses get ':id/repository/commits/:sha/statuses' do - authorize! :read_commit_status, user_project - sha = params[:sha] - ci_commit = user_project.ci_commit(sha) - not_found! 'Commit' unless ci_commit + authorize!(:read_commit_status, user_project) + + not_found!('Commit') unless user_project.commit(params[:sha]) + ci_commit = user_project.ci_commit(params[:sha]) + return [] unless ci_commit + statuses = ci_commit.statuses statuses = statuses.latest unless parse_boolean(params[:all]) statuses = statuses.where(ref: params[:ref]) if params[:ref].present? diff --git a/lib/banzai/filter/gollum_tags_filter.rb b/lib/banzai/filter/gollum_tags_filter.rb index fe01dae4850..bcf5297e382 100644 --- a/lib/banzai/filter/gollum_tags_filter.rb +++ b/lib/banzai/filter/gollum_tags_filter.rb @@ -50,21 +50,30 @@ module Banzai # See https://github.com/gollum/gollum/wiki # # Rubular: http://rubular.com/r/7dQnE5CUCH - TAGS_PATTERN = %r{\[\[(.+?)\]\]} + TAGS_PATTERN = %r{\[\[(.+?)\]\]}.freeze # Pattern to match allowed image extensions - ALLOWED_IMAGE_EXTENSIONS = %r{.+(jpg|png|gif|svg|bmp)\z}i + ALLOWED_IMAGE_EXTENSIONS = %r{.+(jpg|png|gif|svg|bmp)\z}i.freeze def call search_text_nodes(doc).each do |node| - content = node.content + # A Gollum ToC tag is `[[_TOC_]]`, but due to MarkdownFilter running + # before this one, it will be converted into `[[<em>TOC</em>]]`, so it + # needs special-case handling + if toc_tag?(node) + next unless result[:toc].present? - next unless content.match(TAGS_PATTERN) + process_toc_tag(node) + else + content = node.content - html = process_tag($1) + next unless content =~ TAGS_PATTERN - if html && html != node.content - node.replace(html) + html = process_tag($1) + + if html && html != node.content + node.replace(html) + end end end @@ -73,6 +82,12 @@ module Banzai private + # Replace an entire `[[<em>TOC</em>]]` node with the result generated by + # TableOfContentsFilter + def process_toc_tag(node) + node.parent.parent.replace(result[:toc]) + end + # Process a single tag into its final HTML form. # # tag - The String tag contents (the stuff inside the double brackets). @@ -108,6 +123,12 @@ module Banzai end end + def toc_tag?(node) + node.content == 'TOC' && + node.parent.name == 'em' && + node.parent.parent.text == '[[TOC]]' + end + def image?(path) path =~ ALLOWED_IMAGE_EXTENSIONS end diff --git a/lib/banzai/filter_array.rb b/lib/banzai/filter_array.rb new file mode 100644 index 00000000000..77835a14027 --- /dev/null +++ b/lib/banzai/filter_array.rb @@ -0,0 +1,27 @@ +module Banzai + class FilterArray < Array + # Insert a value immediately after another value + # + # If the preceding value does not exist, the new value is added to the end + # of the Array. + def insert_after(after_value, value) + i = index(after_value) || length - 1 + + insert(i + 1, value) + end + + # Insert a value immediately before another value + # + # If the succeeding value does not exist, the new value is added to the + # beginning of the Array. + def insert_before(before_value, value) + i = index(before_value) || -1 + + if i < 0 + unshift(value) + else + insert(i, value) + end + end + end +end diff --git a/lib/banzai/pipeline/base_pipeline.rb b/lib/banzai/pipeline/base_pipeline.rb index db5177db7b3..f60966c3c0f 100644 --- a/lib/banzai/pipeline/base_pipeline.rb +++ b/lib/banzai/pipeline/base_pipeline.rb @@ -4,7 +4,7 @@ module Banzai module Pipeline class BasePipeline def self.filters - [] + FilterArray[] end def self.transform_context(context) diff --git a/lib/banzai/pipeline/broadcast_message_pipeline.rb b/lib/banzai/pipeline/broadcast_message_pipeline.rb index 4bb85e24c38..adc09c8afbd 100644 --- a/lib/banzai/pipeline/broadcast_message_pipeline.rb +++ b/lib/banzai/pipeline/broadcast_message_pipeline.rb @@ -2,7 +2,7 @@ module Banzai module Pipeline class BroadcastMessagePipeline < DescriptionPipeline def self.filters - @filters ||= [ + @filters ||= FilterArray[ Filter::MarkdownFilter, Filter::SanitizationFilter, diff --git a/lib/banzai/pipeline/combined_pipeline.rb b/lib/banzai/pipeline/combined_pipeline.rb index 9485199132e..60190f8d9dd 100644 --- a/lib/banzai/pipeline/combined_pipeline.rb +++ b/lib/banzai/pipeline/combined_pipeline.rb @@ -10,7 +10,7 @@ module Banzai end def self.filters - pipelines.flat_map(&:filters) + FilterArray.new(pipelines.flat_map(&:filters)) end def self.transform_context(context) diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb index b7a38ea8427..8cd4b50e65a 100644 --- a/lib/banzai/pipeline/gfm_pipeline.rb +++ b/lib/banzai/pipeline/gfm_pipeline.rb @@ -2,7 +2,7 @@ module Banzai module Pipeline class GfmPipeline < BasePipeline def self.filters - @filters ||= [ + @filters ||= FilterArray[ Filter::SyntaxHighlightFilter, Filter::SanitizationFilter, diff --git a/lib/banzai/pipeline/plain_markdown_pipeline.rb b/lib/banzai/pipeline/plain_markdown_pipeline.rb index 3fbc681457b..3f45db21869 100644 --- a/lib/banzai/pipeline/plain_markdown_pipeline.rb +++ b/lib/banzai/pipeline/plain_markdown_pipeline.rb @@ -2,7 +2,7 @@ module Banzai module Pipeline class PlainMarkdownPipeline < BasePipeline def self.filters - [ + FilterArray[ Filter::MarkdownFilter ] end diff --git a/lib/banzai/pipeline/post_process_pipeline.rb b/lib/banzai/pipeline/post_process_pipeline.rb index bd338c045f3..ecff094b1e5 100644 --- a/lib/banzai/pipeline/post_process_pipeline.rb +++ b/lib/banzai/pipeline/post_process_pipeline.rb @@ -2,7 +2,7 @@ module Banzai module Pipeline class PostProcessPipeline < BasePipeline def self.filters - [ + FilterArray[ Filter::RelativeLinkFilter, Filter::RedactorFilter ] diff --git a/lib/banzai/pipeline/reference_extraction_pipeline.rb b/lib/banzai/pipeline/reference_extraction_pipeline.rb index eaddccd30a5..919998380e4 100644 --- a/lib/banzai/pipeline/reference_extraction_pipeline.rb +++ b/lib/banzai/pipeline/reference_extraction_pipeline.rb @@ -2,7 +2,7 @@ module Banzai module Pipeline class ReferenceExtractionPipeline < BasePipeline def self.filters - [ + FilterArray[ Filter::ReferenceGathererFilter ] end diff --git a/lib/banzai/pipeline/single_line_pipeline.rb b/lib/banzai/pipeline/single_line_pipeline.rb index 8b84ab401df..ba2555df98d 100644 --- a/lib/banzai/pipeline/single_line_pipeline.rb +++ b/lib/banzai/pipeline/single_line_pipeline.rb @@ -2,7 +2,7 @@ module Banzai module Pipeline class SingleLinePipeline < GfmPipeline def self.filters - @filters ||= [ + @filters ||= FilterArray[ Filter::SanitizationFilter, Filter::EmojiFilter, diff --git a/lib/banzai/pipeline/wiki_pipeline.rb b/lib/banzai/pipeline/wiki_pipeline.rb index 50b5450e70b..9b4ff0f0f80 100644 --- a/lib/banzai/pipeline/wiki_pipeline.rb +++ b/lib/banzai/pipeline/wiki_pipeline.rb @@ -4,7 +4,8 @@ module Banzai module Pipeline class WikiPipeline < FullPipeline def self.filters - super.insert(1, Filter::GollumTagsFilter) + @filters ||= super.insert_after(Filter::TableOfContentsFilter, + Filter::GollumTagsFilter) end end end diff --git a/lib/gitlab/gitlab_import/importer.rb b/lib/gitlab/gitlab_import/importer.rb index 59926084d07..850b73244c6 100644 --- a/lib/gitlab/gitlab_import/importer.rb +++ b/lib/gitlab/gitlab_import/importer.rb @@ -12,7 +12,7 @@ module Gitlab end def execute - project_identifier = CGI.escape(project.import_source, '/') + project_identifier = CGI.escape(project.import_source) #Issues && Comments issues = client.issues(project_identifier) diff --git a/lib/tasks/cache.rake b/lib/tasks/cache.rake index 9e2fb429d57..f221afcf73a 100644 --- a/lib/tasks/cache.rake +++ b/lib/tasks/cache.rake @@ -1,5 +1,5 @@ namespace :cache do - CLEAR_BATCH_SIZE = 1000 # The more the faster, but having too many can crash Ruby + CLEAR_BATCH_SIZE = 1000 # There seems to be no speedup when pushing beyond 1,000 REDIS_SCAN_START_STOP = '0' # Magic value, see http://redis.io/commands/scan desc "GitLab | Clear redis cache" diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index d59872dc3a2..581ab26db79 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -728,13 +728,15 @@ namespace :gitlab do def check_imap_authentication print "IMAP server credentials are correct? ... " - config = Gitlab.config.incoming_email + config_path = Rails.root.join('config', 'mail_room.yml') + config_file = YAML.load(ERB.new(File.read(config_path)).result) + config = config_file[:mailboxes].first if config begin - imap = Net::IMAP.new(config.host, port: config.port, ssl: config.ssl) - imap.starttls if config.start_tls - imap.login(config.user, config.password) + imap = Net::IMAP.new(config[:host], port: config[:port], ssl: config[:ssl]) + imap.starttls if config[:start_tls] + imap.login(config[:email], config[:password]) connected = true rescue connected = false diff --git a/spec/config/mail_room_spec.rb b/spec/config/mail_room_spec.rb new file mode 100644 index 00000000000..462afb24f08 --- /dev/null +++ b/spec/config/mail_room_spec.rb @@ -0,0 +1,56 @@ +require "spec_helper" + +describe "mail_room.yml" do + let(:config_path) { "config/mail_room.yml" } + let(:configuration) { YAML.load(ERB.new(File.read(config_path)).result) } + + context "when incoming email is disabled" do + before do + ENV["MAIL_ROOM_GITLAB_CONFIG_FILE"] = Rails.root.join("spec/fixtures/mail_room_disabled.yml").to_s + end + + after do + ENV["MAIL_ROOM_GITLAB_CONFIG_FILE"] = nil + end + + it "contains no configuration" do + expect(configuration[:mailboxes]).to be_nil + end + end + + context "when incoming email is enabled" do + before do + ENV["MAIL_ROOM_GITLAB_CONFIG_FILE"] = Rails.root.join("spec/fixtures/mail_room_enabled.yml").to_s + end + + after do + ENV["MAIL_ROOM_GITLAB_CONFIG_FILE"] = nil + end + + it "contains the intended configuration" do + expect(configuration[:mailboxes].length).to eq(1) + + mailbox = configuration[:mailboxes].first + + expect(mailbox[:host]).to eq("imap.gmail.com") + expect(mailbox[:port]).to eq(993) + expect(mailbox[:ssl]).to eq(true) + expect(mailbox[:start_tls]).to eq(false) + expect(mailbox[:email]).to eq("gitlab-incoming@gmail.com") + expect(mailbox[:password]).to eq("[REDACTED]") + expect(mailbox[:name]).to eq("inbox") + + redis_config_file = Rails.root.join('config', 'resque.yml') + + redis_url = + if File.exists?(redis_config_file) + YAML.load_file(redis_config_file)[Rails.env] + else + "redis://localhost:6379" + end + + expect(mailbox[:delivery_options][:redis_url]).to eq(redis_url) + expect(mailbox[:arbitration_options][:redis_url]).to eq(redis_url) + end + end +end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 104a5f50143..7337ff58be1 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -41,6 +41,7 @@ describe UsersController do end describe 'GET #calendar' do + it 'renders calendar' do sign_in(user) @@ -48,6 +49,23 @@ describe UsersController do expect(response).to render_template('calendar') end + + context 'forked project' do + let!(:project) { create(:project) } + let!(:forked_project) { Projects::ForkService.new(project, user).execute } + + before do + sign_in(user) + project.team << [user, :developer] + EventCreateService.new.push(project, user, []) + EventCreateService.new.push(forked_project, user, []) + end + + it 'includes forked projects' do + get :calendar, username: user.username + expect(assigns(:contributions_calendar).projects.count).to eq(2) + end + end end describe 'GET #calendar_activities' do diff --git a/spec/factories.rb b/spec/factories.rb index 264e3ed2c8d..cd57661c1cd 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -32,6 +32,7 @@ FactoryGirl.define do before(:create) do |user| user.two_factor_enabled = true user.otp_secret = User.generate_otp_secret(32) + user.otp_grace_period_started_at = Time.now user.generate_otp_backup_codes! end end diff --git a/spec/fixtures/mail_room_disabled.yml b/spec/fixtures/mail_room_disabled.yml new file mode 100644 index 00000000000..97f8cff051f --- /dev/null +++ b/spec/fixtures/mail_room_disabled.yml @@ -0,0 +1,11 @@ +test: + incoming_email: + enabled: false + address: "gitlab-incoming+%{key}@gmail.com" + user: "gitlab-incoming@gmail.com" + password: "[REDACTED]" + host: "imap.gmail.com" + port: 993 + ssl: true + start_tls: false + mailbox: "inbox" diff --git a/spec/fixtures/mail_room_enabled.yml b/spec/fixtures/mail_room_enabled.yml new file mode 100644 index 00000000000..9c94649244d --- /dev/null +++ b/spec/fixtures/mail_room_enabled.yml @@ -0,0 +1,11 @@ +test: + incoming_email: + enabled: true + address: "gitlab-incoming+%{key}@gmail.com" + user: "gitlab-incoming@gmail.com" + password: "[REDACTED]" + host: "imap.gmail.com" + port: 993 + ssl: true + start_tls: false + mailbox: "inbox" diff --git a/spec/lib/banzai/filter/gollum_tags_filter_spec.rb b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb index 38baa819957..11cc290d6d0 100644 --- a/spec/lib/banzai/filter/gollum_tags_filter_spec.rb +++ b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb @@ -86,4 +86,56 @@ describe Banzai::Filter::GollumTagsFilter, lib: true do expect(doc.at_css('a')['href']).to eq 'wiki-slug' end end + + context 'table of contents' do + let(:pipeline) { Banzai::Pipeline[:wiki] } + + it 'replaces the tag with the TableOfContentsFilter result' do + markdown = <<-MD.strip_heredoc + [[_TOC_]] + + ## Header + + Foo + MD + + result = pipeline.call(markdown, project_wiki: project_wiki, project: project) + + aggregate_failures do + expect(result[:output].text).not_to include '[[_TOC_]]' + expect(result[:output].text).not_to include '[[' + expect(result[:output].to_html).to include(result[:toc]) + end + end + + it 'is case-sensitive' do + markdown = <<-MD.strip_heredoc + [[_toc_]] + + # Header 1 + + Foo + MD + + output = pipeline.to_html(markdown, project_wiki: project_wiki, project: project) + + expect(output).to include('[[<em>toc</em>]]') + end + + it 'handles an empty pipeline result' do + # No Markdown headers in this doc, so `result[:toc]` will be empty + markdown = <<-MD.strip_heredoc + [[_TOC_]] + + Foo + MD + + output = pipeline.to_html(markdown, project_wiki: project_wiki, project: project) + + aggregate_failures do + expect(output).not_to include('<ul>') + expect(output).to include('[[<em>TOC</em>]]') + end + end + end end diff --git a/spec/lib/banzai/filter_array_spec.rb b/spec/lib/banzai/filter_array_spec.rb new file mode 100644 index 00000000000..ea84005e7f8 --- /dev/null +++ b/spec/lib/banzai/filter_array_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe Banzai::FilterArray do + describe '#insert_after' do + it 'inserts an element after a provided element' do + filters = described_class.new(%w(a b c)) + + filters.insert_after('b', '1') + + expect(filters).to eq %w(a b 1 c) + end + + it 'inserts an element at the end when the provided element does not exist' do + filters = described_class.new(%w(a b c)) + + filters.insert_after('d', '1') + + expect(filters).to eq %w(a b c 1) + end + end + + describe '#insert_before' do + it 'inserts an element before a provided element' do + filters = described_class.new(%w(a b c)) + + filters.insert_before('b', '1') + + expect(filters).to eq %w(a 1 b c) + end + + it 'inserts an element at the beginning when the provided element does not exist' do + filters = described_class.new(%w(a b c)) + + filters.insert_before('d', '1') + + expect(filters).to eq %w(1 a b c) + end + end +end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 51ae2c04ed0..1c7d66398cb 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -457,11 +457,38 @@ describe Repository, models: true do end end - describe '#revert_merge' do - it 'should revert the changes' do - repository.revert(user, merge_commit, 'master') + describe '#revert' do + let(:new_image_commit) { repository.commit('33f3729a45c02fc67d00adb1b8bca394b0e761d9') } + let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } - expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present + context 'when there is a conflict' do + it 'should abort the operation' do + expect(repository.revert(user, new_image_commit, 'master')).to eq(false) + end + end + + context 'when commit was already reverted' do + it 'should abort the operation' do + repository.revert(user, update_image_commit, 'master') + + expect(repository.revert(user, update_image_commit, 'master')).to eq(false) + end + end + + context 'when commit can be reverted' do + it 'should revert the changes' do + expect(repository.revert(user, update_image_commit, 'master')).to be_truthy + end + end + + context 'reverting a merge commit' do + it 'should revert the changes' do + merge_commit + expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).to be_present + + repository.revert(user, merge_commit, 'master') + expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 88821dd0dad..412101ac9f9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -176,7 +176,7 @@ describe User, models: true do end describe 'avatar' do - it 'only validates when avatar is present' do + it 'only validates when avatar is present and changed' do user = build(:user, :with_avatar) user.avatar_crop_x = nil @@ -184,6 +184,20 @@ describe User, models: true do user.avatar_crop_size = nil expect(user).not_to be_valid + expect(user.errors.keys). + to match_array %i(avatar_crop_x avatar_crop_y avatar_crop_size) + end + + it 'does not validate when avatar has not changed' do + user = create(:user, :with_avatar) + + expect { user.avatar_crop_x = nil }.not_to change(user, :valid?) + end + + it 'does not validate when avatar is not present' do + user = create(:user) + + expect { user.avatar_crop_y = nil }.not_to change(user, :valid?) end end end @@ -268,6 +282,7 @@ describe User, models: true do expect(user).to be_two_factor_enabled expect(user.encrypted_otp_secret).not_to be_nil expect(user.otp_backup_codes).not_to be_nil + expect(user.otp_grace_period_started_at).not_to be_nil user.disable_two_factor! @@ -276,6 +291,7 @@ describe User, models: true do expect(user.encrypted_otp_secret_iv).to be_nil expect(user.encrypted_otp_secret_salt).to be_nil expect(user.otp_backup_codes).to be_nil + expect(user.otp_grace_period_started_at).to be_nil end end diff --git a/spec/requests/api/commit_status_spec.rb b/spec/requests/api/commit_status_spec.rb index 89b554622ef..8017ed97d88 100644 --- a/spec/requests/api/commit_status_spec.rb +++ b/spec/requests/api/commit_status_spec.rb @@ -2,88 +2,125 @@ require 'spec_helper' describe API::CommitStatus, api: true do include ApiHelpers + let!(:project) { create(:project) } let(:commit) { project.repository.commit } - let!(:ci_commit) { project.ensure_ci_commit(commit.id) } let(:commit_status) { create(:commit_status, commit: ci_commit) } let(:guest) { create_user(ProjectMember::GUEST) } let(:reporter) { create_user(ProjectMember::REPORTER) } let(:developer) { create_user(ProjectMember::DEVELOPER) } + let(:sha) { commit.id } + describe "GET /projects/:id/repository/commits/:sha/statuses" do - it_behaves_like 'a paginated resources' do - let(:request) { get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", reporter) } - end + let(:get_url) { "/projects/#{project.id}/repository/commits/#{sha}/statuses" } - context "reporter user" do - let(:statuses_id) { json_response.map { |status| status['id'] } } + context 'ci commit exists' do + let!(:ci_commit) { project.ensure_ci_commit(commit.id) } - before do - @status1 = create(:commit_status, commit: ci_commit, status: 'running') - @status2 = create(:commit_status, commit: ci_commit, name: 'coverage', status: 'pending') - @status3 = create(:commit_status, commit: ci_commit, name: 'coverage', ref: 'develop', status: 'running', allow_failure: true) - @status4 = create(:commit_status, commit: ci_commit, name: 'coverage', status: 'success') - @status5 = create(:commit_status, commit: ci_commit, ref: 'develop', status: 'success') - @status6 = create(:commit_status, commit: ci_commit, status: 'success') + it_behaves_like 'a paginated resources' do + let(:request) { get api(get_url, reporter) } end - it "should return latest commit statuses" do - get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", reporter) - expect(response.status).to eq(200) + context "reporter user" do + let(:statuses_id) { json_response.map { |status| status['id'] } } - expect(json_response).to be_an Array - expect(statuses_id).to contain_exactly(@status3.id, @status4.id, @status5.id, @status6.id) - json_response.sort_by!{ |status| status['id'] } - expect(json_response.map{ |status| status['allow_failure'] }).to eq([true, false, false, false]) - end + def create_status(opts = {}) + create(:commit_status, { commit: ci_commit }.merge(opts)) + end - it "should return all commit statuses" do - get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses?all=1", reporter) - expect(response.status).to eq(200) + let!(:status1) { create_status(status: 'running') } + let!(:status2) { create_status(name: 'coverage', status: 'pending') } + let!(:status3) { create_status(ref: 'develop', status: 'running', allow_failure: true) } + let!(:status4) { create_status(name: 'coverage', status: 'success') } + let!(:status5) { create_status(name: 'coverage', ref: 'develop', status: 'success') } + let!(:status6) { create_status(status: 'success') } - expect(json_response).to be_an Array - expect(statuses_id).to contain_exactly(@status1.id, @status2.id, @status3.id, @status4.id, @status5.id, @status6.id) - end + context 'latest commit statuses' do + before { get api(get_url, reporter) } - it "should return latest commit statuses for specific ref" do - get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses?ref=develop", reporter) - expect(response.status).to eq(200) + it 'returns latest commit statuses' do + expect(response.status).to eq(200) - expect(json_response).to be_an Array - expect(statuses_id).to contain_exactly(@status3.id, @status5.id) + expect(json_response).to be_an Array + expect(statuses_id).to contain_exactly(status3.id, status4.id, status5.id, status6.id) + json_response.sort_by!{ |status| status['id'] } + expect(json_response.map{ |status| status['allow_failure'] }).to eq([true, false, false, false]) + end + end + + context 'all commit statuses' do + before { get api(get_url, reporter), all: 1 } + + it 'returns all commit statuses' do + expect(response.status).to eq(200) + + expect(json_response).to be_an Array + expect(statuses_id).to contain_exactly(status1.id, status2.id, + status3.id, status4.id, + status5.id, status6.id) + end + end + + context 'latest commit statuses for specific ref' do + before { get api(get_url, reporter), ref: 'develop' } + + it 'returns latest commit statuses for specific ref' do + expect(response.status).to eq(200) + + expect(json_response).to be_an Array + expect(statuses_id).to contain_exactly(status3.id, status5.id) + end + end + + context 'latest commit statues for specific name' do + before { get api(get_url, reporter), name: 'coverage' } + + it 'return latest commit statuses for specific name' do + expect(response.status).to eq(200) + + expect(json_response).to be_an Array + expect(statuses_id).to contain_exactly(status4.id, status5.id) + end + end end + end - it "should return latest commit statuses for specific name" do - get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses?name=coverage", reporter) - expect(response.status).to eq(200) + context 'ci commit does not exist' do + before { get api(get_url, reporter) } + it 'returns empty array' do + expect(response.status).to eq 200 expect(json_response).to be_an Array - expect(statuses_id).to contain_exactly(@status3.id, @status4.id) + expect(json_response).to be_empty end end context "guest user" do + before { get api(get_url, guest) } + it "should not return project commits" do - get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", guest) expect(response.status).to eq(403) end end context "unauthorized user" do + before { get api(get_url) } + it "should not return project commits" do - get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses") expect(response.status).to eq(401) end end end describe 'POST /projects/:id/statuses/:sha' do - let(:post_url) { "/projects/#{project.id}/statuses/#{commit.id}" } + let(:post_url) { "/projects/#{project.id}/statuses/#{sha}" } context 'developer user' do - context 'should create commit status' do - it 'with only required parameters' do - post api(post_url, developer), state: 'success' + context 'only required parameters' do + before { post api(post_url, developer), state: 'success' } + + it 'creates commit status' do expect(response.status).to eq(201) expect(json_response['sha']).to eq(commit.id) expect(json_response['status']).to eq('success') @@ -92,9 +129,17 @@ describe API::CommitStatus, api: true do expect(json_response['target_url']).to be_nil expect(json_response['description']).to be_nil end + end - it 'with all optional parameters' do - post api(post_url, developer), state: 'success', context: 'coverage', ref: 'develop', target_url: 'url', description: 'test' + context 'with all optional parameters' do + before do + optional_params = { state: 'success', context: 'coverage', + ref: 'develop', target_url: 'url', description: 'test' } + + post api(post_url, developer), optional_params + end + + it 'creates commit status' do expect(response.status).to eq(201) expect(json_response['sha']).to eq(commit.id) expect(json_response['status']).to eq('success') @@ -105,41 +150,52 @@ describe API::CommitStatus, api: true do end end - context 'should not create commit status' do - it 'with invalid state' do - post api(post_url, developer), state: 'invalid' + context 'invalid status' do + before { post api(post_url, developer), state: 'invalid' } + + it 'does not create commit status' do expect(response.status).to eq(400) end + end - it 'without state' do - post api(post_url, developer) + context 'request without state' do + before { post api(post_url, developer) } + + it 'does not create commit status' do expect(response.status).to eq(400) end + end - it 'invalid commit' do - post api("/projects/#{project.id}/statuses/invalid_sha", developer), state: 'running' + context 'invalid commit' do + let(:sha) { 'invalid_sha' } + before { post api(post_url, developer), state: 'running' } + + it 'returns not found error' do expect(response.status).to eq(404) end end end context 'reporter user' do + before { post api(post_url, reporter) } + it 'should not create commit status' do - post api(post_url, reporter) expect(response.status).to eq(403) end end context 'guest user' do + before { post api(post_url, guest) } + it 'should not create commit status' do - post api(post_url, guest) expect(response.status).to eq(403) end end context 'unauthorized user' do + before { post api(post_url) } + it 'should not create commit status' do - post api(post_url) expect(response.status).to eq(401) end end |