Skip to content

Commit

Permalink
Merge pull request #303 from hennevogel/bugfix/csrf_protection
Browse files Browse the repository at this point in the history
Fixes two sources of InvalidAuthenticityToken exceptions
  • Loading branch information
David Kang committed Jul 9, 2018
2 parents 3e43441 + b41763a commit 0680129
Show file tree
Hide file tree
Showing 14 changed files with 98 additions and 69 deletions.
43 changes: 14 additions & 29 deletions app/controllers/projects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ class ProjectsController < ApplicationController
skip_before_filter :authenticate_user!, only: [ :index, :show, :archived, :finished, :newest, :popular, :biggest, :random ]
skip_before_filter :store_location, only: [:join, :leave, :like, :dislike, :add_keyword, :delete_keyword ]
skip_before_action :verify_authenticity_token, only: [:add_keyword, :delete_keyword ]
skip_load_and_authorize_resource only: :old_archived
before_action :load_episode
before_action :username_array, only: [:new, :edit, :show]
autocomplete :project, :title
Expand Down Expand Up @@ -56,17 +55,6 @@ def show
@new_comment = Comment.new
end

# GET /archive/projects/:id
def old_archived
begin
@project = Project.find_by!(title: params[:id])
redirect_to @project
rescue ActiveRecord::RecordNotFound
flash[:notice] = "Can't find project with title #{params[:id]}"
redirect_to action: :archived
end
end

# GET /projects/new
def new
@project = Project.new
Expand Down Expand Up @@ -117,29 +105,26 @@ def recess

# PUT /projects/1/join
def join
# FIXME: This is a validation
if @project.aasm_state == 'invention'
redirect_to project, error: "You can't join this project as it's finished."
end

if @project.join! current_user
message = "Welcome to the project #{current_user.name}!"
else
message = 'You already joined this project'
respond_to do |format|
if @project.join!(current_user)
format.js { flash['success'] = "Welcome to the project #{current_user.name}." }
else
format.js { flash['error'] = "#{@project.errors.full_messages.to_sentence}" }
end
end

redirect_to project_path(@episode, @project), notice: message
flash.discard
end

# PUT /projects/1/leave
def leave
# FIXME: This is a validation
if @project.aasm_state == 'invention'
redirect_to @project, error: "You can't leave this project as it's finished."
respond_to do |format|
if @project.leave!(current_user)
format.js { flash['success'] = "Sorry to see you go #{current_user.name}." }
else
format.js { flash['error'] = "#{@project.errors.full_messages.to_sentence}" }
end
end

@project.leave! current_user
redirect_to project_path(@episode, @project), notice: "Goodbye #{current_user.name}..."
flash.discard
end

# PUT /projects/1/like
Expand Down
4 changes: 2 additions & 2 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ def bootstrap_class_for flash_type
when 'success'
'alert-success'
when 'error'
'alert-error'
'alert-danger'
when 'alert'
'alert-block'
'alert-warning'
when 'notice'
'alert-info'
else
Expand Down
25 changes: 24 additions & 1 deletion app/models/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,22 @@ def active?
self.idea? || self.project?
end

def joined(user)
return false if users.empty?
return false if !users.include? user
return true if users.include? user
end

def join! user
return if self.users.include?(user)
if users.include?(user)
errors.add(:base, "You already joined this project.")
return false
end

if aasm_state == 'invention'
errors.add(:base, "You can't join this project as it's finished.")
return false
end

if self.users.empty?
self.advance!
Expand All @@ -106,6 +120,15 @@ def join! user
end

def leave! user
if !users.include?(user)
errors.add(:base, "You are not member of this project.")
return false
end

if aasm_state == 'invention'
errors.add(:base, "You can't leave this project as it's finished.")
return false
end
self.users.delete(user)

