From 0ae574007d2118fc6e291591121ceca2da6fc22e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Onur=20K=C3=BC=C3=A7=C3=BCk?= Date: Sun, 3 May 2015 00:43:46 +0300 Subject: [PATCH] add common method to force utf8 and force oauth properties to be utf8 --- CHANGELOG | 1 + lib/gitlab/o_auth/auth_hash.rb | 19 ++-- lib/gitlab/utils.rb | 4 + spec/lib/gitlab/o_auth/auth_hash_spec.rb | 111 +++++++++++++++++------ 4 files changed, 100 insertions(+), 35 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 3af83ddc256..a58a5cca969 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -27,6 +27,7 @@ v 7.11.0 (unreleased) - Move snippets UI to fluid layout - Improve UI for sidebar. Increase separation between navigation and content - Improve new project command options (Ben Bodenmiller) + - Add common method to force UTF-8 and use it to properly handle non-ascii OAuth user properties (Onur Küçük) - Prevent sending empty messages to HipChat (Chulki Lee) - Improve UI for mobile phones on dashboard and project pages - Add room notification and message color option for HipChat diff --git a/lib/gitlab/o_auth/auth_hash.rb b/lib/gitlab/o_auth/auth_hash.rb index ce52beec78e..0f16c925900 100644 --- a/lib/gitlab/o_auth/auth_hash.rb +++ b/lib/gitlab/o_auth/auth_hash.rb @@ -9,11 +9,11 @@ module Gitlab end def uid - auth_hash.uid.to_s + Gitlab::Utils.force_utf8(auth_hash.uid.to_s) end def provider - auth_hash.provider + Gitlab::Utils.force_utf8(auth_hash.provider.to_s) end def info @@ -21,23 +21,28 @@ module Gitlab end def name - (info.try(:name) || full_name).to_s.force_encoding('utf-8') + Gitlab::Utils.force_utf8((info.try(:name) || full_name).to_s) end def full_name - "#{info.first_name} #{info.last_name}" + Gitlab::Utils.force_utf8("#{info.first_name} #{info.last_name}") end def username - (info.try(:nickname) || generate_username).to_s.force_encoding('utf-8') + Gitlab::Utils.force_utf8( + (info.try(:nickname) || generate_username).to_s + ) end def email - (info.try(:email) || generate_temporarily_email).downcase + Gitlab::Utils.force_utf8( + (info.try(:email) || generate_temporarily_email).downcase + ) end def password - @password ||= Devise.friendly_token[0, 8].downcase + devise_friendly_token = Devise.friendly_token[0, 8].downcase + @password ||= Gitlab::Utils.force_utf8(devise_friendly_token) end # Get the first part of the email address (before @) diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index bd184c27187..d13fe0ef8a9 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -9,5 +9,9 @@ module Gitlab def system_silent(cmd) Popen::popen(cmd).last.zero? end + + def force_utf8(str) + str.force_encoding(Encoding::UTF_8) + end end end diff --git a/spec/lib/gitlab/o_auth/auth_hash_spec.rb b/spec/lib/gitlab/o_auth/auth_hash_spec.rb index 5eb77b492b2..678086ffa14 100644 --- a/spec/lib/gitlab/o_auth/auth_hash_spec.rb +++ b/spec/lib/gitlab/o_auth/auth_hash_spec.rb @@ -2,54 +2,109 @@ require 'spec_helper' describe Gitlab::OAuth::AuthHash do let(:auth_hash) do - Gitlab::OAuth::AuthHash.new(double({ - provider: 'twitter', - uid: uid, - info: double(info_hash) - })) + Gitlab::OAuth::AuthHash.new( + double({ + provider: provider_ascii, + uid: uid_ascii, + info: double(info_hash) + }) + ) end - let(:uid) { 'my-uid' } - let(:email) { 'my-email@example.com' } - let(:nickname) { 'my-nickname' } + + let(:uid_raw) { + "CN=Onur K\xC3\xBC\xC3\xA7\xC3\xBCk,OU=Test,DC=example,DC=net" + } + let(:email_raw) { "onur.k\xC3\xBC\xC3\xA7\xC3\xBCk@example.net" } + let(:nickname_raw) { "ok\xC3\xBC\xC3\xA7\xC3\xBCk" } + let(:first_name_raw) { 'Onur' } + let(:last_name_raw) { "K\xC3\xBC\xC3\xA7\xC3\xBCk" } + let(:name_raw) { "Onur K\xC3\xBC\xC3\xA7\xC3\xBCk" } + + let(:provider_ascii) { 'ldap'.force_encoding(Encoding::ASCII_8BIT) } + let(:uid_ascii) { uid_raw.force_encoding(Encoding::ASCII_8BIT) } + let(:email_ascii) { email_raw.force_encoding(Encoding::ASCII_8BIT) } + let(:nickname_ascii) { nickname_raw.force_encoding(Encoding::ASCII_8BIT) } + let(:first_name_ascii) { first_name_raw.force_encoding(Encoding::ASCII_8BIT) } + let(:last_name_ascii) { last_name_raw.force_encoding(Encoding::ASCII_8BIT) } + let(:name_ascii) { name_raw.force_encoding(Encoding::ASCII_8BIT) } + + let(:provider_utf8) { provider_ascii.force_encoding(Encoding::UTF_8) } + let(:uid_utf8) { uid_ascii.force_encoding(Encoding::UTF_8) } + let(:email_utf8) { email_ascii.force_encoding(Encoding::UTF_8) } + let(:nickname_utf8) { nickname_ascii.force_encoding(Encoding::UTF_8) } + let(:name_utf8) { name_ascii.force_encoding(Encoding::UTF_8) } + let(:info_hash) { { - email: email, - nickname: nickname, - name: 'John', - first_name: "John", - last_name: "Who" + email: email_ascii, + first_name: first_name_ascii, + last_name: last_name_ascii, + name: name_ascii, + nickname: nickname_ascii, + uid: uid_ascii } } - context "defaults" do - it { expect(auth_hash.provider).to eql 'twitter' } - it { expect(auth_hash.uid).to eql uid } - it { expect(auth_hash.email).to eql email } - it { expect(auth_hash.username).to eql nickname } - it { expect(auth_hash.name).to eql "John" } + context 'defaults' do + it { expect(auth_hash.provider).to eql provider_utf8 } + it { expect(auth_hash.uid).to eql uid_utf8 } + it { expect(auth_hash.email).to eql email_utf8 } + it { expect(auth_hash.username).to eql nickname_utf8 } + it { expect(auth_hash.name).to eql name_utf8 } it { expect(auth_hash.password).to_not be_empty } end - context "email not provided" do + context 'email not provided' do before { info_hash.delete(:email) } - it "generates a temp email" do + + it 'generates a temp email' do expect( auth_hash.email).to start_with('temp-email-for-oauth') end end - context "username not provided" do + context 'username not provided' do before { info_hash.delete(:nickname) } - it "takes the first part of the email as username" do - expect( auth_hash.username ).to eql "my-email" + it 'takes the first part of the email as username' do + expect(auth_hash.username).to eql 'onur-kucuk' end end - context "name not provided" do + context 'name not provided' do before { info_hash.delete(:name) } - it "concats first and lastname as the name" do - expect( auth_hash.name ).to eql "John Who" + it 'concats first and lastname as the name' do + expect(auth_hash.name).to eql name_utf8 end end -end \ No newline at end of file + + context 'auth_hash constructed with ASCII-8BIT encoding' do + it 'forces utf8 encoding on uid' do + auth_hash.uid.encoding.should eql Encoding::UTF_8 + end + + it 'forces utf8 encoding on provider' do + auth_hash.provider.encoding.should eql Encoding::UTF_8 + end + + it 'forces utf8 encoding on name' do + auth_hash.name.encoding.should eql Encoding::UTF_8 + end + + it 'forces utf8 encoding on full_name' do + auth_hash.full_name.encoding.should eql Encoding::UTF_8 + end + + it 'forces utf8 encoding on username' do + auth_hash.username.encoding.should eql Encoding::UTF_8 + end + + it 'forces utf8 encoding on email' do + auth_hash.email.encoding.should eql Encoding::UTF_8 + end + + it 'forces utf8 encoding on password' do + auth_hash.password.encoding.should eql Encoding::UTF_8 + end + end +end