From 5afa0ac793928f87631034fa0bec7018557f04df Mon Sep 17 00:00:00 2001 From: Priit Tark Date: Wed, 20 May 2015 19:58:00 +0300 Subject: [PATCH 1/2] Added Registrar roles with super, epp, billing --- app/controllers/admin/api_users_controller.rb | 4 +- .../registrar/sessions_controller.rb | 1 + app/controllers/registrar_controller.rb | 5 ++ app/models/ability.rb | 52 +++++++++++++------ app/models/admin_user.rb | 2 +- app/models/api_user.rb | 4 +- app/views/admin/api_users/_form.haml | 17 ++++-- app/views/admin/api_users/show.haml | 3 ++ db/migrate/20150520163237_add_defalut_role.rb | 7 +++ db/schema.rb | 2 +- spec/fabricators/api_user_fabricator.rb | 1 + spec/models/api_user_spec.rb | 3 +- 12 files changed, 77 insertions(+), 24 deletions(-) create mode 100644 db/migrate/20150520163237_add_defalut_role.rb diff --git a/app/controllers/admin/api_users_controller.rb b/app/controllers/admin/api_users_controller.rb index 96ca2bc98..be2170016 100644 --- a/app/controllers/admin/api_users_controller.rb +++ b/app/controllers/admin/api_users_controller.rb @@ -55,6 +55,8 @@ class Admin::ApiUsersController < AdminController end def api_user_params - params.require(:api_user).permit(:username, :password, :active, :registrar_id, :registrar_typeahead, :identity_code) + params.require(:api_user).permit(:username, :password, :active, + :registrar_id, :registrar_typeahead, + :identity_code, { roles: [] }) end end diff --git a/app/controllers/registrar/sessions_controller.rb b/app/controllers/registrar/sessions_controller.rb index ce19773e0..57f92334a 100644 --- a/app/controllers/registrar/sessions_controller.rb +++ b/app/controllers/registrar/sessions_controller.rb @@ -145,6 +145,7 @@ class Registrar::SessionsController < Devise::SessionsController private def check_ip + return if Rails.env.development? return if WhiteIp.registrar_ip_white?(request.ip) render text: t('ip_is_not_whitelisted') and return end diff --git a/app/controllers/registrar_controller.rb b/app/controllers/registrar_controller.rb index 8da12c3c1..5ac47d06d 100644 --- a/app/controllers/registrar_controller.rb +++ b/app/controllers/registrar_controller.rb @@ -12,6 +12,11 @@ class RegistrarController < ApplicationController def check_ip return unless current_user + unless current_user.is_a? ApiUser + sign_out(current_user) + return + end + return if current_user.registrar.registrar_ip_white?(request.ip) flash[:alert] = t('ip_is_not_whitelisted') sign_out(current_user) diff --git a/app/models/ability.rb b/app/models/ability.rb index 742463c37..04598ee1a 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -1,6 +1,8 @@ class Ability include CanCan::Ability - + # rubocop: disable Metrics/CyclomaticComplexity + # rubocop: disable Metrics/PerceivedComplexity + # rubocop: disable Metrics/LineLength def initialize(user) alias_action :show, to: :view alias_action :show, :create, :update, :destroy, to: :crud @@ -11,11 +13,11 @@ class Ability when 'AdminUser' @user.roles.each { |role| send(role) } if @user.roles when 'ApiUser' - epp - registrar - registrant # refactor + @user.roles.each { |role| send(role) } if @user.roles + static_epp + static_registrar when 'RegistrantUser' - registrant + static_registrant end # Public user @@ -23,10 +25,7 @@ class Ability can :create, :registrant_domain_update_confirm end - # rubocop: disable Metrics/CyclomaticComplexity - # rubocop: disable Metrics/PerceivedComplexity - # rubocop: disable Metrics/LineLength - def epp + def static_epp # Epp::Domain can(:info, Epp::Domain) { |d, pw| d.registrar_id == @user.registrar_id || pw.blank? ? true : d.auth_info == pw } can(:check, Epp::Domain) @@ -47,12 +46,8 @@ class Ability can(:renew, Epp::Contact) can(:view_password, Epp::Contact) { |c, pw| c.registrar_id == @user.registrar_id || c.auth_info == pw } end - # rubocop: enable Metrics/LineLength - # rubocop: enable Metrics/CyclomaticComplexity - # rubocop: enable Metrics/PerceivedComplexity - def registrar - can :manage, Invoice + def static_registrar can :read, AccountActivity can :manage, Nameserver can :view, :registrar_dashboard @@ -68,15 +63,38 @@ class Ability can :manage, :deposit end - def registrant + def static_registrant can :manage, :registrant_whois can :manage, Depp::Domain end + def static_billing + can :manage, Invoice + end + def user can :show, :dashboard end + # api_user dynamic role + def super + static_epp + static_registrar + end + + # api_user dynamic role + def epp + static_epp + static_registrar + end + + # api_user dynamic role + def billing + static_registrar + static_billing + end + + # admin dynamic role def customer_service user can :manage, Domain @@ -84,6 +102,7 @@ class Ability can :manage, Registrar end + # admin dynamic role def admin customer_service can :manage, Setting @@ -105,4 +124,7 @@ class Ability can :create, :zonefile can :access, :settings_menu end + # rubocop: enable Metrics/LineLength + # rubocop: enable Metrics/CyclomaticComplexity + # rubocop: enable Metrics/PerceivedComplexity end diff --git a/app/models/admin_user.rb b/app/models/admin_user.rb index 26c969f70..79e8f5649 100644 --- a/app/models/admin_user.rb +++ b/app/models/admin_user.rb @@ -6,7 +6,7 @@ class AdminUser < User validate :validate_identity_code, if: -> { country_code == 'EE' } - ROLES = %w(user customer_service admin) + ROLES = %w(user customer_service admin) # should not match to api_users roles devise :database_authenticatable, :rememberable, :trackable, :validatable, :lockable diff --git a/app/models/api_user.rb b/app/models/api_user.rb index 4dca33a18..79810eeab 100644 --- a/app/models/api_user.rb +++ b/app/models/api_user.rb @@ -15,11 +15,13 @@ class ApiUser < User belongs_to :registrar has_many :certificates - validates :username, :password, :registrar, presence: true + validates :username, :password, :registrar, :roles, presence: true validates :username, uniqueness: true attr_accessor :registrar_typeahead + ROLES = %w(super epp billing) # should not match to admin roles + def ability @ability ||= Ability.new(self) end diff --git a/app/views/admin/api_users/_form.haml b/app/views/admin/api_users/_form.haml index dd6fffe8c..24ca8465b 100644 --- a/app/views/admin/api_users/_form.haml +++ b/app/views/admin/api_users/_form.haml @@ -29,11 +29,20 @@ %span.glyphicon.glyphicon-ok.form-control-feedback.js-typeahead-ok.hidden %span.glyphicon.glyphicon-remove.form-control-feedback.js-typeahead-remove = f.hidden_field(:registrar_id, class: 'js-registrar-id') - .checkbox - %label{for: 'api_user_active'} - = f.check_box(:active) - = t(:active) + .form-group + .col-md-4.control-label + = f.label :role + .col-md-7 + = select_tag 'api_user[roles][]', + options_for_select(ApiUser::ROLES.map {|x| [t(x), x] }, @api_user.roles.try(:first)), + class: 'form-control selectize' + .checkbox + %label{for: 'api_user_active'} + = f.check_box(:active) + = t(:active) + %hr + .row .col-md-8.text-right = button_tag(t(:save), class: 'btn btn-primary') diff --git a/app/views/admin/api_users/show.haml b/app/views/admin/api_users/show.haml index 1cb4423c3..87e6bce5c 100644 --- a/app/views/admin/api_users/show.haml +++ b/app/views/admin/api_users/show.haml @@ -26,6 +26,9 @@ %dt= t(:registrar) %dd= link_to(@api_user.registrar, admin_registrar_path(@api_user.registrar)) + %dt= t(:role) + %dd= @api_user.roles.join(', ') + %dt= t(:active) %dd= @api_user.active .row diff --git a/db/migrate/20150520163237_add_defalut_role.rb b/db/migrate/20150520163237_add_defalut_role.rb new file mode 100644 index 000000000..35b675541 --- /dev/null +++ b/db/migrate/20150520163237_add_defalut_role.rb @@ -0,0 +1,7 @@ +class AddDefalutRole < ActiveRecord::Migration + def change + ApiUser.all.each do |u| + u.update_column :roles, ['super'] if u.roles.blank? + end + end +end diff --git a/db/schema.rb b/db/schema.rb index f35a9555d..b0b227c79 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150519144118) do +ActiveRecord::Schema.define(version: 20150520163237) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/spec/fabricators/api_user_fabricator.rb b/spec/fabricators/api_user_fabricator.rb index 65da9fbab..43fc9b553 100644 --- a/spec/fabricators/api_user_fabricator.rb +++ b/spec/fabricators/api_user_fabricator.rb @@ -5,6 +5,7 @@ Fabricator(:api_user) do identity_code '14212128025' registrar active true + roles ['super'] end # use dedicated fabricator for fixed one diff --git a/spec/models/api_user_spec.rb b/spec/models/api_user_spec.rb index 30c800cb6..62dd07364 100644 --- a/spec/models/api_user_spec.rb +++ b/spec/models/api_user_spec.rb @@ -13,7 +13,8 @@ describe ApiUser do @api_user.errors.full_messages.should match_array([ "Password Password is missing", "Registrar Registrar is missing", - "Username Username is missing" + "Username Username is missing", + "Roles is missing" ]) end From 84cd0b8378c67398cee7f0fc47c48076f72c7424 Mon Sep 17 00:00:00 2001 From: Priit Tark Date: Wed, 20 May 2015 20:58:57 +0300 Subject: [PATCH 2/2] Added role base redirect and updated abilities --- app/api/repp/api.rb | 5 ++- .../registrar/sessions_controller.rb | 16 ++++++--- app/controllers/registrar_controller.rb | 2 +- app/models/ability.rb | 34 +++++++++---------- config/locales/en.yml | 1 + 5 files changed, 34 insertions(+), 24 deletions(-) diff --git a/app/api/repp/api.rb b/app/api/repp/api.rb index 40472e781..13712f997 100644 --- a/app/api/repp/api.rb +++ b/app/api/repp/api.rb @@ -12,6 +12,10 @@ module Repp error! I18n.t('ip_is_not_whitelisted'), 401 unless @current_user.registrar.api_ip_white?(request.ip) end + if @current_user.cannot?(:view, :repp) + error! I18n.t('no_permission'), 401 unless @current_user.registrar.api_ip_white?(request.ip) + end + next if Rails.env.test? || Rails.env.development? message = 'Certificate mismatch! Cert common name should be:' request_name = env['HTTP_SSL_CLIENT_S_DN_CN'] @@ -22,7 +26,6 @@ module Repp else error! "#{message} #{@current_user.username}", 401 if @current_user.username != request_name end - end helpers do diff --git a/app/controllers/registrar/sessions_controller.rb b/app/controllers/registrar/sessions_controller.rb index 57f92334a..0fa45ac6e 100644 --- a/app/controllers/registrar/sessions_controller.rb +++ b/app/controllers/registrar/sessions_controller.rb @@ -35,7 +35,7 @@ class Registrar::SessionsController < Devise::SessionsController @api_user = ApiUser.find_by(username: params[:depp_user][:tag]) if @api_user.active? sign_in @api_user - redirect_to registrar_root_url + redirect_to role_base_root_url(@api_user) else @depp_user.errors.add(:base, :not_active) render 'login' @@ -52,7 +52,7 @@ class Registrar::SessionsController < Devise::SessionsController if @user sign_in(@user, event: :authentication) - redirect_to registrar_root_url + redirect_to role_base_root_url(@user) else flash[:alert] = t('no_such_user') redirect_to registrar_login_url @@ -70,7 +70,7 @@ class Registrar::SessionsController < Devise::SessionsController if Rails.env.test? && phone == "123" @user = ApiUser.find_by(identity_code: "14212128025") sign_in(@user, event: :authentication) - return redirect_to registrar_root_url + return redirect_to role_base_root_url(@user) end # country_codes = {'+372' => 'EST'} @@ -112,7 +112,7 @@ class Registrar::SessionsController < Devise::SessionsController sign_in @user flash[:notice] = t(:welcome) flash.keep(:notice) - render js: "window.location = '#{registrar_root_path}'" + render js: "window.location = '#{role_base_root_url(@user)}'" when 'NOT_VALID' render json: { message: t(:user_signature_is_invalid) }, status: :bad_request when 'EXPIRED_TRANSACTION' @@ -149,4 +149,12 @@ class Registrar::SessionsController < Devise::SessionsController return if WhiteIp.registrar_ip_white?(request.ip) render text: t('ip_is_not_whitelisted') and return end + + def role_base_root_url(user) + if user.try(:roles) == ['billing'] + registrar_invoices_url + else + registrar_root_url + end + end end diff --git a/app/controllers/registrar_controller.rb b/app/controllers/registrar_controller.rb index 5ac47d06d..0bc56c356 100644 --- a/app/controllers/registrar_controller.rb +++ b/app/controllers/registrar_controller.rb @@ -16,7 +16,7 @@ class RegistrarController < ApplicationController sign_out(current_user) return end - + return if Rails.env.development? return if current_user.registrar.registrar_ip_white?(request.ip) flash[:alert] = t('ip_is_not_whitelisted') sign_out(current_user) diff --git a/app/models/ability.rb b/app/models/ability.rb index 04598ee1a..3d8a93eba 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -7,15 +7,13 @@ class Ability alias_action :show, to: :view alias_action :show, :create, :update, :destroy, to: :crud - @user = user || AdminUser.new + @user = user || User.new case @user.class.to_s when 'AdminUser' @user.roles.each { |role| send(role) } if @user.roles when 'ApiUser' @user.roles.each { |role| send(role) } if @user.roles - static_epp - static_registrar when 'RegistrantUser' static_registrant end @@ -45,10 +43,12 @@ class Ability can(:delete, Epp::Contact) { |c, pw| c.registrar_id == @user.registrar_id || c.auth_info == pw } can(:renew, Epp::Contact) can(:view_password, Epp::Contact) { |c, pw| c.registrar_id == @user.registrar_id || c.auth_info == pw } + + # REPP + can(:manage, :repp) end def static_registrar - can :read, AccountActivity can :manage, Nameserver can :view, :registrar_dashboard can :delete, :registrar_poll @@ -60,7 +60,6 @@ class Ability can :manage, Depp::Keyrelay can :confirm, :keyrelay can :confirm, :transfer - can :manage, :deposit end def static_registrant @@ -68,33 +67,32 @@ class Ability can :manage, Depp::Domain end - def static_billing - can :manage, Invoice - end - def user can :show, :dashboard end - # api_user dynamic role + # Registrar/api_user dynamic role def super - static_epp static_registrar + billing + epp end - # api_user dynamic role + # Registrar/api_user dynamic role def epp + static_registrar static_epp - static_registrar end - # api_user dynamic role + # Registrar/api_user dynamic role def billing - static_registrar - static_billing + can :view, :registrar_dashboard + can :manage, Invoice + can :manage, :deposit + can :read, AccountActivity end - # admin dynamic role + # Admin/admin_user dynamic role def customer_service user can :manage, Domain @@ -102,7 +100,7 @@ class Ability can :manage, Registrar end - # admin dynamic role + # Admin/admin_user dynamic role def admin customer_service can :manage, Setting diff --git a/config/locales/en.yml b/config/locales/en.yml index c3bd4256e..0e5bdfc56 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -796,3 +796,4 @@ en: registrant_domain_verification_rejected: 'Domain owner change has been rejected successfully.' registrant_domain_verification_rejected_failed: 'Something went wrong' ip_is_not_whitelisted: 'IP is not whitelisted' + no_permission: 'No permission'