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.

diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb
index 2e2f112cd416441b62543023ecd85a355cfa4826..8db813b38602e1c5348de7a20017f7851909c902 100644 (file)
--- a/app/controllers/amf_controller.rb
+++ b/app/controllers/amf_controller.rb
@@ -736,12 +736,13 @@ def array2tag(a)
 end
 
 def getuserid(token)
 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
   else

-    return ActiveRecord::Base.connection.select_value("SELECT id FROM users WHERE active=1 AND token='#{token}'")
+    user = User.authenticate(:token => token)

   end
   end

+
+  return user ? user.id : nil;

 end
 
 
 end
 
 

diff --git a/app/controllers/application.rb b/app/controllers/application.rb
index 085a3fa9fe2dcf0ebe37c0e99c28935500bb9b74..bc61db5f95493e2c51dfcc2378b85883661d5c02 100644 (file)
--- a/app/controllers/application.rb
+++ b/app/controllers/application.rb
@@ -3,7 +3,12 @@
 class ApplicationController < ActionController::Base
 
   def authorize_web
 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
   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' 
     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
     else

-      @user = User.authenticate(username, passwd) # basic auth
+      @user = User.authenticate(:username => username, :password => passwd) # basic auth

     end
 
     # handle authenticate pass/fail
     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}\"" 
       # 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
 
     response.headers['Error'] = message
   end
 

+private 
+

   # extract authorisation credentials from headers, returns user = nil if none
   # 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 
   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 4e7e1c8ee85b601ce5fc5954f030eed14e984d2a..b8208050c02187ea56de55bf049594ca3d5ece95 100644 (file)
--- a/app/controllers/swf_controller.rb
+++ b/app/controllers/swf_controller.rb
@@ -46,12 +46,11 @@ class SwfController < ApplicationController
                lastfile='-1'
        
                if params['token']
                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 "+
                        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 "+
                                 "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) "+
                                 "  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 d945e53a24be6c0a186d346d75715fdb13123e13..5c95de0c9cad315e69830f3dff116e198e4386f4 100644 (file)
--- 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])
   def save
     @title = 'create account'
     @user = User.new(params[:user])

-    @user.set_defaults

 
     if @user.save
 
     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."
       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'
       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]
   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
       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."
         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']
   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.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
         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]
     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
         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."
         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]
 
   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
       end

+      session[:token] = nil

     end
     end

-    session[:token] = nil
+    session[:user] = nil

     if params[:referer]
       redirect_to params[:referer]
     else
     if params[:referer]
       redirect_to params[:referer]
     else


@@ -144,14 +141,14 @@ class UserController < ApplicationController
   end
 
   def confirm
   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.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!'
       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.'
       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 b064ee1f0350ff348251668de20a3e80c26fbdf8..aaad67d0ea68a95497ced34dc9111b7eac2baefb 100644 (file)
--- a/app/models/notifier.rb
+++ b/app/models/notifier.rb
@@ -1,17 +1,17 @@
 class Notifier < ActionMailer::Base
 
 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'
     @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
 
   end
 

-  def lost_password( user )
+  def lost_password( user, token )

     @recipients = user.email
     @from = 'abuse@openstreetmap.org'
     @subject = '[OpenStreetMap] Password reset request'
     @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)
   end
 
   def reset_password(user, pass)

diff --git a/app/models/user.rb b/app/models/user.rb
index 783a9bc3b2f333fe604625424014f07d93a052ad..bc0c9966ce35e28d34e16b6a764e23d64db40075 100644 (file)
--- 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 :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
 
   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
 
 
   before_save :encrypt_password
 

-  def set_defaults
+  def after_initialize

     self.creation_time = Time.now
     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
 
   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
 
     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
 
   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 (file)
index 0000000..9584d13
--- /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 (file)
index 0000000..a795de0
--- /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 08d6133c4aa17ea66c6be1ff09b524b8543c0e46..548a1acaf7b4a96b461389f40a44126ae52d2850 100644 (file)
--- 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();" } %>
 
 <% 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']  %>
 <% 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('lat',lat);
     fo.addVariable('long',lon);
     fo.addVariable('scale',sc);

-    fo.addVariable('token','<%= @user.token %>');
+    fo.addVariable('token','<%= session[:token] %>');

     fo.write("map");
   }
 
     fo.write("map");
   }
 

diff --git a/db/migrate/004_user_enhancements.rb b/db/migrate/004_user_enhancements.rb
new file mode 100644 (file)
index 0000000..92f01bf
--- /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 5f31e31bf3f83f2b232cd1687c0c7b6657d19bff..ea2a581645d797e7fd677cf31629863f569b2967 100644 (file)
--- a/lib/osm.rb
+++ b/lib/osm.rb
@@ -391,4 +391,16 @@ module OSM
   rescue Exception
     return nil
   end
   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
 end