From: Kai Krueger Date: Sat, 23 Jan 2010 15:31:01 +0000 (+0000) Subject: merge 19364:19600 of rails_port into the openID branch X-Git-Tag: live~7275 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/84db1d66b7d819a4a777d2b9e5909cffe82c514e?hp=-c merge 19364:19600 of rails_port into the openID branch renamed migration 049_add_open_id_authentication_tables.rb to 050_add_open_id_authentication_tables.rb --- 84db1d66b7d819a4a777d2b9e5909cffe82c514e diff --combined app/controllers/user_controller.rb index 3a4b00920,ca0046762..f9bbd5dd5 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@@ -15,85 -15,30 +15,87 @@@ class UserController < ApplicationContr before_filter :lookup_this_user, :only => [:activate, :deactivate, :hide, :unhide, :delete] filter_parameter_logging :password, :pass_crypt, :pass_crypt_confirmation - + + cache_sweeper :user_sweeper, :only => [:account, :hide, :unhide, :delete] + def save @title = t 'user.new.title' if Acl.find_by_address(request.remote_ip, :conditions => {:k => "no_account_creation"}) render :action => 'new' else - @user = User.new(params[:user]) - - @user.visible = true - @user.data_public = true - @user.description = "" if @user.description.nil? - @user.creation_ip = request.remote_ip - @user.languages = request.user_preferred_languages + #The redirect from the OpenID provider reenters here again + #and we need to pass the parameters through to the + #open_id_authentication function a second time + if params[:open_id_complete] + openid_verify('', true) + #We have set the user.openid_url to nil beforehand. If it hasn't + #been set to a new valid openid_url, it means the openid couldn't be validated + if @user.nil? or @user.openid_url.nil? + render :action => 'new' + return + end + else + @user = User.new(params[:user]) + + @user.visible = true + @user.data_public = true + @user.description = "" if @user.description.nil? + @user.creation_ip = request.remote_ip + @user.languages = request.user_preferred_languages + #Set the openid_url to nil as for one it is used + #to check if the openid could be validated and secondly + #to not get dupplicate conflicts for an empty openid + @user.openid_url = nil - if @user.save - flash[:notice] = t 'user.new.flash create success message' - Notifier.deliver_signup_confirm(@user, @user.tokens.create(:referer => params[:referer])) - redirect_to :action => 'login' - else - render :action => 'new' - end + if (!params[:user][:openid_url].nil? and params[:user][:openid_url].length > 0) + if @user.pass_crypt.length == 0 + #if the password is empty, but we have a openid + #then generate a random passowrd to disable + #loging in via password + @user.pass_crypt = ActiveSupport::SecureRandom.base64(16) + @user.pass_crypt_confirmation = @user.pass_crypt + end + #Validate all of the other fields before + #redirecting to the openid provider + if !@user.valid? + render :action => 'new' + else + #TODO: Is it a problem to store the user variable with respect to password safty in the session variables? + #Store the user variable in the session for it to be accessible when redirecting back from the openid provider + session[:new_usr] = @user + begin + @norm_openid_url = OpenIdAuthentication.normalize_identifier(params[:user][:openid_url]) + rescue + flash.now[:error] = t 'user.login.openid invalid' + render :action => 'new' + return + end + #Verify that the openid provided is valid and that the user is the owner of the id + openid_verify(@norm_openid_url, true) + #openid_verify can return in two ways: + #Either it returns with a redirect to the openid provider who then freshly + #redirects back to this url if the openid is valid, or if the openid is not plausible + #and no provider for it could be found it just returns + #we want to just let the redirect through + if response.headers["Location"].nil? + render :action => 'new' + end + end + #At this point there was either an error and the page has been rendered, + #or there is a redirect to the openid provider and the rest of the method + #gets executed whenn this method gets reentered after redirecting back + #from the openid provider + return + end + end + if @user.save + flash[:notice] = t 'user.new.flash create success message' + Notifier.deliver_signup_confirm(@user, @user.tokens.create(:referer => params[:referer])) + redirect_to :action => 'login' + else + render :action => 'new' + end end end @@@ -101,15 -46,6 +103,15 @@@ @title = t 'user.account.title' @tokens = @user.oauth_tokens.find :all, :conditions => 'oauth_tokens.invalidated_at is null and oauth_tokens.authorized_at is not null' + #The redirect from the OpenID provider reenters here again + #and we need to pass the parameters through to the + #open_id_authentication function + if params[:open_id_complete] + openid_verify('', false) + @user.save + return + end + if params[:user] and params[:user][:display_name] and params[:user][:description] if params[:user][:email] != @user.email @user.new_email = params[:user][:email] @@@ -121,10 -57,6 +123,10 @@@ @user.pass_crypt = params[:user][:pass_crypt] @user.pass_crypt_confirmation = params[:user][:pass_crypt_confirmation] end + if (params[:user][:openid_url].length == 0) + #Clearing doesn't need OpenID validation, so we can set it here. + @user.openid_url = nil + end @user.description = params[:user][:description] @user.languages = params[:user][:languages].split(",") @@@ -141,61 -73,9 +143,61 @@@ flash.now[:notice] = t 'user.account.flash update success' end end + + if (params[:user][:openid_url].length > 0) + begin + @norm_openid_url = OpenIdAuthentication.normalize_identifier(params[:user][:openid_url]) + if (@norm_openid_url != @user.openid_url) + #If the OpenID has changed, we want to check that it is a valid OpenID and one + #the user has control over before saving the openID as a password equivalent for + #the user. + openid_verify(@norm_openid_url, false) + end + rescue + flash.now[:error] = t 'user.login.openid invalid' + end + end + + end + end + + def openid_specialcase_mapping(openid_url) + #Special case gmail.com, as it is pontentially a popular OpenID provider and unlike + #yahoo.com, where it works automatically, Google have hidden their OpenID endpoint + #somewhere obscure making it less userfriendly. + if (openid_url.match(/(.*)gmail.com(\/?)$/) or openid_url.match(/(.*)googlemail.com(\/?)$/) ) + return 'https://www.google.com/accounts/o8/id' + end + + return nil + end + + def openid_verify(openid_url,account_create) + authenticate_with_open_id(openid_url) do |result, identity_url| + if result.successful? + #We need to use the openid url passed back from the OpenID provider + #rather than the one supplied by the user, as these can be different. + #e.g. one can simply enter yahoo.com in the login box, i.e. no user specific url + #only once it comes back from the OpenID provider do we know the unique address for + #the user. + @user = session[:new_usr] unless @user #this is used for account creation when the user is not yet in the database + @user.openid_url = identity_url + elsif result.missing? + mapped_id = openid_specialcase_mapping(openid_url) + if mapped_id + openid_verify(mapped_id, account_create) + else + flash.now[:error] = t 'user.login.openid missing provider' + end + elsif result.invalid? + flash.now[:error] = t 'user.login.openid invalid' + else + flash.now[:error] = t 'user.login.auth failure' + end end end + def set_home if params[:user][:home_lat] and params[:user][:home_lon] @user.home_lat = params[:user][:home_lat].to_f @@@ -264,40 -144,22 +266,40 @@@ # The user is logged in already, so don't show them the signup page, instead # send them to the home page redirect_to :controller => 'site', :action => 'index' if session[:user] + + @nickname = params['nickname'] + @email = params['email'] + @openID = params['openid'] end def login + + #The redirect from the OpenID provider reenters here again + #and we need to pass the parameters through to the + # open_id_authentication function + if params[:open_id_complete] + open_id_authentication('') + end + + if params[:user] and session[:user].nil? - email_or_display_name = params[:user][:email] - pass = params[:user][:password] - user = User.authenticate(:username => email_or_display_name, :password => pass) - if user - session[:user] = user.id - elsif User.authenticate(:username => email_or_display_name, :password => pass, :inactive => true) - flash.now[:error] = t 'user.login.account not active' + + if !params[:user][:openid_url].nil? and !params[:user][:openid_url].empty? + open_id_authentication(params[:user][:openid_url]) else - flash.now[:error] = t 'user.login.auth failure' + email_or_display_name = params[:user][:email] + pass = params[:user][:password] + user = User.authenticate(:username => email_or_display_name, :password => pass) + if user + session[:user] = user.id + elsif User.authenticate(:username => email_or_display_name, :password => pass, :inactive => true) + flash.now[:error] = t 'user.login.account not active' + else + flash.now[:error] = t 'user.login.auth failure' + end end end - + if session[:user] # The user is logged in, if the referer param exists, redirect them to that # unless they've also got a block on them, in which case redirect them to @@@ -317,51 -179,6 +319,51 @@@ @title = t 'user.login.title' end + def open_id_authentication(openid_url) + #TODO: only ask for nickname and email, if we don't already have a user for that openID, in which case + #email and nickname are already filled out. I don't know how to do that with ruby syntax though, as we + #don't want to duplicate the do block + #On the other hand it also doesn't matter too much if we ask every time, as the OpenID provider should + #remember these results, and shouldn't repromt the user for these data each time. + authenticate_with_open_id(openid_url, :return_to => request.protocol + request.host_with_port + '/login?referer=' + params[:referer], :optional => [:nickname, :email]) do |result, identity_url, registration| + if result.successful? + #We need to use the openid url passed back from the OpenID provider + #rather than the one supplied by the user, as these can be different. + #e.g. one can simply enter yahoo.com in the login box, i.e. no user specific url + #only once it comes back from the OpenID provider do we know the unique address for + #the user. + user = User.find_by_openid_url(identity_url) + if user + if user.visible? and user.active? + session[:user] = user.id + else + user = nil + flash.now[:error] = t 'user.login.account not active' + end + else + #We don't have a user registered to this OpenID. Redirect to the create account page + #with username and email filled in if they have been given by the OpenID provider through + #the simple registration protocol + redirect_to :controller => 'user', :action => 'new', :nickname => registration['nickname'], :email => registration['email'], :openid => identity_url + end + else if result.missing? + #Try and apply some heuristics to make common cases more userfriendly + mapped_id = openid_specialcase_mapping(openid_url) + if mapped_id + open_id_authentication(mapped_id) + else + flash.now[:error] = t 'user.login.openid missing provider' + end + else if result.invalid? + flash.now[:error] = t 'user.login.openid invalid' + else + flash.now[:error] = t 'user.login.auth failure' + end + end + end + end + end + def logout if session[:token] token = UserToken.find_by_token(session[:token]) diff --combined config/environment.rb index da71cc572,ba5241a21..70a1ddbd5 --- a/config/environment.rb +++ b/config/environment.rb @@@ -42,7 -42,7 +42,7 @@@ Rails::Initializer.run do |config config.frameworks -= [ :active_record ] end - # Specify gems that this application depends on. + # Specify gems that this application depends on. # They can then be installed with "rake gems:install" on new installations. # config.gem "bj" # config.gem "hpricot", :version => '0.6', :source => "http://code.whytheluckystiff.net" @@@ -52,9 -52,9 +52,10 @@@ config.gem 'rmagick', :lib => 'RMagick' config.gem 'oauth', :version => '>= 0.3.6' config.gem 'httpclient' + config.gem 'ruby-openid', :lib => 'openid', :version => '>=2.0.4' + config.gem 'SystemTimer', :version => '>= 1.1.3', :lib => 'system_timer' - # Only load the plugins named here, in the order given. By default, all plugins + # Only load the plugins named here, in the order given. By default, all plugins # in vendor/plugins are loaded in alphabetical order. # :all can be used as a placeholder for all plugins not explicitly named # config.plugins = [ :exception_notification, :ssl_requirement, :all ] @@@ -71,7 -71,7 +72,7 @@@ # Your secret key for verifying cookie session data integrity. # If you change this key, all old sessions will become invalid! - # Make sure the secret is at least 30 characters and all random, + # Make sure the secret is at least 30 characters and all random, # no regular words or you'll be exposed to dictionary attacks. config.action_controller.session = { :session_key => '_osm_session', diff --combined config/locales/en.yml index 4ba9c5de2,7819a11f3..5c8e1fe19 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@@ -1235,8 -1235,9 +1235,9 @@@ en trace_not_found: "Trace not found!" visibility: "Visibility:" trace_paging_nav: - showing: "Showing page" - of: "of" + showing_page: "Showing page {{page}}" + next: "Next »" + previous: "« Previous" trace: pending: "PENDING" count_points: "{{count}} points" @@@ -1347,15 -1348,10 +1348,15 @@@ create_account: "create an account" email or username: "Email Address or Username:" password: "Password:" + openid: "OpenID:" + openid description: "Use your OpenID to login" + alternatively: "Alternatively" lost password link: "Lost your password?" login_button: "Login" account not active: "Sorry, your account is not active yet.
Please click on the link in the account confirmation email to activate your account." auth failure: "Sorry, could not log in with those details." + openid missing provider: "Sorry, could not contact your OpenID provider" + openid invalid: "Sorry, your OpenID seems misformed" lost_password: title: "Lost password" heading: "Forgotten Password?" @@@ -1386,8 -1382,6 +1387,8 @@@ display name description: "Your publicly displayed username. You can change this later in the preferences." password: "Password:" confirm password: "Confirm Password:" + openID: "OpenID:" + openID description: '(Optional) If you have an OpenID you can associate it with this account to login' signup: Signup flash create success message: "User was successfully created. Check your email for a confirmation note, and you will be mapping in no time :-)

