Merge branch 'fix-pages-domain' into 'master'

Fixes pages domains abilities and group page address

The `master` can now access the pages configuration, modify domains and add certificates, but can't remove the pages.

The second bug that is fixed is the group page address. The group page project name needs to be in lowercase.

/cc @DouweM @rymai 

See merge request !195

app/controllers/projects/pages_controller.rb

 class Projects::PagesController < Projects::ApplicationController
   layout 'project_settings'
 
-  before_action :authorize_update_pages!
+  before_action :authorize_read_pages!, only: [:show]
+  before_action :authorize_update_pages!, except: [:show]
 
   def show
     @domains = @project.pages_domains.order(:domain)

app/models/ability.rb

@@ -224,9 +224,11 @@ class Ability
 
     def project_master_rules
       @project_master_rules ||= project_dev_rules + [
+        :read_pages,
         :push_code_to_protected_branches,
         :update_project_snippet,
         :update_merge_request,
+        :update_pages,
         :admin_milestone,
         :admin_project_snippet,
         :admin_project_member,
@@ -235,7 +237,8 @@ class Ability
         :admin_wiki,
         :admin_project,
         :admin_commit_status,
-        :admin_build
+        :admin_build,
+        :admin_pages,
       ]
     end
 
@@ -245,7 +248,6 @@ class Ability
         :change_visibility_level,
         :rename_project,
         :remove_project,
-        :update_pages,
         :remove_pages,
         :archive_project,
         :remove_fork_project

app/models/project.rb

@@ -1059,15 +1059,21 @@ class Project < ActiveRecord::Base
     ensure_runners_token!
   end
 
+  def pages_deployed?
+    Dir.exist?(public_pages_path)
+  end
+
   def pages_url
-    return unless Dir.exist?(public_pages_path)
+    # The hostname always needs to be in downcased
+    # All web servers convert hostname to lowercase
+    host = "#{namespace.path}.#{Settings.pages.host}".downcase
 
