From 9aeff5370d7d626c5a1c412b33289df23b8db843 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 23 Feb 2012 12:21:12 -0700 Subject: [PATCH] plumb host and protocol to gravatar fallbacks test plan: * go to /images/users/ for a user without an avatar set, but avatars are enabled * you should end up back on the same host, with ssl * change your avatar on the profile page * the fallback to dashed should also be over ssl Change-Id: I3e3cd322e75c2bd7a329c39887043915f80d2112 Reviewed-on: https://gerrit.instructure.com/8956 Tested-by: Hudson Reviewed-by: Zach Wily --- app/controllers/profile_controller.rb | 2 +- app/controllers/users_controller.rb | 2 +- app/models/user.rb | 8 ++++---- spec/controllers/users_controller_spec.rb | 9 --------- spec/integration/users_controller_spec.rb | 9 +++++++++ 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/controllers/profile_controller.rb b/app/controllers/profile_controller.rb index e5d63d9a690..c4f3f31f3e7 100644 --- a/app/controllers/profile_controller.rb +++ b/app/controllers/profile_controller.rb @@ -161,7 +161,7 @@ class ProfileController < ApplicationController end end @pics << { - :url => @current_user.gravatar_url(50, "http://#{HostUrl.default_host}/images/dotted_pic.png"), + :url => @current_user.gravatar_url(50, "#{request.protocol}#{request.host_with_port}/images/dotted_pic.png"), :type => 'gravatar', :alt => 'gravatar pic' } diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index c018019070a..96e289d95d6 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -912,7 +912,7 @@ class UsersController < ApplicationController url = Rails.cache.fetch(Cacher.avatar_cache_key(user_id)) do user = User.find_by_id(user_id) if user_id.present? if user && service_enabled?(:avatars) - url = user.avatar_url(nil, @domain_root_account && @domain_root_account.settings[:avatars], params[:fallback]) + url = user.avatar_url(nil, @domain_root_account && @domain_root_account.settings[:avatars], params[:fallback], request) end url ||= params[:fallback] || '/images/no_pic.gif' end diff --git a/app/models/user.rb b/app/models/user.rb index 6f8d7e99e3c..117c9e8dd60 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1088,8 +1088,8 @@ class User < ActiveRecord::Base User.max_messages_per_day end - def gravatar_url(size=50, fallback=nil) - fallback ||= "http://#{HostUrl.default_host}/images/no_pic.gif" + def gravatar_url(size=50, fallback=nil, request=nil) + fallback ||= request ? "#{request.protocol}#{request.host_with_port}/images/no_pic.gif" : "http://#{HostUrl.default_host}/images/no_pic.gif" "https://secure.gravatar.com/avatar/#{Digest::MD5.hexdigest(self.email) rescue '000'}?s=#{size}&d=#{CGI::escape(fallback)}" end @@ -1225,14 +1225,14 @@ class User < ActiveRecord::Base }.uniq end - def avatar_url(size=nil, avatar_setting=nil, fallback='/images/no_pic.gif') + def avatar_url(size=nil, avatar_setting=nil, fallback='/images/no_pic.gif', request = nil) size ||= 50 avatar_setting ||= 'enabled' if avatar_setting == 'enabled' || (avatar_setting == 'enabled_pending' && avatar_approved?) || (avatar_setting == 'sis_only') @avatar_url ||= self.avatar_image_url end @avatar_url ||= fallback if self.avatar_image_source == 'no_pic' - @avatar_url ||= gravatar_url(size, fallback) if avatar_setting == 'enabled' + @avatar_url ||= gravatar_url(size, fallback, request) if avatar_setting == 'enabled' @avatar_url ||= fallback end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 1270bac553e..fe10057a9a0 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -370,15 +370,6 @@ describe UsersController do get 'avatar_image_url', :user_id => @user.id, :fallback => "https://test.domain/my/custom/fallback/url.png" response.should redirect_to "https://secure.gravatar.com/avatar/000?s=50&d=https%3A%2F%2Ftest.domain%2Fmy%2Fcustom%2Ffallback%2Furl.png" end - it "should handle receiving an encrypted user id" do - course_with_student_logged_in(:active_all => true) - @account = Account.default - @account.enable_service(:avatars) - @account.save! - @account.service_enabled?(:avatars).should be_true - get 'avatar_image_url', :user_id => CGI.unescape(User.avatar_key(@user.id)) - response.should redirect_to "https://secure.gravatar.com/avatar/000?s=50&d=http%3A%2F%2F#{CGI.escape(HostUrl.default_host)}%2Fimages%2Fno_pic.gif" - end it "should take an invalid id and return no_pic" do @account = Account.default @account.enable_service(:avatars) diff --git a/spec/integration/users_controller_spec.rb b/spec/integration/users_controller_spec.rb index 300f8f3bde4..ffe1bd037dc 100644 --- a/spec/integration/users_controller_spec.rb +++ b/spec/integration/users_controller_spec.rb @@ -94,5 +94,14 @@ describe UsersController do response.body.should match /Olds, JT.*St\. Clair, John/m end end + + describe "#avatar_image_url" do + it "should maintain protocol and domain name in gravatar redirect" do + Account.default.tap { |a| a.enable_service(:avatars) }.save + user + get "https://someschool.instructure.com/images/users/#{User.avatar_key(@user.id)}" + response.should redirect_to "https://secure.gravatar.com/avatar/000?s=50&d=#{CGI::escape("https://someschool.instructure.com/images/no_pic.gif")}" + end + end end