Disallow guests from modifying project (#243)

* don't show projects to guest users

* disallow guests from modifying project

* add items to changelog

* Fix tests + rubocop issues

* Guest user cannot see the add story button

* Disable story attributes for guest user

CHANGELOG.md

@@ -4,6 +4,10 @@
 ### Fixed
 - Allowed params to update project via API
 
+### Added
+- Don't show projects to guests that he's not member of
+- Disallow guests to make changes on projects
+
 ## [1.9.0] 2017-09-11
 ### Changed
 - Changing `webpack-rails` gem to new `webpacker` gem

app/assets/javascripts/models/story.js

@@ -40,9 +40,17 @@ var Story = module.exports = Backbone.Model.extend({
     this.setReadonly();
   },
 
-  setReadonly: function() {
-    if(this.get('state') === 'accepted' && this.get('accepted_at') !== undefined)
-      this.isReadonly = true;
+  setReadonly: function () {
+    var accepted = this.get('state') === 'accepted' && this.get('accepted_at') !== undefined;
+    var isGuest = (
+      this.collection !== undefined &&
+      this.collection.project.current_user !== undefined &&
+      this.collection.project.current_user.get('guest?')
+    );
+
+    if (isGuest || accepted) {
+      this.isReadonly = true
+    }
   },
 
   changeState: function(model, newValue) {

app/assets/javascripts/views/story_view.js

@@ -343,6 +343,12 @@ module.exports = FormView.extend({
       ReactDOM.unmountComponentAtNode(storyControlsContainer);
     }
 
+    var isGuest = (
+      this.model.collection !== undefined &&
+      this.model.collection.project.current_user !== undefined &&
+      this.model.collection.project.current_user.get('guest?')
+    );
+
     if(this.canEdit()) {
 
       this.$el.empty();
@@ -424,9 +430,12 @@ module.exports = FormView.extend({
       }
       this.renderReactComponents();
 
+      if (isGuest) { this.toggleControlButtons(true, false) }
+
     } else {
       this.$el.removeClass('editing');
       this.$el.html(this.template({story: this.model, view: this}));
+      if (isGuest) { this.$el.find('.state-actions').find('.transition').prop('disabled', true) }
     }
     this.hoverBox();
     return this;
@@ -923,9 +932,12 @@ module.exports = FormView.extend({
     }));
   },
 
-  toggleControlButtons: function(isDisabled) {
+  toggleControlButtons: function(isDisabled, changeCancel) {
     var $storyControls = this.$el.find('.story-controls');
-    $storyControls.find('.submit, .destroy, .cancel').prop('disabled', isDisabled);
+    $storyControls.find('.submit, .destroy').prop('disabled', isDisabled);
+
+    if (changeCancel === undefined) { changeCancel = true }
+    if (changeCancel) { $storyControls.find('.cancel').prop('disabled', isDisabled); }
   },
 
   getLocation: function() {

app/controllers/projects_controller.rb

@@ -15,10 +15,13 @@ def index
     projects_joined = policy_scope(Project)
 
     @projects = {
-      joined: serialize_from_collection(projects_joined),
-      unjoined: serialize_from_collection(projects_unjoined.order(:updated_at))
+      joined: serialize_from_collection(projects_joined)
     }
 
+    unless current_user.guest?
+      @projects[:unjoined] = serialize_from_collection(projects_unjoined.order(:updated_at))
+    end
+
     @activities_group = Activity.grouped_activities(projects_joined, 1.week.ago)
   end
 

app/models/user.rb

@@ -37,6 +37,10 @@ def password_required?
     end
   end
 
+  def guest?
+    role == 'guest'
+  end
+
   def to_s
     "#{name} (#{initials}) <#{email}>"
   end
@@ -56,7 +60,7 @@ def tour_steps
   end
 
   def as_json(_options = {})
-    super(only: JSON_ATTRIBUTES, methods: :tour_steps)
+    super(only: JSON_ATTRIBUTES, methods: [:tour_steps, :guest?])
   end
 
   def self.find_first_by_auth_conditions(warden_conditions)

app/policies/application_policy.rb

@@ -11,6 +11,8 @@ def self.included(base)
 
     protected
 
+    delegate :guest?, to: :current_user
+
     def root?
       # this user can do anothing, it goes in AdminUser instead of User and bypasses everything
       context.active_admin

app/policies/project_policy.rb

@@ -12,7 +12,8 @@ def import?
   end
 
   def join?
-    !project_member?
+    return false if project_member? || guest?
+    true
   end
 
   alias archived? update?

app/policies/story_policy.rb

@@ -1,17 +1,21 @@
 class StoryPolicy < ApplicationPolicy
   def index?
+    return false if guest?
     admin? || project_member?
   end
 
   def show?
+    return false if guest?
     admin? || project_member? && current_project.stories.find_by(id: record.id)
   end
 
   def create?
+    return false if guest?
     admin? || project_member?
   end
 
   def update?
+    return false if guest?
     admin? || project_member?
   end
 

app/views/projects/show.html.erb

@@ -14,10 +14,12 @@
 
   <p class="navbar-text velocity" id="velocity" data-step="velocity"></p>
 
-  <button type="button" class="btn btn-primary btn-sm navbar-btn" id="add_story" data-step="add-story">
-    <i class="mi md-light md-18 md-add">add</i>
-    <%= t('add story') %>
-  </button>
+  <% unless current_user.guest? %>
+    <button type="button" class="btn btn-primary btn-sm navbar-btn" id="add_story" data-step="add-story">
+      <i class="mi md-light md-18 md-add">add</i>
+      <%= t('add story') %>
+    </button>
+  <% end %>
 <% end %>
 
 <% content_for :right_menu do %>

spec/features/stories_spec.rb

@@ -11,8 +11,32 @@
   end
 
   describe 'full story life cycle' do
-    before do
-      project
+    context 'when the user is guest' do
+      let!(:story) do
+        create(:story, title: 'Test Story', project: project, requested_by: user, estimate: 2)
+      end
+
+      before do
+        user.update_column(:role, 'guest')
+        visit project_path(project)
+        wait_spinner
+        wait_page_load
+      end
+
+      it 'cannot see the add story button', js: true do
+        expect(page).to_not have_button 'Add story'
+      end
+
+      it 'attributes are disabled', js: true do
+        expect(page).to have_button('start', disabled: true)
+
+        find('.story-title').click
+        expect(page).to have_field('title', with: story.title, disabled: true)
+        expect(page).to have_select('story_type', selected: 'feature', disabled: true)
+        expect(page).to have_button('Save', disabled: true)
+        expect(page).to have_button('Delete', disabled: true)
+        expect(page).to have_button('Cancel', disabled: false)
+      end
     end
 
     it 'steps through the full story life cycle', js: true do

spec/javascripts/models/story_spec.js

@@ -1,4 +1,5 @@
 var Story = require('models/story');
+var User = require('models/user');
 
 describe('Story', function() {
 
@@ -7,7 +8,7 @@ describe('Story', function() {
       name: 'project',
       defaults: {point_values: [0, 1, 2, 3]},
       users: { get: function() {} },
-      current_user: { id: 999 },
+      current_user: new User({ id: 999, 'guest?': false }),
       currentIterationNumber: function() { return 1; },
       getIterationNumberForDate: function() { return 999; }
     });

spec/models/user_spec.rb

@@ -17,7 +17,7 @@
 
     specify do
       expect(subject.as_json['user'].keys.sort).to eq(
-        %w(email finished_tour id initials name tour_steps username)
+        %w(email finished_tour guest? id initials name tour_steps username)
       )
     end
   end

spec/policies/project_policy_spec.rb

@@ -161,5 +161,32 @@
         expect(policy_scope).to eq([])
       end
     end
+
+    context 'for a guest' do
+      let(:current_user) { create :user, :with_team, role: 'guest' }
+
+      %i(index create new update edit reports).each do |action|
+        it { should_not permit(action) }
+      end
+
+      %i(
+        import
+        import_upload
+        archive
+        unarchive
+        destroy
+        share
+        unshare
+        transfer
+        ownership
+        join
+      ).each do |action|
+        it { should_not permit(action) }
+      end
+
+      it 'hides project' do
+        expect(policy_scope).to eq([])
+      end
+    end
   end
 end

spec/policies/story_policy_spec.rb

@@ -35,8 +35,6 @@
     context 'for a user' do
       let(:current_user) { create :user, :with_team }
 
-      it { should permit(:show) }
-
       %i(index show create new update edit destroy).each do |action|
         it { should permit(action) }
       end
@@ -45,6 +43,18 @@
         expect(policy_scope).to eq([story])
       end
     end
+
+    context 'for a guest' do
+      let(:current_user) { create :user, :with_team, role: 'guest' }
+
+      %i(index show create new update edit destroy).each do |action|
+        it { should_not permit(action) }
+      end
+
+      it 'lists all stories of the project' do
+        expect(policy_scope).to eq([story])
+      end
+    end
   end
 
   context 'user not a member of project' do
@@ -71,5 +81,17 @@
         expect(policy_scope).to eq([])
       end
     end
+
+    context 'for a guest' do
+      let(:current_user) { create :user, :with_team, role: 'guest' }
+
+      %i(index create new update edit destroy).each do |action|
+        it { should_not permit(action) }
+      end
+
+      it 'hides stories of project that is not member' do
+        expect(policy_scope).to eq([])
+      end
+    end
   end
 end