-    host = "#{namespace.path}.#{Settings.pages.host}"
+    # The host in URL always needs to be downcased
     url = Gitlab.config.pages.url.sub(/^https?:\/\//) do |prefix|
       "#{prefix}#{namespace.path}."
-    end
+    end.downcase
 
-    # If the project path is the same as host, leave the short version
+    # If the project path is the same as host, we serve it as group page
     return url if host == path
 
     "#{url}/#{path}"

app/views/projects/pages/_access.html.haml

-- if @project.pages_url
+- if @project.pages_deployed?
   .panel.panel-default
     .panel-heading
       Access pages

app/views/projects/pages/_destroy.haml

-- if can?(current_user, :remove_pages, @project) && @project.pages_url
+- if can?(current_user, :remove_pages, @project) && @project.pages_deployed?
   .panel.panel-default.panel.panel-danger
     .panel-heading Remove pages
     .errors-holder
@@ -7,3 +7,5 @@
         Removing the pages will prevent from exposing them to outside world.
       .form-actions
         = link_to 'Remove pages', namespace_project_pages_path(@project.namespace, @project), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove"
+- else
+  .nothing-here-block Only the project owner can remove pages

app/views/projects/pages/_list.html.haml

-- if @domains.any?
+- if can?(current_user, :update_pages, @project) && @domains.any?
   .panel.panel-default
     .panel-heading
       Domains (#{@domains.count})

app/views/projects/pages/_no_domains.html.haml

-.panel.panel-default
+- if can?(current_user, :update_pages, @project)
+  .panel.panel-default
     .panel-heading
       Domains
     .nothing-here-block

app/views/projects/pages/_use.html.haml

-- unless @project.pages_url
+- unless @project.pages_deployed?
   .panel.panel-info
     .panel-heading
       Configure pages

app/views/projects/pages/show.html.haml

@@ -2,7 +2,7 @@
 %h3.page_title
   Pages
 
-  - if Gitlab.config.pages.external_http || Gitlab.config.pages.external_https
+  - if can?(current_user, :update_pages, @project) && (Gitlab.config.pages.external_http || Gitlab.config.pages.external_https)
     = link_to new_namespace_project_pages_domain_path(@project.namespace, @project), class: 'btn btn-new pull-right', title: 'New Domain' do
       %i.fa.fa-plus
       New Domain

doc/pages/README.md

@@ -45,6 +45,13 @@ URL it will be accessible.
 | Specific project under a user's page  | `walter/area51`     | `https://walter.example.com/area51`     |
 | Specific project under a group's page | `therug/welovecats` | `https://therug.example.com/welovecats` |
 
+## Group pages
+
+To create a page for a group, add a new project to it. The project name must be lowercased.
+
+For example, if you have a group called `TheRug` and pages are hosted under `Example.com`,
+create a project named `therug.example.com`.
+
 ## Enable the pages feature in your project
 
 The GitLab Pages feature needs to be explicitly enabled for each project

doc/permissions/permissions.md

@@ -47,9 +47,12 @@ documentation](../workflow/add-user/add-user.md).
 | Manage runners                        |         |            |             | ✓        | ✓      |
 | Manage build triggers                 |         |            |             | ✓        | ✓      |
 | Manage variables                      |         |            |             | ✓        | ✓      |
+| Manage pages                          |         |            |             | ✓        | ✓      |
+| Manage pages domains and certificates |         |            |             | ✓        | ✓      |
 | Switch visibility level               |         |            |             |          | ✓      |
 | Transfer project to another namespace |         |            |             |          | ✓      |
 | Remove project                        |         |            |             |          | ✓      |
+| Remove pages                          |         |            |             |          | ✓      |
 | Force push to protected branches      |         |            |             |          |        |
 | Remove protected branches             |         |            |             |          |        |
 

features/steps/project/pages.rb

@@ -134,6 +134,6 @@ nNp/xedE1YxutQ==
   end
 
   step 'The Pages should get removed' do
-    expect(@project.pages_url).to be_nil
+    expect(@project.pages_deployed?).to be_falsey
   end
 end

spec/models/project_spec.rb

@@ -620,6 +620,48 @@ describe Project, models: true do
       it { expect(forked_project.visibility_level_allowed?(Gitlab::VisibilityLevel::INTERNAL)).to be_truthy }
       it { expect(forked_project.visibility_level_allowed?(Gitlab::VisibilityLevel::PUBLIC)).to be_falsey }
     end
+  end
+
+  describe '#pages_deployed?' do
+    let(:project) { create :empty_project }
+
+    subject { project.pages_deployed? }
+
+    context 'if public folder does exist' do
+      before { allow(Dir).to receive(:exist?).with(project.public_pages_path).and_return(true) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context "if public folder doesn't exist" do
+      it { is_expected.to be_falsey }
+    end
+  end
 
+  describe '#pages_url' do
+    let(:group) { create :group, name: group_name }
+    let(:project) { create :empty_project, namespace: group, name: project_name }
+    let(:domain) { 'Example.com' }
+
+    subject { project.pages_url }
+
+    before do
+      allow(Settings.pages).to receive(:host).and_return(domain)
+      allow(Gitlab.config.pages).to receive(:url).and_return('http://example.com')
+    end
+
+    context 'group page' do
+      let(:group_name) { 'Group' }
+      let(:project_name) { 'group.example.com' }
+
+      it { is_expected.to eq("http://group.example.com") }
+    end
+
+    context 'project page' do
+      let(:group_name) { 'Group' }
+      let(:project_name) { 'Project' }
+
+      it { is_expected.to eq("http://group.example.com/project") }
+    end
   end
 end

spec/services/projects/update_pages_service_spec.rb

@@ -27,9 +27,9 @@ describe Projects::UpdatePagesService do
       end
 
       it 'succeeds' do
-        expect(project.pages_url).to be_nil
+        expect(project.pages_deployed?).to be_falsey
         expect(execute).to eq(:success)
-        expect(project.pages_url).to_not be_nil
+        expect(project.pages_deployed?).to be_truthy
       end
 
       it 'limits pages size' do
@@ -39,11 +39,11 @@ describe Projects::UpdatePagesService do
 
       it 'removes pages after destroy' do
         expect(PagesWorker).to receive(:perform_in)
-        expect(project.pages_url).to be_nil
+        expect(project.pages_deployed?).to be_falsey
         expect(execute).to eq(:success)
-        expect(project.pages_url).to_not be_nil
+        expect(project.pages_deployed?).to be_truthy
         project.destroy
-        expect(Dir.exist?(project.public_pages_path)).to be_falsey
+        expect(project.pages_deployed?).to be_falsey
       end
 
       it 'fails if sha on branch is not latest' do
@@ -61,7 +61,7 @@ describe Projects::UpdatePagesService do
 
   it 'fails to remove project pages when no pages is deployed' do
     expect(PagesWorker).to_not receive(:perform_in)
-    expect(project.pages_url).to be_nil
+    expect(project.pages_deployed?).to be_falsey
     project.destroy
   end