From 0ba0eb633e8c24086405c53f3d8a8e747f3382e4 Mon Sep 17 00:00:00 2001
From: Azul <azul@riseup.net>
Date: Sun, 22 May 2016 21:12:42 +0200
Subject: restrict user_params in user_controller

Actually this should live in a service_level_controller.
For now fix the security issue.
---
 app/controllers/users_controller.rb | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 1404b0e..225584f 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -39,7 +39,7 @@ class UsersController < ApplicationController
 
   ## added so updating service level works, but not sure we will actually want this. also not sure that this is place to prevent user from updating own effective service level, but here as placeholder:
   def update
-    @user.update_attributes(params[:user]) unless (!admin? and params[:user][:effective_service_level])
+    @user.update_attributes(user_params)
     if @user.valid?
       flash[:notice] = I18n.t(:changes_saved)
     end
@@ -79,4 +79,11 @@ class UsersController < ApplicationController
     end
   end
 
+  def user_params
+    if admin?
+      params.require(:user).permit(:effective_service_level)
+    else
+      params.require(:user).permit(:password, :password_confirmation)
+    end
+  end
 end
-- 
cgit v1.2.3


From 7f0c42a5fa6d6c1952e53b9b73bad0202f746f3e Mon Sep 17 00:00:00 2001
From: Azul <azul@riseup.net>
Date: Mon, 23 May 2016 11:59:41 +0200
Subject: cleanup: remove service level code from users_controller

There's no route to this right now and it also seems to be tested
nowhere. Since i am about to split up the users_controller let's
get rid of this and put it in the place we want it once we actually finish
the implementation
---
 app/controllers/users_controller.rb             | 11 +----------
 app/views/users/_change_service_level.html.haml | 15 ++++++++++-----
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 225584f..2816a64 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -8,7 +8,7 @@ class UsersController < ApplicationController
   before_filter :require_login, :except => [:new]
   before_filter :redirect_if_logged_in, :only => [:new]
   before_filter :require_admin, :only => [:index, :deactivate, :enable]
-  before_filter :fetch_user, :only => [:show, :edit, :update, :destroy, :deactivate, :enable]
+  before_filter :fetch_user, :only => [:show, :edit, :destroy, :deactivate, :enable]
   before_filter :require_registration_allowed, only: :new
 
   respond_to :html
@@ -37,15 +37,6 @@ class UsersController < ApplicationController
   def edit
   end
 
-  ## added so updating service level works, but not sure we will actually want this. also not sure that this is place to prevent user from updating own effective service level, but here as placeholder:
-  def update
-    @user.update_attributes(user_params)
-    if @user.valid?
-      flash[:notice] = I18n.t(:changes_saved)
-    end
-    respond_with @user, :location => edit_user_path(@user)
-  end
-
   def deactivate
     @user.account.disable
     flash[:notice] = I18n.t("actions.user_disabled_message", username: @user.username)
diff --git a/app/views/users/_change_service_level.html.haml b/app/views/users/_change_service_level.html.haml
index a2e9956..32ea8c0 100644
--- a/app/views/users/_change_service_level.html.haml
+++ b/app/views/users/_change_service_level.html.haml
@@ -1,8 +1,13 @@
--# TODO: probably won't want here, but here for now. Also, we will need way to ensure payment if they pick a non-free plan.
--#
--# SERVICE LEVEL
--#
-- if APP_CONFIG[:service_levels]
+:ruby
+  # DISABLED! this form points to a route that does not exist.
+  #   It's a draft for implementing service levels.
+  # TODO: probably won't want here, but here for now.
+  #       We will need way to ensure payment for a non-free plan.
+  #
+  # SERVICE LEVEL
+  #
+  #
+- if APP_CONFIG[:service_levels] && false
   - form_options = {:html => {:class => user_form_class('form-horizontal'), :id => 'update_service_level', :data => {token: session[:token]}}, :validate => true}
   = simple_form_for @user, form_options do |f|
     %legend= t(:service_level)
-- 
cgit v1.2.3


From f47fc9d6522886cf81cfea26ec1f396219c539ba Mon Sep 17 00:00:00 2001
From: Azul <azul@riseup.net>
Date: Mon, 23 May 2016 12:17:31 +0200
Subject: move signup from users to account_controller