Please note that you will not be able to login until you've received and confirmed your email address.

If you use an antispam system which sends confirmation requests then please make sure you whitelist webmaster@openstreetmap.org as we are unable to reply to any confirmation requests." no_such_user: @@@ -1454,10 -1448,6 +1455,10 @@@ title: "Edit account" my settings: My settings email never displayed publicly: "(never displayed publicly)" + openid: + openid: "OpenID:" + link: "http://wiki.openstreetmap.org/wiki/OpenID" + link text: "what is this?" public editing: heading: "Public editing:" enabled: "Enabled. Not anonymous and can edit data." diff --combined db/migrate/050_add_open_id_authentication_tables.rb index 7dfff209d,000000000..7dfff209d mode 100644,000000..100644 --- a/db/migrate/050_add_open_id_authentication_tables.rb +++ b/db/migrate/050_add_open_id_authentication_tables.rb @@@ -1,30 -1,0 +1,30 @@@ +class AddOpenIdAuthenticationTables < ActiveRecord::Migration + def self.up + create_table :open_id_authentication_associations, :force => true do |t| + t.integer :issued, :lifetime + t.string :handle, :assoc_type + t.binary :server_url, :secret + end + + create_table :open_id_authentication_nonces, :force => true do |t| + t.integer :timestamp, :null => false + t.string :server_url, :null => true + t.string :salt, :null => false + end + + add_column :users, :openid_url, :string + + add_index :users, [:openid_url], :name => "user_openid_unique_idx", :unique => true + add_index :open_id_authentication_associations, [:server_url], :name => "open_id_associations_server_url_idx" + add_index :open_id_authentication_nonces, [:timestamp], :name => "open_id_nonces_timestamp_idx" + end + + def self.down + remove_index :users, :name => "user_openid_unique_idx" + remove_index :open_id_authentication_associations, :name => "open_id_associations_server_url_idx" + remove_index :open_id_authentication_nonces, :name => "open_id_nonces_timestamp_idx" + remove_column :users, :openid_url + drop_table :open_id_authentication_associations + drop_table :open_id_authentication_nonces + end +end