From 38e7ba3a7c7a414c5b087f7f32df1a09403fff89 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 16 Jul 2013 19:38:37 +0200
Subject: first take on identity model - still broken
---
users/app/models/identity.rb | 20 ++++++++++++++++++++
users/test/unit/identity_test.rb | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+)
create mode 100644 users/app/models/identity.rb
create mode 100644 users/test/unit/identity_test.rb
(limited to 'users')
diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
new file mode 100644
index 0000000..a081394
--- /dev/null
+++ b/users/app/models/identity.rb
@@ -0,0 +1,20 @@
+class Identity < CouchRest::Model::Base
+
+ use_database :identities
+
+ belongs_to :user
+
+ property :address
+ property :destination
+
+ def initialize(attribs = {}, &block):q
+ attribs.reverse_merge! user_id: user.id,
+ address: user.main_email_address,
+ destination: user.main_email_address
+ Identity.new attribs
+ end
+
+ design do
+ view :by_user_id
+ end
+end
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
new file mode 100644
index 0000000..389ef89
--- /dev/null
+++ b/users/test/unit/identity_test.rb
@@ -0,0 +1,34 @@
+require 'test_helper'
+
+class IdentityTest < ActiveSupport::TestCase
+
+ setup do
+ @user = FactoryGirl.create(:user)
+ end
+
+ test "user has identity to start with" do
+ id = Identity.new user_id: @user.id
+ id.save
+ assert_equal 1, Identity.by_user_id.key(@user.id).count
+ identity = Identity.find_by_user_id(@user.id)
+ assert_equal @user.email_address, identity.address
+ assert_equal @user.email_address, identity.destination
+ assert_equal @user, identity.user
+ end
+
+ test "add alias" do
+ skip
+ @user.create_identity address: @alias
+ end
+
+ test "add forward" do
+ skip
+ @user.create_identity destination: @external
+ end
+
+ test "forward alias" do
+ skip
+ @user.create_identity address: @alias, destination: @external
+ end
+
+end
--
cgit v1.2.3
From 4a071ef1b33525fa2d1052aa7f21b549447fe767 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 18 Jul 2013 11:31:25 +0200
Subject: move identity creation into user class
It's always based on a user and most default values are based on user properties.
---
users/app/models/identity.rb | 7 -------
users/app/models/user.rb | 13 +++++++++++++
users/test/unit/identity_test.rb | 11 ++++-------
3 files changed, 17 insertions(+), 14 deletions(-)
(limited to 'users')
diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index a081394..9fd0cad 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -7,13 +7,6 @@ class Identity < CouchRest::Model::Base
property :address
property :destination
- def initialize(attribs = {}, &block):q
- attribs.reverse_merge! user_id: user.id,
- address: user.main_email_address,
- destination: user.main_email_address
- Identity.new attribs
- end
-
design do
view :by_user_id
end
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index 413b4ac..dda5a41 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -73,6 +73,19 @@ class User < CouchRest::Model::Base
alias_method :find_by_param, :find
end
+ def create_identity(attribs = {}, &block)
+ build_identity(attribs, &block)
+ identity.save
+ identity
+ end
+
+ def build_identity(attribs = {}, &block)
+ attribs.reverse_merge! user_id: self.id,
+ address: self.email_address,
+ destination: self.email_address
+ Identity.new(attribs, &block)
+ end
+
def to_param
self.id
end
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 389ef89..3130ddc 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -7,13 +7,10 @@ class IdentityTest < ActiveSupport::TestCase
end
test "user has identity to start with" do
- id = Identity.new user_id: @user.id
- id.save
- assert_equal 1, Identity.by_user_id.key(@user.id).count
- identity = Identity.find_by_user_id(@user.id)
- assert_equal @user.email_address, identity.address
- assert_equal @user.email_address, identity.destination
- assert_equal @user, identity.user
+ id = @user.build_identity
+ assert_equal @user.email_address, id.address
+ assert_equal @user.email_address, id.destination
+ assert_equal @user, id.user
end
test "add alias" do
--
cgit v1.2.3
From cc96c60c4617c09379d5e1ddbefa42407329c19a Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 18 Jul 2013 12:12:07 +0200
Subject: testing all versions of emial identities, emails are now strings
---
users/app/models/email.rb | 21 ++++-----------------
users/app/models/identity.rb | 4 ++--
users/app/models/local_email.rb | 20 +++-----------------
users/test/unit/identity_test.rb | 32 +++++++++++++++++++++++++-------
4 files changed, 34 insertions(+), 43 deletions(-)
(limited to 'users')
diff --git a/users/app/models/email.rb b/users/app/models/email.rb
index 6d82f2a..90fc645 100644
--- a/users/app/models/email.rb
+++ b/users/app/models/email.rb
@@ -1,6 +1,5 @@
-module Email
- extend ActiveSupport::Concern
-
+class Email < String
+=begin
included do
validates :email,
:format => {
@@ -8,26 +7,14 @@ module Email
:message => "needs to be a valid email address"
}
end
-
- def initialize(attributes = nil, &block)
- attributes = {:email => attributes} if attributes.is_a? String
- super(attributes, &block)
- end
-
- def to_s
- email
- end
-
- def ==(other)
- other.is_a?(Email) ? self.email == other.email : self.email == other
- end
+=end
def to_partial_path
"emails/email"
end
def to_param
- email
+ to_s
end
end
diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index 9fd0cad..c9c8b73 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -4,8 +4,8 @@ class Identity < CouchRest::Model::Base
belongs_to :user
- property :address
- property :destination
+ property :address, LocalEmail
+ property :destination, Email
design do
view :by_user_id
diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb
index 69cba01..1cadc71 100644
--- a/users/app/models/local_email.rb
+++ b/users/app/models/local_email.rb
@@ -1,25 +1,11 @@
-class LocalEmail
- include CouchRest::Model::Embeddable
- include Email
-
- property :username, String
-
- before_validation :strip_domain_if_needed
-
- validates :username,
- :presence => true,
- :format => { :with => /\A([^@\s]+)(@#{APP_CONFIG[:domain]})?\Z/i, :message => "needs to be a valid login or email address @#{APP_CONFIG[:domain]}"}
+class LocalEmail < Email
+=begin
validate :unique_on_server
validate :unique_alias_for_user
validate :differs_from_login
+=end
- validates :casted_by, :presence => true
-
- def email
- return '' if username.nil?
- username + '@' + APP_CONFIG[:domain]
- end
def email=(value)
return if value.blank?
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 3130ddc..a5d30b0 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -6,7 +6,11 @@ class IdentityTest < ActiveSupport::TestCase
@user = FactoryGirl.create(:user)
end
- test "user has identity to start with" do
+ teardown do
+ @user.destroy
+ end
+
+ test "initial identity for a user" do
id = @user.build_identity
assert_equal @user.email_address, id.address
assert_equal @user.email_address, id.destination
@@ -14,18 +18,32 @@ class IdentityTest < ActiveSupport::TestCase
end
test "add alias" do
- skip
- @user.create_identity address: @alias
+ id = @user.build_identity address: alias_name
+ assert_equal LocalEmail.new(alias_name), id.address
+ assert_equal @user.email_address, id.destination
+ assert_equal @user, id.user
end
test "add forward" do
- skip
- @user.create_identity destination: @external
+ id = @user.build_identity destination: forward_address
+ assert_equal @user.email_address, id.address
+ assert_equal Email.new(forward_address), id.destination
+ assert_equal @user, id.user
end
test "forward alias" do
- skip
- @user.create_identity address: @alias, destination: @external
+ id = @user.build_identity address: alias_name, destination: forward_address
+ assert_equal LocalEmail.new(alias_name), id.address
+ assert_equal Email.new(forward_address), id.destination
+ assert_equal @user, id.user
+ id.save
end
+ def alias_name
+ @alias_name ||= Faker::Internet.user_name
+ end
+
+ def forward_address
+ @forward_address ||= Faker::Internet.email
+ end
end
--
cgit v1.2.3
From a2e49d1b946fa34dd41ce1f07920515df13e09db Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 18 Jul 2013 12:28:51 +0200
Subject: local email adds domain if needed
---
users/app/models/local_email.rb | 20 ++++++++++++--------
users/test/unit/local_email_test.rb | 27 +++++++++++++++++++++++++++
2 files changed, 39 insertions(+), 8 deletions(-)
create mode 100644 users/test/unit/local_email_test.rb
(limited to 'users')
diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb
index 1cadc71..d919102 100644
--- a/users/app/models/local_email.rb
+++ b/users/app/models/local_email.rb
@@ -6,15 +6,17 @@ class LocalEmail < Email
validate :differs_from_login
=end
-
- def email=(value)
- return if value.blank?
- self.username = value
- strip_domain_if_needed
+ def initialize(s)
+ super
+ append_domain_if_needed
end
def to_key
- [username]
+ [handle]
+ end
+
+ def handle
+ gsub(/@#{APP_CONFIG[:domain]}/i, '')
end
protected
@@ -42,8 +44,10 @@ class LocalEmail < Email
end
end
- def strip_domain_if_needed
- self.username.gsub! /@#{APP_CONFIG[:domain]}/i, ''
+ def append_domain_if_needed
+ unless self.index('@')
+ self << "@#{APP_CONFIG[:domain]}"
+ end
end
end
diff --git a/users/test/unit/local_email_test.rb b/users/test/unit/local_email_test.rb
new file mode 100644
index 0000000..9031a98
--- /dev/null
+++ b/users/test/unit/local_email_test.rb
@@ -0,0 +1,27 @@
+require 'test_helper'
+
+class LocalEmailTest < ActiveSupport::TestCase
+
+ test "appends domain" do
+ local = LocalEmail.new(handle)
+ assert_equal LocalEmail.new(email), local
+ end
+
+ test "returns handle" do
+ local = LocalEmail.new(email)
+ assert_equal handle, local.handle
+ end
+
+ test "prints full email" do
+ local = LocalEmail.new(handle)
+ assert_equal email, "#{local}"
+ end
+
+ def handle
+ "asdf"
+ end
+
+ def email
+ "asdf@" + APP_CONFIG[:domain]
+ end
+end
--
cgit v1.2.3
From d96fac2de074bbe3a44d888af5ceaff45b1b9b27 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 18 Jul 2013 12:52:02 +0200
Subject: validations of email format and local domain moved over
---
users/app/models/email.rb | 20 +++++++++++---------
users/app/models/local_email.rb | 20 ++++++++++++++++++--
users/test/unit/email_test.rb | 19 +++++++++++++++++++
users/test/unit/local_email_test.rb | 11 +++++++++--
4 files changed, 57 insertions(+), 13 deletions(-)
create mode 100644 users/test/unit/email_test.rb
(limited to 'users')
diff --git a/users/app/models/email.rb b/users/app/models/email.rb
index 90fc645..1bcff1c 100644
--- a/users/app/models/email.rb
+++ b/users/app/models/email.rb
@@ -1,13 +1,11 @@
class Email < String
-=begin
- included do
- validates :email,
- :format => {
- :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/,
- :message => "needs to be a valid email address"
- }
- end
-=end
+ include ActiveModel::Validations
+
+ validates :email,
+ :format => {
+ :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/,
+ :message => "needs to be a valid email address"
+ }
def to_partial_path
"emails/email"
@@ -17,4 +15,8 @@ class Email < String
to_s
end
+ def email
+ self
+ end
+
end
diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb
index d919102..e71d494 100644
--- a/users/app/models/local_email.rb
+++ b/users/app/models/local_email.rb
@@ -6,6 +6,18 @@ class LocalEmail < Email
validate :differs_from_login
=end
+ def self.domain
+ APP_CONFIG[:domain]
+ end
+
+ validates :email,
+ :format => {
+ :with => /@#{domain}\Z/i,
+ :message => "needs to end in @#{domain}"
+ }
+
+
+
def initialize(s)
super
append_domain_if_needed
@@ -16,7 +28,11 @@ class LocalEmail < Email
end
def handle
- gsub(/@#{APP_CONFIG[:domain]}/i, '')
+ gsub(/@#{domain}/i, '')
+ end
+
+ def domain
+ LocalEmail.domain
end
protected
@@ -46,7 +62,7 @@ class LocalEmail < Email
def append_domain_if_needed
unless self.index('@')
- self << "@#{APP_CONFIG[:domain]}"
+ self << '@' + domain
end
end
diff --git a/users/test/unit/email_test.rb b/users/test/unit/email_test.rb
new file mode 100644
index 0000000..7cfbc84
--- /dev/null
+++ b/users/test/unit/email_test.rb
@@ -0,0 +1,19 @@
+require 'test_helper'
+
+class EmailTest < ActiveSupport::TestCase
+
+ test "valid format" do
+ email = Email.new(email_string)
+ assert email.valid?
+ end
+
+ test "validates format" do
+ email = Email.new("email")
+ assert !email.valid?
+ assert_equal ["needs to be a valid email address"], email.errors[:email]
+ end
+
+ def email_string
+ @email_string ||= Faker::Internet.email
+ end
+end
diff --git a/users/test/unit/local_email_test.rb b/users/test/unit/local_email_test.rb
index 9031a98..b25f46f 100644
--- a/users/test/unit/local_email_test.rb
+++ b/users/test/unit/local_email_test.rb
@@ -5,6 +5,7 @@ class LocalEmailTest < ActiveSupport::TestCase
test "appends domain" do
local = LocalEmail.new(handle)
assert_equal LocalEmail.new(email), local
+ assert local.valid?
end
test "returns handle" do
@@ -17,11 +18,17 @@ class LocalEmailTest < ActiveSupport::TestCase
assert_equal email, "#{local}"
end
+ test "validates domain" do
+ local = LocalEmail.new(Faker::Internet.email)
+ assert !local.valid?
+ assert_equal ["needs to end in @#{LocalEmail.domain}"], local.errors[:email]
+ end
+
def handle
- "asdf"
+ @handle ||= Faker::Internet.user_name
end
def email
- "asdf@" + APP_CONFIG[:domain]
+ handle + "@" + APP_CONFIG[:domain]
end
end
--
cgit v1.2.3
From 0acace58c31c96fc1f8836167aeb4f204f72617f Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 18 Jul 2013 17:17:36 +0200
Subject: allow available and unique forwards only
---
users/app/models/identity.rb | 22 ++++++++++++++++++++++
users/app/models/local_email.rb | 30 ------------------------------
users/app/models/user.rb | 8 +++-----
users/test/unit/identity_test.rb | 18 ++++++++++++++++++
4 files changed, 43 insertions(+), 35 deletions(-)
(limited to 'users')
diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index c9c8b73..4dff93a 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -7,7 +7,29 @@ class Identity < CouchRest::Model::Base
property :address, LocalEmail
property :destination, Email
+ validate :unique_forward
+ validate :alias_available
+
design do
view :by_user_id
+ view :by_address_and_destination
+ view :by_address
+ end
+
+ protected
+
+ def unique_forward
+ same = Identity.find_by_address_and_destination([address, destination])
+ if same && same != self
+ errors.add :base, "This alias already exists"
+ end
end
+
+ def alias_available
+ same = Identity.find_by_address(address)
+ if same && same.user != self.user
+ errors.add :base, "This email has already been taken"
+ end
+ end
+
end
diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb
index e71d494..c1f7c11 100644
--- a/users/app/models/local_email.rb
+++ b/users/app/models/local_email.rb
@@ -1,10 +1,5 @@
class LocalEmail < Email
-=begin
- validate :unique_on_server
- validate :unique_alias_for_user
- validate :differs_from_login
-=end
def self.domain
APP_CONFIG[:domain]
@@ -16,8 +11,6 @@ class LocalEmail < Email
:message => "needs to end in @#{domain}"
}
-
-
def initialize(s)
super
append_domain_if_needed
@@ -37,29 +30,6 @@ class LocalEmail < Email
protected
- def unique_on_server
- has_email = User.find_by_login_or_alias(username)
- if has_email && has_email != self.casted_by
- errors.add :username, "has already been taken"
- end
- end
-
- def unique_alias_for_user
- aliases = self.casted_by.email_aliases
- if aliases.select{|a|a.username == self.username}.count > 1
- errors.add :username, "is already your alias"
- end
- end
-
- def differs_from_login
- # If this has not changed but the email let's mark the email invalid instead.
- return if self.persisted?
- user = self.casted_by
- if user.login == self.username
- errors.add :username, "may not be the same as your email address"
- end
- end
-
def append_domain_if_needed
unless self.index('@')
self << '@' + domain
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index dda5a41..5707f24 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -74,7 +74,7 @@ class User < CouchRest::Model::Base
end
def create_identity(attribs = {}, &block)
- build_identity(attribs, &block)
+ identity = build_identity(attribs, &block)
identity.save
identity
end
@@ -135,12 +135,10 @@ class User < CouchRest::Model::Base
##
def login_is_unique_alias
- has_alias = User.find_by_login_or_alias(username)
+ alias_identity = Identity.find_by_address(self.email_address)
return if has_alias.nil?
- if has_alias != self
+ if alias_identity.user != self
errors.add(:login, "has already been taken")
- elsif has_alias.login != self.login
- errors.add(:login, "may not be the same as one of your aliases")
end
end
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index a5d30b0..4ebd72e 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -39,6 +39,24 @@ class IdentityTest < ActiveSupport::TestCase
id.save
end
+ test "prevents duplicates" do
+ id = @user.create_identity address: alias_name, destination: forward_address
+ dup = @user.build_identity address: alias_name, destination: forward_address
+ assert !dup.valid?
+ assert_equal ["This alias already exists"], dup.errors[:base]
+ end
+
+ test "validates availability" do
+ other_user = FactoryGirl.create(:user)
+ id = @user.create_identity address: alias_name, destination: forward_address
+ taken = other_user.build_identity address: alias_name
+ assert !taken.valid?
+ assert_equal ["This email has already been taken"], taken.errors[:base]
+ other_user.destroy
+ end
+
+
+
def alias_name
@alias_name ||= Faker::Internet.user_name
end
--
cgit v1.2.3
From 495c97fbf400ed44a1e64692f2c04d2155a326e7 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 18 Jul 2013 18:09:12 +0200
Subject: remove the remainders of email aliases and forward from user
---
users/app/models/user.rb | 28 +---------------------------
1 file changed, 1 insertion(+), 27 deletions(-)
(limited to 'users')
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index 5707f24..62fb7ec 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -6,9 +6,6 @@ class User < CouchRest::Model::Base
property :password_verifier, String, :accessible => true
property :password_salt, String, :accessible => true
- property :email_forward, String, :accessible => true
- property :email_aliases, [LocalEmail]
-
property :public_key, :accessible => true
property :enabled, TrueClass, :default => true
@@ -43,10 +40,6 @@ class User < CouchRest::Model::Base
:confirmation => true,
:format => { :with => /.{8}.*/, :message => "needs to be at least 8 characters long" }
- validates :email_forward,
- :allow_blank => true,
- :format => { :with => /\A(([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,}))?\Z/, :message => "needs to be a valid email address"}
-
timestamps!
design do
@@ -54,19 +47,6 @@ class User < CouchRest::Model::Base
load_views(own_path.join('..', 'designs', 'user'))
view :by_login
view :by_created_at
- view :pgp_key_by_handle,
- map: <<-EOJS
- function(doc) {
- if (doc.type != 'User') {
- return;
- }
- emit(doc.login, doc.public_key);
- doc.email_aliases.forEach(function(alias){
- emit(alias.username, doc.public_key);
- });
- }
- EOJS
-
end # end of design
class << self
@@ -118,12 +98,6 @@ class User < CouchRest::Model::Base
APP_CONFIG['admins'].include? self.login
end
- # this currently only adds the first email address submitted.
- # All the ui needs for now.
- def email_aliases_attributes=(attrs)
- email_aliases.build(attrs.values.first) if attrs
- end
-
def most_recent_tickets(count=3)
Ticket.for_user(self).limit(count).all #defaults to having most recent updated first
end
@@ -136,7 +110,7 @@ class User < CouchRest::Model::Base
def login_is_unique_alias
alias_identity = Identity.find_by_address(self.email_address)
- return if has_alias.nil?
+ return if alias_identity.blank?
if alias_identity.user != self
errors.add(:login, "has already been taken")
end
--
cgit v1.2.3
From dcaf6232dbd1131cdbd12ac5c2ef997979fd01ff Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 09:50:02 +0200
Subject: add keys to identity
---
users/app/models/identity.rb | 11 +++++++++++
users/app/models/user.rb | 3 ++-
users/test/unit/identity_test.rb | 19 +++++++++++++++++++
3 files changed, 32 insertions(+), 1 deletion(-)
(limited to 'users')
diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index 4dff93a..b862590 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -6,6 +6,7 @@ class Identity < CouchRest::Model::Base
property :address, LocalEmail
property :destination, Email
+ property :keys, Hash
validate :unique_forward
validate :alias_available
@@ -14,6 +15,16 @@ class Identity < CouchRest::Model::Base
view :by_user_id
view :by_address_and_destination
view :by_address
+ view :pgp_key_by_email,
+ map: <<-EOJS
+ function(doc) {
+ if (doc.type != 'Identity') {
+ return;
+ }
+ emit(doc.address, doc.keys["pgp"]);
+ }
+ EOJS
+
end
protected
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index 62fb7ec..fb7f0aa 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -62,7 +62,8 @@ class User < CouchRest::Model::Base
def build_identity(attribs = {}, &block)
attribs.reverse_merge! user_id: self.id,
address: self.email_address,
- destination: self.email_address
+ destination: self.email_address,
+ keys: {}
Identity.new(attribs, &block)
end
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 4ebd72e..6b0a6b1 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -55,7 +55,22 @@ class IdentityTest < ActiveSupport::TestCase
other_user.destroy
end
+ test "setting and getting pgp key" do
+ id = @user.build_identity
+ id.keys[:pgp] = pgp_key_string
+ assert_equal pgp_key_string, id.keys[:pgp]
+ end
+ test "querying pgp key via couch" do
+ id = @user.build_identity
+ id.keys[:pgp] = pgp_key_string
+ id.save
+ view = Identity.pgp_key_by_email.key(id.address)
+ assert_equal 1, view.rows.count
+ assert result = view.rows.first
+ assert_equal id.address, result["key"]
+ assert_equal id.keys[:pgp], result["value"]
+ end
def alias_name
@alias_name ||= Faker::Internet.user_name
@@ -64,4 +79,8 @@ class IdentityTest < ActiveSupport::TestCase
def forward_address
@forward_address ||= Faker::Internet.email
end
+
+ def pgp_key_string
+ @pgp_key ||= "DUMMY PGP KEY ... "+SecureRandom.base64(4096)
+ end
end
--
cgit v1.2.3
From b6242bbefc1e9fe193bbf3479e8fa822829c6d1a Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 11:07:53 +0200
Subject: remove email aliases test - we'll move them to identities
---
users/test/unit/email_aliases_test.rb | 66 -----------------------------------
1 file changed, 66 deletions(-)
delete mode 100644 users/test/unit/email_aliases_test.rb
(limited to 'users')
diff --git a/users/test/unit/email_aliases_test.rb b/users/test/unit/email_aliases_test.rb
deleted file mode 100644
index 86d14aa..0000000
--- a/users/test/unit/email_aliases_test.rb
+++ /dev/null
@@ -1,66 +0,0 @@
-require 'test_helper'
-
-class EmailAliasTest < ActiveSupport::TestCase
-
- setup do
- @user = FactoryGirl.build :user
- @alias = "valid_alias"
- # make sure no existing records are in the way...
- User.find_by_login_or_alias(@alias).try(:destroy)
- end
-
- test "no email aliases set in the beginning" do
- assert_equal [], @user.email_aliases
- end
-
- test "adding email alias through params" do
- @user.attributes = {:email_aliases_attributes => {"0" => {:email => @alias}}}
- assert @user.changed?
- assert @user.save
- assert_equal @alias, @user.email_aliases.first.username
- end
-
- test "adding email alias directly" do
- @user.email_aliases.build :email => @alias
- assert @user.save
- assert_equal @alias, @user.email_aliases.first.username
- end
-
- test "duplicated email aliases are invalid" do
- @user.email_aliases.build :email => @alias
- @user.save
- assert_invalid_alias @alias
- end
-
- test "email alias needs to be different from other peoples login" do
- other_user = FactoryGirl.create :user, :login => @alias
- assert_invalid_alias @alias
- other_user.destroy
- end
-
- test "email needs to be different from other peoples email aliases" do
- other_user = FactoryGirl.create :user, :email_aliases_attributes => {'1' => @alias}
- assert_invalid_alias @alias
- other_user.destroy
- end
-
- test "login is invalid as email alias" do
- @user.login = @alias
- assert_invalid_alias @alias
- end
-
- test "find user by email alias" do
- @user.email_aliases.build :email => @alias
- assert @user.save
- assert_equal @user, User.find_by_login_or_alias(@alias)
- assert_equal @user, User.find_by_alias(@alias)
- assert_nil User.find_by_login(@alias)
- end
-
- def assert_invalid_alias(string)
- email_alias = @user.email_aliases.build :email => string
- assert !email_alias.valid?
- assert !@user.valid?
- end
-
-end
--
cgit v1.2.3
From 8ddbaa6184e4dbcc6ef7e81cf555cc18d3822dce Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 11:09:25 +0200
Subject: support deprecated API to set users main identity pgp key
We'll want to get rid of the #public_key and #public_key= functions but they are still used from the users controller. We'll probably have an identity controller instead at some point.
---
users/app/models/user.rb | 44 +++++++++++++++++++++++++++++++++++-----
users/test/unit/identity_test.rb | 6 +++---
users/test/unit/user_test.rb | 14 ++++++-------
3 files changed, 48 insertions(+), 16 deletions(-)
(limited to 'users')
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index fb7f0aa..546d571 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -6,8 +6,6 @@ class User < CouchRest::Model::Base
property :password_verifier, String, :accessible => true
property :password_salt, String, :accessible => true
- property :public_key, :accessible => true
-
property :enabled, TrueClass, :default => true
validates :login, :password_salt, :password_verifier,
@@ -49,14 +47,50 @@ class User < CouchRest::Model::Base
view :by_created_at
end # end of design
+ # We proxy access to the pgp_key. So we need to make sure
+ # the updated identity actually gets saved.
+ def save(*args)
+ super
+ identity.user_id ||= self.id
+ identity.save if identity.changed?
+ end
+
+ # So far this only works for creating a new user.
+ # TODO: Create an alias for the old login when changing the login
+ def login=(value)
+ write_attribute 'login', value
+ @identity = build_identity
+ end
+
+ # DEPRECATED
+ #
+ # Please access identity.keys[:pgp] directly
+ def public_key=(value)
+ identity.keys[:pgp] = value
+ end
+
+ # DEPRECATED
+ #
+ # Please access identity.keys[:pgp] directly
+ def public_key
+ identity.keys[:pgp]
+ end
+
class << self
alias_method :find_by_param, :find
end
+ # this is the main identity. login@domain.tld
+ # aliases and forwards are represented in other identities.
+ def identity
+ @identity ||=
+ Identity.find_by_address_and_destination([email_address, email_address])
+ end
+
def create_identity(attribs = {}, &block)
- identity = build_identity(attribs, &block)
- identity.save
- identity
+ new_identity = build_identity(attribs, &block)
+ new_identity.save
+ new_identity
end
def build_identity(attribs = {}, &block)
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 6b0a6b1..d20ad93 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -11,7 +11,7 @@ class IdentityTest < ActiveSupport::TestCase
end
test "initial identity for a user" do
- id = @user.build_identity
+ id = @user.identity
assert_equal @user.email_address, id.address
assert_equal @user.email_address, id.destination
assert_equal @user, id.user
@@ -56,13 +56,13 @@ class IdentityTest < ActiveSupport::TestCase
end
test "setting and getting pgp key" do
- id = @user.build_identity
+ id = @user.identity
id.keys[:pgp] = pgp_key_string
assert_equal pgp_key_string, id.keys[:pgp]
end
test "querying pgp key via couch" do
- id = @user.build_identity
+ id = @user.identity
id.keys[:pgp] = pgp_key_string
id.save
view = Identity.pgp_key_by_email.key(id.address)
diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb
index c8c837b..5d1fe06 100644
--- a/users/test/unit/user_test.rb
+++ b/users/test/unit/user_test.rb
@@ -56,23 +56,21 @@ class UserTest < ActiveSupport::TestCase
other_user.destroy
end
- test "login needs to be different from other peoples email aliases" do
- other_user = FactoryGirl.create :user
- other_user.email_aliases.build :email => @user.login
- other_user.save
- assert !@user.valid?
- other_user.destroy
+ test "deprecated public key api still works" do
+ key = SecureRandom.base64(4096)
+ @user.public_key = key
+ assert_equal key, @user.public_key
end
test "pgp key view" do
@user.public_key = SecureRandom.base64(4096)
@user.save
- view = User.pgp_key_by_handle.key(@user.login)
+ view = Identity.pgp_key_by_email.key(@user.email_address)
assert_equal 1, view.rows.count
assert result = view.rows.first
- assert_equal @user.login, result["key"]
+ assert_equal @user.email_address, result["key"]
assert_equal @user.public_key, result["value"]
end
end
--
cgit v1.2.3
From 34c06b49f26bbffe2e29477f0b55a56a38aa0ce7 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 11:14:43 +0200
Subject: no need for a remote email class
---
users/app/models/remote_email.rb | 14 --------------
1 file changed, 14 deletions(-)
delete mode 100644 users/app/models/remote_email.rb
(limited to 'users')
diff --git a/users/app/models/remote_email.rb b/users/app/models/remote_email.rb
deleted file mode 100644
index 4fe7425..0000000
--- a/users/app/models/remote_email.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-class RemoteEmail
- include CouchRest::Model::Embeddable
- include Email
-
- property :email, String
-
- def username
- email.spilt('@').first
- end
-
- def domain
- email.split('@').last
- end
-end
--
cgit v1.2.3
From 51582a668b04d2c1322ad1babe8599ae8797cd3b Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 11:20:01 +0200
Subject: test user validates uniqueness of login amongst aliases
---
users/test/unit/user_test.rb | 7 +++++++
1 file changed, 7 insertions(+)
(limited to 'users')
diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb
index 5d1fe06..f303c8d 100644
--- a/users/test/unit/user_test.rb
+++ b/users/test/unit/user_test.rb
@@ -56,6 +56,13 @@ class UserTest < ActiveSupport::TestCase
other_user.destroy
end
+ test "login needs to be unique amongst aliases" do
+ other_user = FactoryGirl.create :user
+ other_user.create_identity address: @user.login
+ assert !@user.valid?
+ other_user.destroy
+ end
+
test "deprecated public key api still works" do
key = SecureRandom.base64(4096)
@user.public_key = key
--
cgit v1.2.3
From c0b88d9e8fe574d6164f48211db50f3b8a4c4d93 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 12:21:40 +0200
Subject: setter for keys for dirty tracking, more robust tests
Just altering identity.keys did not mark identities as changed. Also we now have a sane default for keys.
---
users/app/models/identity.rb | 11 ++++++++++-
users/app/models/user.rb | 21 ++++++++++++++-------
users/test/integration/api/account_flow_test.rb | 5 +++++
users/test/unit/identity_test.rb | 4 ++--
4 files changed, 31 insertions(+), 10 deletions(-)
(limited to 'users')
diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index b862590..73531ec 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -6,7 +6,7 @@ class Identity < CouchRest::Model::Base
property :address, LocalEmail
property :destination, Email
- property :keys, Hash
+ property :keys, HashWithIndifferentAccess
validate :unique_forward
validate :alias_available
@@ -27,6 +27,15 @@ class Identity < CouchRest::Model::Base
end
+ def keys
+ read_attribute('keys') || HashWithIndifferentAccess.new
+ end
+
+ def set_key(type, value)
+ return if keys[type] == value
+ write_attribute('keys', keys.merge(type => value))
+ end
+
protected
def unique_forward
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index 546d571..c791069 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -59,14 +59,19 @@ class User < CouchRest::Model::Base
# TODO: Create an alias for the old login when changing the login
def login=(value)
write_attribute 'login', value
- @identity = build_identity
+ if @identity
+ @identity.address = email_address
+ @identity.destination = email_address
+ else
+ build_identity
+ end
end
# DEPRECATED
#
- # Please access identity.keys[:pgp] directly
+ # Please set the key on the identity directly
def public_key=(value)
- identity.keys[:pgp] = value
+ identity.set_key(:pgp, value)
end
# DEPRECATED
@@ -83,8 +88,7 @@ class User < CouchRest::Model::Base
# this is the main identity. login@domain.tld
# aliases and forwards are represented in other identities.
def identity
- @identity ||=
- Identity.find_by_address_and_destination([email_address, email_address])
+ @identity ||= find_identity || build_identity
end
def create_identity(attribs = {}, &block)
@@ -96,8 +100,7 @@ class User < CouchRest::Model::Base
def build_identity(attribs = {}, &block)
attribs.reverse_merge! user_id: self.id,
address: self.email_address,
- destination: self.email_address,
- keys: {}
+ destination: self.email_address
Identity.new(attribs, &block)
end
@@ -139,6 +142,10 @@ class User < CouchRest::Model::Base
protected
+ def find_identity
+ Identity.find_by_address_and_destination([email_address, email_address])
+ end
+
##
# Validation Functions
##
diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb
index 4c94389..93b6507 100644
--- a/users/test/integration/api/account_flow_test.rb
+++ b/users/test/integration/api/account_flow_test.rb
@@ -79,8 +79,13 @@ class AccountFlowTest < RackTest
test_public_key = 'asdlfkjslfdkjasd'
original_login = @user.login
new_login = 'zaph'
+ User.find_by_login(new_login).try(:destroy)
+ Identity.by_address.key(new_login + '@' + APP_CONFIG[:domain]).each do |identity|
+ identity.destroy
+ end
put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => test_public_key, :login => new_login}, :format => :json
@user.reload
+ assert last_response.successful?
assert_equal test_public_key, @user.public_key
assert_equal new_login, @user.login
# eventually probably want to remove most of this into a non-integration functional test
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index d20ad93..40b1296 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -57,13 +57,13 @@ class IdentityTest < ActiveSupport::TestCase
test "setting and getting pgp key" do
id = @user.identity
- id.keys[:pgp] = pgp_key_string
+ id.set_key(:pgp, pgp_key_string)
assert_equal pgp_key_string, id.keys[:pgp]
end
test "querying pgp key via couch" do
id = @user.identity
- id.keys[:pgp] = pgp_key_string
+ id.set_key(:pgp, pgp_key_string)
id.save
view = Identity.pgp_key_by_email.key(id.address)
assert_equal 1, view.rows.count
--
cgit v1.2.3
From c0527cc18788fc60180d9293a546c93e6a77b788 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 13:06:57 +0200
Subject: removed email settings controller and views
PGP setting has been moved into account settings. It's using the API now issueing an Ajax request without any visual feedback.
This obviously is not what we want but it hopefully suffices for uploading gpg keys for testing purposes before the Identity UI is in place.
---
users/app/controllers/email_settings_controller.rb | 41 ----------------------
users/app/controllers/v1/users_controller.rb | 3 --
users/app/views/email_settings/edit.html.haml | 38 --------------------
users/app/views/users/_edit.html.haml | 16 ++++++++-
4 files changed, 15 insertions(+), 83 deletions(-)
delete mode 100644 users/app/controllers/email_settings_controller.rb
delete mode 100644 users/app/views/email_settings/edit.html.haml
(limited to 'users')
diff --git a/users/app/controllers/email_settings_controller.rb b/users/app/controllers/email_settings_controller.rb
deleted file mode 100644
index f7d85be..0000000
--- a/users/app/controllers/email_settings_controller.rb
+++ /dev/null
@@ -1,41 +0,0 @@
-class EmailSettingsController < UsersBaseController
-
- before_filter :authorize
- before_filter :fetch_user
-
- def edit
- @email_alias = LocalEmail.new
- end
-
- def update
- @user.attributes = cleanup_params(params[:user])
- if @user.changed?
- if @user.save
- flash[:notice] = t(:changes_saved)
- redirect
- else
- if @user.email_aliases.last && !@user.email_aliases.last.valid?
- # display bad alias in text field:
- @email_alias = @user.email_aliases.pop
- end
- render 'email_settings/edit'
- end
- else
- redirect
- end
- end
-
- private
-
- def redirect
- redirect_to edit_user_email_settings_url(@user)
- end
-
- def cleanup_params(user)
- if !user['email_forward'].nil? && user['email_forward'].empty?
- user.delete('email_forward') # don't allow "" as an email forward
- end
- user
- end
-
-end
diff --git a/users/app/controllers/v1/users_controller.rb b/users/app/controllers/v1/users_controller.rb
index fda56f2..467c9b4 100644
--- a/users/app/controllers/v1/users_controller.rb
+++ b/users/app/controllers/v1/users_controller.rb
@@ -24,9 +24,6 @@ module V1
def update
@user.update_attributes params[:user]
- if @user.valid?
- flash[:notice] = t(:user_updated_successfully)
- end
respond_with @user
end
diff --git a/users/app/views/email_settings/edit.html.haml b/users/app/views/email_settings/edit.html.haml
deleted file mode 100644
index 7757a31..0000000
--- a/users/app/views/email_settings/edit.html.haml
+++ /dev/null
@@ -1,38 +0,0 @@
-- form_options = {:url => user_email_settings_path(@user), :html => {:class => 'form-horizontal'}, :validate => true}
-- alias_error_class = @email_alias.username && !@email_alias.valid? ? 'error' : ''
-
-- content_for :head do
- :css
- table.aliases tr:first-child td {
- border-top: none;
- }
-
-= simple_form_for @user, form_options.dup do |f|
- %legend= t(:email_aliases)
- .control-group
- %label.control-label= t(:current_aliases)
- .controls
- %table.table.table-condensed.no-header.slim.aliases
- - if @user.email_aliases.any?
- - @user.email_aliases.each do |email|
- %tr
- %td= email
- %td= link_to(icon(:remove) + t(:remove), user_email_alias_path(@user, email), :method => :delete)
- - else
- %tr
- %td{:colspan=>2}= t(:none)
- .control-group{:class => alias_error_class}
- %label.control-label= t(:add_email_alias)
- .controls
- = f.simple_fields_for :email_aliases, @email_alias do |e|
- .input-append
- = e.input_field :username
- = e.submit t(:add), :class => 'btn'
- = e.error :username
-
-= simple_form_for @user, form_options do |f|
- %legend= t(:advanced_options)
- = f.input :email_forward
- = f.input :public_key, :as => :text, :hint => t(:use_ascii_key), :input_html => {:class => "full-width", :rows => 4}
- .form-actions
- = f.submit t(:save), :class => 'btn btn-primary'
diff --git a/users/app/views/users/_edit.html.haml b/users/app/views/users/_edit.html.haml
index 0402f37..5f74d32 100644
--- a/users/app/views/users/_edit.html.haml
+++ b/users/app/views/users/_edit.html.haml
@@ -22,6 +22,20 @@
.controls
= f.submit t(:save), :class => 'btn btn-primary'
+-#
+-# CHANGE PGP KEY
+-#
+-# this will be replaced by a identities controller/view at some point
+-#
+
+- form_options = {:html => {:class => user_form_class('form-horizontal'), :id => 'update_pgp_key'}, :validate => true}
+= simple_form_for [:api, @user], form_options do |f|
+ %legend= t(:advanced_options)
+ = f.input :public_key, :as => :text, :hint => t(:use_ascii_key), :input_html => {:class => "full-width", :rows => 4}
+ .control-group
+ .controls
+ = f.submit t(:save), :class => 'btn'
+
-#
-# DESTROY ACCOUNT
-#
@@ -48,4 +62,4 @@
%p= t(:enable_description)
= link_to enable_user_path(@user), :method => :post, :class => "btn btn-warning" do
%i.icon-ok.icon-white
- = t(:enable)
\ No newline at end of file
+ = t(:enable)
--
cgit v1.2.3
From d70c5796a2989b7b43f7e287899fb1394ae28280 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Fri, 19 Jul 2013 16:02:02 +0200
Subject: separate signup and settings service objects for user
---
users/app/controllers/v1/users_controller.rb | 13 +++++-
users/app/models/account_settings.rb | 36 ++++++++++++++
users/app/models/identity.rb | 27 +++++++++++
users/app/models/signup_service.rb | 9 ++++
users/app/models/user.rb | 57 -----------------------
users/test/functional/v1/users_controller_test.rb | 8 +++-
users/test/integration/api/account_flow_test.rb | 13 ++----
users/test/unit/identity_test.rb | 20 ++++----
users/test/unit/user_test.rb | 2 +-
9 files changed, 105 insertions(+), 80 deletions(-)
create mode 100644 users/app/models/account_settings.rb
create mode 100644 users/app/models/signup_service.rb
(limited to 'users')
diff --git a/users/app/controllers/v1/users_controller.rb b/users/app/controllers/v1/users_controller.rb
index 467c9b4..f380c19 100644
--- a/users/app/controllers/v1/users_controller.rb
+++ b/users/app/controllers/v1/users_controller.rb
@@ -18,14 +18,23 @@ module V1
end
def create
- @user = User.create(params[:user])
+ @user = signup_service.register(params[:user])
respond_with @user # return ID instead?
end
def update
- @user.update_attributes params[:user]
+ account_settings.update params[:user]
respond_with @user
end
+ protected
+
+ def account_settings
+ AccountSettings.new(@user)
+ end
+
+ def signup_service
+ SignupService.new
+ end
end
end
diff --git a/users/app/models/account_settings.rb b/users/app/models/account_settings.rb
new file mode 100644
index 0000000..a73a95a
--- /dev/null
+++ b/users/app/models/account_settings.rb
@@ -0,0 +1,36 @@
+class AccountSettings
+
+ def initialize(user)
+ @user = user
+ end
+
+ def update(attrs)
+ if attrs[:password_verifier].present?
+ update_login(attrs[:login])
+ @user.update_attributes attrs.slice(:password_verifier, :password_salt)
+ end
+ # TODO: move into identity controller
+ update_pgp_key(attrs[:public_key]) if attrs.has_key? :public_key
+ @user.save && save_identities
+ end
+
+ protected
+
+ def update_login(login, verifier)
+ return unless login.present?
+ @old_identity = Identity.for(@user)
+ @user.login = login
+ @new_identity = Identity.for(@user) # based on the new login
+ @old_identity.destination = @user.email_address # alias old -> new
+ end
+
+ def update_pgp_key(key)
+ @new_identity ||= Identity.for(@user)
+ @new_identity.set_key(:pgp, key)
+ end
+
+ def save_identities
+ @new_identity.try(:save) && @old_identity.try(:save)
+ end
+
+end
diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb
index 73531ec..355f67a 100644
--- a/users/app/models/identity.rb
+++ b/users/app/models/identity.rb
@@ -27,6 +27,33 @@ class Identity < CouchRest::Model::Base
end
+ def self.for(user, attributes = {})
+ find_for(user, attributes) || build_for(user, attributes)
+ end
+
+ def self.find_for(user, attributes = {})
+ attributes.reverse_merge! attributes_from_user(user)
+ find_by_address_and_destination [attributes[:address], attributes[:destination]]
+ end
+
+ def self.build_for(user, attributes = {})
+ attributes.reverse_merge! attributes_from_user(user)
+ Identity.new(attributes)
+ end
+
+ def self.create_for(user, attributes = {})
+ identity = build_for(user, attributes)
+ identity.save
+ identity
+ end
+
+ def self.attributes_from_user(user)
+ { user_id: user.id,
+ address: user.email_address,
+ destination: user.email_address
+ }
+ end
+
def keys
read_attribute('keys') || HashWithIndifferentAccess.new
end
diff --git a/users/app/models/signup_service.rb b/users/app/models/signup_service.rb
new file mode 100644
index 0000000..f316ca9
--- /dev/null
+++ b/users/app/models/signup_service.rb
@@ -0,0 +1,9 @@
+class SignupService
+
+ def register(attrs)
+ User.create(attrs).tap do |user|
+ Identity.create_for user
+ end
+ end
+
+end
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index c791069..f78f290 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -47,63 +47,10 @@ class User < CouchRest::Model::Base
view :by_created_at
end # end of design
- # We proxy access to the pgp_key. So we need to make sure
- # the updated identity actually gets saved.
- def save(*args)
- super
- identity.user_id ||= self.id
- identity.save if identity.changed?
- end
-
- # So far this only works for creating a new user.
- # TODO: Create an alias for the old login when changing the login
- def login=(value)
- write_attribute 'login', value
- if @identity
- @identity.address = email_address
- @identity.destination = email_address
- else
- build_identity
- end
- end
-
- # DEPRECATED
- #
- # Please set the key on the identity directly
- def public_key=(value)
- identity.set_key(:pgp, value)
- end
-
- # DEPRECATED
- #
- # Please access identity.keys[:pgp] directly
- def public_key
- identity.keys[:pgp]
- end
-
class << self
alias_method :find_by_param, :find
end
- # this is the main identity. login@domain.tld
- # aliases and forwards are represented in other identities.
- def identity
- @identity ||= find_identity || build_identity
- end
-
- def create_identity(attribs = {}, &block)
- new_identity = build_identity(attribs, &block)
- new_identity.save
- new_identity
- end
-
- def build_identity(attribs = {}, &block)
- attribs.reverse_merge! user_id: self.id,
- address: self.email_address,
- destination: self.email_address
- Identity.new(attribs, &block)
- end
-
def to_param
self.id
end
@@ -142,10 +89,6 @@ class User < CouchRest::Model::Base
protected
- def find_identity
- Identity.find_by_address_and_destination([email_address, email_address])
- end
-
##
# Validation Functions
##
diff --git a/users/test/functional/v1/users_controller_test.rb b/users/test/functional/v1/users_controller_test.rb
index 0d44e50..a330bf3 100644
--- a/users/test/functional/v1/users_controller_test.rb
+++ b/users/test/functional/v1/users_controller_test.rb
@@ -5,7 +5,9 @@ class V1::UsersControllerTest < ActionController::TestCase
test "user can change settings" do
user = find_record :user
changed_attribs = record_attributes_for :user_with_settings
- user.expects(:update_attributes).with(changed_attribs)
+ account_settings = stub
+ account_settings.expects(:update).with(changed_attribs)
+ AccountSettings.expects(:new).with(user).returns(account_settings)
login user
put :update, :user => changed_attribs, :id => user.id, :format => :json
@@ -18,7 +20,9 @@ class V1::UsersControllerTest < ActionController::TestCase
test "admin can update user" do
user = find_record :user
changed_attribs = record_attributes_for :user_with_settings
- user.expects(:update_attributes).with(changed_attribs)
+ account_settings = stub
+ account_settings.expects(:update).with(changed_attribs)
+ AccountSettings.expects(:new).with(user).returns(account_settings)
login :is_admin? => true
put :update, :user => changed_attribs, :id => user.id, :format => :json
diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb
index 93b6507..c09dcb6 100644
--- a/users/test/integration/api/account_flow_test.rb
+++ b/users/test/integration/api/account_flow_test.rb
@@ -84,20 +84,17 @@ class AccountFlowTest < RackTest
identity.destroy
end
put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => test_public_key, :login => new_login}, :format => :json
- @user.reload
assert last_response.successful?
- assert_equal test_public_key, @user.public_key
- assert_equal new_login, @user.login
+ assert_equal test_public_key, Identity.for(@user).keys[:pgp]
+ # does not change login if no password_verifier is present
+ assert_equal original_login, @user.login
# eventually probably want to remove most of this into a non-integration functional test
# should not overwrite public key:
put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:blee => :blah}, :format => :json
- @user.reload
- assert_equal test_public_key, @user.public_key
+ assert_equal test_public_key, Identity.for(@user).keys[:pgp]
# should overwrite public key:
put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => nil}, :format => :json
- # TODO: not sure why i need this, but when public key is removed, the DB is updated but @user.reload doesn't seem to actually reload.
- @user = User.find(@user.id) # @user.reload
- assert_nil @user.public_key
+ assert_nil Identity.for(@user).keys[:pgp]
end
end
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 40b1296..bf24f02 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -11,28 +11,28 @@ class IdentityTest < ActiveSupport::TestCase
end
test "initial identity for a user" do
- id = @user.identity
+ id = Identity.for(@user)
assert_equal @user.email_address, id.address
assert_equal @user.email_address, id.destination
assert_equal @user, id.user
end
test "add alias" do
- id = @user.build_identity address: alias_name
+ id = Identity.for @user, address: alias_name
assert_equal LocalEmail.new(alias_name), id.address
assert_equal @user.email_address, id.destination
assert_equal @user, id.user
end
test "add forward" do
- id = @user.build_identity destination: forward_address
+ id = Identity.for @user, destination: forward_address
assert_equal @user.email_address, id.address
assert_equal Email.new(forward_address), id.destination
assert_equal @user, id.user
end
test "forward alias" do
- id = @user.build_identity address: alias_name, destination: forward_address
+ id = Identity.for @user, address: alias_name, destination: forward_address
assert_equal LocalEmail.new(alias_name), id.address
assert_equal Email.new(forward_address), id.destination
assert_equal @user, id.user
@@ -40,29 +40,29 @@ class IdentityTest < ActiveSupport::TestCase
end
test "prevents duplicates" do
- id = @user.create_identity address: alias_name, destination: forward_address
- dup = @user.build_identity address: alias_name, destination: forward_address
+ id = Identity.create_for @user, address: alias_name, destination: forward_address
+ dup = Identity.build_for @user, address: alias_name, destination: forward_address
assert !dup.valid?
assert_equal ["This alias already exists"], dup.errors[:base]
end
test "validates availability" do
other_user = FactoryGirl.create(:user)
- id = @user.create_identity address: alias_name, destination: forward_address
- taken = other_user.build_identity address: alias_name
+ id = Identity.create_for @user, address: alias_name, destination: forward_address
+ taken = Identity.build_for other_user, address: alias_name
assert !taken.valid?
assert_equal ["This email has already been taken"], taken.errors[:base]
other_user.destroy
end
test "setting and getting pgp key" do
- id = @user.identity
+ id = Identity.for(@user)
id.set_key(:pgp, pgp_key_string)
assert_equal pgp_key_string, id.keys[:pgp]
end
test "querying pgp key via couch" do
- id = @user.identity
+ id = Identity.for(@user)
id.set_key(:pgp, pgp_key_string)
id.save
view = Identity.pgp_key_by_email.key(id.address)
diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb
index f303c8d..477c727 100644
--- a/users/test/unit/user_test.rb
+++ b/users/test/unit/user_test.rb
@@ -58,7 +58,7 @@ class UserTest < ActiveSupport::TestCase
test "login needs to be unique amongst aliases" do
other_user = FactoryGirl.create :user
- other_user.create_identity address: @user.login
+ Identity.create_for other_user, address: @user.login
assert !@user.valid?
other_user.destroy
end
--
cgit v1.2.3
From 370be6caa8a366774fba15d09fb03ee4d923b861 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 24 Jul 2013 11:09:04 +0200
Subject: keeping the pgp_key accessors for User so views still work
---
users/app/models/user.rb | 20 ++++++++++++++++++++
users/test/unit/user_test.rb | 8 +++++---
2 files changed, 25 insertions(+), 3 deletions(-)
(limited to 'users')
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index f78f290..0a89f7c 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -87,6 +87,26 @@ class User < CouchRest::Model::Base
Ticket.for_user(self).limit(count).all #defaults to having most recent updated first
end
+ # DEPRECATED
+ #
+ # Please set the key on the identity directly
+ # WARNING: This will not be serialized with the user record!
+ # It is only a workaround for the key form.
+ def public_key=(value)
+ identity.set_key(:pgp, value)
+ end
+
+ # DEPRECATED
+ #
+ # Please access identity.keys[:pgp] directly
+ def public_key
+ identity.keys[:pgp]
+ end
+
+ def identity
+ @identity ||= Identity.for(self)
+ end
+
protected
##
diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb
index 477c727..89ee749 100644
--- a/users/test/unit/user_test.rb
+++ b/users/test/unit/user_test.rb
@@ -70,14 +70,16 @@ class UserTest < ActiveSupport::TestCase
end
test "pgp key view" do
- @user.public_key = SecureRandom.base64(4096)
- @user.save
+ key = SecureRandom.base64(4096)
+ identity = Identity.create_for @user
+ identity.set_key('pgp', key)
+ identity.save
view = Identity.pgp_key_by_email.key(@user.email_address)
assert_equal 1, view.rows.count
assert result = view.rows.first
assert_equal @user.email_address, result["key"]
- assert_equal @user.public_key, result["value"]
+ assert_equal key, result["value"]
end
end
--
cgit v1.2.3
From b1065710102193ba11e2a18b5f6506ba1d5b31f9 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 24 Jul 2013 12:30:59 +0200
Subject: also destroy the identity for a test user during teardown
---
users/test/integration/api/account_flow_test.rb | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
(limited to 'users')
diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb
index c09dcb6..ec7753c 100644
--- a/users/test/integration/api/account_flow_test.rb
+++ b/users/test/integration/api/account_flow_test.rb
@@ -18,7 +18,10 @@ class AccountFlowTest < RackTest
end
teardown do
- @user.destroy if @user
+ if @user
+ @user.identity.destroy
+ @user.destroy
+ end
Warden.test_reset!
end
--
cgit v1.2.3
From 8e2bff3fb077410fd7facc41e4a460b402e08045 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 7 Aug 2013 17:45:03 +0200
Subject: integration test exploiting srp vulnerability
---
users/test/integration/browser/account_test.rb | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
(limited to 'users')
diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb
index ce63baf..b5776ff 100644
--- a/users/test/integration/browser/account_test.rb
+++ b/users/test/integration/browser/account_test.rb
@@ -20,4 +20,23 @@ class AccountTest < BrowserIntegrationTest
assert_equal '/', current_path
end
+ # trying to seed an invalid A for srp login
+ test "detects attempt to circumvent SRP" do
+ user = FactoryGirl.create :user
+ visit '/sessions/new'
+ fill_in 'Username', with: user.login
+ fill_in 'Password', with: "password"
+ inject_malicious_js
+ click_on 'Log In'
+ assert !page.has_content?("Welcome")
+ end
+
+ def inject_malicious_js
+ page.execute_script <<-EOJS
+ var calc = new srp.Calculate();
+ calc.A = function(_a) {return "00";};
+ calc.S = calc.A;
+ srp.session = new srp.Session(null, calc);
+ EOJS
+ end
end
--
cgit v1.2.3
From a0b276e4b8ae86dec7deee898e85b65784d89933 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 7 Aug 2013 18:09:20 +0200
Subject: close srp vulnerability and report error in webapp
---
users/config/locales/en.yml | 1 +
users/leap_web_users.gemspec | 2 +-
users/lib/warden/strategies/secure_remote_password.rb | 2 ++
users/test/integration/browser/account_test.rb | 1 +
4 files changed, 5 insertions(+), 1 deletion(-)
(limited to 'users')
diff --git a/users/config/locales/en.yml b/users/config/locales/en.yml
index 1aa7005..62f822c 100644
--- a/users/config/locales/en.yml
+++ b/users/config/locales/en.yml
@@ -12,6 +12,7 @@ en:
change_password: "Change Password"
login_message: "Please log in with your account."
invalid_user_pass: "Not a valid username/password combination"
+ invalid_ephemeral: "Invalid random key used. This looked like an attempt to hack the site to us. If it wasn't please contact support so we can look into the issue."
all_strategies_failed: "Could not understand your login attempt. Please first send your login and a SRP ephemeral value A and then send the client_auth in the same session (using cookies)."
update_login_and_password: "Update Login and Password"
destroy_my_account: "Destroy my account"
diff --git a/users/leap_web_users.gemspec b/users/leap_web_users.gemspec
index d33328a..7d1f220 100644
--- a/users/leap_web_users.gemspec
+++ b/users/leap_web_users.gemspec
@@ -17,6 +17,6 @@ Gem::Specification.new do |s|
s.add_dependency "leap_web_core", LeapWeb::VERSION
- s.add_dependency "ruby-srp", "~> 0.2.0"
+ s.add_dependency "ruby-srp", "~> 0.2.1"
s.add_dependency "rails_warden"
end
diff --git a/users/lib/warden/strategies/secure_remote_password.rb b/users/lib/warden/strategies/secure_remote_password.rb
index 2c681be..4688fcd 100644
--- a/users/lib/warden/strategies/secure_remote_password.rb
+++ b/users/lib/warden/strategies/secure_remote_password.rb
@@ -49,6 +49,8 @@ module Warden
else
fail! :base => 'invalid_user_pass'
end
+ rescue SRP::InvalidEphemeral
+ fail!(:base => "invalid_ephemeral")
end
def json_response(object)
diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb
index b5776ff..c65c491 100644
--- a/users/test/integration/browser/account_test.rb
+++ b/users/test/integration/browser/account_test.rb
@@ -29,6 +29,7 @@ class AccountTest < BrowserIntegrationTest
inject_malicious_js
click_on 'Log In'
assert !page.has_content?("Welcome")
+ assert page.has_content?("Invalid random key")
end
def inject_malicious_js
--
cgit v1.2.3
From c073099f0283492b30e702d833721206ab9986cc Mon Sep 17 00:00:00 2001
From: jessib <jessib@riseup.net>
Date: Mon, 19 Aug 2013 10:30:00 -0700
Subject: Change JS warning message per https://leap.se/code/issues/3492 Key
must end in _html so the html doesn't get escaped.
---
users/app/views/users/_warnings.html.haml | 2 +-
users/config/locales/en.yml | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
(limited to 'users')
diff --git a/users/app/views/users/_warnings.html.haml b/users/app/views/users/_warnings.html.haml
index 7e0b2ce..79ab103 100644
--- a/users/app/views/users/_warnings.html.haml
+++ b/users/app/views/users/_warnings.html.haml
@@ -1,5 +1,5 @@
%noscript
- %div.alert.alert-error=t :js_required
+ %div.alert.alert-error=t :js_required_html
#cookie_warning.alert.alert-error{:style => "display:none"}
=t :cookie_disabled_warning
:javascript
diff --git a/users/config/locales/en.yml b/users/config/locales/en.yml
index 62f822c..55ba3a1 100644
--- a/users/config/locales/en.yml
+++ b/users/config/locales/en.yml
@@ -32,7 +32,7 @@ en:
not_authorized_login: "Please log in to perform that action."
search: "Search"
cookie_disabled_warning: "You have cookies disabled. You will not be able to login until you enable cookies."
- js_required: "We are sorry, but this doesn't work without javascript enabled. This is for security reasons."
+ js_required_html: "We are sorry, but this doesn't work without javascript enabled. This is because the authentication system used, <a href='http://srp.stanford.edu/'>SRP</a>, requires javascript."
enable_account: "Enable the account %{username}"
enable_description: "This will restore the account to full functionality"
deactivate_account: "Deactivate the account %{username}"
--
cgit v1.2.3
From 115d96398246dcda23a51728dfafe1ea3c8ede88 Mon Sep 17 00:00:00 2001
From: jessib <jessib@riseup.net>
Date: Tue, 20 Aug 2013 12:37:58 -0700
Subject: Tweak to parameters to fix wrong-number-of-arguments error blocking
other work.
---
users/app/models/account_settings.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
(limited to 'users')
diff --git a/users/app/models/account_settings.rb b/users/app/models/account_settings.rb
index a73a95a..27fa227 100644
--- a/users/app/models/account_settings.rb
+++ b/users/app/models/account_settings.rb
@@ -16,7 +16,7 @@ class AccountSettings
protected
- def update_login(login, verifier)
+ def update_login(login)
return unless login.present?
@old_identity = Identity.for(@user)
@user.login = login
--
cgit v1.2.3
From 54243290c1550d88be0a39cdabeaf47a4a13075c Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 21 Aug 2013 07:30:36 +0200
Subject: integration test updating users password
---
users/test/integration/api/account_flow_test.rb | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
(limited to 'users')
diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb
index ec7753c..507f0b9 100644
--- a/users/test/integration/api/account_flow_test.rb
+++ b/users/test/integration/api/account_flow_test.rb
@@ -5,6 +5,7 @@ class AccountFlowTest < RackTest
setup do
@login = "integration_test_user"
+ Identity.find_by_address(@login + '@' + APP_CONFIG[:domain]).tap{|i| i.destroy if i}
User.find_by_login(@login).tap{|u| u.destroy if u}
@password = "srp, verify me!"
@srp = SRP::Client.new @login, :password => @password
@@ -18,7 +19,7 @@ class AccountFlowTest < RackTest
end
teardown do
- if @user
+ if @user.reload
@user.identity.destroy
@user.destroy
end
@@ -77,6 +78,25 @@ class AccountFlowTest < RackTest
assert_nil server_auth
end
+ test "update password via api" do
+ @srp.authenticate(self)
+ @password = "No! Verify me instead."
+ @srp = SRP::Client.new @login, :password => @password
+ @user_params = {
+ # :login => @login,
+ :password_verifier => @srp.verifier.to_s(16),
+ :password_salt => @srp.salt.to_s(16)
+ }
+ puts @user_params.inspect
+ put "http://api.lvh.me:3000/1/users/" + @user.id + '.json',
+ :user => @user_params,
+ :format => :json
+ server_auth = @srp.authenticate(self)
+ assert last_response.successful?
+ assert_nil server_auth["errors"]
+ assert server_auth["M2"]
+ end
+
test "update user" do
server_auth = @srp.authenticate(self)
test_public_key = 'asdlfkjslfdkjasd'
--
cgit v1.2.3
From 76bc9d708b119d8c5047b487ccedaa9c70fec78b Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 21 Aug 2013 07:58:52 +0200
Subject: also test updating the user password in python against dev.bm
---
users/test/integration/api/python/flow_with_srp.py | 63 +++++++++++++---------
1 file changed, 39 insertions(+), 24 deletions(-)
(limited to 'users')
diff --git a/users/test/integration/api/python/flow_with_srp.py b/users/test/integration/api/python/flow_with_srp.py
index 7b741d6..d37c6af 100755
--- a/users/test/integration/api/python/flow_with_srp.py
+++ b/users/test/integration/api/python/flow_with_srp.py
@@ -16,24 +16,24 @@ def id_generator(size=6, chars=string.ascii_lowercase + string.digits):
return ''.join(random.choice(chars) for x in range(size))
# using globals for a start
-server = 'https://api.bitmask.net:4430/1'
-login = id_generator()
+server = 'https://dev.bitmask.net/1'
+login = 'test_' + id_generator()
password = id_generator() + id_generator()
-# print ' username = "' + login + '"'
-# print ' password = "' + password + '"'
-
# log the server communication
def print_and_parse(response):
- print response.request.method + ': ' + response.url
- print " " + json.dumps(response.request.data)
+ request = response.request
+ print request.method + ': ' + response.url
+ if hasattr(request, 'data'):
+ print " " + json.dumps(response.request.data)
print " -> " + response.text
- return json.loads(response.text)
+ try:
+ return json.loads(response.text)
+ except ValueError:
+ return None
def signup(session):
salt, vkey = srp.create_salted_verification_key( login, password, srp.SHA256, srp.NG_1024 )
- # print ' salt = "' + binascii.hexlify(salt) + '"'
- # print ' v = "' + binascii.hexlify(vkey) + '"'
user_params = {
'user[login]': login,
'user[password_verifier]': binascii.hexlify(vkey),
@@ -41,38 +41,53 @@ def signup(session):
}
return session.post(server + '/users.json', data = user_params, verify = False)
-usr = srp.User( login, password, srp.SHA256, srp.NG_1024 )
+def change_password(session):
+ password = id_generator() + id_generator()
+ salt, vkey = srp.create_salted_verification_key( login, password, srp.SHA256, srp.NG_1024 )
+ user_params = {
+ 'user[password_verifier]': binascii.hexlify(vkey),
+ 'user[password_salt]': binascii.hexlify(salt)
+ }
+ print user_params
+ print_and_parse(session.put(server + '/users/' + auth['id'] + '.json', data = user_params, verify = False))
+ return srp.User( login, password, srp.SHA256, srp.NG_1024 )
+
def authenticate(session, login):
uname, A = usr.start_authentication()
- # print ' aa = "' + binascii.hexlify(A) + '"'
params = {
'login': uname,
'A': binascii.hexlify(A)
}
init = print_and_parse(session.post(server + '/sessions', data = params, verify=False))
- # print ' b = "' + init['b'] + '"'
- # print ' bb = "' + init['B'] + '"'
M = usr.process_challenge( safe_unhexlify(init['salt']), safe_unhexlify(init['B']) )
- # print ' m = "' + binascii.hexlify(M) + '"'
return session.put(server + '/sessions/' + login, verify = False,
data = {'client_auth': binascii.hexlify(M)})
+def verify_or_debug(auth):
+ if ( 'errors' in auth ):
+ print ' u = "%x"' % usr.u
+ print ' x = "%x"' % usr.x
+ print ' v = "%x"' % usr.v
+ print ' S = "%x"' % usr.S
+ print ' K = "' + binascii.hexlify(usr.K) + '"'
+ print ' M = "' + binascii.hexlify(usr.M) + '"'
+ else:
+ usr.verify_session( safe_unhexlify(auth["M2"]) )
+
+usr = srp.User( login, password, srp.SHA256, srp.NG_1024 )
session = requests.session()
user = print_and_parse(signup(session))
# SRP signup would happen here and calculate M hex
auth = print_and_parse(authenticate(session, user['login']))
-if ( 'errors' in auth ):
- print ' u = "%x"' % usr.u
- print ' x = "%x"' % usr.x
- print ' v = "%x"' % usr.v
- print ' S = "%x"' % usr.S
- print ' K = "' + binascii.hexlify(usr.K) + '"'
- print ' M = "%x"' % usr.M
-else:
- usr.verify_session( safe_unhexlify(auth["M2"]) )
+verify_or_debug(auth)
+assert usr.authenticated()
+
+usr = change_password(session)
+auth = print_and_parse(authenticate(session, user['login']))
+verify_or_debug(auth)
# At this point the authentication process is complete.
assert usr.authenticated()
--
cgit v1.2.3
From 75db45671d432a0d81805ad50c6cc9f8f7eff7a7 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 21 Aug 2013 09:49:26 +0200
Subject: use the same login validations on sessions and users
The session ones were outdated so valid usernames could not login if they contained a '.'
Refactored so both models use the same module for this validation to ensure consistency.
---
users/app/models/login_format_validation.rb | 19 +++++++++++++++++++
users/app/models/session.rb | 6 ++----
users/app/models/user.rb | 15 +--------------
users/test/integration/browser/account_test.rb | 2 +-
4 files changed, 23 insertions(+), 19 deletions(-)
create mode 100644 users/app/models/login_format_validation.rb
(limited to 'users')
diff --git a/users/app/models/login_format_validation.rb b/users/app/models/login_format_validation.rb
new file mode 100644
index 0000000..1d02bd1
--- /dev/null
+++ b/users/app/models/login_format_validation.rb
@@ -0,0 +1,19 @@
+module LoginFormatValidation
+ extend ActiveSupport::Concern
+
+ included do
+ # Have multiple regular expression validations so we can get specific error messages:
+ validates :login,
+ :format => { :with => /\A.{2,}\z/,
+ :message => "Login must have at least two characters"}
+ validates :login,
+ :format => { :with => /\A[a-z\d_\.-]+\z/,
+ :message => "Only lowercase letters, digits, . - and _ allowed."}
+ validates :login,
+ :format => { :with => /\A[a-z].*\z/,
+ :message => "Login must begin with a lowercase letter"}
+ validates :login,
+ :format => { :with => /\A.*[a-z\d]\z/,
+ :message => "Login must end with a letter or digit"}
+ end
+end
diff --git a/users/app/models/session.rb b/users/app/models/session.rb
index a9fdb1b..0d7e10e 100644
--- a/users/app/models/session.rb
+++ b/users/app/models/session.rb
@@ -1,12 +1,10 @@
class Session < SRP::Session
include ActiveModel::Validations
+ include LoginFormatValidation
attr_accessor :login
- validates :login,
- :presence => true,
- :format => { :with => /\A[A-Za-z\d_]+\z/,
- :message => "Only letters, digits and _ allowed" }
+ validates :login, :presence => true
def initialize(user = nil, aa = nil)
super(user, aa) if user
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index 0a89f7c..c1988f3 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -1,4 +1,5 @@
class User < CouchRest::Model::Base
+ include LoginFormatValidation
use_database :users
@@ -15,20 +16,6 @@ class User < CouchRest::Model::Base
:uniqueness => true,
:if => :serverside?
- # Have multiple regular expression validations so we can get specific error messages:
- validates :login,
- :format => { :with => /\A.{2,}\z/,
- :message => "Login must have at least two characters"}
- validates :login,
- :format => { :with => /\A[a-z\d_\.-]+\z/,
- :message => "Only lowercase letters, digits, . - and _ allowed."}
- validates :login,
- :format => { :with => /\A[a-z].*\z/,
- :message => "Login must begin with a lowercase letter"}
- validates :login,
- :format => { :with => /\A.*[a-z\d]\z/,
- :message => "Login must end with a letter or digit"}
-
validate :login_is_unique_alias
validates :password_salt, :password_verifier,
diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb
index c65c491..b412980 100644
--- a/users/test/integration/browser/account_test.rb
+++ b/users/test/integration/browser/account_test.rb
@@ -28,8 +28,8 @@ class AccountTest < BrowserIntegrationTest
fill_in 'Password', with: "password"
inject_malicious_js
click_on 'Log In'
- assert !page.has_content?("Welcome")
assert page.has_content?("Invalid random key")
+ assert page.has_no_content?("Welcome")
end
def inject_malicious_js
--
cgit v1.2.3
From e325de14dd0d9878a3392051dc98a14d9811d2c9 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Wed, 21 Aug 2013 20:35:50 +0200
Subject: return 204 NO CONTENT on API logout
That's the only meaningful response.
---
users/app/controllers/v1/sessions_controller.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
(limited to 'users')
diff --git a/users/app/controllers/v1/sessions_controller.rb b/users/app/controllers/v1/sessions_controller.rb
index e3459d6..c99d1f3 100644
--- a/users/app/controllers/v1/sessions_controller.rb
+++ b/users/app/controllers/v1/sessions_controller.rb
@@ -29,7 +29,7 @@ module V1
def destroy
logout
- redirect_to root_path
+ head :no_content
end
protected
--
cgit v1.2.3
From 441db4736e0cd003caf9c8f7b3fbdb1ffa72b969 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 27 Aug 2013 14:57:18 +0200
Subject: minor: remove puts line
---
users/test/integration/api/account_flow_test.rb | 1 -
1 file changed, 1 deletion(-)
(limited to 'users')
diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb
index 507f0b9..e41befa 100644
--- a/users/test/integration/api/account_flow_test.rb
+++ b/users/test/integration/api/account_flow_test.rb
@@ -87,7 +87,6 @@ class AccountFlowTest < RackTest
:password_verifier => @srp.verifier.to_s(16),
:password_salt => @srp.salt.to_s(16)
}
- puts @user_params.inspect
put "http://api.lvh.me:3000/1/users/" + @user.id + '.json',
:user => @user_params,
:format => :json
--
cgit v1.2.3
From 53a8481e1b2307c772220293a9a4e1cc939b7e07 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Thu, 22 Aug 2013 12:53:19 +0200
Subject: sort authentication controller extension
---
.../controller_extension/authentication.rb | 47 +++++++++++-----------
1 file changed, 23 insertions(+), 24 deletions(-)
(limited to 'users')
diff --git a/users/app/controllers/controller_extension/authentication.rb b/users/app/controllers/controller_extension/authentication.rb
index 5fac884..1b17589 100644
--- a/users/app/controllers/controller_extension/authentication.rb
+++ b/users/app/controllers/controller_extension/authentication.rb
@@ -7,30 +7,6 @@ module ControllerExtension::Authentication
helper_method :current_user, :logged_in?, :admin?
end
- def authentication_errors
- return unless attempted_login?
- errors = get_warden_errors
- errors.inject({}) do |translated,err|
- translated[err.first] = I18n.t(err.last)
- translated
- end
- end
-
- def get_warden_errors
- if strategy = warden.winning_strategy
- message = strategy.message
- # in case we get back the default message to fail!
- message.respond_to?(:inject) ? message : { base: message }
- else
- { login: :all_strategies_failed }
- end
- end
-
- def attempted_login?
- request.env['warden.options'] &&
- request.env['warden.options'][:attempted_path]
- end
-
def logged_in?
!!current_user
end
@@ -62,4 +38,27 @@ module ControllerExtension::Authentication
access_denied unless admin?
end
+ def authentication_errors
+ return unless attempted_login?
+ errors = get_warden_errors
+ errors.inject({}) do |translated,err|
+ translated[err.first] = I18n.t(err.last)
+ translated
+ end
+ end
+
+ def get_warden_errors
+ if strategy = warden.winning_strategy
+ message = strategy.message
+ # in case we get back the default message to fail!
+ message.respond_to?(:inject) ? message : { base: message }
+ else
+ { login: :all_strategies_failed }
+ end
+ end
+
+ def attempted_login?
+ request.env['warden.options'] &&
+ request.env['warden.options'][:attempted_path]
+ end
end
--
cgit v1.2.3
From 7ad6d054d72d3c76098f689e4e7890265a3604c8 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Mon, 26 Aug 2013 10:59:18 +0200
Subject: first steps towards enabling token based auth
---
.../controllers/controller_extension/authentication.rb | 4 ++++
.../controller_extension/token_authentication.rb | 12 ++++++++++++
users/config/initializers/add_controller_methods.rb | 1 +
users/test/functional/v1/sessions_controller_test.rb | 17 ++++++++++++++---
4 files changed, 31 insertions(+), 3 deletions(-)
create mode 100644 users/app/controllers/controller_extension/token_authentication.rb
(limited to 'users')
diff --git a/users/app/controllers/controller_extension/authentication.rb b/users/app/controllers/controller_extension/authentication.rb
index 1b17589..dca3664 100644
--- a/users/app/controllers/controller_extension/authentication.rb
+++ b/users/app/controllers/controller_extension/authentication.rb
@@ -7,6 +7,10 @@ module ControllerExtension::Authentication
helper_method :current_user, :logged_in?, :admin?
end
+ def current_user
+ @current_user ||= token_authenticate || warden.user
+ end
+
def logged_in?
!!current_user
end
diff --git a/users/app/controllers/controller_extension/token_authentication.rb b/users/app/controllers/controller_extension/token_authentication.rb
new file mode 100644
index 0000000..71dbc50
--- /dev/null
+++ b/users/app/controllers/controller_extension/token_authentication.rb
@@ -0,0 +1,12 @@
+module ControllerExtension::TokenAuthentication
+ extend ActiveSupport::Concern
+
+ def token_authenticate
+ token = nil
+ authenticate_or_request_with_http_token do |token, options|
+ token = Token.find(token)
+ end
+ User.find(token.user_id) if token
+ end
+end
+
diff --git a/users/config/initializers/add_controller_methods.rb b/users/config/initializers/add_controller_methods.rb
index 2579176..f572ecb 100644
--- a/users/config/initializers/add_controller_methods.rb
+++ b/users/config/initializers/add_controller_methods.rb
@@ -1,3 +1,4 @@
ActiveSupport.on_load(:application_controller) do
include ControllerExtension::Authentication
+ include ControllerExtension::TokenAuthentication
end
diff --git a/users/test/functional/v1/sessions_controller_test.rb b/users/test/functional/v1/sessions_controller_test.rb
index 0c4e325..8a16997 100644
--- a/users/test/functional/v1/sessions_controller_test.rb
+++ b/users/test/functional/v1/sessions_controller_test.rb
@@ -7,7 +7,7 @@ class V1::SessionsControllerTest < ActionController::TestCase
setup do
@request.env['HTTP_HOST'] = 'api.lvh.me'
- @user = stub_record :user
+ @user = stub_record :user, {}, true
@client_hex = 'a123'
end
@@ -48,13 +48,24 @@ class V1::SessionsControllerTest < ActionController::TestCase
assert_response :success
assert json_response.keys.include?("id")
assert json_response.keys.include?("token")
+ assert token = Token.find(json_response['token'])
+ assert_equal @user.id, token.user_id
end
test "logout should reset warden user" do
expect_warden_logout
delete :destroy
- assert_response :redirect
- assert_redirected_to root_url
+ assert_response 204
+ end
+
+ test "logout should remove token" do
+ login
+ expect_warden_logout
+ skip "TODO: implement token removal"
+ assert_difference "Token.count", -1 do
+ delete :destroy
+ assert_response 204
+ end
end
def expect_warden_logout
--
cgit v1.2.3
From e60ee749cab0f80cf23ca57e28c7de6d1b3a395b Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 27 Aug 2013 11:14:30 +0200
Subject: basic testing for token based auth in tests
---
.../controller_extension/token_authentication.rb | 7 ++--
users/test/factories.rb | 3 ++
users/test/functional/helper_methods_test.rb | 2 +-
users/test/functional/test_helpers_test.rb | 38 ++++++++++++++++++++++
users/test/support/auth_test_helper.rb | 9 ++++-
users/test/support/stub_record_helper.rb | 2 +-
6 files changed, 54 insertions(+), 7 deletions(-)
create mode 100644 users/test/functional/test_helpers_test.rb
(limited to 'users')
diff --git a/users/app/controllers/controller_extension/token_authentication.rb b/users/app/controllers/controller_extension/token_authentication.rb
index 71dbc50..06e9e04 100644
--- a/users/app/controllers/controller_extension/token_authentication.rb
+++ b/users/app/controllers/controller_extension/token_authentication.rb
@@ -2,11 +2,10 @@ module ControllerExtension::TokenAuthentication
extend ActiveSupport::Concern
def token_authenticate
- token = nil
- authenticate_or_request_with_http_token do |token, options|
- token = Token.find(token)
+ authenticate_or_request_with_http_token do |token_id, options|
+ @token = Token.find(token_id)
end
- User.find(token.user_id) if token
+ User.find_by_param(@token.user_id) if @token
end
end
diff --git a/users/test/factories.rb b/users/test/factories.rb
index 777704b..c87e290 100644
--- a/users/test/factories.rb
+++ b/users/test/factories.rb
@@ -18,4 +18,7 @@ FactoryGirl.define do
end
end
end
+
+ factory :token
+
end
diff --git a/users/test/functional/helper_methods_test.rb b/users/test/functional/helper_methods_test.rb
index 2b2375c..44226ae 100644
--- a/users/test/functional/helper_methods_test.rb
+++ b/users/test/functional/helper_methods_test.rb
@@ -11,7 +11,7 @@ class HelperMethodsTest < ActionController::TestCase
# we test them right in here...
include ApplicationController._helpers
- # they all reference the controller.
+ # the helpers all reference the controller.
def controller
@controller
end
diff --git a/users/test/functional/test_helpers_test.rb b/users/test/functional/test_helpers_test.rb
new file mode 100644
index 0000000..d1bdb64
--- /dev/null
+++ b/users/test/functional/test_helpers_test.rb
@@ -0,0 +1,38 @@
+#
+# There are a few test helpers for dealing with login etc.
+# We test them here and also document their behaviour.
+#
+
+require 'test_helper'
+
+class TestHelpersTest < ActionController::TestCase
+ tests ApplicationController # testing no controller in particular
+
+ def test_login_stubs_warden
+ login
+ assert_equal @current_user, request.env['warden'].user
+ end
+
+ def test_login_token_authenticates
+ login
+ assert_equal @current_user, @controller.send(:token_authenticate)
+ end
+
+ def test_login_stubs_token
+ login
+ assert @token
+ assert_equal @current_user.id, @token.user_id
+ end
+
+ def test_login_adds_token_header
+ login
+ token_present = @controller.authenticate_with_http_token do |token, options|
+ assert_equal @token.id, token
+ end
+ # authenticate_with_http_token just returns nil and does not
+ # execute the block if there is no token. So we have to also
+ # ensure it was run:
+ assert token_present
+ end
+end
+
diff --git a/users/test/support/auth_test_helper.rb b/users/test/support/auth_test_helper.rb
index 555b5db..ab6b1ac 100644
--- a/users/test/support/auth_test_helper.rb
+++ b/users/test/support/auth_test_helper.rb
@@ -13,8 +13,9 @@ module AuthTestHelper
if user_or_method_hash.respond_to?(:reverse_merge)
user_or_method_hash.reverse_merge! :is_admin? => false
end
- @current_user = stub_record(:user, user_or_method_hash, true)
+ @current_user = find_record(:user, user_or_method_hash)
request.env['warden'] = stub :user => @current_user
+ request.env['HTTP_AUTHORIZATION'] = header_for_token_auth
return @current_user
end
@@ -37,6 +38,12 @@ module AuthTestHelper
end
end
+ protected
+
+ def header_for_token_auth
+ @token = find_record(:token, :user_id => @current_user.id)
+ ActionController::HttpAuthentication::Token.encode_credentials @token.id
+ end
end
class ActionController::TestCase
diff --git a/users/test/support/stub_record_helper.rb b/users/test/support/stub_record_helper.rb
index 8aa1973..b3460d2 100644
--- a/users/test/support/stub_record_helper.rb
+++ b/users/test/support/stub_record_helper.rb
@@ -1,7 +1,7 @@
module StubRecordHelper
#
- # We will stub find_by_param or find_by_id to be called on klass and
+ # We will stub find_by_param or find to be called on klass and
# return the record given.
#
# If no record is given but a hash or nil will create a stub based on
--
cgit v1.2.3
From 420bfb326f974eec14b04d6a170ed2d28c14180f Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 27 Aug 2013 14:36:27 +0200
Subject: clear token on logout with test
---
.../controller_extension/token_authentication.rb | 12 ++++++++++++
users/test/functional/v1/sessions_controller_test.rb | 13 +++++--------
2 files changed, 17 insertions(+), 8 deletions(-)
(limited to 'users')
diff --git a/users/app/controllers/controller_extension/token_authentication.rb b/users/app/controllers/controller_extension/token_authentication.rb
index 06e9e04..e1c92e7 100644
--- a/users/app/controllers/controller_extension/token_authentication.rb
+++ b/users/app/controllers/controller_extension/token_authentication.rb
@@ -7,5 +7,17 @@ module ControllerExtension::TokenAuthentication
end
User.find_by_param(@token.user_id) if @token
end
+
+ def logout
+ super
+ clear_token
+ end
+
+ def clear_token
+ authenticate_with_http_token do |token_id, options|
+ @token = Token.find(token_id)
+ @token.destroy if @token
+ end
+ end
end
diff --git a/users/test/functional/v1/sessions_controller_test.rb b/users/test/functional/v1/sessions_controller_test.rb
index 8a16997..ff9fca1 100644
--- a/users/test/functional/v1/sessions_controller_test.rb
+++ b/users/test/functional/v1/sessions_controller_test.rb
@@ -52,20 +52,18 @@ class V1::SessionsControllerTest < ActionController::TestCase
assert_equal @user.id, token.user_id
end
- test "logout should reset warden user" do
+ test "logout should reset session" do
expect_warden_logout
delete :destroy
assert_response 204
end
- test "logout should remove token" do
+ test "logout should destroy token" do
login
expect_warden_logout
- skip "TODO: implement token removal"
- assert_difference "Token.count", -1 do
- delete :destroy
- assert_response 204
- end
+ @token.expects(:destroy)
+ delete :destroy
+ assert_response 204
end
def expect_warden_logout
@@ -76,5 +74,4 @@ class V1::SessionsControllerTest < ActionController::TestCase
request.env['warden'].expects(:logout)
end
-
end
--
cgit v1.2.3
From 0b8df3c03f440147f36858246e1003a2d0e2e54a Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 27 Aug 2013 14:51:56 +0200
Subject: make sure find_record still works with real records
---
users/test/support/stub_record_helper.rb | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
(limited to 'users')
diff --git a/users/test/support/stub_record_helper.rb b/users/test/support/stub_record_helper.rb
index b3460d2..5bccb66 100644
--- a/users/test/support/stub_record_helper.rb
+++ b/users/test/support/stub_record_helper.rb
@@ -1,15 +1,14 @@
module StubRecordHelper
#
- # We will stub find_by_param or find to be called on klass and
+ # We will stub find_by_param or find_by_id to be called on klass and
# return the record given.
#
# If no record is given but a hash or nil will create a stub based on
# that instead and returns the stub.
#
- def find_record(factory, attribs_hash = {})
- attribs_hash = attribs_hash.reverse_merge(:id => Random.rand(10000).to_s)
- record = stub_record factory, attribs_hash
+ def find_record(factory, record_or_attribs_hash = {})
+ record = stub_record factory, record_or_attribs_hash, true
klass = record.class
finder = klass.respond_to?(:find_by_param) ? :find_by_param : :find
klass.stubs(finder).with(record.to_param.to_s).returns(record)
--
cgit v1.2.3
From 147ccec989672f9b1314aa6dcc5ce8578e841370 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 27 Aug 2013 14:53:35 +0200
Subject: do not redirect if no token present
So far we allow two mechanisms of authentication:
* session based
* token based
If token fails session will be atempted in most cases. So we can't just redirect here or we get a double render error.
---
users/app/controllers/controller_extension/token_authentication.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
(limited to 'users')
diff --git a/users/app/controllers/controller_extension/token_authentication.rb b/users/app/controllers/controller_extension/token_authentication.rb
index e1c92e7..82df314 100644
--- a/users/app/controllers/controller_extension/token_authentication.rb
+++ b/users/app/controllers/controller_extension/token_authentication.rb
@@ -2,7 +2,7 @@ module ControllerExtension::TokenAuthentication
extend ActiveSupport::Concern
def token_authenticate
- authenticate_or_request_with_http_token do |token_id, options|
+ authenticate_with_http_token do |token_id, options|
@token = Token.find(token_id)
end
User.find_by_param(@token.user_id) if @token
--
cgit v1.2.3
From 5e6a2a2995598489372676bf8e045dc2dfda6c81 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 27 Aug 2013 14:55:43 +0200
Subject: token.user will get you the right user
This way we can stub the token to return the user directly. Stubbing User.find_by_param is not a good idea as it will make all calls to User#find_by_param with a different id fail.
---
users/app/controllers/controller_extension/token_authentication.rb | 2 +-
users/app/models/token.rb | 4 ++++
users/test/functional/test_helpers_test.rb | 2 +-
users/test/support/auth_test_helper.rb | 4 ++--
4 files changed, 8 insertions(+), 4 deletions(-)
(limited to 'users')
diff --git a/users/app/controllers/controller_extension/token_authentication.rb b/users/app/controllers/controller_extension/token_authentication.rb
index 82df314..3e2816d 100644
--- a/users/app/controllers/controller_extension/token_authentication.rb
+++ b/users/app/controllers/controller_extension/token_authentication.rb
@@ -5,7 +5,7 @@ module ControllerExtension::TokenAuthentication
authenticate_with_http_token do |token_id, options|
@token = Token.find(token_id)
end
- User.find_by_param(@token.user_id) if @token
+ @token.user if @token
end
def logout
diff --git a/users/app/models/token.rb b/users/app/models/token.rb
index cc62778..514b97f 100644
--- a/users/app/models/token.rb
+++ b/users/app/models/token.rb
@@ -6,6 +6,10 @@ class Token < CouchRest::Model::Base
validates :user_id, presence: true
+ def user
+ User.find(self.user_id)
+ end
+
def initialize(*args)
super
self.id = SecureRandom.urlsafe_base64(32).gsub(/^_*/, '')
diff --git a/users/test/functional/test_helpers_test.rb b/users/test/functional/test_helpers_test.rb
index d1bdb64..9bd01ad 100644
--- a/users/test/functional/test_helpers_test.rb
+++ b/users/test/functional/test_helpers_test.rb
@@ -21,7 +21,7 @@ class TestHelpersTest < ActionController::TestCase
def test_login_stubs_token
login
assert @token
- assert_equal @current_user.id, @token.user_id
+ assert_equal @current_user, @token.user
end
def test_login_adds_token_header
diff --git a/users/test/support/auth_test_helper.rb b/users/test/support/auth_test_helper.rb
index ab6b1ac..47147fc 100644
--- a/users/test/support/auth_test_helper.rb
+++ b/users/test/support/auth_test_helper.rb
@@ -13,7 +13,7 @@ module AuthTestHelper
if user_or_method_hash.respond_to?(:reverse_merge)
user_or_method_hash.reverse_merge! :is_admin? => false
end
- @current_user = find_record(:user, user_or_method_hash)
+ @current_user = stub_record(:user, user_or_method_hash)
request.env['warden'] = stub :user => @current_user
request.env['HTTP_AUTHORIZATION'] = header_for_token_auth
return @current_user
@@ -41,7 +41,7 @@ module AuthTestHelper
protected
def header_for_token_auth
- @token = find_record(:token, :user_id => @current_user.id)
+ @token = find_record(:token, :user => @current_user)
ActionController::HttpAuthentication::Token.encode_credentials @token.id
end
end
--
cgit v1.2.3
From 1e6e6d82ed53b1db558b7c1f4e14740962cc406a Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 27 Aug 2013 14:56:41 +0200
Subject: separate different tests for showing non existant user
This way the failed stubbing errors were more telling
---
users/test/functional/users_controller_test.rb | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
(limited to 'users')
diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb
index 0ce5cc2..96ae48c 100644
--- a/users/test/functional/users_controller_test.rb
+++ b/users/test/functional/users_controller_test.rb
@@ -59,19 +59,23 @@ class UsersControllerTest < ActionController::TestCase
assert_access_denied
end
- test "show for non-existing user" do
+ test "may not show non-existing user without auth" do
nonid = 'thisisnotanexistinguserid'
- # when unauthenticated:
get :show, :id => nonid
assert_access_denied(true, false)
+ end
- # when authenticated but not admin:
+ test "may not show non-existing user without admin" do
+ nonid = 'thisisnotanexistinguserid'
login
+
get :show, :id => nonid
assert_access_denied
+ end
- # when authenticated as admin:
+ test "redirect admin to user list for non-existing user" do
+ nonid = 'thisisnotanexistinguserid'
login :is_admin? => true
get :show, :id => nonid
assert_response :redirect
--
cgit v1.2.3
From d4a253d0564d4b1735fb8be5faac6a0fed174238 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 27 Aug 2013 16:20:27 +0200
Subject: use token to update user password
---
users/test/integration/api/python/flow_with_srp.py | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
(limited to 'users')
diff --git a/users/test/integration/api/python/flow_with_srp.py b/users/test/integration/api/python/flow_with_srp.py
index d37c6af..72c513f 100755
--- a/users/test/integration/api/python/flow_with_srp.py
+++ b/users/test/integration/api/python/flow_with_srp.py
@@ -16,7 +16,8 @@ def id_generator(size=6, chars=string.ascii_lowercase + string.digits):
return ''.join(random.choice(chars) for x in range(size))
# using globals for a start
-server = 'https://dev.bitmask.net/1'
+# server = 'https://dev.bitmask.net/1'
+server = 'http://api.lvh.me:3000/1'
login = 'test_' + id_generator()
password = id_generator() + id_generator()
@@ -41,15 +42,16 @@ def signup(session):
}
return session.post(server + '/users.json', data = user_params, verify = False)
-def change_password(session):
+def change_password(token):
password = id_generator() + id_generator()
salt, vkey = srp.create_salted_verification_key( login, password, srp.SHA256, srp.NG_1024 )
user_params = {
'user[password_verifier]': binascii.hexlify(vkey),
'user[password_salt]': binascii.hexlify(salt)
}
+ auth_headers = { 'Authorization': 'Token token="' + token + '"'}
print user_params
- print_and_parse(session.put(server + '/users/' + auth['id'] + '.json', data = user_params, verify = False))
+ print_and_parse(requests.put(server + '/users/' + auth['id'] + '.json', data = user_params, verify = False, headers = auth_headers))
return srp.User( login, password, srp.SHA256, srp.NG_1024 )
@@ -84,7 +86,7 @@ auth = print_and_parse(authenticate(session, user['login']))
verify_or_debug(auth)
assert usr.authenticated()
-usr = change_password(session)
+usr = change_password(auth['token'])
auth = print_and_parse(authenticate(session, user['login']))
verify_or_debug(auth)
--
cgit v1.2.3
From fdf9c5f9ea605020ea371de8e221efe8e5d5ba32 Mon Sep 17 00:00:00 2001
From: Azul <azul@leap.se>
Date: Tue, 27 Aug 2013 16:35:09 +0200
Subject: refactor: Changing the py test to use less globals and session only
locally.
---
users/test/integration/api/python/flow_with_srp.py | 59 +++++++++++-----------
1 file changed, 30 insertions(+), 29 deletions(-)
(limited to 'users')
diff --git a/users/test/integration/api/python/flow_with_srp.py b/users/test/integration/api/python/flow_with_srp.py
index 72c513f..9fc168b 100755
--- a/users/test/integration/api/python/flow_with_srp.py
+++ b/users/test/integration/api/python/flow_with_srp.py
@@ -11,16 +11,31 @@ import binascii
safe_unhexlify = lambda x: binascii.unhexlify(x) if (len(x) % 2 == 0) else binascii.unhexlify('0'+x)
+# using globals for now
+# server = 'https://dev.bitmask.net/1'
+server = 'http://api.lvh.me:3000/1'
+
+def run_tests():
+ login = 'test_' + id_generator()
+ password = id_generator() + id_generator()
+ usr = srp.User( login, password, srp.SHA256, srp.NG_1024 )
+ print_and_parse(signup(login, password))
+
+ auth = print_and_parse(authenticate(usr))
+ verify_or_debug(auth, usr)
+ assert usr.authenticated()
+
+ usr = change_password(auth['id'], login, auth['token'])
+
+ auth = print_and_parse(authenticate(usr))
+ verify_or_debug(auth, usr)
+ # At this point the authentication process is complete.
+ assert usr.authenticated()
+
# let's have some random name
def id_generator(size=6, chars=string.ascii_lowercase + string.digits):
return ''.join(random.choice(chars) for x in range(size))
-# using globals for a start
-# server = 'https://dev.bitmask.net/1'
-server = 'http://api.lvh.me:3000/1'
-login = 'test_' + id_generator()
-password = id_generator() + id_generator()
-
# log the server communication
def print_and_parse(response):
request = response.request
@@ -33,16 +48,16 @@ def print_and_parse(response):
except ValueError:
return None
-def signup(session):
+def signup(login, password):
salt, vkey = srp.create_salted_verification_key( login, password, srp.SHA256, srp.NG_1024 )
user_params = {
'user[login]': login,
'user[password_verifier]': binascii.hexlify(vkey),
'user[password_salt]': binascii.hexlify(salt)
}
- return session.post(server + '/users.json', data = user_params, verify = False)
+ return requests.post(server + '/users.json', data = user_params, verify = False)
-def change_password(token):
+def change_password(user_id, login, token):
password = id_generator() + id_generator()
salt, vkey = srp.create_salted_verification_key( login, password, srp.SHA256, srp.NG_1024 )
user_params = {
@@ -51,11 +66,12 @@ def change_password(token):
}
auth_headers = { 'Authorization': 'Token token="' + token + '"'}
print user_params
- print_and_parse(requests.put(server + '/users/' + auth['id'] + '.json', data = user_params, verify = False, headers = auth_headers))
+ print_and_parse(requests.put(server + '/users/' + user_id + '.json', data = user_params, verify = False, headers = auth_headers))
return srp.User( login, password, srp.SHA256, srp.NG_1024 )
-def authenticate(session, login):
+def authenticate(usr):
+ session = requests.session()
uname, A = usr.start_authentication()
params = {
'login': uname,
@@ -63,10 +79,10 @@ def authenticate(session, login):
}
init = print_and_parse(session.post(server + '/sessions', data = params, verify=False))
M = usr.process_challenge( safe_unhexlify(init['salt']), safe_unhexlify(init['B']) )
- return session.put(server + '/sessions/' + login, verify = False,
+ return session.put(server + '/sessions/' + uname, verify = False,
data = {'client_auth': binascii.hexlify(M)})
-def verify_or_debug(auth):
+def verify_or_debug(auth, usr):
if ( 'errors' in auth ):
print ' u = "%x"' % usr.u
print ' x = "%x"' % usr.x
@@ -77,19 +93,4 @@ def verify_or_debug(auth):
else:
usr.verify_session( safe_unhexlify(auth["M2"]) )
-usr = srp.User( login, password, srp.SHA256, srp.NG_1024 )
-session = requests.session()
-user = print_and_parse(signup(session))
-
-# SRP signup would happen here and calculate M hex
-auth = print_and_parse(authenticate(session, user['login']))
-verify_or_debug(auth)
-assert usr.authenticated()
-
-usr = change_password(auth['token'])
-
-auth = print_and_parse(authenticate(session, user['login']))
-verify_or_debug(auth)
-# At this point the authentication process is complete.
-assert usr.authenticated()
-
+run_tests()
--
cgit v1.2.3