There was a lot of special case handling going on in the users_controller
for this. Lot simpler this way.
---
 app/controllers/account_controller.rb              | 17 ++++++++++++++
 app/controllers/users_controller.rb                | 14 +----------
 app/views/account/new.html.haml                    | 27 ++++++++++++++++++++++
 app/views/sessions/_warnings.html.haml             | 12 ++++++++++
 app/views/sessions/new.html.haml                   |  2 +-
 app/views/users/_warnings.html.haml                | 12 ----------
 app/views/users/new.html.haml                      | 27 ----------------------
 config/routes.rb                                   |  4 ++--
 test/functional/account_controller_test.rb         | 26 +++++++++++++++++++++
 test/functional/users_controller_test.rb           | 22 +-----------------
 test/integration/browser/account_livecycle_test.rb |  2 +-
 .../browser/password_validation_test.rb            |  8 +++----
 test/support/browser_integration_test.rb           |  4 ++--
 13 files changed, 94 insertions(+), 83 deletions(-)
 create mode 100644 app/controllers/account_controller.rb
 create mode 100644 app/views/account/new.html.haml
 create mode 100644 app/views/sessions/_warnings.html.haml
 delete mode 100644 app/views/users/_warnings.html.haml
 delete mode 100644 app/views/users/new.html.haml
 create mode 100644 test/functional/account_controller_test.rb

diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb
new file mode 100644
index 0000000..ee7cca4
--- /dev/null
+++ b/app/controllers/account_controller.rb
@@ -0,0 +1,17 @@
+class AccountController < ApplicationController
+
+  before_filter :require_registration_allowed
+  before_filter :redirect_if_logged_in
+
+  def new
+    @user = User.new
+  end
+
+  protected
+
+  def require_registration_allowed
+    unless APP_CONFIG[:allow_registration]
+      redirect_to home_path
+    end
+  end
+end
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 2816a64..4d198b9 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -5,11 +5,9 @@
 class UsersController < ApplicationController
   include ControllerExtension::FetchUser
 
-  before_filter :require_login, :except => [:new]
-  before_filter :redirect_if_logged_in, :only => [:new]
+  before_filter :require_login
   before_filter :require_admin, :only => [:index, :deactivate, :enable]
   before_filter :fetch_user, :only => [:show, :edit, :destroy, :deactivate, :enable]
-  before_filter :require_registration_allowed, only: :new
 
   respond_to :html
 
@@ -27,10 +25,6 @@ class UsersController < ApplicationController
     @users = @users.limit(100)
   end
 
-  def new
-    @user = User.new
-  end
-
   def show
   end
 
@@ -64,12 +58,6 @@ class UsersController < ApplicationController
 
   protected
 
-  def require_registration_allowed
-    unless APP_CONFIG[:allow_registration]
-      redirect_to home_path
-    end
-  end
-
   def user_params
     if admin?
       params.require(:user).permit(:effective_service_level)
diff --git a/app/views/account/new.html.haml b/app/views/account/new.html.haml
new file mode 100644
index 0000000..d40259e
--- /dev/null
+++ b/app/views/account/new.html.haml
@@ -0,0 +1,27 @@
+-#
+-# This form is handled entirely by javascript
+-# Please take care when changing element ids.
+-#
+-# The form is hidden when no js is available
+-# to prevent submission in the clear.
+-#
+
+- form_options = {url: '/not-used', html: {id: 'new_user', class: user_form_class('form-horizontal'), style: 'display:none'}, validate: true}
+
+.col-md-1
+.col-md-9
+  %h2=t :signup
+  .lead=t :signup_info
+  = render "sessions/warnings"
+  = simple_form_for(@user, form_options) do |f|
+    = f.input :login, :label => t(:username), :required => false, :input_html => { :id => :srp_username }
+    = f.input :password, :label => t(:password), :required => false, :validate => true, :input_html => { :id => :srp_password }
+    = f.input :password_confirmation, :label => t(:password_confirmation), :required => false, :validate => true, :input_html => { :id => :srp_password_confirmation }
+
+    - if APP_CONFIG[:invite_required]
+      = f.input :invite_code, :label => t(:invite_code), :input_html => { :id => :srp_invite_code }
+    - else
+      = f.input :invite_code, :as => "hidden", :input_html => { :value => " ", :id => :srp_invite_code }
+
+    = f.button :wrapped, cancel: home_path
+-#
diff --git a/app/views/sessions/_warnings.html.haml b/app/views/sessions/_warnings.html.haml
new file mode 100644
index 0000000..baf80a4
--- /dev/null
+++ b/app/views/sessions/_warnings.html.haml
@@ -0,0 +1,12 @@
+%noscript
+  %div.alert.alert-error=t :js_required_html
+#cookie_warning.alert.alert-error{:style => "display:none"}
+  =t :cookie_disabled_warning
+:javascript
+  document.cookie = "testing=cookies_enabled; path=/";
+  if(document.cookie.indexOf("testing=cookies_enabled") < 0)
+  {
+    document.getElementById('cookie_warning').style.display = 'block';
+  } else {
+    document.getElementById('cookie_warning').style.display = 'none';
+  }
diff --git a/app/views/sessions/new.html.haml b/app/views/sessions/new.html.haml
index 942c485..6695123 100644
--- a/app/views/sessions/new.html.haml
+++ b/app/views/sessions/new.html.haml
@@ -2,7 +2,7 @@
 .col-md-9
   %h2=t :login
   .lead=t :login_info
