From 2b2264ddab13ee8ffa7e90d8b803d1a0296a7cff Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Mon, 27 Apr 2015 22:52:42 +0100 Subject: [PATCH] Add support for Windows Live authentication --- Gemfile | 1 + Gemfile.lock | 4 + app/assets/images/windowslive.png | Bin 0 -> 2868 bytes app/views/user/login.html.erb | 3 + config/example.application.yml | 4 + config/initializers/omniauth.rb | 2 + config/locales/en.yml | 4 + lib/auth.rb | 1 + test/controllers/user_controller_test.rb | 2 +- test/fixtures/users.yml | 16 ++- test/integration/user_creation_test.rb | 118 +++++++++++++++++++++++ test/integration/user_login_test.rb | 93 ++++++++++++++++++ test/models/user_test.rb | 6 +- 13 files changed, 249 insertions(+), 5 deletions(-) create mode 100644 app/assets/images/windowslive.png diff --git a/Gemfile b/Gemfile index 09fa2d89b..b5547dc9f 100644 --- a/Gemfile +++ b/Gemfile @@ -59,6 +59,7 @@ gem "omniauth" gem "omniauth-openid" gem "openstreetmap-omniauth-google-oauth2", ">= 0.2.6.1", :require => "omniauth-google-oauth2" gem "omniauth-facebook" +gem "omniauth-windowslive" # Markdown formatting support gem "redcarpet" diff --git a/Gemfile.lock b/Gemfile.lock index 05b873399..333fb2529 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -158,6 +158,9 @@ GEM omniauth-openid (1.0.1) omniauth (~> 1.0) rack-openid (~> 1.3.1) + omniauth-windowslive (0.0.9) + multi_json (>= 1.0.3) + omniauth-oauth2 (~> 1.0) openstreetmap-i18n-js (3.0.0.rc5.3) i18n openstreetmap-omniauth-google-oauth2 (0.2.6.1) @@ -315,6 +318,7 @@ DEPENDENCIES omniauth omniauth-facebook omniauth-openid + omniauth-windowslive openstreetmap-i18n-js (>= 3.0.0.rc5.3) openstreetmap-omniauth-google-oauth2 (>= 0.2.6.1) paperclip (~> 4.0) diff --git a/app/assets/images/windowslive.png b/app/assets/images/windowslive.png new file mode 100644 index 0000000000000000000000000000000000000000..8df253c2b44b90b6559b059824321fbe48dc55e0 GIT binary patch literal 2868 zcmV-43(NG0P)K1|4#h*uVe)3c*Q4 zK~z}7?U-$h9o2cqf9K4cnYs7wy?1xLFL=Gqf^8t)cyVlF>`=SZNlkc3Oj^=Nw2;=J zQL21s`=M2(s!fAbMO7=csnn_|O-qQHz9goC&=?wFhf-og0NdafSbP1pUa#%-zTG?b zWoFKqbNXRzLt>{Ag^&4vd~{~!r|17XN6$0xGrb2@m3mj_S5(z|{-(HAu%rZy?GGRx z{g=0clK%Cb#i5as91NuB(hkwGZvMc0e)7#mdlYC~KKkLFkOR}{`v-JcLxm{XSSfDU zR*>ubz+YuO2Bq(MbLA>{hyY>`XRTy(Z^JzISBGc*2dKaA8M>U4Us>F}`p!z}uNOu;3?yFUaFHSiJ(TZoo`66ifXL-EMaNt{6CdBy zB@eDW+=%x5V08XnBu4?-|H)|c67jG1cXba$-W_=e#e%p5B9RgJ5vWk$dxw$w4@+%by{@|Kw7!Gt+L?(N@dMZa> zp4{*MG~AJH)!!F-n~5N-gb+bkc8EfNANu%#!})-72%r#_Id(eTdHp~A_l2*&y6vt; zGx&|38)WU&!DQF6wUwvmE=C8n<09lXOI)W5-EvtCHiW&=N2@*Mf!>yT^8W&gS|r2E zyGq>z5LLldB7#sLi$KBQ0>E1+!U#YId9};!>E3*A#ZPTt5ucv!s9L>4-gooHp-;|C zqo-!m*)%0e9Z>_+ymOw2d|x;@esa&$UVtA15(1oqt%Lnb0#Dsu2%#M=z=RzF(Xrr= zDuRl@2@nb*g*`PYUaCzrX8!08M>ZjoAtjqsB1;0EbD38u3q=?d;}~Ks$l98qo#ST^ z`r`E7J*Qs0?Bx}$UQ`8O+Xp%Zt}CmLhzNo*1fhTus1AtA;8%bO&;hb6C{&ewWxT#? z^EVI9FCQr1)#(?PMXE>;R)|Cg9Z^6>-1LFhp#@`#XaKQ>1l_RLioIxt-W zperydhstW$MFLS!h!O-9fnNrq1-qmI6#yZ$o4FmGYCHqjVptr`r6`C*BAi-i<4j`| z6bqUZ2s0>1u%RS1?n3Q&{0cz#U=P=*p-2#mnFeEj(S`Ii9V z3o};(eSWy>=5i?31|HN+el)aZ+@-P++suT1prKV0Wl* zTDf$aBIuFACsh^?O_+tCa?(6OOP^30A`kpPB8;%U3RF|#djUa^;Yp?HhusygaCnO?CEodn{NWFxP1j&nmqQ#Tc6o- z`|scSo6E!#U;~uq*op5QJPa_lXABmuHW0!C zEQ>aLa&x%pm;cmTJoJ!fldhhE61@USK~87j(V-NTz?6&V+j`2;_M2{8`g75u8+Yz_ z_)GElx%lXzne*C(c1@)ima0*R=R5FnJdu-2OWQR`jp0HIPM^xoo;}fe{M_poe)!HB z_5qw*u&{IG<~0qchLc32ln{x43<1KSQ_uy-3?fUNmn0s|xY_?7-~QKI*247P|MvFs z`t6VW?M;vNJTX2sw>r$sovRT zlHTPbA}B6$SsSY&675LC6rBcW4|F2PLa?DAX{DrTHh*Hz&i!9|Wb>M}zxMNg<39b= zQ?zPd6aAlk8u&h+_;6=ss4L1m&nDB>aHyK{#%;Ahz9PQ*(2`QTDze+3`|&a43Q$jR zV3l_6dh7X+Tby>JDkIj4*oT5Ogf=D80VO=z^%`eSjgM`CnT@N5KTkVdpEmO$qur)_ z{Hrt4#k1(vB=y<4n>*u)>U@M8k7$lwKYL+m-@fDCw?6#`>yhS)dKyp<4N>M1csYKeiL|2~x6kZ)_6INi`_uPym2OzJO~(yu zY|`gMa#x-kL~bBY8ZKzH^3F{9A_-g@UnUf5O)$>xfR|fWO@+;=OViv3ZrPX$f57Mr zWi>K4=u}WQhPhNz&vNQ5%WTsyT~C>B+7r+I!c?6=az{X{o3Ds zKe+&~#OI#1$c|H-2lrvPfT+SNee>djkqE9=gs zKzK4YGO$+qj>LhrLi<+Erb4C;k%bcKwZr=!d+b$v|D7xP2GreoItl6JwWk5JfF|V4 z1(;?^+7uKr7(34xzJCm(w9!z|D1~LzrAp%+4_I2 z($~_HPj2K>pLz)(-1?!;p)cOsvu@Q5`Nn2uM~Vf1X~7p|C37#!Y(5{;Y_-Hx;K{Mf zm#=^4mGp(#Ci@=SdB*hhm6)1pzIPy{1i!kulgD?@y)#_A@7Auh_ipOGad?U7T2vMO zTrBk7(JcPnf%!AL_nX7$#>C~rUw&|qFaPtz`~SP<6YHw)`k8ers{BmvMgI${HZ?Kf S0I*&F0000
  • <%= auth_button "facebook", "facebook" %>
    • <% end -%> + <% if defined?(WINDOWSLIVE_AUTH_ID) -%> +
  • <%= auth_button "windowslive", "windowslive" %>
    • + <% end -%>
  • <%= auth_button "yahoo", "openid", :openid_url => "yahoo.com" %>
  • <%= auth_button "wordpress", "openid", :openid_url => "wordpress.com" %>
  • <%= auth_button "aol", "openid", :openid_url => "aol.com" %>
    • diff --git a/config/example.application.yml b/config/example.application.yml index dd2a174e9..0af929e76 100644 --- a/config/example.application.yml +++ b/config/example.application.yml @@ -94,6 +94,8 @@ defaults: &defaults #google_openid_realm: "" #facebook_auth_id: "" #facebook_auth_secret: "" + #windowslive_auth_id: "" + #windowslive_auth_secret: "" development: <<: *defaults @@ -111,3 +113,5 @@ test: google_openid_realm: "https://www.openstreetmap.org" facebook_auth_id: "dummy" facebook_auth_secret: "dummy" + windowslive_auth_id: "dummy" + windowslive_auth_secret: "dummy" diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 508d326d6..a654f2aa2 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -22,6 +22,7 @@ end openid_options = { :name => "openid", :store => openid_store } google_options = { :name => "google", :scope => "email", :access_type => "online" } facebook_options = { :name => "facebook", :scope => "email" } +windowslive_options = { :name => "windowslive", :scope => "wl.signin,wl.emails" } if defined?(GOOGLE_OPENID_REALM) google_options[:openid_realm] = GOOGLE_OPENID_REALM @@ -31,6 +32,7 @@ Rails.application.config.middleware.use OmniAuth::Builder do provider :openid, openid_options provider :google_oauth2, GOOGLE_AUTH_ID, GOOGLE_AUTH_SECRET, google_options if defined?(GOOGLE_AUTH_ID) provider :facebook, FACEBOOK_AUTH_ID, FACEBOOK_AUTH_SECRET, facebook_options if defined?(FACEBOOK_AUTH_ID) + provider :windowslive, WINDOWSLIVE_AUTH_ID, WINDOWSLIVE_AUTH_SECRET, windowslive_options if defined?(WINDOWSLIVE_AUTH_ID) end # Pending fix for: https://github.com/intridea/omniauth/pull/795 diff --git a/config/locales/en.yml b/config/locales/en.yml index e554832f8..531c70f46 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1723,6 +1723,9 @@ en: facebook: title: Login with Facebook alt: Login with a Facebook Account + windowslive: + title: Login with Windows Live + alt: Login with a Windows Live Account yahoo: title: Login with Yahoo alt: Login with a Yahoo OpenID @@ -1996,6 +1999,7 @@ en: invalid_credentials: Invalid authentication credentials no_authorization_code: No authorization code unknown_signature_algorithm: Unknown signature algorithm + invalid_scope: Invalid scope user_role: filter: not_an_administrator: "Only administrators can perform user role management, and you are not an administrator." diff --git a/lib/auth.rb b/lib/auth.rb index b5ac0d2eb..636173928 100644 --- a/lib/auth.rb +++ b/lib/auth.rb @@ -2,4 +2,5 @@ module Auth PROVIDERS = { "None" => "", "OpenID" => "openid" } PROVIDERS["Google"] = "google" if defined?(GOOGLE_AUTH_ID) PROVIDERS["Facebook"] = "facebook" if defined?(FACEBOOK_AUTH_ID) + PROVIDERS["Windows Live"] = "windowslive" if defined?(WINDOWSLIVE_AUTH_ID) end diff --git a/test/controllers/user_controller_test.rb b/test/controllers/user_controller_test.rb index 0f08ced7c..03238ea28 100644 --- a/test/controllers/user_controller_test.rb +++ b/test/controllers/user_controller_test.rb @@ -1321,7 +1321,7 @@ class UserControllerTest < ActionController::TestCase get :list, :page => 3 assert_response :success assert_template :list - assert_select "table#user_list tr", :count => 21 + assert_select "table#user_list tr", :count => 22 end def test_list_post_confirm diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 30b27c7bd..cb861ab93 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -232,7 +232,7 @@ north_pole_user: languages: en home_lat: 89.9 home_lon: 146.8 - + german_user: id: 18 email: german@example.com @@ -276,3 +276,17 @@ facebook_user: terms_agreed: "2010-01-01 11:22:33" terms_seen: true languages: en + +windowslive_user: + id: 21 + email: windowslive-user@example.com + status: active + pass_crypt: <%= Digest::MD5.hexdigest('test') %> + creation_time: "2008-05-01 01:23:45" + display_name: windowsliveuser + data_public: true + auth_provider: windowslive + auth_uid: 123456789 + terms_agreed: "2010-01-01 11:22:33" + terms_seen: true + languages: en diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb index 15fcaf236..940eb3044 100644 --- a/test/integration/user_creation_test.rb +++ b/test/integration/user_creation_test.rb @@ -15,6 +15,7 @@ class UserCreationTest < ActionDispatch::IntegrationTest OmniAuth.config.mock_auth[:openid] = nil OmniAuth.config.mock_auth[:google] = nil OmniAuth.config.mock_auth[:facebook] = nil + OmniAuth.config.mock_auth[:windowslive] = nil OmniAuth.config.test_mode = false end @@ -499,4 +500,121 @@ class UserCreationTest < ActionDispatch::IntegrationTest assert_response :success assert_template "site/welcome" end + + def test_user_create_windowslive_success + OmniAuth.config.add_mock(:windowslive, :uid => "123454321", :extra => { + :id_info => { "openid_id" => "http://localhost:1123/new.tester" } + }) + + new_email = "newtester-windowslive@osm.org" + display_name = "new_tester-windowslive" + password = "testtest" + assert_difference("User.count") do + assert_difference("ActionMailer::Base.deliveries.size", 1) do + post "/user/new", + :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "windowslive", :pass_crypt => "", :pass_crypt_confirmation => "" } + assert_response :redirect + assert_redirected_to auth_path(:provider => "windowslive", :origin => "/user/new") + follow_redirect! + assert_response :redirect + assert_redirected_to auth_success_path(:provider => "windowslive") + follow_redirect! + assert_response :redirect + assert_redirected_to "/user/terms" + post "/user/save", + :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "windowslive", :auth_uid => "123454321", :pass_crypt => password, :pass_crypt_confirmation => password } + assert_response :redirect + follow_redirect! + end + end + + # Check the page + assert_response :success + assert_template "user/confirm" + + ActionMailer::Base.deliveries.clear + end + + def test_user_create_windowslive_failure + OmniAuth.config.mock_auth[:windowslive] = :connection_failed + + new_email = "newtester-windowslive2@osm.org" + display_name = "new_tester-windowslive2" + assert_difference("User.count", 0) do + assert_difference("ActionMailer::Base.deliveries.size", 0) do + post "/user/new", + :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "windowslive", :pass_crypt => "", :pass_crypt_confirmation => "" } + assert_response :redirect + assert_redirected_to auth_path(:provider => "windowslive", :origin => "/user/new") + follow_redirect! + assert_response :redirect + assert_redirected_to auth_success_path(:provider => "windowslive") + follow_redirect! + assert_response :redirect + assert_redirected_to auth_failure_path(:strategy => "windowslive", :message => "connection_failed", :origin => "/user/new") + follow_redirect! + assert_response :redirect + follow_redirect! + assert_response :success + assert_template "user/new" + end + end + + ActionMailer::Base.deliveries.clear + end + + def test_user_create_windowslive_redirect + OmniAuth.config.add_mock(:windowslive, :uid => "123454321", :extra => { + :id_info => { "openid_id" => "http://localhost:1123/new.tester" } + }) + + new_email = "redirect_tester_windowslive@osm.org" + display_name = "redirect_tester_windowslive" + # nothing special about this page, just need a protected page to redirect back to. + referer = "/traces/mine" + assert_difference("User.count") do + assert_difference("ActionMailer::Base.deliveries.size", 1) do + post "/user/new", + :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "windowslive", :pass_crypt => "", :pass_crypt_confirmation => "" }, :referer => referer + assert_response :redirect + assert_redirected_to auth_path(:provider => "windowslive", :origin => "/user/new") + follow_redirect! + assert_response :redirect + assert_redirected_to auth_success_path(:provider => "windowslive") + follow_redirect! + assert_response :redirect + assert_redirected_to "/user/terms" + post_via_redirect "/user/save", + :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "windowslive", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" } + end + end + + # Check the e-mail + register_email = ActionMailer::Base.deliveries.first + + assert_equal register_email.to[0], new_email + # Check that the confirm account url is correct + confirm_regex = Regexp.new("/user/redirect_tester_windowslive/confirm\\?confirm_string=([a-zA-Z0-9]*)") + register_email.parts.each do |part| + assert_match confirm_regex, part.body.to_s + end + confirm_string = register_email.parts[0].body.match(confirm_regex)[1] + + # Check the page + assert_response :success + assert_template "user/confirm" + + ActionMailer::Base.deliveries.clear + + # Go to the confirmation page + get "/user/#{display_name}/confirm", :confirm_string => confirm_string + assert_response :success + assert_template "user/confirm" + + post "/user/#{display_name}/confirm", :confirm_string => confirm_string + assert_response :redirect + follow_redirect! + assert_response :success + assert_template "site/welcome" + end end diff --git a/test/integration/user_login_test.rb b/test/integration/user_login_test.rb index 9cab196f2..ae1c3cfd6 100644 --- a/test/integration/user_login_test.rb +++ b/test/integration/user_login_test.rb @@ -11,6 +11,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest OmniAuth.config.mock_auth[:openid] = nil OmniAuth.config.mock_auth[:google] = nil OmniAuth.config.mock_auth[:facebook] = nil + OmniAuth.config.mock_auth[:windowslive] = nil OmniAuth.config.test_mode = false end @@ -1061,4 +1062,96 @@ class UserLoginTest < ActionDispatch::IntegrationTest assert_template "user/new" assert_select "span.username", false end + + def test_login_windowslive_success + OmniAuth.config.add_mock(:windowslive, :uid => "123456789", :extra => { + :id_info => { "openid_id" => "http://localhost:1123/fred.bloggs" } + }) + + get "/login", :referer => "/history" + assert_response :redirect + assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true", "referer" => "/history" + follow_redirect! + assert_response :success + assert_template "user/login" + get auth_path(:provider => "windowslive", :origin => "/login") + assert_response :redirect + assert_redirected_to auth_success_path(:provider => "windowslive") + follow_redirect! + assert_response :redirect + follow_redirect! + assert_response :success + assert_template "changeset/history" + assert_select "span.username", "windowsliveuser" + end + + def test_login_windowslive_connection_failed + OmniAuth.config.mock_auth[:windowslive] = :connection_failed + + get "/login", :referer => "/history" + assert_response :redirect + assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true", "referer" => "/history" + follow_redirect! + assert_response :success + assert_template "user/login" + get auth_path(:provider => "windowslive", :origin => "/login") + assert_response :redirect + assert_redirected_to auth_success_path(:provider => "windowslive") + follow_redirect! + assert_response :redirect + assert_redirected_to auth_failure_path(:strategy => "windowslive", :message => "connection_failed", :origin => "/login") + follow_redirect! + assert_response :redirect + follow_redirect! + assert_response :success + assert_template "login" + assert_select "div.flash.error", "Connection to authentication provider failed" + assert_select "span.username", false + end + + def test_login_windowslive_invalid_credentials + OmniAuth.config.mock_auth[:windowslive] = :invalid_credentials + + get "/login", :referer => "/history" + assert_response :redirect + assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true", "referer" => "/history" + follow_redirect! + assert_response :success + assert_template "user/login" + get auth_path(:provider => "windowslive", :origin => "/login") + assert_response :redirect + assert_redirected_to auth_success_path(:provider => "windowslive") + follow_redirect! + assert_response :redirect + assert_redirected_to auth_failure_path(:strategy => "windowslive", :message => "invalid_credentials", :origin => "/login") + follow_redirect! + assert_response :redirect + follow_redirect! + assert_response :success + assert_template "login" + assert_select "div.flash.error", "Invalid authentication credentials" + assert_select "span.username", false + end + + def test_login_windowslive_unknown + OmniAuth.config.add_mock(:windowslive, :uid => "987654321", :extra => { + :id_info => { "openid_id" => "http://localhost:1123/fred.bloggs" } + }) + + get "/login", :referer => "/history" + assert_response :redirect + assert_redirected_to "controller" => "user", "action" => "login", "cookie_test" => "true", "referer" => "/history" + follow_redirect! + assert_response :success + assert_template "user/login" + get auth_path(:provider => "windowslive", :origin => "/login") + assert_response :redirect + assert_redirected_to auth_success_path(:provider => "windowslive") + follow_redirect! + assert_response :redirect + follow_redirect! + assert_response :success + assert_template "user/new" + assert_select "span.username", false + end end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 14e00cc99..b3e6eae8f 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -163,7 +163,7 @@ class UserTest < ActiveSupport::TestCase end def test_visible - assert_equal 18, User.visible.count + assert_equal 19, User.visible.count assert_raise ActiveRecord::RecordNotFound do User.visible.find(users(:suspended_user).id) end @@ -173,7 +173,7 @@ class UserTest < ActiveSupport::TestCase end def test_active - assert_equal 17, User.active.count + assert_equal 18, User.active.count assert_raise ActiveRecord::RecordNotFound do User.active.find(users(:inactive_user).id) end @@ -186,7 +186,7 @@ class UserTest < ActiveSupport::TestCase end def test_identifiable - assert_equal 19, User.identifiable.count + assert_equal 20, User.identifiable.count assert_raise ActiveRecord::RecordNotFound do User.identifiable.find(users(:normal_user).id) end -- 2.39.5