Revert "Block access to private and public pages for locked users"

pull/13056/head
Alex Martín 7 years ago committed by GitHub
parent 64d6716a42
commit 317c85521f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -227,7 +227,6 @@ WORKING_SPECS_9 = \
spec/requests/carto/api/overlays_controller_spec.rb \
spec/models/carto/user_creation_spec.rb \
spec/requests/carto/api/invitations_controller_spec.rb \
spec/requests/user_state_spec.rb \
spec/models/carto/invitation_spec.rb \
spec/models/carto/user_service_spec.rb \
spec/models/carto/user_spec.rb \

@ -83,7 +83,6 @@ ion for time-series (#12670)
* Improve legends error (cartodb.js#1758)
* Updates Dataservices API client default version to `0.21.0` (#12942)
* Now is possible to use wildcard character (*) in the whitelist emails for organization signups (#12991)
* User accounts in locked state returns 404 for resources like maps or visualizatios and redirection for private endpoints (#13030)
### Bug fixes / enhancements
* Fix dashboard redirections (#12775)

@ -20,7 +20,6 @@ class ApplicationController < ActionController::Base
before_filter :browser_is_html5_compliant?
before_filter :set_asset_debugging
before_filter :cors_preflight_check
before_filter :check_user_state
after_filter :allow_cross_domain_access
after_filter :remove_flash_cookie
after_filter :add_revision_header
@ -162,17 +161,6 @@ class ApplicationController < ActionController::Base
right_referer && right_origin
end
def check_user_state
return unless (request.path =~ %r{^\/(upgrade_trial|login|logout|unauthenticated)}).nil?
viewed_username = CartoDB.extract_subdomain(request)
if current_user.nil? || current_user.username != viewed_username
user = Carto::User.find_by_username(viewed_username)
render_locked_owner if user.try(:locked?)
elsif current_user.locked?
render_locked_user
end
end
def render_403
respond_to do |format|
format.html { render(file: 'public/403.html', status: 403, layout: false) }
@ -225,28 +213,6 @@ class ApplicationController < ActionController::Base
end
end
def render_locked_user
respond_to do |format|
format.html do
redirect_to CartoDB.path(self, 'upgrade_trial')
end
format.json do
head 404
end
end
end
def render_locked_owner
respond_to do |format|
format.html do
render_404
end
format.json do
head 404
end
end
end
def not_authorized
respond_to do |format|
format.html do

@ -107,11 +107,6 @@ class SignupController < ApplicationController
end
end
def upgrade_trial
# TODO Render the personalized upgrade trial view
render action: 'upgrade_trial'
end
private
def oauth_provider

@ -27,9 +27,6 @@ class Carto::User < ActiveRecord::Base
OBS_GENERAL_BLOCK_SIZE = 1000
MAPZEN_ROUTING_BLOCK_SIZE = 1000
STATE_ACTIVE = 'active'.freeze
STATE_LOCKED = 'locked'.freeze
# INFO: select filter is done for security and performance reasons. Add new columns if needed.
DEFAULT_SELECT = "users.email, users.username, users.admin, users.organization_id, users.id, users.avatar_url," \
"users.api_key, users.database_schema, users.database_name, users.name, users.location," \
@ -594,14 +591,6 @@ class Carto::User < ActiveRecord::Base
account_url(request_protocol) + '/plan'
end
def active?
state == STATE_ACTIVE
end
def locked?
state == STATE_LOCKED
end
private
def set_database_host

@ -112,7 +112,7 @@ module Concerns
:salesforce_datasource_enabled, :viewer, :geocoder_provider,
:isolines_provider, :routing_provider, :engine_enabled, :builder_enabled,
:mapzen_routing_quota, :mapzen_routing_block_price, :soft_mapzen_routing_limit, :no_map_logo,
:user_render_timeout, :database_render_timeout, :state]
:user_render_timeout, :database_render_timeout]
end
end

@ -21,9 +21,9 @@ class SearchTweet < Sequel::Model
def self.get_twitter_imports_count(dataset, date_from, date_to)
dataset
.where('search_tweets.state = ?', ::SearchTweet::STATE_COMPLETE)
.where('search_tweets.created_at >= ? AND search_tweets.created_at <= ?', date_from, date_to + 1.days)
.sum("retrieved_items".lit).to_i
.where(state: ::SearchTweet::STATE_COMPLETE)
.where('search_tweets.created_at >= ? AND search_tweets.created_at <= ?', date_from, date_to + 1.days)
.sum("retrieved_items".lit).to_i
end
def set_importing_state

@ -123,9 +123,6 @@ class User < Sequel::Model
COMMON_DATA_ACTIVE_DAYS = 31
STATE_ACTIVE = 'active'.freeze
STATE_LOCKED = 'locked'.freeze
self.raise_on_typecast_failure = false
self.raise_on_save_failure = false
@ -1667,14 +1664,6 @@ class User < Sequel::Model
frontend_version || CartoDB::Application.frontend_version
end
def active?
state == STATE_ACTIVE
end
def locked?
state == STATE_LOCKED
end
private
def common_data_outdated?

@ -26,7 +26,7 @@ module Carto
:salesforce_datasource_enabled, :builder_enabled, :geocoder_provider, :isolines_provider, :routing_provider,
:github_user_id, :engine_enabled, :mapzen_routing_quota, :mapzen_routing_block_price, :soft_mapzen_routing_limit,
:no_map_logo, :org_admin, :last_name, :user_render_timeout, :database_render_timeout, :frontend_version,
:asset_host, :state
:asset_host
].freeze
def compatible_version?(version)

@ -29,8 +29,6 @@ CartoDB::Application.routes.draw do
get '(/user/:user_domain)(/u/:user_domain)/status' => 'home#app_status'
get '(/user/:user_domain)(/u/:user_domain)/diagnosis' => 'home#app_diagnosis'
get '(/user/:user_domain)/upgrade_trial' => 'signup#upgrade_trial', as: :upgrade_trial
# Explore
get '(/user/:user_domain)(/u/:user_domain)/explore' => 'explore#index', as: :explore_index
get '(/user/:user_domain)(/u/:user_domain)/search' => 'explore#search', as: :explore_search

@ -1,12 +0,0 @@
require 'carto/db/migration_helper'
include Carto::Db::MigrationHelper
migration(
Proc.new do
add_column :users, :state, :text, default: 'active', null: false
end,
Proc.new do
drop_column :users, :state
end
)

@ -25,8 +25,6 @@ module Carto
visualization_attributes: {}
)
carto_user = Carto::User.find(carto_user.id) unless carto_user.is_a? Carto::User
table_visualization = table.visualization || create_table_visualization(carto_user, table)
visualization = FactoryGirl.create(:carto_visualization, { user: carto_user, map: map }
.merge(visualization_attributes))

@ -20,9 +20,11 @@ FactoryGirl.define do
builder_enabled nil # Most tests still assume editor
trait :admin_privileges do
username 'Admin'
email 'admin@example.com'
admin true
end
trait :private_tables do
@ -42,22 +44,18 @@ FactoryGirl.define do
mobile_max_private_users 20000
end
trait :locked do
state 'locked'
end
factory :user_with_private_tables, traits: [:enabled, :private_tables]
factory :admin, traits: [:admin]
trait :valid do
factory :valid_user do
username { unique_name('user') }
email { unique_email }
password 'kkkkkkkkk'
password_confirmation 'kkkkkkkkk'
salt 'kkkkkkkkk'
crypted_password 'kkkkkkkkk'
end
factory :user_with_private_tables, traits: [:enabled, :private_tables]
factory :admin, traits: [:admin]
factory :valid_user, traits: [:valid]
factory :locked_user, traits: [:valid, :locked]
before(:create) do
CartoDB::UserModule::DBService.any_instance.stubs(:enable_remote_db_user).returns(true)
end
@ -90,12 +88,6 @@ FactoryGirl.define do
::User.where(id: carto_user.id).first.after_create
CartoDB::UserModule::DBService.any_instance.unstub
end
trait :locked do
state 'locked'
end
factory :carto_locked_user, traits: [:locked]
end
end

@ -0,0 +1,136 @@
# encoding: utf-8
require_relative '../acceptance_helper'
feature 'Dashboard' do
background do
@user = FactoryGirl.create(:user, :table_quota => 6)
@tables = []
@tables << FactoryGirl.create(:table, :user_id => @user.id, :tags => 'tag 1, wadus')
@tables << FactoryGirl.create(:table, :user_id => @user.id, :tags => 'wadus')
@tables << FactoryGirl.create(:table, :user_id => @user.id, :tags => 'tag 2')
@tables << FactoryGirl.create(:table, :user_id => @user.id, :tags => 'tag 1, tag 2')
@tables << FactoryGirl.create(:table, :user_id => @user.id, :tags => 'tag 2, wadus')
end
scenario 'allows user to login and see their tables' do
log_in_as(@user)
page.should have_content "Welcome #{@user.username}"
page.should have_content '5 tables in your account'
page.should have_css 'div.table_info div.left hgroup h3 a', :length => 5
end
scenario 'allows users to browse their tables by tags' do
log_in_as(@user)
page.should have_content '5 tables in your account'
click_on 'wadus'
page.should have_content '3 tables with tag wadus'
page.should have_link @tables[0].name
page.should have_link @tables[1].name
page.should have_link @tables[4].name
page.should_not have_link @tables[2].name
page.should_not have_link @tables[3].name
click_on 'tag 1'
page.should have_content '2 tables with tag tag 1'
page.should have_link @tables[0].name
page.should have_link @tables[3].name
page.should_not have_link @tables[1].name
page.should_not have_link @tables[2].name
page.should_not have_link @tables[4].name
end
scenario 'allows users to remove a table' do
log_in_as(@user)
page.should have_content '5 tables in your account'
within '.table_info' do
click_on 'delete'
end
expect do
click_on 'Delete this table'
end.to change{ Table.count }.from(5).to(4)
page.should have_content '4 tables in your account'
end
scenario 'allows users to create a table with default attributes' do
log_in_as(@user)
click_on 'Create a new table'
click_on 'Start from scratch'
expect do
click_on 'Create table'
sleep 3
end.to change{ Table.count }.from(5).to(6)
created_table = Table.all.last
page.should have_link created_table.name
current_path.should match table_path(created_table)
end
scenario 'shows users a link to upgrade their accounts' do
log_in_as(@user)
page.should have_content "Hey #{@user.username}, looks like you're about to reach your account limit. Start thinking about upgrading your plan."
page.should have_link 'upgrading your plan', :href => "http://localhost:3000/account/#{@user.username}/upgrade"
end
scenario 'allows users to obtain their security keys' do
log_in_as(@user)
click_on @user.username
click_on 'Your API keys'
page.should have_content 'Your Api key'
click_on 'Request a new key'
click_on 'Regenerate Api key'
page.should have_content @user.api_key
click_on 'OAUTH'
click_on 'Request a new key'
click_on 'Regenerate Oauth and secret keys'
page.should have_content @user.client_application.key
page.should have_content @user.client_application.secret
end
scenario 'allows users to create a table from a imported file'
scenario 'allows users to create a table from a URL pointing to a data file'
scenario 'allows to import a previously exported table as SHP (.zip extension)'
scenario 'allows to import a previously exported table as SHP (.tgz extension)'
scenario 'allows to import a previously exported table as SHP (.tar.gz extension)'
scenario 'allows to import a previously exported table as CSV'
scenario 'allows to import a previously exported table as KML'
scenario 'allows to import a previously exported table as SQL'
end

@ -1,254 +0,0 @@
# encoding: utf-8
require_relative '../spec_helper'
include Warden::Test::Helpers
include Carto::Factories::Visualizations
def login(user)
login_as(user, scope: user.username)
host! "#{user.username}.localhost.lan"
end
def follow_redirects(limit = 10)
while response.redirect? && (limit -= 1) > 0
follow_redirect!
end
end
describe "UserState" do
before(:all) do
@locked_user = FactoryGirl.create(:locked_user)
@map, @table, @table_visualization, @visualization = create_full_builder_vis(@locked_user)
@visualization.create_mapcap!
@non_locked_user = FactoryGirl.create(:valid_user)
@dashboard_endpoints = ['/dashboard', '/dashboard/tables', '/dashboard/datasets', '/dashboard/visualizations',
'/dashboard/maps'].freeze
@public_user_endpoints = ['/me'].freeze
@user_endpoints = ['/account', '/profile'].freeze
@tables_endpoints = ["/tables/#{@table.id}", "/tables/#{@table.id}/public",
"/tables/#{@table.id}/embed_map"].freeze
@viz_endpoints = ["/viz/#{@visualization.id}/public",
"/viz/#{@visualization.id}/embed_map", "/viz/#{@visualization.id}/public_map",
"/builder/#{@visualization.id}", "/builder/#{@visualization.id}/embed"].freeze
@public_api_endpoints = ["/api/v1/viz", "/api/v1/viz/#{@visualization.id}",
"/api/v2/viz/#{@visualization.id}/viz",
"/api/v3/me", "/api/v3/viz/#{@visualization.id}/viz"].freeze
@private_api_endpoints = ["/api/v1/tables/#{@table.id}", "/api/v1/tables/#{@table.id}/columns",
"/api/v1/imports", "/api/v1/users/#{@locked_user.id}/layers",
"/api/v1/synchronizations", "/api/v1/geocodings",
"/api/v1/users/#{@locked_user.id}"]
@headers = {}
@api_headers = { 'CONTENT_TYPE' => 'application/json', :format => "json" }
end
after(:all) do
@locked_user.destroy
@non_locked_user.destroy
end
describe '#locked user' do
it 'owner accessing their resources' do
login(@locked_user)
@dashboard_endpoints.each do |endpoint|
get endpoint, {}, @headers
response.status.should == 302
follow_redirect!
request.path.should == '/upgrade_trial'
response.status.should == 200
end
@user_endpoints.each do |endpoint|
get endpoint, {}, @headers
response.status.should == 302
follow_redirect!
request.path.should == '/upgrade_trial'
response.status.should == 200
end
@public_user_endpoints.each do |endpoint|
get endpoint, {}, @headers
response.status.should == 302
follow_redirect!
request.path.should == '/upgrade_trial'
response.status.should == 200
end
@tables_endpoints.each do |endpoint|
get endpoint, {}, @headers
response.status.should == 302
follow_redirect!
request.path.should == '/upgrade_trial'
response.status.should == 200
end
@viz_endpoints.each do |endpoint|
get endpoint, {}, @headers
response.status.should == 302
follow_redirect!
request.path.should == '/upgrade_trial'
response.status.should == 200
end
@private_api_endpoints.each do |endpoint|
get "#{endpoint}?api_key=#{@locked_user.api_key}", {}, @api_headers
request.path == endpoint
response.status.should == 404
end
@public_api_endpoints.each do |endpoint|
get endpoint, {}, @api_headers
request.path == endpoint
response.status.should == 404
end
end
it 'user accessing a locked user resources' do
login(@non_locked_user)
host! "#{@locked_user.username}.localhost.lan"
@user_endpoints.each do |endpoint|
get endpoint, {}, @headers
response.status.should == 404
end
@public_user_endpoints.each do |endpoint|
get endpoint, {}, @headers
response.status.should == 404
end
@tables_endpoints.each do |endpoint|
get endpoint, {}, @headers
response.status.should == 404
end
@viz_endpoints.each do |endpoint|
get endpoint, {}, @headers
response.status.should == 404
end
@public_api_endpoints.each do |endpoint|
get endpoint, {}, @api_headers
request.path == endpoint
response.status.should == 404
end
end
it 'non-logged user accessing a locked user resources' do
host! "#{@locked_user.username}.localhost.lan"
@public_user_endpoints.each do |endpoint|
get endpoint, {}, @headers
response.status.should == 404
end
@tables_endpoints.each do |endpoint|
get endpoint, {}, @headers
response.status.should == 404
end
@viz_endpoints.each do |endpoint|
get endpoint, {}, @headers
response.status.should == 404
end
@public_api_endpoints.each do |endpoint|
get endpoint, {}, @api_headers
request.path == endpoint
response.status.should == 404
end
end
end
describe '#non locked user' do
before(:all) do
@locked_user.state = 'active'
@locked_user.save
end
after(:all) do
@locked_user.state = 'locked'
@locked_user.save
end
it 'owner accessing their resources' do
login(@locked_user)
host! "#{@locked_user.username}.localhost.lan"
@dashboard_endpoints.each do |endpoint|
get endpoint, {}, @headers
follow_redirects
request.path.should_not == '/upgrade_trial'
response.status.should == 200
end
@user_endpoints.each do |endpoint|
get endpoint, {}, @headers
follow_redirects
request.path.should_not == '/upgrade_trial'
response.status.should == 200
end
@public_user_endpoints.each do |endpoint|
get endpoint, {}, @headers
follow_redirects
request.path.should_not == '/upgrade_trial'
response.status.should == 200
end
@tables_endpoints.each do |endpoint|
get endpoint, {}, @headers
follow_redirects
request.path.should_not == '/upgrade_trial'
response.status.should == 200
end
@viz_endpoints.each do |endpoint|
get endpoint, {}, @headers
follow_redirects
request.path.should_not == '/upgrade_trial'
response.status.should == 200
end
@private_api_endpoints.each do |endpoint|
get "#{endpoint}?api_key=#{@locked_user.api_key}", {}, @api_headers
request.path == endpoint
response.status.should == 200
end
@public_api_endpoints.each do |endpoint|
get endpoint, {}, @api_headers
request.path == endpoint
response.status.should == 200
end
end
it 'non locked user accessing a locked user resources' do
login(@non_locked_user)
@user_endpoints.each do |endpoint|
host! "#{@locked_user.username}.localhost.lan"
get endpoint, {}, @headers
follow_redirects
response.status.should == 200
end
@public_user_endpoints.each do |endpoint|
host! "#{@locked_user.username}.localhost.lan"
get endpoint, {}, @headers
follow_redirects
response.status.should == 200
end
@tables_endpoints.each do |endpoint|
host! "#{@locked_user.username}.localhost.lan"
get endpoint, {}, @headers
follow_redirects
response.status.should == 200
end
@viz_endpoints.each do |endpoint|
host! "#{@locked_user.username}.localhost.lan"
get endpoint, {}, @headers
follow_redirects
response.status.should == 200
end
@public_api_endpoints.each do |endpoint|
get endpoint, {}, @api_headers
request.path == endpoint
response.status.should == 200
end
end
it 'non-logged user accessing a locked user resources' do
host! "#{@locked_user.username}.localhost.lan"
@public_user_endpoints.each do |endpoint|
get endpoint, {}, @headers
follow_redirects
response.status.should == 200
end
@tables_endpoints.each do |endpoint|
get endpoint, {}, @headers
follow_redirects
response.status.should == 200
end
@viz_endpoints.each do |endpoint|
get endpoint, {}, @headers
follow_redirects
response.status.should == 200
end
@public_api_endpoints.each do |endpoint|
get endpoint, {}, @api_headers
request.path == endpoint
response.status.should == 200
end
end
end
end

@ -263,7 +263,6 @@ describe Carto::UserMetadataExportService do
salt: "kkkkkkkkk",
database_name: "cartodb_test_user_5be8c3d4-49f0-11e7-8698-bc5ff4c95cd0_db",
username: "user00000001",
state: 'active',
admin: nil,
enabled: true,
invite_token: nil,

Loading…
Cancel
Save