-  = render :partial => 'users/warnings'
+  = render 'warnings'
   = simple_form_for [:api, @session], validate: true, html: { id: :new_session, class: 'form-horizontal hidden js-show', style: "display:none;" } do |f|
     = f.input :login, :required => false, :label => t(:username), :input_html => { :id => :srp_username }
     = f.input :password, :required => false, :input_html => { :id => :srp_password }
diff --git a/app/views/users/_warnings.html.haml b/app/views/users/_warnings.html.haml
deleted file mode 100644
index baf80a4..0000000
--- a/app/views/users/_warnings.html.haml
+++ /dev/null
@@ -1,12 +0,0 @@
-%noscript
-  %div.alert.alert-error=t :js_required_html
-#cookie_warning.alert.alert-error{:style => "display:none"}
-  =t :cookie_disabled_warning
-:javascript
-  document.cookie = "testing=cookies_enabled; path=/";
-  if(document.cookie.indexOf("testing=cookies_enabled") < 0)
-  {
-    document.getElementById('cookie_warning').style.display = 'block';
-  } else {
-    document.getElementById('cookie_warning').style.display = 'none';
-  }
diff --git a/app/views/users/new.html.haml b/app/views/users/new.html.haml
deleted file mode 100644
index 1b257d9..0000000
--- a/app/views/users/new.html.haml
+++ /dev/null
@@ -1,27 +0,0 @@
--#
--# This form is handled entirely by javascript
--# Please take care when changing element ids.
--# 
--# The form is hidden when no js is available 
--# to prevent submission in the clear.
--#
-
-- form_options = {url: '/not-used', html: {id: 'new_user', class: user_form_class('form-horizontal'), style: 'display:none'}, validate: true}
-
-.col-md-1
-.col-md-9
-  %h2=t :signup
-  .lead=t :signup_info
-  = render :partial => 'warnings'
-  = simple_form_for(@user, form_options) do |f|
-    = f.input :login, :label => t(:username), :required => false, :input_html => { :id => :srp_username }
-    = f.input :password, :label => t(:password), :required => false, :validate => true, :input_html => { :id => :srp_password }
-    = f.input :password_confirmation, :label => t(:password_confirmation), :required => false, :validate => true, :input_html => { :id => :srp_password_confirmation }
-
-    - if APP_CONFIG[:invite_required]
-      = f.input :invite_code, :label => t(:invite_code), :input_html => { :id => :srp_invite_code }
-    - else
-      = f.input :invite_code, :as => "hidden", :input_html => { :value => " ", :id => :srp_invite_code }
-
-    = f.button :wrapped, cancel: home_path
--#
diff --git a/config/routes.rb b/config/routes.rb
index 7fbedf2..b152c9c 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -44,8 +44,8 @@ LeapWeb::Application.routes.draw do
     get "login" => "sessions#new", :as => "login"
     delete "logout" => "sessions#destroy", :as => "logout"
 
