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(-)

(limited to 'app')

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(-)

(limited to 'app')

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 ---------------------------
 7 files changed, 58 insertions(+), 53 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

(limited to 'app')

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
--#
-- 
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(-)

(limited to 'app')

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