diff --git a/db/manifest b/db/manifest index ec3b567a2..25fd574e1 100755 --- a/db/manifest +++ b/db/manifest @@ -96,4 +96,5 @@ ms_user_history_add_instruments.sql icecast_config_changed.sql invited_users_facebook_support.sql first_recording_at.sql -share_token.sql \ No newline at end of file +share_token.sql +facebook_signup.sql \ No newline at end of file diff --git a/db/up/facebook_signup.sql b/db/up/facebook_signup.sql new file mode 100644 index 000000000..d27ec0cdd --- /dev/null +++ b/db/up/facebook_signup.sql @@ -0,0 +1,16 @@ +-- when a user authorizes our application to signup, we create this row +CREATE UNLOGGED TABLE facebook_signups ( + id VARCHAR(64) PRIMARY KEY DEFAULT uuid_generate_v4(), + lookup_id VARCHAR(255) UNIQUE NOT NULL, + last_name VARCHAR(100), + first_name VARCHAR(100), + gender VARCHAR(1), + email VARCHAR(1024), + uid VARCHAR(1024), + token VARCHAR(1024), + token_expires_at TIMESTAMP, + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP +); + +ALTER TABLE user_authorizations ADD CONSTRAINT user_authorizations_uniqkey UNIQUE (provider, uid); \ No newline at end of file diff --git a/ruby/lib/jam_ruby.rb b/ruby/lib/jam_ruby.rb index 6ab7a51bb..aa036793d 100755 --- a/ruby/lib/jam_ruby.rb +++ b/ruby/lib/jam_ruby.rb @@ -35,6 +35,7 @@ require "jam_ruby/resque/resque_hooks" require "jam_ruby/resque/scheduled/audiomixer_retry" require "jam_ruby/resque/scheduled/icecast_config_retry" require "jam_ruby/resque/scheduled/icecast_source_check" +require "jam_ruby/resque/scheduled/cleanup_facebook_signup" require "jam_ruby/mq_router" require "jam_ruby/base_manager" require "jam_ruby/connection_manager" @@ -117,6 +118,7 @@ require "jam_ruby/models/icecast_server_socket" require "jam_ruby/models/icecast_template_socket" require "jam_ruby/models/icecast_server_group" require "jam_ruby/models/icecast_mount_template" +require "jam_ruby/models/facebook_signup" include Jampb diff --git a/ruby/lib/jam_ruby/models/facebook_signup.rb b/ruby/lib/jam_ruby/models/facebook_signup.rb new file mode 100644 index 000000000..9e232b2a1 --- /dev/null +++ b/ruby/lib/jam_ruby/models/facebook_signup.rb @@ -0,0 +1,15 @@ +module JamRuby + class FacebookSignup < ActiveRecord::Base + + before_create :generate_lookup_id + + def self.delete_old + FacebookSignup.where("created_at < :week", {:week => 1.week.ago}).delete_all + end + + private + def generate_lookup_id + self.lookup_id = SecureRandom.urlsafe_base64 + end + end +end diff --git a/ruby/lib/jam_ruby/models/user.rb b/ruby/lib/jam_ruby/models/user.rb index 00011b407..e137b8a81 100644 --- a/ruby/lib/jam_ruby/models/user.rb +++ b/ruby/lib/jam_ruby/models/user.rb @@ -697,8 +697,23 @@ module JamRuby # throws ActiveRecord::RecordNotFound if instrument is invalid # throws an email delivery error if unable to connect out to SMTP - def self.signup(first_name, last_name, email, password, password_confirmation, terms_of_service, - location, instruments, birth_date, musician, photo_url, invited_user, signup_confirm_url) + def self.signup(options) + + first_name = options[:first_name] + last_name = options[:last_name] + email = options[:email] + password = options[:password] + password_confirmation = options[:password_confirmation] + terms_of_service = options[:terms_of_service] + location = options[:location] + instruments = options[:instruments] + birth_date = options[:birth_date] + musician = options[:musician] + photo_url = options[:photo_url] + invited_user = options[:invited_user] + fb_signup = options[:fb_signup] + signup_confirm_url = options[:signup_confirm_url] + user = User.new UserManager.active_record_transaction do |user_manager| @@ -744,11 +759,25 @@ module JamRuby user.photo_url = photo_url + unless fb_signup.nil? + user.update_fb_authorization(fb_signup) + + if fb_signup.email.casecmp(user.email).zero? + user.email_confirmed = true + user.signup_token = nil + else + user.email_confirmed = false + user.signup_token = SecureRandom.urlsafe_base64 + end + end if invited_user.nil? user.can_invite = Limits::USERS_CAN_INVITE - user.email_confirmed = false - user.signup_token = SecureRandom.urlsafe_base64 + + unless user.email_confirmed # important that the only time this goes true is if some other mechanism, like fb_signup, set this high + user.email_confirmed = false + user.signup_token = SecureRandom.urlsafe_base64 + end else # if you are invited by an admin, we'll say you can invite too. # but if not, then you can not invite @@ -785,15 +814,10 @@ module JamRuby if user.errors.any? raise ActiveRecord::Rollback else - # don't send an signup email if the user was invited already *and* they used the same email that they were invited with - if !invited_user.nil? && invited_user.email.casecmp(user.email).zero? + # don't send an signup email if email is already confirmed + if user.email_confirmed UserMailer.welcome_message(user).deliver else - - # FIXME: - # It's not standard to require a confirmation when a user signs up with Facebook. - # We should stop asking for it. - # # any errors here should also rollback the transaction; that's OK. If emails aren't going to be delivered, # it's already a really bad situation; make user signup again UserMailer.confirm_email(user, signup_confirm_url.nil? ? nil : (signup_confirm_url + "/" + user.signup_token) ).deliver @@ -941,6 +965,30 @@ module JamRuby end end + # updates an existing user_authorization for facebook, or creates a new one if none exist + def update_fb_authorization(fb_signup) + if fb_signup.uid && fb_signup.token && fb_signup.token_expires_at + + user_authorization = nil + + unless self.new_record? + # see if this user has an existing user_authorization for this provider + user_authorization = UserAuthorization.find_by_user_id_and_provider(self.id, 'facebook') + end + + if user_authorization.nil? + self.user_authorizations.build provider: 'facebook', + uid: fb_signup.uid, + token: fb_signup.token, + token_expiration: fb_signup.token_expires_at + else + user_authorization.uid = fb_signup.uid + user_authorization.token = fb_signup.token + user_authorization.token_expiration = fb_signup.token_expires_at + end + end + end + def provides_location? !self.city.blank? && (!self.state.blank? || !self.country.blank?) end diff --git a/ruby/lib/jam_ruby/models/user_authorization.rb b/ruby/lib/jam_ruby/models/user_authorization.rb index 5f7c97e25..47061e17e 100644 --- a/ruby/lib/jam_ruby/models/user_authorization.rb +++ b/ruby/lib/jam_ruby/models/user_authorization.rb @@ -9,8 +9,10 @@ module JamRuby belongs_to :user, :class_name => "JamRuby::User", :foreign_key => "user_id" validates :provider, :uid, :presence => true + validates_uniqueness_of :uid, scope: :provider # token and token_expiration can be missing - + + end end diff --git a/ruby/lib/jam_ruby/resque/scheduled/cleanup_facebook_signup.rb b/ruby/lib/jam_ruby/resque/scheduled/cleanup_facebook_signup.rb new file mode 100644 index 000000000..a2cbab476 --- /dev/null +++ b/ruby/lib/jam_ruby/resque/scheduled/cleanup_facebook_signup.rb @@ -0,0 +1,19 @@ + +module JamRuby + class CleanupFacebookSignup + + @queue = :cleanup_facebook_signup + + @@log = Logging.logger[CleanupFacebookSignup] + + + def self.perform + @@log.debug("waking up") + + FacebookSignup.delete_old + + @@log.debug("done") + end + + end +end diff --git a/ruby/spec/factories.rb b/ruby/spec/factories.rb index 68b71c553..a8eda4e0a 100644 --- a/ruby/spec/factories.rb +++ b/ruby/spec/factories.rb @@ -317,4 +317,14 @@ FactoryGirl.define do association :authentication, :factory => :icecast_user_authentication end + factory :facebook_signup, :class => JamRuby::FacebookSignup do + sequence(:lookup_id) { |n| "lookup-#{n}"} + sequence(:first_name) { |n| "first-#{n}"} + sequence(:last_name) { |n| "last-#{n}"} + gender 'M' + sequence(:email) { |n| "jammin-#{n}@jamkazam.com"} + sequence(:uid) { |n| "uid-#{n}"} + sequence(:token) { |n| "token-#{n}"} + token_expires_at Time.now + end end diff --git a/ruby/spec/jam_ruby/models/facebook_signup_spec.rb b/ruby/spec/jam_ruby/models/facebook_signup_spec.rb new file mode 100644 index 000000000..6edbab157 --- /dev/null +++ b/ruby/spec/jam_ruby/models/facebook_signup_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe FacebookSignup do + + it "does not delete new one" do + new_signup = FactoryGirl.create(:facebook_signup) + + FacebookSignup.delete_old + + FacebookSignup.find(new_signup) + end + + it "does delete old one" do + old_signup = FactoryGirl.create(:facebook_signup, :created_at => 10.days.ago) + + FacebookSignup.delete_old + + FacebookSignup.find_by_id(old_signup.id).should be_nil + end +end diff --git a/ruby/spec/jam_ruby/models/user_spec.rb b/ruby/spec/jam_ruby/models/user_spec.rb index a29179a8a..a069e7ee0 100644 --- a/ruby/spec/jam_ruby/models/user_spec.rb +++ b/ruby/spec/jam_ruby/models/user_spec.rb @@ -397,6 +397,34 @@ describe User do end end + describe "user_authorizations" do + + it "can create" do + @user.user_authorizations.build provider: 'facebook', + uid: '1', + token: '1', + token_expiration: Time.now + @user.save! + end + + it "fails on duplicate" do + @user.user_authorizations.build provider: 'facebook', + uid: '1', + token: '1', + token_expiration: Time.now + @user.save! + + @user2 = FactoryGirl.create(:user) + @user2.user_authorizations.build provider: 'facebook', + uid: '1', + token: '1', + token_expiration: Time.now + @user2.save.should be_false + @user2.errors[:user_authorizations].should == ['is invalid'] + end + + + end =begin describe "update avatar" do diff --git a/web/app/assets/javascripts/jamkazam.js b/web/app/assets/javascripts/jamkazam.js index d9ec02fba..108c7753a 100644 --- a/web/app/assets/javascripts/jamkazam.js +++ b/web/app/assets/javascripts/jamkazam.js @@ -322,10 +322,15 @@ var hash = context.location.hash; + if(!this.layout.isScreenName(hash)) { + hash = null; + } + var url = '#/home'; if (hash) { url = hash; } + logger.debug("Changing screen to " + url); context.location = url; } diff --git a/web/app/assets/javascripts/layout.js b/web/app/assets/javascripts/layout.js index 53bac88fa..979a73d5d 100644 --- a/web/app/assets/javascripts/layout.js +++ b/web/app/assets/javascripts/layout.js @@ -428,6 +428,18 @@ dialogEvent(dialog, 'afterHide'); } + function isScreenName(screenName) { + if(!screenName) return false; + + var hashIndex = screenName.indexOf('#'); + + if(hashIndex > -1) { + screenName = screenName.substr(hashIndex); + } + + return screenBindings[screenName]; + } + function screenEvent(screen, evtName, data) { if (screen && screen in screenBindings) { if (evtName in screenBindings[screen]) { @@ -752,6 +764,10 @@ }; }; + this.isScreenName = function(screenName) { + return isScreenName(screenName); + } + this.bindScreen = function(screen, handler) { screenBindings[screen] = handler; }; diff --git a/web/app/assets/javascripts/web/signinDialog.js b/web/app/assets/javascripts/web/signinDialog.js new file mode 100644 index 000000000..aded10b15 --- /dev/null +++ b/web/app/assets/javascripts/web/signinDialog.js @@ -0,0 +1,43 @@ +(function(context,$) { + + "use strict"; + + context.JK = context.JK || {}; + + context.JK.SigninDialog = function(app) { + var logger = context.JK.logger; + var rest = context.JK.Rest(); + var dialogId = '#signin-dialog'; + + function events() { + $(dialogId + ' .signin-cancel').click(function(e) { + app.layout.closeDialog('signin-dialog'); + e.stopPropagation(); + return false; + }); + } + + function beforeShow() { + + } + + function afterHide() { + + } + + function initialize(){ + + var dialogBindings = { + 'beforeShow' : beforeShow, + 'afterHide': afterHide + }; + + app.bindDialog('signin-dialog', dialogBindings); + + events(); + } + + this.initialize = initialize; + + } +})(window, jQuery); \ No newline at end of file diff --git a/web/app/assets/javascripts/web/signupDialog.js b/web/app/assets/javascripts/web/signupDialog.js index 459833503..2c85ac2f7 100644 --- a/web/app/assets/javascripts/web/signupDialog.js +++ b/web/app/assets/javascripts/web/signupDialog.js @@ -10,8 +10,11 @@ var dialogId = '#signup-dialog'; function events() { - - + $(dialogId + ' .signup-cancel').click(function(e) { + app.layout.closeDialog('signup-dialog'); + e.stopPropagation(); + return false; + }); } function beforeShow() { @@ -31,7 +34,6 @@ app.bindDialog('signup-dialog', dialogBindings); - console.log("honuth") events(); } diff --git a/web/app/assets/javascripts/web/web.js b/web/app/assets/javascripts/web/web.js index c6489a530..ea049bbd4 100644 --- a/web/app/assets/javascripts/web/web.js +++ b/web/app/assets/javascripts/web/web.js @@ -6,6 +6,7 @@ //= require AAC_underscore //= require globals //= require web/signupDialog +//= require web/signinDialog //= require invitationDialog //= require shareDialog //= require layout diff --git a/web/app/assets/javascripts/web/welcome.js b/web/app/assets/javascripts/web/welcome.js index 7d4728e03..55ed205a1 100644 --- a/web/app/assets/javascripts/web/welcome.js +++ b/web/app/assets/javascripts/web/welcome.js @@ -10,6 +10,12 @@ e.preventDefault(); return false; }); + + $('#signin').click(function(e) { + context.JK.app.layout.showDialog('signin-dialog'); + e.preventDefault(); + return false; + }); } initialize() diff --git a/web/app/controllers/api_users_controller.rb b/web/app/controllers/api_users_controller.rb index 0565d95b5..01eb5107e 100644 --- a/web/app/controllers/api_users_controller.rb +++ b/web/app/controllers/api_users_controller.rb @@ -27,32 +27,6 @@ class ApiUsersController < ApiController respond_with @user, responder: ApiResponder, :status => 200 end - # this API call is disabled by virtue of it being commented out in routes.rb - # the reason is that it has no captcha, and is therefore a bit abuseable - # if someone wants to use it, please add in captcha or some other bot-protector - def create - # sends email to email account for confirmation - @user = UserManager.new.signup(params[:first_name], - params[:last_name], - params[:email], - params[:password], - params[:password_confirmation], - params[:city], - params[:state], - params[:country], - params[:instruments], - params[:photo_url], - ApplicationHelper.base_uri(request) + "/confirm") - - # check for errors - unless @user.errors.any? - render :json => {}, :status => :ok # an empty response, but 200 OK - else - response.status = :unprocessable_entity - respond_with @user, responder: ApiResponder - end - end - def update @user = User.find(params[:id]) diff --git a/web/app/controllers/sessions_controller.rb b/web/app/controllers/sessions_controller.rb index 45e019b27..019c52b99 100644 --- a/web/app/controllers/sessions_controller.rb +++ b/web/app/controllers/sessions_controller.rb @@ -48,15 +48,10 @@ class SessionsController < ApplicationController # an email and whatnot. # # Also, should we grab their photo from facebook? - user = UserManager.new.signup(remote_ip(), - auth_hash[:info][:first_name], - auth_hash[:info][:last_name], - auth_hash[:info][:email], - nil, - nil, - nil, # instruments - nil, # photo_url - nil) + user = UserManager.new.signup(remote_ip: remote_ip(), + first_name: auth_hash[:info][:first_name], + last_name: auth_hash[:info][:last_name], + email: auth_hash[:info][:email]) # Users who sign up using oauth are presumed to have valid email adddresses. user.confirm_email! @@ -72,18 +67,49 @@ class SessionsController < ApplicationController def oauth_callback + + auth_hash = request.env['omniauth.auth'] + + provider = auth_hash[:provider] + + if provider == 'facebook' + fb_uid = auth_hash[:uid] + token = auth_hash[:credentials][:token] + token_expiration = Time.at(auth_hash[:credentials][:expires_at]) + first_name = auth_hash[:extra][:raw_info][:first_name] + last_name = auth_hash[:extra][:raw_info][:last_name] + email = auth_hash[:extra][:raw_info][:email] + gender = auth_hash[:extra][:raw_info][:gender] + + fb_signup = FacebookSignup.new + fb_signup.uid = fb_uid + fb_signup.token = token + fb_signup.token_expires_at = token_expiration + fb_signup.first_name = first_name + fb_signup.last_name = last_name + fb_signup.email = email + if gender == 'male' + fb_signup.gender = 'M' + elsif gender == 'female' + fb_signup.gender = 'F' + end + fb_signup.save! + + redirect_to "#{signup_path}?facebook_signup=#{fb_signup.lookup_id}" + return + end + if current_user.nil? render :nothing => true, :status => 404 return end - auth_hash = request.env['omniauth.auth'] #authorization = UserAuthorization.find_by_provider_and_uid(auth_hash["provider"], auth_hash["uid"]) # Always make and save a new authorization. This is because they expire, and honestly there's no cost # to just making and saving it. #if authorization.nil? - authorization = current_user.user_authorizations.build :provider => auth_hash[:provider], + authorization = current_user.user_authorizations.build :provider => auth_hash[:provider], :uid => auth_hash[:uid], :token => auth_hash[:credentials][:token], :token_expiration => Time.at(auth_hash[:credentials][:expires_at]) diff --git a/web/app/controllers/users_controller.rb b/web/app/controllers/users_controller.rb index e2aae2132..6c98a031d 100644 --- a/web/app/controllers/users_controller.rb +++ b/web/app/controllers/users_controller.rb @@ -27,6 +27,33 @@ class UsersController < ApplicationController return end + @fb_signup = load_facebook_signup(params) + + + # check if the email specified by @fb_signup already exists in the databse--if so, log them in and redirect + if @fb_signup && @fb_signup.email + user = User.find_by_email_and_email_confirmed(@fb_signup, true) + if user + # update user_authorization for user because this is fresher + user.update_fb_authorization(@fb_signup) + sign_in(user) + redirect_to client_url + return + end + end + + # check if the uid specified by @fb_signup already exists in the databse--if so, log them in and redirect + if @fb_signup && @fb_signup.uid + user_authorization = UserAuthorization.find_by_uid_and_provider(@fb_signup.uid, 'facebook') + # update user_authorization for user because this is fresher + if user_authorization + user_authorization.user.update_fb_authorization(@fb_signup) + sign_in(user_authorization.user) + redirect_to client_url + return + end + end + @invited_user = load_invited_user(params) if !@invited_user.nil? && @invited_user.has_required_email? && @invited_user.accepted @@ -34,7 +61,7 @@ class UsersController < ApplicationController render "already_signed_up", :layout => 'landing' return end - @signup_postback = load_postback(@invited_user) + @signup_postback = load_postback(@invited_user, @fb_signup) load_location(request.remote_ip) @@ -42,21 +69,54 @@ class UsersController < ApplicationController @user.musician = true # default the UI to musician as selected option # preseed the form with the invited email as a convenience to the user - unless @invited_user.nil? - @user.email = @invited_user.email + @user.email = @invited_user.email unless @invited_user.nil? + + if @fb_signup + @user.email = @fb_signup.email + @user.first_name = @fb_signup.first_name + @user.last_name = @fb_signup.last_name + @user.gender = @fb_signup.gender end render :layout => 'web' end def create + if current_user redirect_to client_url return end + @fb_signup = load_facebook_signup(params) + + # check if the email specified by @fb_signup already exists in the databse--if so, log them in and redirect + if @fb_signup && @fb_signup.email + user = User.find_by_email_and_email_confirmed(@fb_signup, true) + if user + # update user_authorization for user because this is fresher + user.update_fb_authorization(@fb_signup) + sign_in(user) + redirect_to client_url + return + end + end + + # check if the uid specified by @fb_signup already exists in the databse--if so, log them in and redirect + if @fb_signup && @fb_signup.uid + user_authorization = UserAuthorization.find_by_uid_and_provider(@fb_signup.uid, 'facebook') + # update user_authorization for user because this is fresher + if user_authorization + user_authorization.user.update_fb_authorization(@fb_signup) + sign_in(user_authorization.user) + redirect_to client_url + return + end + + end + @invited_user = load_invited_user(params) - @signup_postback = load_postback(@invited_user) + @signup_postback = load_postback(@invited_user, @fb_signup) @user = User.new @@ -73,21 +133,20 @@ class UsersController < ApplicationController terms_of_service = params[:jam_ruby_user][:terms_of_service].nil? ? false : true musician = params[:jam_ruby_user][:musician] - - @user = UserManager.new.signup(request.remote_ip, - params[:jam_ruby_user][:first_name], - params[:jam_ruby_user][:last_name], - params[:jam_ruby_user][:email], - params[:jam_ruby_user][:password], - params[:jam_ruby_user][:password_confirmation], - terms_of_service, - instruments, - birth_date, - location, - musician, - nil, # we don't accept photo url on the signup form yet - @invited_user, - ApplicationHelper.base_uri(request) + "/confirm") + @user = UserManager.new.signup(remote_ip: request.remote_ip, + first_name: params[:jam_ruby_user][:first_name], + last_name: params[:jam_ruby_user][:last_name], + email: params[:jam_ruby_user][:email], + password: params[:jam_ruby_user][:password], + password_confirmation: params[:jam_ruby_user][:password_confirmation], + terms_of_service: terms_of_service, + instruments: instruments, + birth_date: birth_date, + location: location, + musician: musician, + invited_user: @invited_user, + fb_signup: @fb_signup, + signup_confirm_url: ApplicationHelper.base_uri(request) + "/confirm") # check for errors if @user.errors.any? @@ -308,6 +367,12 @@ class UsersController < ApplicationController return Date.new(year.to_i, month.to_i, day.to_i) end + def load_facebook_signup(params) + lookup_id = params[:facebook_signup] + + FacebookSignup.find_by_lookup_id(lookup_id) + end + def load_invited_user(params) # check if this an anonymous request, or result of invitation code invitation_code = params[:invitation_code] @@ -336,11 +401,10 @@ class UsersController < ApplicationController @cities = @location[:state].nil? ? [] : MaxMindManager.cities(@location[:country], @location[:state]) end - def load_postback(invited_user) - if invited_user.nil? - signup_path - else - signup_path + "?invitation_code=" + invited_user.invitation_code - end + def load_postback(invited_user, fb_signup) + query = {} + query[:invitation_code] = invited_user.invitation_code if invited_user + query[:facebook_signup] = fb_signup.lookup_id if fb_signup + signup_path + "?" + params.to_query end end diff --git a/web/app/views/layouts/web.erb b/web/app/views/layouts/web.erb index 58c07dd86..28f55c2f9 100644 --- a/web/app/views/layouts/web.erb +++ b/web/app/views/layouts/web.erb @@ -50,6 +50,7 @@ <%= render "clients/invitationDialog" %> <%= render "users/signupDialog" %> + <%= render "users/signinDialog" %> diff --git a/web/app/views/users/_signinDialog.html.erb b/web/app/views/users/_signinDialog.html.erb new file mode 100644 index 000000000..9dc8df8a0 --- /dev/null +++ b/web/app/views/users/_signinDialog.html.erb @@ -0,0 +1,58 @@ +
\ No newline at end of file diff --git a/web/app/views/users/_signupDialog.html.erb b/web/app/views/users/_signupDialog.html.erb index 9484902a5..6b19e7b1e 100644 --- a/web/app/views/users/_signupDialog.html.erb +++ b/web/app/views/users/_signupDialog.html.erb @@ -8,7 +8,7 @@