-    get "signup" => "users#new", :as => "signup"
-    resources :users, :except => [:create, :update] do
+    get "signup" => "account#new", :as => "signup"
+    resources :users, :except => [:new, :create, :update] do
       # resource :email_settings, :only => [:edit, :update]
       # resources :email_aliases, :only => [:destroy], :id => /.*/
       post 'deactivate', on: :member
diff --git a/test/functional/account_controller_test.rb b/test/functional/account_controller_test.rb
new file mode 100644
index 0000000..f5f1446
--- /dev/null
+++ b/test/functional/account_controller_test.rb
@@ -0,0 +1,26 @@
+require 'test_helper'
+
+class AccountControllerTest < ActionController::TestCase
+
+  test "should get new" do
+    get :new
+    assert_equal User, assigns(:user).class
+    assert_response :success
+  end
+
+  test "new should redirect logged in users" do
+    login
+    get :new
+    assert_response :redirect
+    assert_redirected_to home_path
+  end
+
+  test "new redirects if registration is closed" do
+    with_config(allow_registration: false) do
+      get :new
+      assert_response :redirect
+      assert_redirected_to home_path
+    end
+  end
+end
+
diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb
index 6029c83..2794422 100644
--- a/test/functional/users_controller_test.rb
+++ b/test/functional/users_controller_test.rb
@@ -1,20 +1,7 @@
-require_relative '../test_helper'
+require 'test_helper'
 
 class UsersControllerTest < ActionController::TestCase
 
-  test "should get new" do
-    get :new
-    assert_equal User, assigns(:user).class
-    assert_response :success
-  end
-
-  test "new should redirect logged in users" do
-    login
-    get :new
-    assert_response :redirect
-    assert_redirected_to home_path
-  end
-
   test "failed show without login" do
     user = find_record :user
     get :show, :id => user.id
@@ -163,11 +150,4 @@ class UsersControllerTest < ActionController::TestCase
     assert !assigns(:user).enabled?
   end
 
-  test "new redirects if registration is closed" do
-    with_config(allow_registration: false) do
-      get :new
-      assert_response :redirect
-      assert_redirected_to home_path
-    end
-  end
 end
diff --git a/test/integration/browser/account_livecycle_test.rb b/test/integration/browser/account_livecycle_test.rb
index 604f456..85dbf13 100644
--- a/test/integration/browser/account_livecycle_test.rb
+++ b/test/integration/browser/account_livecycle_test.rb
@@ -22,7 +22,7 @@ class AccountLivecycleTest < BrowserIntegrationTest
       username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
       password ||= SecureRandom.base64
 
-      visit '/users/new'
+      visit '/signup'
       fill_in 'Username', with: username
       fill_in 'Password', with: password
       fill_in 'Password confirmation', with: password
diff --git a/test/integration/browser/password_validation_test.rb b/test/integration/browser/password_validation_test.rb
index 45eb0bf..51fcc5d 100644
--- a/test/integration/browser/password_validation_test.rb
+++ b/test/integration/browser/password_validation_test.rb
@@ -5,26 +5,26 @@ class PasswordValidationTest < BrowserIntegrationTest
   test "password confirmation is validated" do
     username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
     password ||= SecureRandom.base64
-    visit '/users/new'
+    visit '/signup'
     fill_in 'Username', with: username
     fill_in 'Password', with: password
     fill_in 'Password confirmation', with: password + "-typo"
     click_on 'Sign Up'
     assert page.has_content? "does not match."
-    assert_equal '/users/new', current_path
+    assert_equal '/signup', current_path
     assert page.has_selector? ".error #srp_password_confirmation"
   end
 
   test "password needs to be at least 8 chars long" do
     username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
     password ||= SecureRandom.base64[0,7]
-    visit '/users/new'
+    visit '/signup'
     fill_in 'Username', with: username
     fill_in 'Password', with: password
     fill_in 'Password confirmation', with: password
     click_on 'Sign Up'
     assert page.has_content? "needs to be at least 8 characters long"
-    assert_equal '/users/new', current_path
+    assert_equal '/signup', current_path
     assert page.has_selector? ".error #srp_password"
   end
 end
diff --git a/test/support/browser_integration_test.rb b/test/support/browser_integration_test.rb
index 84440a1..70161f9 100644
--- a/test/support/browser_integration_test.rb
+++ b/test/support/browser_integration_test.rb
@@ -52,7 +52,7 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest
 
       username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
       password ||= SecureRandom.base64
