From 3f161a3a3d1c2411cd80260a6eaf13eb608e0ce3 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Tue, 19 Aug 2014 01:37:33 +0200 Subject: [PATCH 1/5] Update omniauth-google-oauth2 gem Udate doc --- Gemfile.lock | 14 +++++++------- doc/integration/google.md | 14 +++++++++----- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index f102150bc1..205599e898 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -129,7 +129,7 @@ GEM factory_girl_rails (4.3.0) factory_girl (~> 4.3.0) railties (>= 3.0.0) - faraday (0.8.8) + faraday (0.8.9) multipart-post (~> 1.2.0) faraday_middleware (0.9.0) faraday (>= 0.7.4, < 0.9) @@ -234,7 +234,7 @@ GEM activesupport (>= 4.0.1) haml (>= 3.1, < 5.0) railties (>= 4.0.1) - hashie (2.0.5) + hashie (2.1.2) hike (1.2.3) hipchat (0.14.0) httparty @@ -243,7 +243,7 @@ GEM httparty (0.13.0) json (~> 1.8) multi_xml (>= 0.5.2) - httpauth (0.2.0) + httpauth (0.2.1) i18n (0.6.11) ice_nine (0.10.0) jasmine (2.0.2) @@ -264,7 +264,7 @@ GEM jquery-ui-rails (4.2.1) railties (>= 3.2.16) json (1.8.1) - jwt (0.1.8) + jwt (0.1.13) multi_json (>= 1.5) kaminari (0.15.1) actionpack (>= 3.0.0) @@ -311,9 +311,9 @@ GEM omniauth-github (1.1.1) omniauth (~> 1.0) omniauth-oauth2 (~> 1.1) - omniauth-google-oauth2 (0.2.1) - omniauth (~> 1.0) - omniauth-oauth2 + omniauth-google-oauth2 (0.2.5) + omniauth (> 1.0) + omniauth-oauth2 (~> 1.1) omniauth-oauth (1.0.1) oauth omniauth (~> 1.0) diff --git a/doc/integration/google.md b/doc/integration/google.md index 3cf546bd77..7a78aff8ea 100644 --- a/doc/integration/google.md +++ b/doc/integration/google.md @@ -2,17 +2,21 @@ To enable the Google OAuth2 OmniAuth provider you must register your application with Google. Google will generate a client ID and secret key for you to use. -1. Sign in to the [Google Developers Console](https://console.developers.google.com/) with the Google account you want to use to register GitLab. +1. Sign in to the [Google Developers Console](https://console.developers.google.com/) with the Google account you want to use to register GitLab. 1. Select "Create Project". 1. Provide the project information - - Project name: 'GitLab' works just fine here. + - Project name: 'GitLab' works just fine here. - Project ID: Must be unique to all Google Developer registered applications. Google provides a randomly generated Project ID by default. You can use the randomly generated ID or choose a new one. 1. Refresh the page. You should now see your new project in the list. Click on the project. 1. Select "APIs & auth" in the left menu. +1. Select "APIs" in the submenu. + - Enable `Contacts API` + - Enable `Google+ API` + 1. Select "Credentials" in the submenu. 1. Select "Create New Client ID". @@ -41,7 +45,7 @@ To enable the Google OAuth2 OmniAuth provider you must register your application args: { access_type: 'offline', approval_prompt: '' } } ``` -1. Change 'YOUR APP ID' to the client ID from the GitHub application page from step 7. +1. Change 'YOUR APP ID' to the client ID from the GitHub application page from step 7. 1. Change 'YOUR APP SECRET' to the client secret from the GitHub application page from step 7. @@ -51,12 +55,12 @@ To enable the Google OAuth2 OmniAuth provider you must register your application On the sign in page there should now be a Google icon below the regular sign in form. Click the icon to begin the authentication process. Google will ask the user to sign in and authorize the GitLab application. If everything goes well the user will be returned to GitLab and will be signed in. -## Further Configuration +## Further Configuration This further configuration is not required for Google authentication to function but it is strongly recommended. Taking these steps will increase usability for users by providing a little more recognition and branding. At this point, when users first try to authenticate to your GitLab installation with Google they will see a generic application name on the prompt screen. The prompt informs the user that "Project Default Service Account" would like to access their account. "Project Default Service Account" isn't very recognizable and may confuse or cause users to be concerned. This is easily changeable. 1. Select 'Consent screen' in the left menu. (See steps 1, 4 and 5 above for instructions on how to get here if you closed your window). -1. Scroll down until you find "Product Name". Change the product name to something more descriptive. +1. Scroll down until you find "Product Name". Change the product name to something more descriptive. 1. Add any additional information as you wish - homepage, logo, privacy policy, etc. None of this is required, but it may help your users. From 794a8d8187a9fefc7908307ac9d995f4262aae8a Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Tue, 19 Aug 2014 14:53:37 +0200 Subject: [PATCH 2/5] Mention Google OAuth API change in update guides Add oauth doc to 6.0 to 7.2 upgrader --- doc/update/6.0-to-7.2.md | 10 ++++++++-- doc/update/7.1-to-7.2.md | 6 ++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/doc/update/6.0-to-7.2.md b/doc/update/6.0-to-7.2.md index 41ff628360..913768aee0 100644 --- a/doc/update/6.0-to-7.2.md +++ b/doc/update/6.0-to-7.2.md @@ -84,7 +84,7 @@ sudo chmod u+rwx,g+rx,o-rwx /home/git/gitlab-satellites ## 6. Update config files -TIP: to see what changed in gitlab.yml.example in this release use next command: +TIP: to see what changed in gitlab.yml.example in this release use next command: ``` git diff 6-0-stable:config/gitlab.yml.example 7-2-stable:config/gitlab.yml.example @@ -129,6 +129,12 @@ To make sure you didn't miss anything run a more thorough check with: If all items are green, then congratulations upgrade complete! +## 10. Update OmniAuth configuration + +When using Google omniauth login, changes of the Google account required. +Ensure that `Contacts API` and the `Google+ API` are enabled in the [Google Developers Console](https://console.developers.google.com/). +More details can be found at the [integration documentation](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/integration/google.md). + ## Things went south? Revert to previous version (6.0) ### 1. Revert the code to the previous version @@ -144,4 +150,4 @@ sudo -u git -H bundle exec rake gitlab:backup:restore RAILS_ENV=production ## Login issues after upgrade? -If running in HTTPS mode, be sure to read [Can't Verify CSRF token authenticity](https://github.com/gitlabhq/gitlab-public-wiki/wiki/Trouble-Shooting-Guide#cant-verify-csrf-token-authenticitycant-get-past-login-pageredirected-to-login-page) +If running in HTTPS mode, be sure to read [Can't Verify CSRF token authenticity](https://github.com/gitlabhq/gitlab-public-wiki/wiki/Trouble-Shooting-Guide#cant-verify-csrf-token-authenticitycant-get-past-login-pageredirected-to-login-page) diff --git a/doc/update/7.1-to-7.2.md b/doc/update/7.1-to-7.2.md index be28944a8f..40698e8092 100644 --- a/doc/update/7.1-to-7.2.md +++ b/doc/update/7.1-to-7.2.md @@ -91,6 +91,12 @@ To make sure you didn't miss anything run a more thorough check with: If all items are green, then congratulations upgrade is complete! +### 8. Update OmniAuth configuration + +When using Google omniauth login, changes of the Google account required. +Ensure that `Contacts API` and the `Google+ API` are enabled in the [Google Developers Console](https://console.developers.google.com/). +More details can be found at the [integration documentation](../integration/google.md). + ## Things went south? Revert to previous version (7.1) ### 1. Revert the code to the previous version From 7ad93ab250019d7737186a0bf8884faf2db2b625 Mon Sep 17 00:00:00 2001 From: jubianchi Date: Sun, 17 Aug 2014 22:22:01 +0200 Subject: [PATCH 3/5] Improve labels validation and expose error messages --- app/models/concerns/issuable.rb | 4 +++ app/models/label.rb | 4 +-- lib/api/helpers.rb | 8 +++-- lib/api/issues.rb | 11 ++++--- spec/requests/api/issues_spec.rb | 56 ++++++++++++++++++++++++++++---- spec/requests/api/labels_spec.rb | 16 +++++++++ 6 files changed, 84 insertions(+), 15 deletions(-) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 0a5fe24b5a..5c9b44812b 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -138,6 +138,10 @@ module Issuable labels.order('title ASC').pluck(:title) end + def remove_labels + labels.delete_all + end + def add_labels_by_names(label_names) label_names.each do |label_name| label = project.labels.create_with( diff --git a/app/models/label.rb b/app/models/label.rb index 3ff52416c2..233f477444 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -6,14 +6,14 @@ class Label < ActiveRecord::Base has_many :issues, through: :label_links, source: :target, source_type: 'Issue' validates :color, - format: { with: /\A\#[0-9A-Fa-f]{6}+\Z/ }, + format: { with: /\A#[0-9A-Fa-f]{6}\Z/ }, allow_blank: false validates :project, presence: true # Don't allow '?', '&', and ',' for label titles validates :title, presence: true, - format: { with: /\A[^&\?,&]*\z/ }, + format: { with: /\A[^&\?,&]+\z/ }, uniqueness: { scope: :project_id } scope :order_by_name, -> { reorder("labels.title ASC") } diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index d36b29a00b..6af0f6d1b2 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -114,17 +114,21 @@ module API # Helper method for validating all labels against its names def validate_label_params(params) + errors = {} + if params[:labels].present? params[:labels].split(',').each do |label_name| label = user_project.labels.create_with( color: Label::DEFAULT_COLOR).find_or_initialize_by( title: label_name.strip) + if label.invalid? - return true + errors[label.title] = label.errors end end end - false + + errors end # error helpers diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 055529ccbd..eb6a74cd2b 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -52,8 +52,8 @@ module API attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id] # Validate label names in advance - if validate_label_params(params) - return render_api_error!('Label names invalid', 405) + if (errors = validate_label_params(params)).any? + render_api_error!({ labels: errors }, 400) end issue = ::Issues::CreateService.new(user_project, current_user, attrs).execute @@ -90,8 +90,8 @@ module API attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id, :state_event] # Validate label names in advance - if validate_label_params(params) - return render_api_error!('Label names invalid', 405) + if (errors = validate_label_params(params)).any? + render_api_error!({ labels: errors }, 400) end issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue) @@ -99,7 +99,8 @@ module API if issue.valid? # Find or create labels and attach to issue. Labels are valid because # we already checked its name, so there can't be an error here - if params[:labels].present? + unless params[:labels].nil? + issue.remove_labels # Create and add labels to the new created issue issue.add_labels_by_names(params[:labels].split(',')) end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index d8e8e4f503..08dc94ebdf 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -73,12 +73,12 @@ describe API::API, api: true do response.status.should == 400 end - it 'should return 405 on invalid label names' do + it 'should return 400 on invalid label names' do post api("/projects/#{project.id}/issues", user), title: 'new issue', labels: 'label, ?' - response.status.should == 405 - json_response['message'].should == 'Label names invalid' + response.status.should == 400 + json_response['message']['labels']['?']['title'].should == ['is invalid'] end end @@ -97,12 +97,56 @@ describe API::API, api: true do response.status.should == 404 end - it 'should return 405 on invalid label names' do + it 'should return 400 on invalid label names' do put api("/projects/#{project.id}/issues/#{issue.id}", user), title: 'updated title', labels: 'label, ?' - response.status.should == 405 - json_response['message'].should == 'Label names invalid' + response.status.should == 400 + json_response['message']['labels']['?']['title'].should == ['is invalid'] + end + end + + describe 'PUT /projects/:id/issues/:issue_id to update labels' do + let!(:label) { create(:label, title: 'dummy', project: project) } + let!(:label_link) { create(:label_link, label: label, target: issue) } + + it 'should not update labels if not present' do + put api("/projects/#{project.id}/issues/#{issue.id}", user), + title: 'updated title' + response.status.should == 200 + json_response['labels'].should == [label.title] + end + + it 'should remove all labels' do + put api("/projects/#{project.id}/issues/#{issue.id}", user), + labels: '' + response.status.should == 200 + json_response['labels'].should == [] + end + + it 'should update labels' do + put api("/projects/#{project.id}/issues/#{issue.id}", user), + labels: 'foo,bar' + response.status.should == 200 + json_response['labels'].should include 'foo' + json_response['labels'].should include 'bar' + end + + it 'should return 400 on invalid label names' do + put api("/projects/#{project.id}/issues/#{issue.id}", user), + labels: 'label, ?' + response.status.should == 400 + json_response['message']['labels']['?']['title'].should == ['is invalid'] + end + + it 'should allow special label names' do + put api("/projects/#{project.id}/issues/#{issue.id}", user), + labels: 'label:foo, label-bar,label_bar,label/bar' + response.status.should == 200 + json_response['labels'].should include 'label:foo' + json_response['labels'].should include 'label-bar' + json_response['labels'].should include 'label_bar' + json_response['labels'].should include 'label/bar' end end diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index cd1b84c53c..ee9088933a 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -50,6 +50,14 @@ describe API::API, api: true do json_response['message'].should == 'Color is invalid' end + it 'should return 400 for too long color code' do + post api("/projects/#{project.id}/labels", user), + name: 'Foo', + color: '#FFAAFFFF' + response.status.should == 400 + json_response['message'].should == 'Color is invalid' + end + it 'should return 400 for invalid name' do post api("/projects/#{project.id}/labels", user), name: '?', @@ -147,5 +155,13 @@ describe API::API, api: true do response.status.should == 400 json_response['message'].should == 'Color is invalid' end + + it 'should return 400 for too long color code' do + post api("/projects/#{project.id}/labels", user), + name: 'Foo', + color: '#FFAAFFFF' + response.status.should == 400 + json_response['message'].should == 'Color is invalid' + end end end From 9d5c69019371379d055be8ba963fde78d3e0c7f6 Mon Sep 17 00:00:00 2001 From: jubianchi Date: Wed, 20 Aug 2014 12:31:16 +0200 Subject: [PATCH 4/5] Add spec on labels validation --- spec/models/label_spec.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index 5098af84cc..1d273e59bd 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -5,4 +5,27 @@ describe Label do it { label.should be_valid } it { should belong_to(:project) } + + describe 'Validation' do + it 'should validate color code' do + build(:label, color: 'G-ITLAB').should_not be_valid + build(:label, color: 'AABBCC').should_not be_valid + build(:label, color: '#AABBCCEE').should_not be_valid + build(:label, color: '#GGHHII').should_not be_valid + build(:label, color: '#').should_not be_valid + build(:label, color: '').should_not be_valid + + build(:label, color: '#AABBCC').should be_valid + end + + it 'should validate title' do + build(:label, title: 'G,ITLAB').should_not be_valid + build(:label, title: 'G?ITLAB').should_not be_valid + build(:label, title: 'G&ITLAB').should_not be_valid + build(:label, title: '').should_not be_valid + + build(:label, title: 'GITLAB').should be_valid + build(:label, title: 'gitlab').should be_valid + end + end end From 87efd92d8cc378697359012226d2679cab7e1e9e Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Wed, 20 Aug 2014 17:13:03 +0200 Subject: [PATCH 5/5] Bump version to 7.2.0.rc5 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 35eaced7f8..8fb668f3e9 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.2.0.rc4 +7.2.0.rc5