# If the last user has left...
Expand Down
2 changes: 2 additions & 0 deletions app/views/announcements/_announcement_toggle.js.erb
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
$('input[name="authenticity_token"]').val('<%= form_authenticity_token %>');
$('meta[name="csrf-token"]').attr('content','<%= form_authenticity_token %>');
$("#announcement-<%= @announcement.id %>").toggle();
3 changes: 2 additions & 1 deletion app/views/layouts/application.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
%div#loader
#content
- if flash
= render partial: "layouts/alert", flash: flash
#flash
= render partial: "layouts/alert", flash: flash
- if @news
= render partial: "layouts/news"
= yield
Expand Down
3 changes: 3 additions & 0 deletions app/views/projects/_hackers.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- @project.users.each do |user|
= link_to(user_path(user)) do
= image_tag user.gravatar_url(:size => "64"), alt: user.name, title: user.name, class: "img-thumbnail", id: "user#{user.id}-gravatar"
8 changes: 8 additions & 0 deletions app/views/projects/_membership_buttons.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
- joined = @project.joined(current_user) ? 'hidden': ''
= link_to(join_project_path(@episode, @project), :method => :post, :class => "btn btn-success #{joined}", remote: true) do
%i.fa.fa-plus
Join this project
- joined = @project.joined(current_user) ? '': 'hidden'
= link_to(leave_project_path(@episode, @project), :method => :post, :class => "btn btn-warning #{joined}", remote: true) do
%i.fa.fa-minus
Leave this project
5 changes: 5 additions & 0 deletions app/views/projects/join.js.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$('input[name="authenticity_token"]').val('<%= form_authenticity_token %>');
$('meta[name="csrf-token"]').attr('content','<%= form_authenticity_token %>');
$('#flash').html("<%= escape_javascript render partial: "layouts/alert", flash: flash %>");
$('#hackers').html("<%= escape_javascript render partial: 'hackers' %>");
$('#membership_buttons').html("<%= escape_javascript render partial: 'membership_buttons' %>");
5 changes: 5 additions & 0 deletions app/views/projects/leave.js.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$('input[name="authenticity_token"]').val('<%= form_authenticity_token %>');
$('meta[name="csrf-token"]').attr('content','<%= form_authenticity_token %>');
$('#flash').html("<%= escape_javascript render partial: "layouts/alert", flash: flash %>");
$('#hackers').html("<%= escape_javascript render partial: 'hackers' %>");
$('#membership_buttons').html("<%= escape_javascript render partial: 'membership_buttons' %>");
15 changes: 3 additions & 12 deletions app/views/projects/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,10 @@
No
Hackers:
.well.well-sm#hackers
- @project.users.each do |user|
= link_to(user_path(user)) do
= image_tag user.gravatar_url(:size => "64"), alt: user.name, title: user.name, class: "img-thumbnail", id: "user#{user.id}-gravatar"
= render partial: 'hackers'
- unless @project.aasm_state == "invention"
%p.pull-right
- if @project.users.empty? or !@project.users.include? current_user
= link_to(join_project_path(@episode, @project), :method => :post, :class => "btn btn-success") do
%i.fa.fa-plus
Join this project
- if @project.users.include? current_user
= link_to(leave_project_path(@episode, @project), :method => :post, :class => "btn btn-warning") do
%i.fa.fa-minus
Leave this project
%p#membership_buttons.pull-right
= render partial: 'membership_buttons'
.clearfix
.row
.col-sm-12
Expand Down
3 changes: 0 additions & 3 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@
end
end

# compat routes
get 'archive/projects/:id', to: "projects#old_archived"

scope '(:episode)' do
resources :projects do
collection do
Expand Down
19 changes: 4 additions & 15 deletions spec/controllers/projects_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,23 +171,18 @@

it 'ads an user to the project' do
expect {
post :join, id:project.id
post :join, id:project.id, format: 'js'
}.to change(project.users, :count).by(1)
end

it 'redirects to the project' do
post :join, id:project.id
expect(response).to redirect_to(project)
end

context 'when the user already joined' do
before do
project.join!(admin)
end

it 'does not add the user again' do
expect {
post :join, id:project.id
post :join, id:project.id, format: 'js'
}.not_to change(project.users, :count)
end
end
Expand All @@ -196,17 +191,11 @@
describe 'GET leave_project' do
it 'deletes an user from the project' do
project = create(:idea)
post :join, id:project.id
project.join!(admin)
expect {
post :leave, id:project.id
post :leave, id:project.id, format: 'js'
}.to change(project.users, :count).by(-1)
end

it 'redirects to the project' do
project = create(:project)
post :leave, id:project.id
expect(response).to redirect_to(project)
end
end

describe 'GET like_project' do
Expand Down
11 changes: 5 additions & 6 deletions spec/features/collaboration_spec.rb
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
require 'rails_helper'

feature 'Collaboration' do
scenario 'User joins a project' do
scenario 'User joins a project', :js do
user = create(:user)
project = create(:idea)
sign_in user

visit project_path(nil, project)

expect {
click_link 'Join this project'
}.to change(Project.populated, :count).by(1)
click_link 'Join this project'
expect(page).to have_css("#user#{user.id}-gravatar")
expect(page).to have_text("Welcome to the project #{user.name}!")
expect(page).to have_text("Welcome to the project #{user.name}.")
end

scenario 'User leaves a project' do
scenario 'User leaves a project', :js do
user = create(:user)
project = create(:project, users: [user])
sign_in user
Expand All @@ -24,6 +22,7 @@
click_link 'Leave this project'

expect(page).not_to have_css("#user#{user.id}-gravatar")
expect(page).to have_text("Sorry to see you go #{user.name}.")
end

scenario 'User likes a project' do
Expand Down
21 changes: 21 additions & 0 deletions spec/models/project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,25 @@
expect(project.similar_projects).to include(project2)
end
end

describe '.joined' do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:user2) { create(:user) }

it 'returns true if user is there' do
project.users = [user]
expect(project.joined(user)).to eq(true)
end

it 'returns false if user the project has no users' do
project.users = []
expect(project.joined(user)).to eq(false)
end

it 'returns false if user is not there' do
project.users = [user2]
expect(project.joined(user)).to eq(false)
end
end
end

0 comments on commit 0680129

Please sign in to comment.