From 0a8c26e596b40da6d428a5c91db0d815d13c89e4 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Tue, 14 Aug 2007 23:07:38 +0000 Subject: [PATCH] Various updates to the user management, including the creation of a preferences table and moving tokens into a tokens table so that a user can have more than one. --- app/controllers/amf_controller.rb | 9 +++-- app/controllers/application.rb | 24 ++++++------ app/controllers/swf_controller.rb | 7 ++-- app/controllers/user_controller.rb | 55 +++++++++++++-------------- app/models/notifier.rb | 8 ++-- app/models/user.rb | 35 +++++++++-------- app/models/user_preference.rb | 3 ++ app/models/user_token.rb | 8 ++++ app/views/site/edit.rhtml | 4 +- db/migrate/004_user_enhancements.rb | 58 +++++++++++++++++++++++++++++ lib/osm.rb | 12 ++++++ 11 files changed, 150 insertions(+), 73 deletions(-) create mode 100644 app/models/user_preference.rb create mode 100644 app/models/user_token.rb create mode 100644 db/migrate/004_user_enhancements.rb diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index 2e2f112cd..8db813b38 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -736,12 +736,13 @@ def array2tag(a) end def getuserid(token) - token=sqlescape(token) - if (token=~/^(.+)\+(.+)$/) then - return ActiveRecord::Base.connection.select_value("SELECT id FROM users WHERE active=1 AND email='#{$1}' AND pass_crypt=MD5('#{$2}')") + if (token =~ /^(.+)\+(.+)$/) then + user = User.authenticate(:username => $1, :password => $2) else - return ActiveRecord::Base.connection.select_value("SELECT id FROM users WHERE active=1 AND token='#{token}'") + user = User.authenticate(:token => token) end + + return user ? user.id : nil; end diff --git a/app/controllers/application.rb b/app/controllers/application.rb index 085a3fa9f..bc61db5f9 100644 --- a/app/controllers/application.rb +++ b/app/controllers/application.rb @@ -3,7 +3,12 @@ class ApplicationController < ActionController::Base def authorize_web - @user = User.find_by_token(session[:token]) + if session[:user] + @user = User.find(session[:user]) + elsif session[:token] + @user = User.authenticate(:token => session[:token]) + session[:user] = @user.id + end end def require_user @@ -16,21 +21,13 @@ class ApplicationController < ActionController::Base if username.nil? @user = nil # no authentication provided - perhaps first connect (client should retry after 401) elsif username == 'token' - @user = User.authenticate_token(passwd) # preferred - random token for user from db, passed in basic auth + @user = User.authenticate(:token => passwd) # preferred - random token for user from db, passed in basic auth else - @user = User.authenticate(username, passwd) # basic auth + @user = User.authenticate(:username => username, :password => passwd) # basic auth end # handle authenticate pass/fail - if @user - # user exists and password is correct ... horray! - if @user.methods.include? 'lastlogin' # note last login - @session['lastlogin'] = user.lastlogin - @user.last.login = Time.now - @user.save() - @session["User.id"] = @user.id - end - else + unless @user # no auth, the user does not exist or the password was wrong response.headers["Status"] = "Unauthorized" response.headers["WWW-Authenticate"] = "Basic realm=\"#{realm}\"" @@ -58,8 +55,9 @@ class ApplicationController < ActionController::Base response.headers['Error'] = message end +private + # extract authorisation credentials from headers, returns user = nil if none - private def get_auth_data if request.env.has_key? 'X-HTTP_AUTHORIZATION' # where mod_rewrite might have put it authdata = request.env['X-HTTP_AUTHORIZATION'].to_s.split diff --git a/app/controllers/swf_controller.rb b/app/controllers/swf_controller.rb index 4e7e1c8ee..b8208050c 100644 --- a/app/controllers/swf_controller.rb +++ b/app/controllers/swf_controller.rb @@ -46,12 +46,11 @@ class SwfController < ApplicationController lastfile='-1' if params['token'] - token=sqlescape(params['token']) + user=User.authenticate(:token => params[:token]) sql="SELECT gps_points.latitude*0.000001 AS lat,gps_points.longitude*0.000001 AS lon,gpx_files.id AS fileid,UNIX_TIMESTAMP(gps_points.timestamp) AS ts "+ - " FROM gpx_files,gps_points,users "+ + " FROM gpx_files,gps_points "+ "WHERE gpx_files.id=gpx_id "+ - " AND gpx_files.user_id=users.id "+ - " AND token='#{token}' "+ + " AND gpx_files.user_id=#{user.id} "+ " AND (gps_points.longitude BETWEEN #{xminr} AND #{xmaxr}) "+ " AND (gps_points.latitude BETWEEN #{yminr} AND #{ymaxr}) "+ " AND (gps_points.timestamp IS NOT NULL) "+ diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index d945e53a2..5c95de0c9 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -10,11 +10,11 @@ class UserController < ApplicationController def save @title = 'create account' @user = User.new(params[:user]) - @user.set_defaults if @user.save + token = @user.tokens.create flash[:notice] = "User was successfully created. Check your email for a confirmation note, and you\'ll be mapping in no time :-)
Please note that you won't be able to login until you've received and confirmed your email address." - Notifier::deliver_signup_confirm(@user) + Notifier::deliver_signup_confirm(@user, token) redirect_to :action => 'login' else render :action => 'new' @@ -64,11 +64,10 @@ class UserController < ApplicationController def lost_password @title = 'lost password' if params[:user] and params[:user][:email] - user = User.find_by_email(params['user']['email']) + user = User.find_by_email(params[:user][:email]) if user - user.token = User.make_token - user.save - Notifier::deliver_lost_password(user) + token = user.tokens.create + Notifier::deliver_lost_password(user, token) flash[:notice] = "Sorry you lost it :-( but an email is on its way so you can reset it soon." else flash[:notice] = "Couldn't find that email address, sorry." @@ -81,13 +80,15 @@ class UserController < ApplicationController def reset_password @title = 'reset password' if params['token'] - user = User.find_by_token(params['token']) - if user - pass = User.make_token(8) + token = UserToken.find_by_token(params[:token]) + if token + pass = OSM::make_token(8) + user = token.user user.pass_crypt = pass user.pass_crypt_confirmation = pass user.active = true - user.save + user.save! + token.destroy Notifier::deliver_reset_password(user, pass) flash[:notice] = "Your password has been changed and is on its way to your mailbox :-)" else @@ -106,19 +107,16 @@ class UserController < ApplicationController if params[:user] email = params[:user][:email] pass = params[:user][:password] - u = User.authenticate(email, pass) - if u - u.token = User.make_token - u.timeout = 1.day.from_now - u.save - session[:token] = u.token + user = User.authenticate(:username => email, :password => pass) + if user + session[:user] = user.id if params[:referer] redirect_to params[:referer] else redirect_to :controller => 'site', :action => 'index' end return - elsif User.authenticate(email, pass, false) + elsif User.authenticate(:username => email, :password => pass, :invalid => true) flash[:notice] = "Sorry, your account is not active yet.
Please click on the link in the account confirmation email to activate your account." else flash[:notice] = "Sorry, couldn't log in with those details." @@ -128,14 +126,13 @@ class UserController < ApplicationController def logout if session[:token] - u = User.find_by_token(session[:token]) - if u - u.token = User.make_token - u.timeout = Time.now - u.save + token = UserToken.find_by_token(session[:token]) + if token + token.destroy end + session[:token] = nil end - session[:token] = nil + session[:user] = nil if params[:referer] redirect_to params[:referer] else @@ -144,14 +141,14 @@ class UserController < ApplicationController end def confirm - @user = User.find_by_token(params[:confirm_string]) - if @user && @user.active == 0 + token = UserToken.find_by_token(params[:confirm_string]) + if token and !token.user.active? + @user = token.user @user.active = true - @user.token = User.make_token - @user.timeout = 1.day.from_now - @user.save + @user.save! + token.destroy flash[:notice] = 'Confirmed your account, thanks for signing up!' - session[:token] = @user.token + session[:user] = @user.id redirect_to :action => 'account', :display_name => @user.display_name else flash[:notice] = 'Something went wrong confirming that user.' diff --git a/app/models/notifier.rb b/app/models/notifier.rb index b064ee1f0..aaad67d0e 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -1,17 +1,17 @@ class Notifier < ActionMailer::Base - def signup_confirm( user ) + def signup_confirm( user, token ) @recipients = user.email @from = 'abuse@openstreetmap.org' @subject = '[OpenStreetMap] Confirm your email address' - @body['url'] = "http://#{SERVER_URL}/user/confirm?confirm_string=#{user.token}" + @body['url'] = "http://#{SERVER_URL}/user/confirm?confirm_string=#{token.token}" end - def lost_password( user ) + def lost_password( user, token ) @recipients = user.email @from = 'abuse@openstreetmap.org' @subject = '[OpenStreetMap] Password reset request' - @body['url'] = "http://#{SERVER_URL}/user/reset_password?email=#{user.email}&token=#{user.token}" + @body['url'] = "http://#{SERVER_URL}/user/reset_password?email=#{user.email}&token=#{token.token}" end def reset_password(user, pass) diff --git a/app/models/user.rb b/app/models/user.rb index 783a9bc3b..bc0c9966c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -7,6 +7,8 @@ class User < ActiveRecord::Base has_many :messages, :foreign_key => :to_user_id has_many :new_messages, :class_name => "Message", :foreign_key => :to_user_id, :conditions => "message_read = 0" has_many :friends + has_many :tokens, :class_name => "UserToken" + has_many :preferences, :class_name => "UserPreference" validates_confirmation_of :pass_crypt, :message => 'Password must match the confirmation password' validates_uniqueness_of :display_name, :allow_nil => true @@ -18,34 +20,31 @@ class User < ActiveRecord::Base before_save :encrypt_password - def set_defaults + def after_initialize self.creation_time = Time.now - self.timeout = Time.now - self.token = User.make_token() end def encrypt_password self.pass_crypt = Digest::MD5.hexdigest(pass_crypt) unless pass_crypt_confirmation.nil? end - def self.authenticate(email, passwd, active = true) - find(:first, :conditions => [ "email = ? AND pass_crypt = ? AND active = ?", email, Digest::MD5.hexdigest(passwd), active]) - end - - def self.authenticate_token(token) - find(:first, :conditions => [ "token = ? ", token]) - end - - def self.make_token(length=30) - chars = 'abcdefghijklmnopqrtuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' - confirmstring = '' + def self.authenticate(options) + if options[:username] and options[:password] + user = find(:first, :conditions => ["email = ? OR display_name = ?", options[:username], options[:username]]) + user = nil unless user.pass_crypt == Digest::MD5.hexdigest(options[:password]) + elsif options[:token] + token = UserToken.find(:first, :include => :user, :conditions => ["user_tokens.token = ?", options[:token]]) + user = token.user if token + end - length.times do - confirmstring += chars[(rand * chars.length).to_i].chr + if user + user = nil unless user.active? or options[:inactive] end - return confirmstring - end + token.update_attribute(:expiry, 1.week.from_now) if token and user + + return user + end def to_xml doc = OSM::API.new.get_xml_doc diff --git a/app/models/user_preference.rb b/app/models/user_preference.rb new file mode 100644 index 000000000..9584d13e4 --- /dev/null +++ b/app/models/user_preference.rb @@ -0,0 +1,3 @@ +class UserPreference < ActiveRecord::Base + belongs_to :user +end diff --git a/app/models/user_token.rb b/app/models/user_token.rb new file mode 100644 index 000000000..a795de034 --- /dev/null +++ b/app/models/user_token.rb @@ -0,0 +1,8 @@ +class UserToken < ActiveRecord::Base + belongs_to :user + + def after_initialize + self.token = OSM::make_token() if self.token.blank? + self.expiry = 1.week.from_now if self.expiry.blank? + end +end diff --git a/app/views/site/edit.rhtml b/app/views/site/edit.rhtml index 08d6133c4..548a1acaf 100644 --- a/app/views/site/edit.rhtml +++ b/app/views/site/edit.rhtml @@ -5,6 +5,8 @@ <% else %> <%= render :partial => 'search', :locals => { :onopen => "resizeMap();", :onclose => "resizeMap();" } %> +<% session[:token] = @user.tokens.create.token unless session[:token] %> + <% if params['mlon'] and params['mlat'] %> <% lon = params['mlon'] %> <% lat = params['mlat'] %> @@ -34,7 +36,7 @@ fo.addVariable('lat',lat); fo.addVariable('long',lon); fo.addVariable('scale',sc); - fo.addVariable('token','<%= @user.token %>'); + fo.addVariable('token','<%= session[:token] %>'); fo.write("map"); } diff --git a/db/migrate/004_user_enhancements.rb b/db/migrate/004_user_enhancements.rb new file mode 100644 index 000000000..92f01bf5d --- /dev/null +++ b/db/migrate/004_user_enhancements.rb @@ -0,0 +1,58 @@ +class UserEnhancements < ActiveRecord::Migration + def self.up + add_column "diary_entries", "latitude", :double + add_column "diary_entries", "longitude", :double + add_column "diary_entries", "language", :string, :limit => 3 + + create_table "user_preferences", innodb_table do |t| + t.column "user_id", :bigint, :limit => 20, :null => false + t.column "k", :string, :null => false + t.column "v", :string, :null => false + end + + add_primary_key "user_preferences", ["user_id", "k"] + + create_table "user_tokens", innodb_table do |t| + t.column "id", :bigint, :limit => 20, :null => false + t.column "user_id", :bigint, :limit => 20, :null => false + t.column "token", :string, :null => false + t.column "expiry", :datetime, :null => false + end + + add_primary_key "user_tokens", ["id"] + add_index "user_tokens", ["token"], :name => "user_tokens_token_idx", :unique => true + add_index "user_tokens", ["user_id"], :name => "user_tokens_user_id_idx" + + change_column "user_tokens", "id", :bigint, :limit => 20, :null => false, :options => "AUTO_INCREMENT" + + User.find(:all, :conditions => "token is not null").each do |user| + UserToken.create(:user_id => user.id, :token => user.token, :expiry => 1.week.from_now) + end + + remove_column "users", "token" + remove_column "users", "timeout" + remove_column "users", "within_lon" + remove_column "users", "within_lat" + add_column "users", "nearby", :integer, :default => 50 + add_column "users", "pass_salt", :string + + User.update_all("nearby = 50"); + end + + def self.down + remove_column "users", "pass_salt" + remove_column "users", "nearby" + add_column "users", "within_lat", :double + add_column "users", "within_lon", :double + add_column "users", "timeout", :datetime + add_column "users", "token", :string + + drop_table "user_tokens" + + drop_table "user_preferences" + + remove_column "diary_entries", "language" + remove_column "diary_entries", "longitude" + remove_column "diary_entries", "latitude" + end +end diff --git a/lib/osm.rb b/lib/osm.rb index 5f31e31bf..ea2a58164 100644 --- a/lib/osm.rb +++ b/lib/osm.rb @@ -391,4 +391,16 @@ module OSM rescue Exception return nil end + + # Construct a random token of a given length + def self.make_token(length = 30) + chars = 'abcdefghijklmnopqrtuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' + token = '' + + length.times do + token += chars[(rand * chars.length).to_i].chr + end + + return token + end end -- 2.43.2