diff --git a/app/assets/javascripts/auth_providers.js b/app/assets/javascripts/auth_providers.js index 35da7b5e4..975c57a9b 100644 --- a/app/assets/javascripts/auth_providers.js +++ b/app/assets/javascripts/auth_providers.js @@ -12,7 +12,7 @@ $(document).ready(function () { $("#openid_open_url").click(function (e) { e.preventDefault(); $("#openid_url").val("http://"); - $("#login_auth_buttons").hide(); + $("#login_auth_buttons").hide().removeClass("d-flex"); $("#login_openid_url").show(); $("#openid_login_button").show(); }); diff --git a/app/controllers/concerns/session_methods.rb b/app/controllers/concerns/session_methods.rb index cebe932fc..5dcddb82d 100644 --- a/app/controllers/concerns/session_methods.rb +++ b/app/controllers/concerns/session_methods.rb @@ -3,6 +3,18 @@ module SessionMethods private + ## + # Read @preferred_auth_provider and @client_app_name from oauth2 authorization request's referer + def parse_oauth_referer(referer) + referer_query = URI(referer).query if referer + return unless referer_query + + ref_params = CGI.parse referer_query + preferred = ref_params["preferred_auth_provider"].first + @preferred_auth_provider = preferred if preferred && Settings.key?(:"#{preferred}_auth_id") + @client_app_name = Oauth2Application.where(:uid => ref_params["client_id"].first).pick(:name) + end + ## # return the URL to use for authentication def auth_url(provider, uid, referer = nil) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index e57ffc06a..fdf2df6a7 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -15,6 +15,8 @@ class SessionsController < ApplicationController override_content_security_policy_directives(:form_action => []) if Settings.csp_enforce || Settings.key?(:csp_report_url) session[:referer] = safe_referer(params[:referer]) if params[:referer] + + parse_oauth_referer session[:referer] end def create diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 74ec5c0ec..e022ff0c1 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -60,6 +60,8 @@ class UsersController < ApplicationController session[:referer] end + parse_oauth_referer @referer + append_content_security_policy_directives( :form_action => %w[accounts.google.com *.facebook.com login.live.com github.com meta.wikimedia.org] ) diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index d0b2f0be5..0a68e608e 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -60,11 +60,25 @@ module UserHelper link_to( image_tag("#{name}.svg", :alt => t("application.auth_providers.#{name}.alt"), - :class => "rounded-3", - :size => "36"), + :class => "rounded-1", + :size => "24"), auth_path(options.merge(:provider => provider)), :method => :post, - :class => "auth_button", + :class => "auth_button p-2 d-block", + :title => t("application.auth_providers.#{name}.title") + ) + end + + def auth_button_preferred(name, provider, options = {}) + link_to( + image_tag("#{name}.svg", + :alt => t("application.auth_providers.#{name}.alt"), + :class => "rounded-1 me-3", + :width => "24px", + :height => "24px") + t("application.auth_providers.#{name}.title"), + auth_path(options.merge(:provider => provider)), + :method => :post, + :class => "auth_button fs-6 border rounded text-muted text-decoration-none py-2 px-4 d-flex justify-content-center align-items-center", :title => t("application.auth_providers.#{name}.title") ) end diff --git a/app/views/application/_auth_providers.html.erb b/app/views/application/_auth_providers.html.erb index 9c72d7aa0..a79e7b5ce 100644 --- a/app/views/application/_auth_providers.html.erb +++ b/app/views/application/_auth_providers.html.erb @@ -1,33 +1,44 @@
-
- +
-
    -
  • + <% %w[google facebook microsoft github wikipedia].each do |provider| %> + <% if Settings.key?("#{provider}_auth_id".to_sym) -%> + <% if @preferred_auth_provider == provider %> +
    <%= auth_button_preferred provider, provider %>
    + <% end %> + <% end -%> + <% end -%> + +
    +
    <%= link_to image_tag("openid.png", :alt => t("application.auth_providers.openid.title"), - :size => "36"), + :size => "24"), "#", :id => "openid_open_url", - :title => t("application.auth_providers.openid.title") %> -
    • - - <% %w[google facebook microsoft github wikipedia].each do |provider| %> - <% if Settings.key?("#{provider}_auth_id".to_sym) -%> -
  • <%= auth_button provider, provider %>
    • - <% end -%> - <% end -%> -
- - <%= form_tag(auth_path(:provider => "openid"), :id => "openid_login_form") do %> -
- - <%= hidden_field_tag("referer", params[:referer], :autocomplete => "off") %> - <%= text_field_tag("openid_url", "", :tabindex => 5, :autocomplete => "on", :class => "openid_url form-control") %> - (" target="_new"><%= t "accounts.edit.openid.link text" %>) + :title => t("application.auth_providers.openid.title"), + :class => "p-2 d-block" %>
- <%= submit_tag t(".openid_login_button"), :tabindex => 6, :id => "openid_login_button", :class => "btn btn-primary" %> - <% end %> + <% %w[google facebook microsoft github wikipedia].each do |provider| %> + <% unless @preferred_auth_provider == provider %> + <% if Settings.key?("#{provider}_auth_id".to_sym) -%> +
<%= auth_button provider, provider %>
+ <% end -%> + <% end %> + <% end -%> +
+ + <%# :tabindex starts high to allow rendering at the bottom of the template %> + <%= form_tag(auth_path(:provider => "openid"), :id => "openid_login_form") do %> +
+ + <%= hidden_field_tag("referer", params[:referer], :autocomplete => "off") %> + <%= text_field_tag("openid_url", "", :tabindex => 20, :autocomplete => "on", :class => "openid_url form-control") %> + (" target="_new"><%= t "accounts.edit.openid.link text" %>) +
+ + <%= submit_tag t(".openid_login_button"), :tabindex => 21, :id => "openid_login_button", :class => "btn btn-primary" %> + <% end %>
diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index 90611f214..d30eb6697 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -6,6 +6,10 @@ <% content_for :heading_class, "p-0 mw-100" %> <% content_for :heading do %> + <% if @client_app_name %> +

<%= t(".login_to_authorize_html", :client_app_name => @client_app_name) %>

+ <% end %> +
<% end %> -

<%= t(".by_signing_up_html", - :tou_link => link_to(t("layouts.tou"), - "https://wiki.osmfoundation.org/wiki/Terms_of_Use", - :target => :new), - :privacy_policy_link => link_to(t(".privacy_policy"), - t(".privacy_policy_url"), - :title => t(".privacy_policy_title"), - :target => :new), - :contributor_terms_link => link_to(t(".contributor_terms"), - t(".contributor_terms_url"), - :target => :new)) %>

- +

<%= t(".by_signing_up_html", + :tou_link => link_to(t("layouts.tou"), + "https://wiki.osmfoundation.org/wiki/Terms_of_Use", + :target => :new), + :privacy_policy_link => link_to(t(".privacy_policy"), + t(".privacy_policy_url"), + :title => t(".privacy_policy_title"), + :target => :new), + :contributor_terms_link => link_to(t(".contributor_terms"), + t(".contributor_terms_url"), + :target => :new)) %>

<%= f.form_group do %> <%= f.check_box :consider_pd, :tabindex => 5, @@ -84,8 +96,12 @@ <% end %> - <% if current_user.auth_uid.nil? %> -
+ <% if current_user.auth_uid.nil? and @preferred_auth_provider.nil? %> +
+
+
<%= t ".use external auth" %>
+
+
<%= render :partial => "auth_providers" %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 677ff618c..4570183d0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1848,15 +1848,15 @@ en: new: title: "Log in" tab_title: "Log in" - heading: "Log in" + login_to_authorize_html: "Log in to OpenStreetMap to access %{client_app_name}." email or username: "Email Address or Username" password: "Password" remember: "Remember me" lost password link: "Lost your password?" login_button: "Log in" register now: Register now - with external: "Alternatively, use a third party to log in:" - no account: Don't have an account? + with external: "or log in with a third party" + or: "or" auth failure: "Sorry, could not log in with those details." destroy: title: "Logout" @@ -2576,7 +2576,6 @@ en: openid_logo_alt: "Log in with an OpenID" openid_html: "%{logo} OpenID" openid_login_button: "Continue" - with external: "Alternatively, use a third party to login:" openid: title: Log in with OpenID alt: Log in with an OpenID URL @@ -2730,22 +2729,22 @@ en: new: title: "Sign Up" tab_title: "Sign up" + signup_to_authorize_html: "Sign up with OpenStreetMap to access %{client_app_name}." no_auto_account_create: "Unfortunately we are not currently able to create an account for you automatically." please_contact_support_html: 'Please contact %{support_link} to arrange for an account to be created - we will try and deal with the request as quickly as possible.' support: support about: header: Free and editable. paragraph_1: Unlike other maps, OpenStreetMap is completely created by people like you, and it's free for anyone to fix, update, download and use. - paragraph_2: Sign up to get started contributing. We'll send an email to confirm your account. + paragraph_2: Sign up to get started contributing. welcome: "Welcome to OpenStreetMap" duplicate_social_email: "If you already have an OpenStreetMap account and wish to use a 3rd party identity provider, please log in using your password and modify the settings of your account." display name description: "Your publicly displayed username. You can change this later in the preferences." - by_signing_up_html: "By signing up, you agree to our %{contributor_terms_link} and %{tou_link}." + by_signing_up_html: "By signing up, you agree to our %{tou_link}, %{privacy_policy_link} and %{contributor_terms_link}." + tou: "terms of use" contributor_terms_url: "https://wiki.osmfoundation.org/wiki/Licence/Contributor_Terms" - contributor_terms: "Contributor terms" + contributor_terms: "contributor terms" external auth: "Third Party Authentication:" - use external auth: "Alternatively, use a third party to log in" - auth no password: "With third party authentication a password is not required, but some extra tools or server may still need one." continue: Sign Up terms accepted: "Thanks for accepting the new contributor terms!" email_help_html: 'Your address is not displayed publicly, see our %{privacy_policy_link} for more information.' @@ -2755,6 +2754,8 @@ en: consider_pd_html: "I consider my contributions to be in the %{consider_pd_link}." consider_pd: "public domain" consider_pd_url: https://wiki.osmfoundation.org/wiki/Licence_and_Legal_FAQ/Why_would_I_want_my_contributions_to_be_public_domain + or: "or" + use external auth: "or sign up with a third party" terms: title: "Terms" heading: "Terms" diff --git a/test/helpers/user_helper_test.rb b/test/helpers/user_helper_test.rb index f7d2726db..c2883c2c0 100644 --- a/test/helpers/user_helper_test.rb +++ b/test/helpers/user_helper_test.rb @@ -116,8 +116,8 @@ class UserHelperTest < ActionView::TestCase def test_auth_button button = auth_button("google", "google") - img_tag = "\"Log" - assert_equal("#{img_tag}", button) + img_tag = "\"Log" + assert_equal("#{img_tag}", button) end private diff --git a/test/system/user_signup_test.rb b/test/system/user_signup_test.rb index 7e2c6ba54..0835df741 100644 --- a/test/system/user_signup_test.rb +++ b/test/system/user_signup_test.rb @@ -4,7 +4,7 @@ class UserSignupTest < ApplicationSystemTestCase test "Sign up from login page" do visit login_path - click_on "Register now" + click_on "Sign up" assert_content "Confirm Password" end