From a7171af0a34f612d05667f1a5c35a4ca834da082 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 21 Feb 2018 03:40:12 +0100 Subject: Fix avatar and header issues by using custom geometry detector (#6515) * Fix avatar and header issues by using custom geometry detector Revert a part of #6508. The file passed to dynamic styles method was not actually a file, but an instance of Paperclip::Attachment, which broke all styles by always returning {} from the method. One problem with GIF avatars was that Paperclip::GeometryDetector reported wrong dimensions for them, e.g. 120x120 GIF avatar would for some reason be detected as 120x53. By writing our own geometry parser, we can use FastImage, which also happens to be faster than ImageMagick, to detect image dimensions, which are also correct. Unfortunately, this PR does not implement skipping a `convert` entirely if the dimensions are already correct, as I found no easy way to write that behaviour into Paperclip without rewriting the Paperclip::Thumbnail class. * Only invoke convert if dimension or format needs to be changed --- app/models/concerns/account_avatar.rb | 12 +++--------- app/models/concerns/account_header.rb | 12 +++--------- 2 files changed, 6 insertions(+), 18 deletions(-) (limited to 'app/models/concerns') diff --git a/app/models/concerns/account_avatar.rb b/app/models/concerns/account_avatar.rb index 53d0d876f..619644c9a 100644 --- a/app/models/concerns/account_avatar.rb +++ b/app/models/concerns/account_avatar.rb @@ -7,15 +7,9 @@ module AccountAvatar class_methods do def avatar_styles(file) - styles = {} - geometry = Paperclip::Geometry.from_file(file) - - styles[:original] = '120x120#' if geometry.width != geometry.height || geometry.width > 120 || geometry.height > 120 - styles[:static] = { format: 'png', convert_options: '-coalesce' } if file.content_type == 'image/gif' - + styles = { original: { geometry: '120x120#', file_geometry_parser: FastGeometryParser } } + styles[:static] = { format: 'png', convert_options: '-coalesce', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif' styles - rescue Paperclip::Errors::NotIdentifiedByImageMagickError - {} end private :avatar_styles @@ -23,7 +17,7 @@ module AccountAvatar included do # Avatar upload - has_attached_file :avatar, styles: ->(f) { avatar_styles(f) }, convert_options: { all: '-strip' } + has_attached_file :avatar, styles: ->(f) { avatar_styles(f) }, convert_options: { all: '-strip' }, processors: [:lazy_thumbnail] validates_attachment_content_type :avatar, content_type: IMAGE_MIME_TYPES validates_attachment_size :avatar, less_than: 2.megabytes end diff --git a/app/models/concerns/account_header.rb b/app/models/concerns/account_header.rb index 991473d8c..5ed8a9c83 100644 --- a/app/models/concerns/account_header.rb +++ b/app/models/concerns/account_header.rb @@ -7,15 +7,9 @@ module AccountHeader class_methods do def header_styles(file) - styles = {} - geometry = Paperclip::Geometry.from_file(file) - - styles[:original] = '700x335#' unless geometry.width == 700 && geometry.height == 335 - styles[:static] = { format: 'png', convert_options: '-coalesce' } if file.content_type == 'image/gif' - + styles = { original: { geometry: '700x335#', file_geometry_parser: FastGeometryParser } } + styles[:static] = { format: 'png', convert_options: '-coalesce', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif' styles - rescue Paperclip::Errors::NotIdentifiedByImageMagickError - {} end private :header_styles @@ -23,7 +17,7 @@ module AccountHeader included do # Header upload - has_attached_file :header, styles: ->(f) { header_styles(f) }, convert_options: { all: '-strip' } + has_attached_file :header, styles: ->(f) { header_styles(f) }, convert_options: { all: '-strip' }, processors: [:lazy_thumbnail] validates_attachment_content_type :header, content_type: IMAGE_MIME_TYPES validates_attachment_size :header, less_than: 2.megabytes end -- cgit From d3a62d263703142250d7d59335394a8e2a599ed4 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 22 Feb 2018 00:28:19 +0100 Subject: Fix #6525: Make sure file is opened in LazyThumbnail processor (#6529) --- app/models/concerns/account_avatar.rb | 2 +- app/models/concerns/account_header.rb | 2 +- lib/paperclip/lazy_thumbnail.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'app/models/concerns') diff --git a/app/models/concerns/account_avatar.rb b/app/models/concerns/account_avatar.rb index 619644c9a..7712a29fd 100644 --- a/app/models/concerns/account_avatar.rb +++ b/app/models/concerns/account_avatar.rb @@ -8,7 +8,7 @@ module AccountAvatar class_methods do def avatar_styles(file) styles = { original: { geometry: '120x120#', file_geometry_parser: FastGeometryParser } } - styles[:static] = { format: 'png', convert_options: '-coalesce', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif' + styles[:static] = { geometry: '120x120#', format: 'png', convert_options: '-coalesce', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif' styles end diff --git a/app/models/concerns/account_header.rb b/app/models/concerns/account_header.rb index 5ed8a9c83..04c576b28 100644 --- a/app/models/concerns/account_header.rb +++ b/app/models/concerns/account_header.rb @@ -8,7 +8,7 @@ module AccountHeader class_methods do def header_styles(file) styles = { original: { geometry: '700x335#', file_geometry_parser: FastGeometryParser } } - styles[:static] = { format: 'png', convert_options: '-coalesce', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif' + styles[:static] = { geometry: '700x335#', format: 'png', convert_options: '-coalesce', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif' styles end diff --git a/lib/paperclip/lazy_thumbnail.rb b/lib/paperclip/lazy_thumbnail.rb index 594f0ce39..42f9a557a 100644 --- a/lib/paperclip/lazy_thumbnail.rb +++ b/lib/paperclip/lazy_thumbnail.rb @@ -3,7 +3,7 @@ module Paperclip class LazyThumbnail < Paperclip::Thumbnail def make - return @file unless needs_convert? + return File.open(@file.path) unless needs_convert? Paperclip::Thumbnail.make(file, options, attachment) end -- cgit From 3084fe49595f44152e9835bded7490bf84d8edef Mon Sep 17 00:00:00 2001 From: Ghislain Loaec Date: Thu, 22 Feb 2018 23:31:25 +0100 Subject: New env variable: SAML_SECURITY_ASSUME_EMAIL_IS_VERIFIED + fixes #6533 (#6538) --- .env.production.sample | 3 +++ app/models/concerns/omniauthable.rb | 6 ++++-- config/initializers/omniauth.rb | 3 +++ 3 files changed, 10 insertions(+), 2 deletions(-) (limited to 'app/models/concerns') diff --git a/.env.production.sample b/.env.production.sample index 38f7326f0..06606ff25 100644 --- a/.env.production.sample +++ b/.env.production.sample @@ -184,7 +184,10 @@ STREAMING_CLUSTER_NUM=1 # SAML_PRIVATE_KEY= # SAML_SECURITY_WANT_ASSERTION_SIGNED=true # SAML_SECURITY_WANT_ASSERTION_ENCRYPTED=true +# SAML_SECURITY_ASSUME_EMAIL_IS_VERIFIED=true # SAML_ATTRIBUTES_STATEMENTS_UID="urn:oid:0.9.2342.19200300.100.1.1" # SAML_ATTRIBUTES_STATEMENTS_EMAIL="urn:oid:1.3.6.1.4.1.5923.1.1.1.6" # SAML_ATTRIBUTES_STATEMENTS_FULL_NAME="urn:oid:2.5.4.42" # SAML_UID_ATTRIBUTE="urn:oid:0.9.2342.19200300.100.1.1" +# SAML_ATTRIBUTES_STATEMENTS_VERIFIED= +# SAML_ATTRIBUTES_STATEMENTS_VERIFIED_EMAIL= diff --git a/app/models/concerns/omniauthable.rb b/app/models/concerns/omniauthable.rb index a3d55108d..7a396e301 100644 --- a/app/models/concerns/omniauthable.rb +++ b/app/models/concerns/omniauthable.rb @@ -53,8 +53,10 @@ module Omniauthable private def user_params_from_auth(auth) - email_is_verified = auth.info.email && (auth.info.verified || auth.info.verified_email) - email = auth.info.email if email_is_verified && !User.exists?(email: auth.info.email) + assume_verified = Devise.omniauth_configs[:saml].strategy.security.assume_email_is_verified + email_is_verified = auth.info.verified || auth.info.verified_email || assume_verified + email = auth.info.verified_email || auth.info.email + email = email_is_verified && !User.exists?(email: auth.info.email) && email { email: email ? email : "#{TEMP_EMAIL_PREFIX}-#{auth.uid}-#{auth.provider}.com", diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 97f32c0a4..1b650ad09 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -48,10 +48,13 @@ Devise.setup do |config| saml_options[:security] = {} saml_options[:security][:want_assertions_signed] = ENV['SAML_SECURITY_WANT_ASSERTION_SIGNED'] == 'true' saml_options[:security][:want_assertions_encrypted] = ENV['SAML_SECURITY_WANT_ASSERTION_ENCRYPTED'] == 'true' + saml_options[:security][:assume_email_is_verified] = ENV['SAML_SECURITY_ASSUME_EMAIL_IS_VERIFIED'] == 'true' saml_options[:attribute_statements] = {} saml_options[:attribute_statements][:uid] = [ENV['SAML_ATTRIBUTES_STATEMENTS_UID']] if ENV['SAML_ATTRIBUTES_STATEMENTS_UID'] saml_options[:attribute_statements][:email] = [ENV['SAML_ATTRIBUTES_STATEMENTS_EMAIL']] if ENV['SAML_ATTRIBUTES_STATEMENTS_EMAIL'] saml_options[:attribute_statements][:full_name] = [ENV['SAML_ATTRIBUTES_STATEMENTS_FULL_NAME']] if ENV['SAML_ATTRIBUTES_STATEMENTS_FULL_NAME'] + saml_options[:attribute_statements][:verified] = [ENV['SAML_ATTRIBUTES_STATEMENTS_VERIFIED']] if ENV['SAML_ATTRIBUTES_STATEMENTS_VERIFIED'] + saml_options[:attribute_statements][:verified_email] = [ENV['SAML_ATTRIBUTES_STATEMENTS_VERIFIED_EMAIL']] if ENV['SAML_ATTRIBUTES_STATEMENTS_VERIFIED_EMAIL'] saml_options[:uid_attribute] = ENV['SAML_UID_ATTRIBUTE'] if ENV['SAML_UID_ATTRIBUTE'] config.omniauth :saml, saml_options end -- cgit From e668180044560e28bdc5eef94744c210013efcda Mon Sep 17 00:00:00 2001 From: Ghislain Loaec Date: Fri, 23 Feb 2018 01:16:17 +0100 Subject: New variable OAUTH_REDIRECT_AT_SIGN_IN + Ref #6538 (not only SAML strategies) (#6540) --- .env.production.sample | 4 ++++ app/controllers/auth/sessions_controller.rb | 9 +++++++++ app/models/concerns/omniauthable.rb | 3 ++- config/initializers/omniauth.rb | 8 +++++--- 4 files changed, 20 insertions(+), 4 deletions(-) (limited to 'app/models/concerns') diff --git a/.env.production.sample b/.env.production.sample index d74cdb8f8..21d44a416 100644 --- a/.env.production.sample +++ b/.env.production.sample @@ -153,6 +153,10 @@ STREAMING_CLUSTER_NUM=1 # Name of the pam service used for checking if an user can register (pam "account" section is evaluated) # PAM_CONTROLLED_SERVICE=rpam +# Global OAuth settings (optional) : +# If you have only one strategy, you may want to enable this +# OAUTH_REDIRECT_AT_SIGN_IN=true + # Optional CAS authentication (cf. omniauth-cas) : # CAS_ENABLED=true # CAS_URL=https://sso.myserver.com/ diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 4fc41b378..42a3cb62c 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -10,6 +10,15 @@ class Auth::SessionsController < Devise::SessionsController prepend_before_action :authenticate_with_two_factor, if: :two_factor_enabled?, only: [:create] before_action :set_instance_presenter, only: [:new] + def new + Devise.omniauth_configs.each do |provider, config| + if config.strategy.redirect_at_sign_in + return redirect_to(omniauth_authorize_path(resource_name, provider)) + end + end + super + end + def create super do |resource| remember_me(resource) diff --git a/app/models/concerns/omniauthable.rb b/app/models/concerns/omniauthable.rb index 7a396e301..87d93c1fd 100644 --- a/app/models/concerns/omniauthable.rb +++ b/app/models/concerns/omniauthable.rb @@ -53,7 +53,8 @@ module Omniauthable private def user_params_from_auth(auth) - assume_verified = Devise.omniauth_configs[:saml].strategy.security.assume_email_is_verified + strategy = Devise.omniauth_configs[auth.provider.to_sym].strategy + assume_verified = strategy.try(:security).try(:assume_email_is_verified) email_is_verified = auth.info.verified || auth.info.verified_email || assume_verified email = auth.info.verified_email || auth.info.email email = email_is_verified && !User.exists?(email: auth.info.email) && email diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 1b650ad09..92a73d82a 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -4,10 +4,12 @@ end Devise.setup do |config| # Devise omniauth strategies + options = {} + options[:redirect_at_sign_in] = ENV['OAUTH_REDIRECT_AT_SIGN_IN'] == 'true' # CAS strategy if ENV['CAS_ENABLED'] == 'true' - cas_options = {} + cas_options = options cas_options[:url] = ENV['CAS_URL'] if ENV['CAS_URL'] cas_options[:host] = ENV['CAS_HOST'] if ENV['CAS_HOST'] cas_options[:port] = ENV['CAS_PORT'] if ENV['CAS_PORT'] @@ -18,7 +20,7 @@ Devise.setup do |config| cas_options[:login_url] = ENV['CAS_LOGIN_URL'] if ENV['CAS_LOGIN_URL'] cas_options[:uid_field] = ENV['CAS_UID_FIELD'] || 'user' if ENV['CAS_UID_FIELD'] cas_options[:ca_path] = ENV['CAS_CA_PATH'] if ENV['CAS_CA_PATH'] - cas_options[:disable_ssl_verification] = ENV['CAS_DISABLE_SSL_VERIFICATION'] == 'true' if ENV['CAS_DISABLE_SSL_VERIFICATION'] + cas_options[:disable_ssl_verification] = ENV['CAS_DISABLE_SSL_VERIFICATION'] == 'true' cas_options[:uid_key] = ENV['CAS_UID_KEY'] || 'user' cas_options[:name_key] = ENV['CAS_NAME_KEY'] || 'name' cas_options[:email_key] = ENV['CAS_EMAIL_KEY'] || 'email' @@ -33,7 +35,7 @@ Devise.setup do |config| # SAML strategy if ENV['SAML_ENABLED'] == 'true' - saml_options = {} + saml_options = options saml_options[:assertion_consumer_service_url] = ENV['SAML_ACS_URL'] if ENV['SAML_ACS_URL'] saml_options[:issuer] = ENV['SAML_ISSUER'] if ENV['SAML_ISSUER'] saml_options[:idp_sso_target_url] = ENV['SAML_IDP_SSO_TARGET_URL'] if ENV['SAML_IDP_SSO_TARGET_URL'] -- cgit