From 688cd80b44afe441cd43dd3a27231f1c9e1c6107 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Wed, 16 Sep 2020 10:09:49 +0200 Subject: [PATCH] Re-enable rubocop and put it in travis Closes #92 --- .rubocop | 1 - .rubocop.yml | 20 ++++++--- .travis.yml | 1 + Capfile | 2 +- Gemfile | 13 +++--- Gemfile.lock | 5 +++ app/controllers/admin/groups_controller.rb | 10 ++--- app/controllers/admin/settings_controller.rb | 22 +++------- app/controllers/admin/users_controller.rb | 4 +- app/controllers/admin_controller.rb | 26 +++++------ app/controllers/application_controller.rb | 13 +++--- .../concerns/authenticates_with_two_factor.rb | 27 ++++++------ .../oauth_applications_controller.rb | 8 ++-- app/controllers/pages_controller.rb | 5 +-- app/controllers/registrations_controller.rb | 14 +++--- app/controllers/saml_idp_controller.rb | 11 ++--- app/controllers/sessions_controller.rb | 15 +++---- app/controllers/users_controller.rb | 10 +++-- app/helpers/application_helper.rb | 23 +++++----- app/models/application.rb | 2 +- app/models/group.rb | 1 - app/models/saml_service_provider.rb | 2 +- app/models/user.rb | 43 +++++++++---------- config/environments/development.rb | 2 +- config/environments/production.rb | 2 +- config/initializers/assets.rb | 2 +- config/initializers/cookies.rb | 9 ++-- config/initializers/doorkeeper.rb | 4 +- .../initializers/doorkeeper_openid_connect.rb | 23 +++------- config/initializers/email.rb | 4 +- .../initializers/filter_parameter_logging.rb | 2 +- config/initializers/i18n.rb | 2 +- config/initializers/peek.rb | 2 + config/initializers/translation.rb | 2 + config/puma.rb | 8 ++-- config/routes.rb | 6 +-- lib/tasks/auto_annotate_models.rake | 2 +- lib/tasks/eyedp.rake | 17 ++++---- .../admin/dashboard_controller_spec.rb | 12 +++--- .../oauth_applications_controller_spec.rb | 12 +++--- spec/controllers/pages_controller_spec.rb | 11 ++--- spec/controllers/sessions_controller_spec.rb | 11 +++-- spec/models/permission_spec.rb | 2 + spec/models/user_spec.rb | 4 +- spec/spec_helper.rb | 12 +++--- 45 files changed, 211 insertions(+), 218 deletions(-) diff --git a/.rubocop b/.rubocop index 8738cb9..71b4480 100644 --- a/.rubocop +++ b/.rubocop @@ -1,2 +1 @@ ---rails --extra-details diff --git a/.rubocop.yml b/.rubocop.yml index bebcfce..493a711 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,10 +1,11 @@ -Rails: - Enabled: true +require: + - rubocop-rails -Metrics/LineLength: +Layout/LineLength: Max: 120 AllCops: + NewCops: enable Exclude: - db/migrate/* - db/schema.rb @@ -14,6 +15,7 @@ AllCops: - config/initializers/devise.rb - app/models/setting.rb - config/deploy.rb + Style/Documentation: Enabled: false @@ -33,16 +35,24 @@ Rails/InverseOf: Metrics/BlockLength: Exclude: - config/environments/development.rb + - config/routes.rb - Guardfile - lib/tasks/auto_annotate_models.rake - config/initializers/simple_form_bootstrap.rb + - config/initializers/doorkeeper_openid_connect.rb - lib/tasks/* - app/admin/* + - spec/models/*.rb + - spec/controllers/*.rb + - spec/controllers/admin/*.rb + +Naming/AccessorMethodName: + Enabled: false -Layout/AlignHash: +Layout/HashAlignment: Enabled: False -GlobalVars: +Style/GlobalVars: AllowedVariables: - $redis diff --git a/.travis.yml b/.travis.yml index 89953c3..ff786cd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,3 +27,4 @@ script: - bundle exec rake db:create - RAILS_ENV=test bundle exec rake db:migrate --trace - bundle exec rspec + - bundle exec rubocop diff --git a/Capfile b/Capfile index 849c3ae..7319ec2 100644 --- a/Capfile +++ b/Capfile @@ -11,7 +11,7 @@ require 'capistrano/rvm' require 'capistrano/puma' install_plugin Capistrano::Puma -require "capistrano/scm/git" +require 'capistrano/scm/git' install_plugin Capistrano::SCM::Git # Loads custom tasks from `lib/capistrano/tasks' if you have any defined. diff --git a/Gemfile b/Gemfile index 27aab03..7a50fcb 100644 --- a/Gemfile +++ b/Gemfile @@ -25,12 +25,12 @@ gem 'redis', '~> 4.0' gem 'bootsnap', '>= 1.3.2', require: false -gem 'rails-i18n' gem 'dotenv', '~> 2.5.0' +gem 'rails-i18n' gem 'devise' -gem 'devise-i18n' gem 'devise_fido_usf' +gem 'devise-i18n' gem 'friendly_id' # Do TOTP 2FA gem 'devise-two-factor' @@ -55,10 +55,10 @@ gem 'scout_apm' # gem 'rails-settings-cached' gem 'peek' -gem 'peek-pg' +gem 'peek-gc' gem 'peek-performance_bar' +gem 'peek-pg' gem 'tipsy-rails' -gem 'peek-gc' # SAML the things! gem 'saml_idp' @@ -88,6 +88,7 @@ group :development do gem 'brakeman', require: false gem 'overcommit' gem 'rubocop', require: false + gem 'rubocop-rails', require: false gem 'guard' gem 'guard-bundler', require: false @@ -100,15 +101,15 @@ group :development do gem 'bcrypt_pbkdf', require: false gem 'capistrano', require: false + gem 'capistrano3-puma', require: false gem 'capistrano-bundler', require: false gem 'capistrano-rails', require: false gem 'capistrano-rvm', require: false - gem 'capistrano3-puma', require: false gem 'ed25519', require: false end group :test do gem 'faker' - gem 'timecop' gem 'shoulda' + gem 'timecop' end diff --git a/Gemfile.lock b/Gemfile.lock index f91565c..42882cb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -344,6 +344,10 @@ GEM unicode-display_width (>= 1.4.0, < 2.0) rubocop-ast (0.4.0) parser (>= 2.7.1.4) + rubocop-rails (2.7.1) + activesupport (>= 4.2.0) + rack (>= 1.1) + rubocop (>= 0.87.0) ruby-graphviz (1.2.5) rexml ruby-progressbar (1.10.1) @@ -487,6 +491,7 @@ DEPENDENCIES rqrcode rspec-rails (~> 4) rubocop + rubocop-rails saml_idp sass-rails (~> 6.0) scout_apm diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 7d67756..5022c89 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -17,9 +17,9 @@ def form_relations parent: { type: :select, options: { prompt: 'No Parent' }, - finder: -> { - helpers.options_from_collection_for_select(Group.all, :id, :name, @model.parent.try(:id)) - } + finder: lambda { + helpers.options_from_collection_for_select(Group.all, :id, :name, @model.parent.try(:id)) + } }, permissions: { type: :select, @@ -51,7 +51,7 @@ def model_params p end - def sort_whitelist - ['created_at', 'name'] + def sort_whitelist + %w[created_at name] end end diff --git a/app/controllers/admin/settings_controller.rb b/app/controllers/admin/settings_controller.rb index 9b7bda4..fc9a711 100644 --- a/app/controllers/admin/settings_controller.rb +++ b/app/controllers/admin/settings_controller.rb @@ -10,25 +10,15 @@ def index; end # PATCH/PUT /admin/settings/1.json def update opts = setting_params - if opts[:registration_enabled].nil? - opts[:registration_enabled] = false - else - opts[:registration_enabled] = true - end + opts[:registration_enabled] = if opts[:registration_enabled].nil? + false + else + true + end opts.each do |setting, value| Setting.send("#{setting}=", value) - puts "#{setting}: #{value}" end redirect_to admin_settings_url, notice: 'Settings were successfully updated.' - # respond_to do |format| - # if @setting.update(setting_params) - # format.html { redirect_to @setting, notice: 'Setting was successfully updated.' } - # format.json { render :show, status: :ok, location: @setting } - # else - # format.html { render :edit } - # format.json { render json: @setting.errors, status: :unprocessable_entity } - # end - # end end private @@ -46,7 +36,7 @@ def setting_params :oidc_signing_key, :registration_enabled, :logo, :logo_height, :logo_width, - :home_template, :registered_home_template, + :home_template, :registered_home_template ) end end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 5f9661e..5086d85 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -42,11 +42,11 @@ def model_params end def sort_whitelist - ['created_at', 'username', 'email'] + %w[created_at username email] end def filter_whitelist - ['username', 'email'] + %w[username email] end def filter diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index c7614df..571e9cb 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class AdminController < ApplicationController +class AdminController < ApplicationController # rubocop:disable Metrics/ClassLength # before_action :authenticate_user! before_action :ensure_user_is_admin! before_action :set_model, only: %i[show edit update destroy] @@ -8,12 +8,10 @@ class AdminController < ApplicationController # GET /admin/#{model}.json def index @models = model - .page(params[:page] || 1) - .includes(includes) - .order(order) - if filter - @models = @models.where(filter) - end + .page(params[:page] || 1) + .includes(includes) + .order(order) + @models = @models.where(filter) if filter end # GET /admin/#{model}/1 @@ -30,7 +28,6 @@ def edit; end # POST /admin/#{model} # POST /admin/#{model}.json - # rubocop:disable Metrics/AbcSize def create @model = model.new(model_params) respond_to do |format| @@ -43,7 +40,6 @@ def create end end end - # rubocop:enable Metrics/AbcSize # PATCH/PUT /admin/#{model}/1 # PATCH/PUT /admin/#{model}/1.json @@ -94,19 +90,19 @@ def includes def filter if filter_whitelist.include? params[:filter_by] - {params[:filter_by] => params[:filter]} + { params[:filter_by] => params[:filter] } else {} end end - def order + def order # rubocop:disable Metrics/AbcSize sort = { sort_by: :created_at, - sort_dir: :asc, + sort_dir: :asc } sort[:sort_by] = params[:sort_by] if params[:sort_by] && sort_whitelist.include?(params[:sort_by]) - sort[:sort_dir] = params[:sort_dir] if params[:sort_dir] && ['asc', 'desc'].include?(params[:sort_dir]) + sort[:sort_dir] = params[:sort_dir] if params[:sort_dir] && %w[asc desc].include?(params[:sort_dir]) { sort[:sort_by] => sort[:sort_dir] } end @@ -141,7 +137,7 @@ def set_model end def ensure_user_is_admin! - raise ActionController::RoutingError.new('Not Found') and return \ - unless current_user && current_user.admin? + raise(ActionController::RoutingError, 'Not Found') and return \ + unless current_user&.admin? end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4dc7446..b8e9ea7 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -22,11 +22,11 @@ def default_url_options end def after_sign_in_path_for(resource) - request.env['omniauth.origin'] || params[:redirect_to] || stored_location_for(resource) || root_url + request.env['omniauth.origin'] || params[:redirect_to] || stored_location_for(resource) || root_url end def peek_enabled? - super || ( current_user && current_user.admin? ) + super || current_user&.admin? end # U2F (universal 2nd factor) devices need a unique identifier for the application @@ -36,13 +36,12 @@ def u2f_app_id request.base_url end - - protected + protected def configure_permitted_parameters - devise_parameter_sanitizer.permit(:sign_in, keys: [:login, :otp_attempt]) - devise_parameter_sanitizer.permit(:sign_up, keys: [:username, :email]) - devise_parameter_sanitizer.permit(:account_update, keys: [:username, :email, :name]) + devise_parameter_sanitizer.permit(:sign_in, keys: %i[login otp_attempt]) + devise_parameter_sanitizer.permit(:sign_up, keys: %i[username email]) + devise_parameter_sanitizer.permit(:account_update, keys: %i[username email name]) end private diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index be31440..fcb02bf 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -17,7 +17,7 @@ module AuthenticatesWithTwoFactor # Returns nil def prompt_for_two_factor(user) # Set @user for Devise views - @user = user # rubocop:disable Gitlab/ModuleWithInstanceVariables + @user = user session[:otp_user_id] = user.id session[:user_password_hash] = Digest::SHA256.hexdigest(user.encrypted_password) @@ -27,8 +27,7 @@ def prompt_for_two_factor(user) render 'devise/sessions/two_factor' end - - def authenticate_with_two_factor + def authenticate_with_two_factor # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity user = self.resource = find_user return handle_changed_user(user) if user_password_changed?(user) @@ -36,12 +35,12 @@ def authenticate_with_two_factor authenticate_with_two_factor_via_otp(user) elsif user_params[:device_response].present? && session[:otp_user_id] authenticate_with_two_factor_via_u2f(user) - elsif user && user.valid_password?(user_params[:password]) + elsif user&.valid_password?(user_params[:password]) prompt_for_two_factor(user) end end - def authenticate_with_two_factor_via_otp(user) + def authenticate_with_two_factor_via_otp(user) # rubocop:disable Metrics/AbcSize if valid_otp_attempt?(user) # Remove any lingering user data from login clear_two_factor_attempt! @@ -59,18 +58,16 @@ def authenticate_with_two_factor_via_otp(user) # Setup in preparation of communication with a U2F (universal 2nd factor) device # Actual communication is performed using a Javascript API - # rubocop: disable CodeReuse/ActiveRecord def setup_u2f_authentication(user) key_handles = user.fido_usf_devices.pluck(:key_handle) u2f = U2F::U2F.new(u2f_app_id) + return if key_handles.blank? - if key_handles.present? - sign_requests = u2f.authentication_requests(key_handles) - session[:challenge] ||= u2f.challenge - @app_id = u2f_app_id - @sign_requests = sign_requests - @challenge = session[:challenge] - end + sign_requests = u2f.authentication_requests(key_handles) + session[:challenge] ||= u2f.challenge + @app_id = u2f_app_id + @sign_requests = sign_requests + @challenge = session[:challenge] end # Authenticate using the response from a U2F (universal 2nd factor) device @@ -85,7 +82,7 @@ def authenticate_with_two_factor_via_u2f(user) def handle_two_factor_failure(user, method) # user.increment_failed_attempts! Rails.logger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=#{method}") - flash.now[:alert] = _('Authentication via %{method} device failed.') % { method: method } + flash.now[:alert] = format(_('Authentication via %{method} device failed.'), method: method) # rubocop:disable Style/FormatStringToken prompt_for_two_factor(user) end @@ -103,7 +100,7 @@ def clear_two_factor_attempt! session.delete(:challenge) end - def handle_changed_user(user) + def handle_changed_user(_user) clear_two_factor_attempt! redirect_to new_user_session_path, alert: _('An error occurred. Please sign in again.') diff --git a/app/controllers/oauth_applications_controller.rb b/app/controllers/oauth_applications_controller.rb index e1520c3..1c9a3e6 100644 --- a/app/controllers/oauth_applications_controller.rb +++ b/app/controllers/oauth_applications_controller.rb @@ -5,14 +5,16 @@ def new super Login.create( user: current_user, - auth_type: "Existing Login", - service_provider: Application.find_by(uid: params[:client_id])) + auth_type: 'Existing Login', + service_provider: Application.find_by(uid: params[:client_id]) + ) end def create super Login.create( user: current_user, - service_provider: Application.find_by(uid: params[:client_id])) + service_provider: Application.find_by(uid: params[:client_id]) + ) end end diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 6389332..a06bf93 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -6,7 +6,6 @@ class PagesController < ApplicationController def home @template = Liquid::Template.parse(Setting.home_template) - end def user_dashboard @@ -14,12 +13,12 @@ def user_dashboard @logins = current_user.logins.page(params[:page] || 1).includes(:service_provider).order(created_at: :desc) end - def template_variables + def template_variables # rubocop:disable Metrics/MethodLength if current_user { 'user' => { 'username' => current_user.username, - 'email' => current_user.email, + 'email' => current_user.email }, 'groups' => current_user.groups.pluck(:name) } diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index eb9befe..7080fea 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -1,19 +1,21 @@ +# frozen_string_literal: true + class RegistrationsController < Devise::RegistrationsController def new if Setting.registration_enabled - super + super else - flash[:info] = 'Registrations are not open' - redirect_to root_path + flash[:info] = 'Registrations are not open' + redirect_to root_path end end def create if Setting.registration_enabled - super + super else - flash[:info] = 'Registrations are not open' - redirect_to root_path + flash[:info] = 'Registrations are not open' + redirect_to root_path end end end diff --git a/app/controllers/saml_idp_controller.rb b/app/controllers/saml_idp_controller.rb index a239d6a..83e183c 100644 --- a/app/controllers/saml_idp_controller.rb +++ b/app/controllers/saml_idp_controller.rb @@ -32,15 +32,12 @@ def idp_make_saml_response(found_user) encode_response found_user, {} end - - def saml_acs_url - super || "" - end + def saml_acs_url + super || '' + end def idp_logout - if user_signed_in? - sign_out current_user - end + sign_out current_user if user_signed_in? end private :idp_logout end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 6f67bf6..4973510 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -5,11 +5,11 @@ class SessionsController < Devise::SessionsController include AuthenticatesWithTwoFactor prepend_before_action :authenticate_with_two_factor, - if: -> { action_name == 'create' && two_factor_enabled? } + if: -> { action_name == 'create' && two_factor_enabled? } # replaced with :require_no_authentication_without_flash - skip_before_action :require_no_authentication, only: [:new, :create] - prepend_before_action :require_no_authentication_without_flash, only: [:new, :create] + skip_before_action :require_no_authentication, only: %i[new create] # rubocop:disable Rails/LexicallyScopedActionFilter + prepend_before_action :require_no_authentication_without_flash, only: %i[new create] # rubocop:disable Rails/LexicallyScopedActionFilter # protect_from_forgery is already prepended in ApplicationController but # authenticate_with_two_factor which signs in the user is prepended before # that here. @@ -40,8 +40,8 @@ def user_params params.require(:user).permit(:login, :password, :remember_me, :otp_attempt, :device_response) end - def find_user - @user ||= begin + def find_user # rubocop:disable Metrics/AbcSize + @find_user ||= begin if session[:otp_user_id] && user_params[:login] User.by_id_and_login(session[:otp_user_id], user_params[:login]).first elsif session[:otp_user_id] @@ -64,9 +64,6 @@ def valid_otp_attempt?(user) def require_no_authentication_without_flash require_no_authentication - if flash[:alert] == I18n.t('devise.failure.already_authenticated') - flash[:alert] = nil - end + flash[:alert] = nil if flash[:alert] == I18n.t('devise.failure.already_authenticated') end - end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index a898924..3dce487 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,8 +1,8 @@ +# frozen_string_literal: true + class UsersController < ApplicationController def new_2fa - unless current_user.two_factor_otp_enabled? - current_user.otp_secret = User.generate_otp_secret(32) - end + current_user.otp_secret = User.generate_otp_secret(32) unless current_user.two_factor_otp_enabled? current_user.save! @qr_code = build_qr_code @@ -34,7 +34,9 @@ def codes def disable_2fa current_user.disable_two_factor! - redirect_to edit_user_registration_path, status: :found, notice: s_('Two-factor authentication has been disabled successfully!') + redirect_to edit_user_registration_path, + status: :found, + notice: s_('Two-factor authentication has been disabled successfully!') end def account_string diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index c9f512d..0513939 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true module ApplicationHelper - - def asset_available? logical_path + def asset_available?(logical_path) if Rails.configuration.assets.compile Rails.application.precompiled_assets.include? logical_path else @@ -22,34 +21,34 @@ def nav_link(link_text, link_path, base_class = '') current_page = current_page?(link_path) class_name = 'nav-item' class_name = [base_class, class_name].join(' ') - content_tag(:li, class: class_name) do + tag.li(class: class_name) do link_to link_text, link_path, class: current_page ? 'nav-link active' : 'nav-link' end end # rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/MethodLength - def settings_row(head, subline, options = {}) + def settings_row(head, subline, options = {}, &block) options.reverse_merge!(width: 9) heading = if options.key?(:decorate) - content_tag(:h3, head, class: (options[:decorate]).to_s) + tag.h3(head, class: (options[:decorate]).to_s) else - content_tag(:h3, head) + tag.h3(head) end heading_subline = if options.key?(:decorate) - content_tag(:p, subline, class: "text-muted #{options[:decorate]}") + tag.p(subline, class: "text-muted #{options[:decorate]}") else - content_tag(:p, subline, class: 'text-muted') + tag.p(subline, class: 'text-muted') end content = if options.key?(:id) - content_tag(:div, class: "col-lg-#{options[:width]}", id: options[:id]) { yield } + tag.div(class: "col-lg-#{options[:width]}", id: options[:id], &block) else - content_tag(:div, class: "col-lg-#{options[:width]}") { yield } + tag.div(class: "col-lg-#{options[:width]}", &block) end - content_tag(:div, class: 'row') do - content_tag(:div, class: 'col-lg-3') do + tag.div(class: 'row') do + tag.div(class: 'col-lg-3') do heading + heading_subline end + content diff --git a/app/models/application.rb b/app/models/application.rb index 0478996..906f2af 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -20,7 +20,7 @@ # class Application < Doorkeeper::Application - has_many :logins, :as => :service_provider, dependent: :destroy + has_many :logins, as: :service_provider, dependent: :destroy def to_s name diff --git a/app/models/group.rb b/app/models/group.rb index bac8492..347fcd5 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -18,7 +18,6 @@ class Group < ApplicationRecord acts_as_tree order: 'name' belongs_to :parent, - foreign_key: 'parent_id', class_name: 'Group', optional: true, foreign_type: :uuid diff --git a/app/models/saml_service_provider.rb b/app/models/saml_service_provider.rb index 329cf38..936e482 100644 --- a/app/models/saml_service_provider.rb +++ b/app/models/saml_service_provider.rb @@ -18,7 +18,7 @@ # class SamlServiceProvider < ApplicationRecord - has_many :logins, :as => :service_provider, dependent: :destroy + has_many :logins, as: :service_provider, dependent: :destroy def to_s issuer_or_entity_id diff --git a/app/models/user.rb b/app/models/user.rb index fe70066..0681161 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -24,16 +24,16 @@ # index_users_on_reset_password_token (reset_password_token) UNIQUE # -class User < ApplicationRecord +class User < ApplicationRecord # rubocop:disable Metrics/ClassLength devise :two_factor_authenticatable, - :otp_secret_encryption_key => ENV['TOTP_ENCRYPTION_KEY'] + otp_secret_encryption_key: ENV['TOTP_ENCRYPTION_KEY'] devise :two_factor_backupable, otp_number_of_backup_codes: 10 # Include default devise modules. Others available are: # :confirmable, :lockable, :timeoutable, :trackable and :omniauthable devise :registerable, :recoverable, :rememberable, :validatable, - :fido_usf_registerable + :fido_usf_registerable has_many :user_groups, dependent: :destroy has_many :groups, through: :user_groups @@ -52,22 +52,21 @@ class User < ApplicationRecord has_many :logins, dependent: :destroy - validates :username, presence: true, uniqueness: { case_sensitive: false } + validates :username, presence: true, uniqueness: { case_sensitive: false } # rubocop:disable Rails/UniqueValidationWithoutIndex validate :validate_username attr_writer :login + def validate_username - if User.where(email: username).exists? - errors.add(:username, :invalid) - end + errors.add(:username, :invalid) if User.exists?(email: username) end - def self.u2f_authenticate(user, app_id, json_response, challenges) + def self.u2f_authenticate(user, app_id, json_response, challenges) # rubocop:disable Metrics/MethodLength # binding.pry response = U2F::SignResponse.load_from_json(json_response) registration = user.fido_usf_devices - .find_by(key_handle: response.key_handle) + .find_by(key_handle: response.key_handle) # registration = user.fido_usf_devices.find_by_key_handle(response.key_handle) u2f = U2F::U2F.new(app_id) # binding.pry @@ -89,11 +88,11 @@ def disable_two_factor! encrypted_otp_secret_salt: nil, otp_backup_codes: nil ) - self.fido_usf_devices.destroy_all # rubocop: disable Cop/DestroyAll + fido_usf_devices.destroy_all end end - def asserted_attributes + def asserted_attributes # rubocop:disable Metrics/MethodLength { groups: { getter: :groups }, email: { @@ -102,14 +101,14 @@ def asserted_attributes name_id_format: Saml::XML::Namespaces::Formats::NameId::EMAIL_ADDRESS }, username: { - getter: :username, - # name_format: Saml::XML::Namespaces::Formats::NameId::USERNAME, - # name_id_format: Saml::XML::Namespaces::Formats::NameId::USERNAME + getter: :username + # name_format: Saml::XML::Namespaces::Formats::NameId::USERNAME, + # name_id_format: Saml::XML::Namespaces::Formats::NameId::USERNAME }, name: { - getter: :name, - # name_format: Saml::XML::Namespaces::Formats::NameId::NAME, - # name_id_format: Saml::XML::Namespaces::Formats::NameId::NAME + getter: :name + # name_format: Saml::XML::Namespaces::Formats::NameId::NAME, + # name_id_format: Saml::XML::Namespaces::Formats::NameId::NAME } } end @@ -130,17 +129,17 @@ def to_s # end def login - @login || self.username || self.email + @login || username || email end def self.by_id_and_login(id, login) - where(id: id).database_authentication_rel({login: login}) + where(id: id).database_authentication_rel({ login: login }) end def self.database_authentication_rel(conditions) - if login = conditions.delete(:login) - where(conditions.to_h).where(["lower(username) = :value OR lower(email) = :value", { :value => login.downcase }]) - elsif conditions.has_key?(:username) || conditions.has_key?(:email) + if (login = conditions.delete(:login)) + where(conditions.to_h).where(['lower(username) = :value OR lower(email) = :value', { value: login.downcase }]) + elsif conditions.key?(:username) || conditions.key?(:email) where(conditions.to_h) end end diff --git a/config/environments/development.rb b/config/environments/development.rb index dbf0af2..0183ab0 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -16,7 +16,7 @@ # Enable/disable caching. By default caching is disabled. # Run rails dev:cache to toggle caching. - if Rails.root.join('tmp', 'caching-dev.txt').exist? + if Rails.root.join('tmp/caching-dev.txt').exist? config.action_controller.perform_caching = true config.cache_store = :memory_store diff --git a/config/environments/production.rb b/config/environments/production.rb index 757d59b..2e4b0ab 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -85,7 +85,7 @@ # config.logger = ActiveSupport::TaggedLogging.new(Syslog::Logger.new 'app-name') if ENV['RAILS_LOG_TO_STDOUT'].present? - logger = ActiveSupport::Logger.new(STDOUT) + logger = ActiveSupport::Logger.new($stdout) logger.formatter = config.log_formatter config.logger = ActiveSupport::TaggedLogging.new(logger) end diff --git a/config/initializers/assets.rb b/config/initializers/assets.rb index bb1061f..fff82a8 100644 --- a/config/initializers/assets.rb +++ b/config/initializers/assets.rb @@ -12,7 +12,7 @@ Rails.application.config.assets.paths << Rails.root.join('public/images') -Rails.application.config.assets.precompile += %w(*.png *.jpg *.jpeg *.gif) +Rails.application.config.assets.precompile += %w[*.png *.jpg *.jpeg *.gif] # Precompile additional assets. # application.js, application.css, and all non-JS/CSS in the app/assets diff --git a/config/initializers/cookies.rb b/config/initializers/cookies.rb index 34f908a..d920580 100644 --- a/config/initializers/cookies.rb +++ b/config/initializers/cookies.rb @@ -1,6 +1,7 @@ +# frozen_string_literal: true EyedP::Application.config.session_store :cookie_store, - key: '_eyed_p_session', - domain: ENV['SSO_DOMAIN'] || :all, - tld_length: 2, - secure: Rails.env.production? + key: '_eyed_p_session', + domain: ENV['SSO_DOMAIN'] || :all, + tld_length: 2, + secure: Rails.env.production? diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 988902b..a998a86 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -17,7 +17,7 @@ end end - skip_authorization do |resource_owner, client| + skip_authorization do |_resource_owner, client| client.application.internal? end @@ -28,5 +28,5 @@ default_scopes :openid optional_scopes :profile, :email, :address, :phone - grant_flows %w(authorization_code implicit_oidc) + grant_flows %w[authorization_code implicit_oidc] end diff --git a/config/initializers/doorkeeper_openid_connect.rb b/config/initializers/doorkeeper_openid_connect.rb index 178a5e0..f5f7913 100644 --- a/config/initializers/doorkeeper_openid_connect.rb +++ b/config/initializers/doorkeeper_openid_connect.rb @@ -15,16 +15,13 @@ def @config.signing_key User.active.find_by(id: access_token.resource_owner_id) end - auth_time_from_resource_owner do |user| - user.current_sign_in_at - end + auth_time_from_resource_owner(&:current_sign_in_at) subject do |user| # hash the user's ID with the Rails secret_key_base to avoid revealing it Digest::SHA256.hexdigest "#{user.id}-#{Rails.application.secrets.secret_key_base}" end - resource_owner_from_access_token do |access_token| User.find_by(id: access_token.resource_owner_id) end @@ -52,9 +49,7 @@ def @config.signing_key # expiration 600 claims do - claim :email do |resource_owner| - resource_owner.email - end + claim :email, &:email claim :email_verified do |_resource_owner| true @@ -68,13 +63,9 @@ def @config.signing_key resource_owner.real_name or resource_owner.username end - claim :nickname do |resource_owner| - resource_owner.username - end + claim :nickname, &:username - claim :preferred_username do |resource_owner| - resource_owner.username - end + claim :preferred_username, &:username # claim :preferred_username, scope: :openid do |resource_owner, scopes, access_token| # # Pass the resource_owner's preferred_username if the application has @@ -82,15 +73,15 @@ def @config.signing_key # scopes.exists?(:profile) ? resource_owner.username : "summer-sun-9449" # end - claim :profile do |resource_owner| + claim :profile do |_resource_owner| nil end - claim :address do |resource_owner| + claim :address do |_resource_owner| nil end - claim :phone do |resource_owner| + claim :phone do |_resource_owner| nil end end diff --git a/config/initializers/email.rb b/config/initializers/email.rb index b89a764..a3a53be 100644 --- a/config/initializers/email.rb +++ b/config/initializers/email.rb @@ -1,4 +1,6 @@ -EyedP::Application.config.action_mailer.default_url_options = { :host => ENV['EMAIL_DOMAIN'] } +# frozen_string_literal: true + +EyedP::Application.config.action_mailer.default_url_options = { host: ENV['EMAIL_DOMAIN'] } ActionMailer::Base.delivery_method = :smtp diff --git a/config/initializers/filter_parameter_logging.rb b/config/initializers/filter_parameter_logging.rb index d71e04b..f348ba7 100644 --- a/config/initializers/filter_parameter_logging.rb +++ b/config/initializers/filter_parameter_logging.rb @@ -3,4 +3,4 @@ # Be sure to restart your server when you modify this file. # Configure sensitive parameters which will be filtered from the log file. -Rails.application.config.filter_parameters += [:password, :otp_attempt] +Rails.application.config.filter_parameters += %i[password otp_attempt] diff --git a/config/initializers/i18n.rb b/config/initializers/i18n.rb index 6a69b89..30e2f4a 100644 --- a/config/initializers/i18n.rb +++ b/config/initializers/i18n.rb @@ -2,4 +2,4 @@ I18n.default_locale = :en I18n.available_locales = %i[en fr] -I18n.load_path += Dir[Rails.root.join('config', 'locales', '**', '*.{rb,yml}')] +I18n.load_path += Dir[Rails.root.join('config/locales/**/*.{rb,yml}')] diff --git a/config/initializers/peek.rb b/config/initializers/peek.rb index bfb777b..c290d4c 100644 --- a/config/initializers/peek.rb +++ b/config/initializers/peek.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + Peek.into Peek::Views::PerformanceBar Peek.into Peek::Views::PG Peek.into Peek::Views::GC diff --git a/config/initializers/translation.rb b/config/initializers/translation.rb index 23b5e99..a9ffe73 100644 --- a/config/initializers/translation.rb +++ b/config/initializers/translation.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + TranslationIO.configure do |config| config.api_key = Rails.application.credentials.translate_io_key config.source_locale = 'en' diff --git a/config/puma.rb b/config/puma.rb index a21751e..bf98e98 100644 --- a/config/puma.rb +++ b/config/puma.rb @@ -6,15 +6,15 @@ # the maximum value specified for Puma. Default is set to 5 threads for minimum # and maximum; this matches the default thread size of Active Record. # -threads_count = ENV.fetch('RAILS_MAX_THREADS') { 5 } +threads_count = ENV.fetch('RAILS_MAX_THREADS', 5) threads threads_count, threads_count # Specifies the `port` that Puma will listen on to receive requests; default is 3000. # -port ENV.fetch('PORT') { 3000 } +port ENV.fetch('PORT', 3000) # Specifies the `environment` that Puma will run in. -env = ENV.fetch('RAILS_ENV') { 'development' } +env = ENV.fetch('RAILS_ENV', 'development') environment env # Specifies the number of `workers` to boot in clustered mode. @@ -23,7 +23,7 @@ # Workers do not work on JRuby or Windows (both of which do not support # processes). # -workers ENV.fetch("WEB_CONCURRENCY") { 2 } +workers ENV.fetch('WEB_CONCURRENCY', 2) # Use the `preload_app!` method when specifying a `workers` number. # This directive tells Puma to first boot the application and load code diff --git a/config/routes.rb b/config/routes.rb index b7c07ef..888e31b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -19,9 +19,9 @@ post :settings, to: 'settings#update' end - devise_for :users, :controllers => { + devise_for :users, controllers: { registrations: :registrations, - sessions: :sessions, + sessions: :sessions } authenticated do @@ -40,7 +40,7 @@ get '/saml/auth' => 'saml_idp#create' post '/saml/auth' => 'saml_idp#create' get '/saml/metadata' => 'saml_idp#show' - match '/saml/logout' => 'saml_idp#logout', via: [:get, :post, :delete] + match '/saml/logout' => 'saml_idp#logout', via: %i[get post delete] get 'auth/basic/:permission_name', to: 'basic_auth#create' end diff --git a/lib/tasks/auto_annotate_models.rake b/lib/tasks/auto_annotate_models.rake index fd2d626..4596a05 100644 --- a/lib/tasks/auto_annotate_models.rake +++ b/lib/tasks/auto_annotate_models.rake @@ -5,7 +5,7 @@ # NOTE: to have a dev-mode tool do its thing in production. if Rails.env.development? require 'annotate' - task :set_annotation_options do + task set_annotation_options: :environment do # You can override any of these by setting an environment variable of the # same name. Annotate.set_defaults( diff --git a/lib/tasks/eyedp.rake b/lib/tasks/eyedp.rake index 33be2c7..fac2290 100644 --- a/lib/tasks/eyedp.rake +++ b/lib/tasks/eyedp.rake @@ -1,11 +1,12 @@ +# frozen_string_literal: true + namespace :eyedp do - desc "TODO" - task :create_admin_user, [:username] => [:environment] do |task, args| - username = args[:username] || 'admin' - password = SecureRandom.hex - puts "Creating user '#{username}' with password: '#{password}'" - user = User.create(username: username, email: 'admin@localhost', password: password) - user.groups << Group.where(name: 'administrators').find + desc 'TODO' + task :create_admin_user, [:username] => [:environment] do |_task, args| + username = args[:username] || 'admin' + password = SecureRandom.hex + puts "Creating user '#{username}' with password: '#{password}'" + user = User.create(username: username, email: 'admin@localhost', password: password) + user.groups << Group.where(name: 'administrators').find end - end diff --git a/spec/controllers/admin/dashboard_controller_spec.rb b/spec/controllers/admin/dashboard_controller_spec.rb index 920cd91..1f41220 100644 --- a/spec/controllers/admin/dashboard_controller_spec.rb +++ b/spec/controllers/admin/dashboard_controller_spec.rb @@ -12,29 +12,29 @@ end describe 'Dashboard' do - context "signed in admin" do + context 'signed in admin' do before do sign_in(admin) end - it 'Shows the dashboard' do + it 'Shows the dashboard' do get :index expect(response.status).to eq(200) end end - context "signed in user" do + context 'signed in user' do before do sign_in(user) end it 'returns 404 code' do - expect{ get :index }.to raise_error(ActionController::RoutingError) + expect { get :index }.to raise_error(ActionController::RoutingError) end end - context "signed out user" do + context 'signed out user' do it 'returns 404 code' do - expect{ get :index }.to raise_error(ActionController::RoutingError) + expect { get :index }.to raise_error(ActionController::RoutingError) end end end diff --git a/spec/controllers/oauth_applications_controller_spec.rb b/spec/controllers/oauth_applications_controller_spec.rb index 0880f6a..85fe52a 100644 --- a/spec/controllers/oauth_applications_controller_spec.rb +++ b/spec/controllers/oauth_applications_controller_spec.rb @@ -4,22 +4,21 @@ RSpec.describe OauthApplicationsController, type: :controller do let(:user) { User.create!(username: 'example', email: 'test@localhost', password: 'test1234') } - let(:app) { Application.create!(uid: 'test', internal: true, redirect_uri: 'https://example.com', name: 'test' ) } + let(:app) { Application.create!(uid: 'test', internal: true, redirect_uri: 'https://example.com', name: 'test') } let(:params) do { - response_type: "code", + response_type: 'code', client_id: app.uid, redirect_uri: app.redirect_uri, state: 'state' } end - context "signed in user" do + context 'signed in user' do before do sign_in(user) end - describe 'Existing Login' do it 'records the login' do # @request.env['devise.mapping'] = Devise.mappings[:user] @@ -30,7 +29,7 @@ end describe 'New login' do - it 'records the login' do + it 'records the login' do # @request.env['devise.mapping'] = Devise.mappings[:user] get :create, params: params expect(response.status).to eq(302) @@ -39,12 +38,11 @@ end end - context "signed out user" do + context 'signed out user' do it 'returns 301 code and redirects to login' do get :new expect(response.status).to eq(302) expect(response.headers['Location']).to eq('http://test.host/users/sign_in') - end end end diff --git a/spec/controllers/pages_controller_spec.rb b/spec/controllers/pages_controller_spec.rb index ece6208..ed6cdc7 100644 --- a/spec/controllers/pages_controller_spec.rb +++ b/spec/controllers/pages_controller_spec.rb @@ -9,20 +9,21 @@ uid: 'test', internal: true, redirect_uri: 'https://example.com', - name: 'this is a fairly high entropy test string' ) + name: 'this is a fairly high entropy test string' + ) end - context "User dashboard" do + context 'User dashboard' do render_views before do sign_in(user) end - it "shows logins" do - login = Login.create!( + it 'shows logins' do + Login.create!( user: user, service_provider: app, - auth_type: "Existing Login", + auth_type: 'Existing Login' ) get :user_dashboard expect(response.body).to include('this is a fairly high entropy test string') diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 3388b9d..072f692 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -10,14 +10,13 @@ end describe '#create' do - context 'when using standard authentications' do context 'invalid password' do it 'does not authenticate user' do post(:create, params: { user: { login: 'invalid', password: 'invalid' } }) expect(controller) - .to set_flash.now[:alert].to /Invalid Login or password/ + .to set_flash.now[:alert].to(/Invalid Login or password/) end end @@ -77,7 +76,7 @@ def authenticate_2fa(user_params, otp_user_id: user.id) otp_user_id: user.id ) - expect(controller).to set_flash.now[:alert].to /Invalid Login or password/ + expect(controller).to set_flash.now[:alert].to(/Invalid Login or password/) end end @@ -130,7 +129,7 @@ def authenticate_2fa(user_params, otp_user_id: user.id) it 'warns about invalid OTP code' do expect(controller).to set_flash.now[:alert] - .to /Invalid two-factor code/ + .to(/Invalid two-factor code/) end end end @@ -159,7 +158,7 @@ def authenticate_2fa_u2f(user_params) expect(controller) .to receive(:remember_me).with(user).and_call_original - authenticate_2fa_u2f(remember_me: '1', login: user.username, device_response: "{}") + authenticate_2fa_u2f(remember_me: '1', login: user.username, device_response: '{}') expect(response.cookies['remember_user_token']).to be_present end @@ -169,7 +168,7 @@ def authenticate_2fa_u2f(user_params) allow(controller).to receive(:find_user).and_return(user) expect(controller).not_to receive(:remember_me) - authenticate_2fa_u2f(remember_me: '0', login: user.username, device_response: "{}") + authenticate_2fa_u2f(remember_me: '0', login: user.username, device_response: '{}') expect(response.cookies['remember_user_token']).to be_nil end diff --git a/spec/models/permission_spec.rb b/spec/models/permission_spec.rb index 1cfeb81..28fb0d7 100644 --- a/spec/models/permission_spec.rb +++ b/spec/models/permission_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rails_helper' RSpec.describe Permission, type: :model do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 66c3f80..0bdbb0f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -63,6 +63,6 @@ expect(user.login).to eq 'example2' end - it_behaves_like "two_factor_authenticatable" - it_behaves_like "two_factor_backupable" + it_behaves_like 'two_factor_authenticatable' + it_behaves_like 'two_factor_backupable' end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 92b956c..8dde9ff 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,19 +1,19 @@ # frozen_string_literal: true + require 'coveralls' Coveralls.wear! - -ENV["RAILS_ENV"] = 'test' -ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true' -ENV["RSPEC_ALLOW_INVALID_URLS"] = 'true' +ENV['RAILS_ENV'] = 'test' +ENV['IN_MEMORY_APPLICATION_SETTINGS'] = 'true' +ENV['RSPEC_ALLOW_INVALID_URLS'] = 'true' require 'dotenv' Dotenv.load('.env') require File.expand_path('../config/environment', __dir__) -Dir[Rails.root.join("spec/support/helpers/*.rb")].sort.each { |f| require f } -Dir[Rails.root.join("spec/support/**/*.rb")].sort.each { |f| require f } +Dir[Rails.root.join('spec/support/helpers/*.rb')].sort.each { |f| require f } +Dir[Rails.root.join('spec/support/**/*.rb')].sort.each { |f| require f } # This file was generated by the `rails generate rspec:install` command. Conventionally, all # specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`. # The generated `.rspec` file contains `--require spec_helper` which will cause