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

@@ -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
 
 

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 

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) "+

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 :-)<br>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.<br>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.'

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)

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 = ''
-
-    length.times do
-      confirmstring += chars[(rand * chars.length).to_i].chr
+  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
 
-    return confirmstring
-  end
+    if user
+      user = nil unless user.active? or options[:inactive]
+    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

app/models/user_preference.rb

@@ -0,0 +1,3 @@
+class UserPreference < ActiveRecord::Base
+  belongs_to :user
+end

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

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");
   }
 

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

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