diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index f44f5f169..4bf5c75ec 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -75,19 +75,6 @@ class ApplicationController < ActionController::Base
response.headers["Expires"] = "Fri, 01 Jan 1990 00:00:00 GMT"
end
- def tab_list(user)
- tab_list = ["Profile",
- "Integrations",
- "Notifications",
- "Publishing from RSS",
- "Organization",
- "Billing"]
- tab_list << "Membership" if user&.monthly_dues&.positive? && user&.stripe_id_code
- tab_list << "Switch Organizations" if user&.has_role?(:switch_between_orgs)
- tab_list << "Misc"
- tab_list
- end
-
def touch_current_user
current_user.touch
end
diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb
index c3dbdfda7..86ba22efa 100644
--- a/app/controllers/organizations_controller.rb
+++ b/app/controllers/organizations_controller.rb
@@ -4,7 +4,7 @@ class OrganizationsController < ApplicationController
def create
@tab = "organization"
@user = current_user
- @tab_list = tab_list(@user)
+ @tab_list = @user.settings_tab_list
@organization = Organization.new(organization_params)
if @organization.save
current_user.update(organization_id: @organization.id, org_admin: true)
@@ -20,7 +20,7 @@ class OrganizationsController < ApplicationController
def update
@user = current_user
@tab = "organization"
- @tab_list = tab_list(@user)
+ @tab_list = @user.settings_tab_list
raise unless @user.org_admin
@organization = @user.organization
if @organization.update(organization_params)
diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb
index 1ed9d1346..0ce474cf6 100644
--- a/app/controllers/stories_controller.rb
+++ b/app/controllers/stories_controller.rb
@@ -89,7 +89,7 @@ class StoriesController < ApplicationController
@stories = @stories.where(approved: true)
end
- @stories = stories_by_timeframe
+ @stories = stories_by_timeframe
@stories = @stories.decorate
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index ae96ac47e..dbaa9e797 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -1,35 +1,27 @@
class UsersController < ApplicationController
- before_action :set_user, only: %i[show update destroy finish_signup]
- before_action :set_no_cache_header, except: [:index]
-
- def index
- #Soft internal deprecation. Should be removed at some point.
- return redirect_to "/" unless current_user
- @users = case params[:user_board].downcase
- when "following" then current_user.following_users
- when "followers" then current_user.user_followers
- end
- end
+ before_action :set_no_cache_header
+ after_action :verify_authorized, except: [:signout_confirm]
# GET /settings/@tab
def edit
- @user = current_user
- @tab_list = tab_list(@user)
- unless @user
- redirect_to "/enter"
- return
+ unless current_user
+ skip_authorization
+ return redirect_to "/enter"
end
+ @user = current_user
+ @tab_list = @user.settings_tab_list
@tab = params["tab"]
+ authorize @user
handle_settings_tab
- # authorize! :update, @user
end
# PATCH/PUT /users/:id.:format
def update
@user = current_user
- @tab_list = tab_list(@user)
+ @tab_list = @user.settings_tab_list
@tab = params["user"]["tab"] || "profile"
- if !@user.banned && @user.update(user_params)
+ authorize @user
+ if @user.update(user_params)
RssReader.new.delay.fetch_user(@user) if @user.feed_url.present?
notice = "Your profile was successfully updated."
follow_hiring_tag(@user)
@@ -41,6 +33,7 @@ class UsersController < ApplicationController
def onboarding_update
current_user.saw_onboarding = true
+ authorize User
if current_user.save!
respond_to do |format|
format.json { render json: { outcome: "onboarding closed" } }
@@ -53,6 +46,7 @@ class UsersController < ApplicationController
end
def join_org
+ authorize User
if @organization = Organization.find_by_secret(params[:org_secret])
current_user.update(organization_id: @organization.id)
redirect_to "/settings/organization",
@@ -63,15 +57,15 @@ class UsersController < ApplicationController
end
def leave_org
+ authorize User
current_user.update(organization_id: nil, org_admin: nil)
redirect_to "/settings/organization",
notice: "You have left your organization."
end
def add_org_admin
- raise unless current_user.org_admin
user = User.find(params[:user_id])
- raise unless current_user.organization_id == user.organization_id
+ authorize user
user.update(org_admin: true)
user.add_role :analytics_beta_tester if user.organization.approved
redirect_to "/settings/organization",
@@ -79,19 +73,16 @@ class UsersController < ApplicationController
end
def remove_org_admin
- raise unless current_user.org_admin
- raise if current_user.id == params[:user_id]
user = User.find(params[:user_id])
- raise unless current_user.organization_id == user.organization_id
+ authorize user
user.update(org_admin: false)
redirect_to "/settings/organization",
notice: "#{user.name} is no longer an admin."
end
def remove_from_org
- raise unless current_user.org_admin
- raise if current_user.id == params[:user_id]
user = User.find(params[:user_id])
+ authorize user
user.update(organization_id: nil)
redirect_to "/settings/organization",
notice: "#{user.name} is no longer part of your organization."
@@ -99,20 +90,6 @@ class UsersController < ApplicationController
def signout_confirm; end
- # GET/PATCH /users/:id/finish_signup
- def finish_signup
- # authorize! :update, @user
- if request.patch? && params[:user] # && params[:user][:email]
- if @user.update(user_params)
- # @user.skip_reconfirmation!
- sign_in(@user, bypass: true)
- redirect_to "/", notice: "Your profile was successfully updated."
- else
- @show_errors = true
- end
- end
- end
-
def follow_hiring_tag(user)
return unless user.looking_for_work?
user.delay.follow(Tag.find_by(name: "hiring"))
@@ -134,7 +111,7 @@ class UsersController < ApplicationController
@customer = Stripe::Customer.retrieve(current_user.stripe_id_code) if current_user.stripe_id_code
when "membership"
if current_user.monthly_dues.zero?
- redirect_to "/membership"
+ redirect_to "/membership"
return
end
end
@@ -142,10 +119,6 @@ class UsersController < ApplicationController
private
- def set_user
- @user = User.find(params[:id])
- end
-
def user_params
accessible = %i[name
email
diff --git a/app/models/user.rb b/app/models/user.rb
index 1d1112a39..d64a93b6a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -279,6 +279,19 @@ class User < ApplicationRecord
end
end
+ def settings_tab_list
+ tab_list = ["Profile",
+ "Integrations",
+ "Notifications",
+ "Publishing from RSS",
+ "Organization",
+ "Billing"]
+ tab_list << "Membership" if monthly_dues&.positive? && stripe_id_code
+ tab_list << "Switch Organizations" if has_role?(:switch_between_orgs)
+ tab_list << "Misc"
+ tab_list
+ end
+
private
diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb
new file mode 100644
index 000000000..dc4f8263a
--- /dev/null
+++ b/app/policies/user_policy.rb
@@ -0,0 +1,43 @@
+class UserPolicy < ApplicationPolicy
+ def edit?
+ user == record
+ end
+
+ def onboarding_update?
+ true
+ end
+
+ def update?
+ user == record
+ end
+
+ def join_org?
+ !user_is_banned?
+ end
+
+ def leave_org?
+ true
+ end
+
+ def add_org_admin?
+ user.org_admin && within_the_same_org?
+ end
+
+ def remove_org_admin?
+ user.org_admin && not_self? && within_the_same_org?
+ end
+
+ def remove_from_org?
+ user.org_admin && not_self? && within_the_same_org?
+ end
+
+ private
+
+ def within_the_same_org?
+ user.organization == record.organization
+ end
+
+ def not_self?
+ user != record
+ end
+end
diff --git a/app/views/users/finish_signup.html.erb b/app/views/users/finish_signup.html.erb
deleted file mode 100644
index 9e9aa2bd5..000000000
--- a/app/views/users/finish_signup.html.erb
+++ /dev/null
@@ -1,41 +0,0 @@
-
-
- <% if current_user.unconfirmed_email %>
-
An email has been sent to you, please confirm!
- <% else %>
-
Welcome!
-
Just add a couple details to get going
- <% end %>
- <%= form_for(current_user, :as => 'user', :url => finish_signup_path(current_user), :html => { role: 'form'}) do |f| %>
- <% if @show_errors && current_user.errors.any? %>
-
- <% current_user.errors.full_messages.each do |msg| %>
- <%= msg %>
- <% end %>
-
- <% end %>
-
-
-
-
-
- <%= f.submit 'Continue', :class => 'btn btn-primary' %>
-
- <% end %>
-
diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb
deleted file mode 100644
index 3c50bafec..000000000
--- a/app/views/users/index.html.erb
+++ /dev/null
@@ -1,40 +0,0 @@
-<% title params[:user_board].titleize %>
-
-
-
-
- <%=params[:user_board].titleize%>
-
- <% @users.each do |user| %>
-
- <% end %>
-
-
- <% if params[:user_board].downcase == "followers" %>
- When someone follows you, your posts are prioritized on their home page. ❤️
- <% else %>
- When you follow someone, their posts are prioritized on your home page. ❤️
- <% end %>
-
-
diff --git a/config/routes.rb b/config/routes.rb
index 0746f6e53..10f6796de 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -99,7 +99,7 @@ Rails.application.routes.draw do
resources :video_states, only: [:create]
resources :twilio_tokens, only: [:show]
resources :push_notification_subscriptions, only: [:create]
-
+
get "/notifications/:username" => "notifications#index"
patch "/onboarding_update" => "users#onboarding_update"
get "email_subscriptions/unsubscribe"
@@ -193,12 +193,8 @@ Rails.application.routes.draw do
end
end
- get "/:user_board" => "users#index", :constraints => { user_board: /following|followers|leaderboard/}
-
match "/delayed_job_admin" => DelayedJobWeb, :anchor => false, via: [:get, :post]
- match "/users/:id/finish_signup" => "users#finish_signup", via: [:get, :patch], :as => :finish_signup
-
get "/settings/(:tab)" => "users#edit"
get "/signout_confirm" => "users#signout_confirm"
get "/dashboard" => "dashboards#show"
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
deleted file mode 100644
index 083c6b904..000000000
--- a/spec/controllers/users_controller_spec.rb
+++ /dev/null
@@ -1,48 +0,0 @@
-# controller specs are now discouraged in favor of request specs.
-# This file should eventually be removed
-require "rails_helper"
-
-RSpec.describe UsersController, type: :controller do
- let(:user) { create(:user) }
-
- describe "GET #index" do
- context "without current_user" do
- it "returns a 302" do
- get :index, params: { user_board: "following" }
- expect(response.status).to eq(302)
- end
- end
-
- context "with current_user" do
- before { sign_in user }
-
- it "works for followings" do
- get :index, params: { user_board: "following" }
- expect(response.status).to eq(200)
- end
-
- it" works for followers" do
- get :index, params: { user_board: "followers" }
- expect(response.status).to eq(200)
- end
- end
- end
-
- describe "GET #edit" do
- context "without being signed in" do
- it "returns redirect" do
- get :edit
- expect(response).to redirect_to("/enter")
- end
- end
-
- context "with authorized user" do
- before { sign_in user }
-
- it "returns 200" do
- get :edit
- expect(response.status).to eq(200)
- end
- end
- end
-end
diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb
new file mode 100644
index 000000000..6594ddbcd
--- /dev/null
+++ b/spec/policies/user_policy_spec.rb
@@ -0,0 +1,56 @@
+require "rails_helper"
+
+RSpec.describe UserPolicy do
+ subject { described_class.new(user, other_user) }
+
+ let(:other_user) { build(:user) }
+
+ context "when user is not signed-in" do
+ let(:user) { nil }
+
+ it { within_block_is_expected.to raise_error(Pundit::NotAuthorizedError) }
+ end
+
+ context "when user is signed-in" do
+ let(:user) { other_user }
+
+ it { is_expected.to permit_actions(%i[edit update onboarding_update join_org leave_org]) }
+
+ context "with banned status" do
+ before { user.add_role(:banned) }
+
+ it { is_expected.to forbid_actions(%i[join_org]) }
+ end
+ end
+
+ context "when user is org_admin" do
+ let(:org) { build(:organization) }
+ let(:other_org) { build(:organization) }
+ let(:user) { build(:user, org_admin: true, organization: org) }
+
+
+ context "with other_user as org_member of same org" do
+ let(:other_user) { build(:user, organization: org) }
+
+ it { is_expected.to permit_actions(%i[add_org_admin remove_from_org]) }
+ end
+
+ context "with other_user as org_member of a different org" do
+ let(:other_user) { build(:user, organization: other_org) }
+
+ it { is_expected.to forbid_actions(%i[add_org_admin remove_from_org]) }
+ end
+
+ context "with other_user as org admin" do
+ let(:other_user) { build(:user, org_admin: true, organization: org) }
+
+ it { is_expected.to permit_actions(%i[remove_org_admin]) }
+ end
+
+ context "with other_user as org adming of a different org" do
+ let(:other_user) { build(:user, org_admin: true, organization: other_org) }
+
+ it { is_expected.to forbid_actions(%i[remove_org_admin]) }
+ end
+ end
+end
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 831a6d6b1..544854362 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -20,7 +20,6 @@ require "shoulda/matchers"
require "pundit/rspec"
require "pundit/matchers"
-
WebMock.disable_net_connect!(allow_localhost: true)
# Add additional requires below this line. Rails is not loaded until this point!
diff --git a/spec/requests/additional_content_boxes_spec.rb b/spec/requests/additional_content_boxes_spec.rb
index 23da2e473..00689f6cd 100644
--- a/spec/requests/additional_content_boxes_spec.rb
+++ b/spec/requests/additional_content_boxes_spec.rb
@@ -1,19 +1,19 @@
require "rails_helper"
RSpec.describe "AdditionalContentBoxes", type: :request do
+ let(:regular_article) { create(:article) }
+
describe "GET /additional_content_boxes" do
it "returns an article if there is a published/featured one" do
suggestion = create(:article, published: true, featured: true)
- suggestion.update_column(:published, true)
- article = create(:article)
- get "/additional_content_boxes?article_id=#{article.id}"
+ get "/additional_content_boxes?article_id=#{regular_article.id}"
expect(response.body).to include CGI.escapeHTML(suggestion.title)
end
+
it "returns no article if not published/featured" do
suggestion = create(:article)
- article = create(:article)
- get "/additional_content_boxes?article_id=#{article.id}"
- expect(response.body).to_not include CGI.escapeHTML(suggestion.title)
+ get "/additional_content_boxes?article_id=#{regular_article.id}"
+ expect(response.body).not_to include CGI.escapeHTML(suggestion.title)
end
end
-end
\ No newline at end of file
+end
diff --git a/spec/requests/user_organization_spec.rb b/spec/requests/user_organization_spec.rb
index d7d39512a..343546532 100644
--- a/spec/requests/user_organization_spec.rb
+++ b/spec/requests/user_organization_spec.rb
@@ -44,7 +44,7 @@ RSpec.describe "UserOrganization", type: :request do
user.update(organization_id: organization.id)
user2 = create(:user, organization_id: organization.id, org_admin: true)
expect { post "/users/add_org_admin", params: { user_id: user2.id } }.
- to raise_error RuntimeError
+ to raise_error Pundit::NotAuthorizedError
end
it "removes org admin" do
@@ -58,6 +58,6 @@ RSpec.describe "UserOrganization", type: :request do
user.update(organization_id: organization.id)
user2 = create(:user, organization_id: organization.id, org_admin: true)
expect { post "/users/remove_org_admin", params: { user_id: user2.id } }.
- to raise_error RuntimeError
+ to raise_error Pundit::NotAuthorizedError
end
end
diff --git a/spec/requests/user_settings_spec.rb b/spec/requests/user_settings_spec.rb
index 978a11fe2..33d49bbf9 100644
--- a/spec/requests/user_settings_spec.rb
+++ b/spec/requests/user_settings_spec.rb
@@ -1,39 +1,53 @@
require "rails_helper"
RSpec.describe "UserSettings", type: :request do
- before do
- @user = create(:user)
- login_as @user
- end
+ let(:user) { create(:user) }
- describe "GET /settings" do
- it "renders various settings tabs properly" do
- get "/settings/organization"
- expect(response.body).to include("Settings for")
- get "/settings/switch-organizations"
- expect(response.body).to include("Settings for")
- # get "/settings/integrations"
- # expect(response.body).to include("Settings for")
- get "/settings/billing"
- expect(response.body).to include("Settings for")
- get "/settings/misc"
- expect(response.body).to include("Settings for")
- get "/settings/membership"
- expect(response.body).not_to include("Settings for")
- @user.update_column(:monthly_dues, 5)
- get "/settings/membership"
- expect(response.body).to include("Settings for")
+ describe "GET /settings/:tab" do
+ context "when not signed-in" do
+ it "returns not_foudn" do
+ get "/settings"
+ expect(response).to redirect_to("/enter")
+ end
+ end
+
+ context "when signed-in" do
+ before do
+ login_as user
+ end
+
+ it "renders various settings tabs properly" do
+ %w[organization switch-organizations billing misc].each do |tab|
+ get "/settings/#{tab}"
+ expect(response.body).to include("Settings for")
+ end
+ end
+
+ it "doesn't let user access membership if user has no monthly_dues" do
+ get "/settings/membership"
+ expect(response.body).not_to include("Settings for")
+ end
+
+ it "allows user with monthly_dues to access membership" do
+ user.update_column(:monthly_dues, 5)
+ get "/settings/membership"
+ expect(response.body).to include("Settings for")
+ end
end
end
describe "PUT /update/:id" do
+ before do
+ login_as user
+ end
+
it "updates summary" do
- put "/users/#{@user.id}", params: { user: {tab: "profile", summary: "Hello new summary"} }
- expect(@user.summary).to eq("Hello new summary")
+ put "/users/#{user.id}", params: { user: { tab: "profile", summary: "Hello new summary" } }
+ expect(user.summary).to eq("Hello new summary")
end
it "updates username to too short username" do
- put "/users/#{@user.id}", params: { user: {tab: "profile", username: "h"} }
+ put "/users/#{user.id}", params: { user: { tab: "profile", username: "h" } }
expect(response.body).to include("Username is too short")
end
end