Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Adds identites model, please read the 'migrations' part of the readme

  • Loading branch information...
commit 9471aac90173c3dc566afe9cb6b5050ddbe2c692 1 parent c73fd91
@TheEmpty authored
View
15 README.textile
@@ -70,9 +70,18 @@ In @config/application.rb@ or @config/environments/YOUR_ENV.rb@ (to set a differ
</pre>
*Migrations*
-
-Your model needs one attribute/column to store the RPX identifier. By default, this identifier is @rpx_identifier@.
-So don't forget to add that field to your model (using a migration or whatever...).
+Run the following command
+<pre>
+ rails g model identity user_id:integer identifier:string
+</pre>
+Add the following to your user model
+<pre>
+ has_many :identities
+</pre>
+Then commit the model by running
+<pre>
+ rake db:migrate
+</pre>
*Views*
View
11 devise_rpx_connectable_generator.rb
@@ -0,0 +1,11 @@
+puts "loading generator..."
+
+class DeviseRpxConnectableGenerator < Rails::Generators::Base
+ source_root File.expand_path("../templates", __FILE__)
+
+ def copy_migration
+ migration_template "migration.rb", "db/migrate/devise_add_identifiers.rb"
+ end
+end
+
+puts "loaded"
View
5 init.rb
@@ -1,2 +1,5 @@
# encoding: utf-8
-require 'devise_rpx_connectable'
+require 'devise_rpx_connectable'
+puts "trying to require generator"
+require '../devise_rpx_connectable_generator'
+puts "required"
View
1  lib/devise_rpx_connectable.rb
@@ -10,7 +10,6 @@
require 'devise_rpx_connectable/strategy'
Warden::Strategies.add(:rpx_connectable, Devise::RpxConnectable::Strategies::RpxConnectable)
-require 'devise_rpx_connectable/schema'
require 'devise_rpx_connectable/view_helpers'
module Devise
View
16 lib/devise_rpx_connectable/model.rb
@@ -33,8 +33,6 @@ def self.included(base) #:nodoc:
# Store RPX account/session credentials.
#
def store_rpx_credentials!(attributes = {})
- self.send(:"#{self.class.rpx_identifier_field}=", attributes[:identifier])
-
# Confirm without e-mail - if confirmable module is loaded.
self.skip_confirmation! if self.respond_to?(:skip_confirmation!)
@@ -141,13 +139,16 @@ def authenticate_with_rpx(attributes = {})
end
if !user and attributes[:email]
- user = self.find_by_email(attributes[:email])
+ if user = self.find_by_email(attributes[:email])
@tardate
tardate added a note

You might want to reconsider this approach to mapping identities, as I think it creates a pretty serious security vulnerability.

If I'm reading this correctly, it means that a new user's login will be automatically attached to an existing user account if the email addresses match. There are two issues:

  • attributes[:email] is not guaranteed to be verified. There is another attribute[:verified_email] I believe which is supposed to be verified by the provider.
  • but even if you are using attribute[:verified_email] instead, you have still put all your trust in the other provider to have ensured the email matches the user

So if any provider allows an unverified attribute[:email] to be sent, it creates a perfect way for anyone to hijack one of your legitimate user's accounts and gain full control of it.

I think the better approach, which avoids the issue of trusting an email address from a 3rd party provider, is to attach a new credential +after+ the user is signed in (i.e. you "sign in" again with a new credential when already logged in. RPX has a mechanism to send this new request to another url or with additional params so that it can be distinguished). I did something along these lines with https://github.com/tardate/authlogic_rpx (a dusty project - authlogic and rails 2)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ user.identities.new(:identifier => attributes[:identifier]) #build_identity?
+ user.save!
+ end
end
return user
rescue
- raise StandardError, "Oi! error in authenticate_with_rpx()"
+ raise StandardError, "Error in authenticate_with_rpx() -> #{$!}"
end
end
@@ -158,7 +159,12 @@ def authenticate_with_rpx(attributes = {})
# namedscope to filter records while authenticating.
#
def find_for_rpx(identifier)
- self.first(:conditions => { rpx_identifier_field => identifier })
+ #self.first(:conditions => { rpx_identifier_field => identifier })
+ @identity = Identity.first(:conditions => ["identifier = ?", identifier])
+ if @identity
+ return User.find @identity.user_id
+ end
+ return false
end
# Contains the logic used in authentication. Overwritten by other devise modules.
View
14 lib/devise_rpx_connectable/strategy.rb
@@ -24,24 +24,26 @@ def authenticate!
if user = klass.authenticate_with_rpx(rpx_data)
user.on_before_rpx_success(rpx_data)
- success!(user)
- return
+ success!(user) and return
end
begin
-
fail!(:rpx_invalid) and return unless klass.rpx_auto_create_account?
user = klass.new
user.store_rpx_credentials!(rpx_data)
user.on_before_rpx_auto_create(rpx_data)
+ # TODO: create a random password here and email the user if we can so that we can check the validations
+ # if the user doesn't have an email, set it to none@none.none and blank the password and salt
user.save(:validate => false)
- user.on_before_rpx_success(rpx_data)
- success!(user)
+ i = Identity.new(:identifier => rpx_data[:identifer])
+ i.save! if i.save # if we got an error with this, it's fine; we can work through it.
+ user.on_before_rpx_success(rpx_data)
+ success!(user) and return
rescue
- fail!(:email_taken)
+ fail!(:email_taken) and return
end
end
View
12 templates/migration.rb
@@ -0,0 +1,12 @@
+class CreateIdentifiers
+ def self.up
+ create_table(:identifiers) do |t|
+ t.column :user_id, :integer
+ t.column :identifier, :string
+ end
+ end
+
+ def self.down
+ drop_table :identifiers
+ end
+end
View
2  todo.txt
@@ -0,0 +1,2 @@
+REMOVE schema.rb
+ADD migration rb
@tardate

You might want to reconsider this approach to mapping identities, as I think it creates a pretty serious security vulnerability.

If I'm reading this correctly, it means that a new user's login will be automatically attached to an existing user account if the email addresses match. There are two issues:

  • attributes[:email] is not guaranteed to be verified. There is another attribute[:verified_email] I believe which is supposed to be verified by the provider.
  • but even if you are using attribute[:verified_email] instead, you have still put all your trust in the other provider to have ensured the email matches the user

So if any provider allows an unverified attribute[:email] to be sent, it creates a perfect way for anyone to hijack one of your legitimate user's accounts and gain full control of it.

I think the better approach, which avoids the issue of trusting an email address from a 3rd party provider, is to attach a new credential +after+ the user is signed in (i.e. you "sign in" again with a new credential when already logged in. RPX has a mechanism to send this new request to another url or with additional params so that it can be distinguished). I did something along these lines with https://github.com/tardate/authlogic_rpx (a dusty project - authlogic and rails 2)

Please sign in to comment.
Something went wrong with that request. Please try again.