-      visit '/users/new'
+      visit '/signup'
       fill_in 'Username', with: username
       fill_in 'Password', with: password
       fill_in 'Invite code', with: @testcode.invite_code
@@ -65,7 +65,7 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest
 
       username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
       password ||= SecureRandom.base64
-      visit '/users/new'
+      visit '/signup'
       fill_in 'Username', with: username
       fill_in 'Password', with: password
       fill_in 'Password confirmation', with: password
-- 
cgit v1.2.3


From ad208ae3625e67c2551744df7906ebdda94d215e Mon Sep 17 00:00:00 2001
From: Azul <azul@riseup.net>
Date: Mon, 23 May 2016 12:27:14 +0200
Subject: rename destroy_identity to release_handles

This expresses the intent rather than the implementation.
Also replace temp with query refactoring.
---
 app/controllers/api/users_controller.rb |  7 +++++--
 app/models/account.rb                   | 10 ++++------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb
index e64d21f..c79a729 100644
--- a/app/controllers/api/users_controller.rb
+++ b/app/controllers/api/users_controller.rb
@@ -50,8 +50,7 @@ module Api
     end
 
     def destroy
-      destroy_identity = current_user.is_monitor? || params[:identities] == "destroy"
-      @user.account.destroy(destroy_identity)
+      @user.account.destroy(release_handles)
       if @user == current_user
         logout
       end
@@ -60,6 +59,10 @@ module Api
 
     private
 
+    def release_handles
+      current_user.is_monitor? || params[:identities] == "destroy"
+    end
+
     # tester auth can only create test users.
     def create_test_account
       if User::is_test?(params[:user][:login])
diff --git a/app/models/account.rb b/app/models/account.rb
index 7310250..d722caa 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -69,15 +69,13 @@ class Account
     @user.refresh_identity
   end
 
-  def destroy(destroy_identity=false)
+  def destroy(release_handles=false)
     return unless @user
     if !@user.is_tmp?
-      if destroy_identity == false
-        @user.identities.each do |id|
+      @user.identities.each do |id|
+        if release_handles == false
           id.orphan!
-        end
-      else
-        @user.identities.each do |id|
+        else
           id.destroy
         end
       end
-- 
cgit v1.2.3


From 30da8e6ffa1eefafb9762645efb85e0beed236c6 Mon Sep 17 00:00:00 2001
From: Azul <azul@riseup.net>
Date: Mon, 23 May 2016 12:53:23 +0200
Subject: fix config check in submit_signup

with_config is not mean to test the current config. It will
set the config. So instead we need to look into APP_CONFIG.
---
 test/support/browser_integration_test.rb | 33 +++++++++-----------------------
 1 file changed, 9 insertions(+), 24 deletions(-)

diff --git a/test/support/browser_integration_test.rb b/test/support/browser_integration_test.rb
index 70161f9..1f5e3d2 100644
--- a/test/support/browser_integration_test.rb
+++ b/test/support/browser_integration_test.rb
@@ -47,32 +47,17 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest
   end
 
   def submit_signup(username = nil, password = nil)
-
-    with_config invite_required: true do
-
-      username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
-      password ||= SecureRandom.base64
-      visit '/signup'
-      fill_in 'Username', with: username
-      fill_in 'Password', with: password
+    username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
+    password ||= SecureRandom.base64
+    visit '/signup'
+    fill_in 'Username', with: username
+    fill_in 'Password', with: password
+    if APP_CONFIG[:invite_required]
       fill_in 'Invite code', with: @testcode.invite_code
-      fill_in 'Password confirmation', with: password
-      click_on 'Sign Up'
-      return username, password
-    end
-
-    with_config invite_required: false do
-
-      username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
-      password ||= SecureRandom.base64
-      visit '/signup'
-      fill_in 'Username', with: username
-      fill_in 'Password', with: password
-      fill_in 'Password confirmation', with: password
-      click_on 'Sign Up'
-      return username, password
     end
-
+    fill_in 'Password confirmation', with: password
+    click_on 'Sign Up'
+    return username, password
   end
 
   # currently this only works for tests with poltergeist.
-- 
cgit v1.2.3