From 38e7ba3a7c7a414c5b087f7f32df1a09403fff89 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 16 Jul 2013 19:38:37 +0200 Subject: first take on identity model - still broken --- users/app/models/identity.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 users/app/models/identity.rb (limited to 'users/app') 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 -- cgit v1.2.3 From 4a071ef1b33525fa2d1052aa7f21b549447fe767 Mon Sep 17 00:00:00 2001 From: Azul 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 +++++++++++++ 2 files changed, 13 insertions(+), 7 deletions(-) (limited to 'users/app') 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 -- cgit v1.2.3 From cc96c60c4617c09379d5e1ddbefa42407329c19a Mon Sep 17 00:00:00 2001 From: Azul 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 +++----------------- 3 files changed, 9 insertions(+), 36 deletions(-) (limited to 'users/app') 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? -- cgit v1.2.3 From a2e49d1b946fa34dd41ce1f07920515df13e09db Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 18 Jul 2013 12:28:51 +0200 Subject: local email adds domain if needed --- users/app/models/local_email.rb | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'users/app') 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 -- cgit v1.2.3 From d96fac2de074bbe3a44d888af5ceaff45b1b9b27 Mon Sep 17 00:00:00 2001 From: Azul 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 ++++++++++++++++++-- 2 files changed, 29 insertions(+), 11 deletions(-) (limited to 'users/app') 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 -- cgit v1.2.3 From 0acace58c31c96fc1f8836167aeb4f204f72617f Mon Sep 17 00:00:00 2001 From: Azul 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 +++----- 3 files changed, 25 insertions(+), 35 deletions(-) (limited to 'users/app') 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 -- cgit v1.2.3 From 495c97fbf400ed44a1e64692f2c04d2155a326e7 Mon Sep 17 00:00:00 2001 From: Azul 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/app') 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 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 ++- 2 files changed, 13 insertions(+), 1 deletion(-) (limited to 'users/app') 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 -- cgit v1.2.3 From 8ddbaa6184e4dbcc6ef7e81cf555cc18d3822dce Mon Sep 17 00:00:00 2001 From: Azul 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 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 5 deletions(-) (limited to 'users/app') 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) -- cgit v1.2.3 From 34c06b49f26bbffe2e29477f0b55a56a38aa0ce7 Mon Sep 17 00:00:00 2001 From: Azul 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/app') 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 c0b88d9e8fe574d6164f48211db50f3b8a4c4d93 Mon Sep 17 00:00:00 2001 From: Azul 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 ++++++++++++++------- 2 files changed, 24 insertions(+), 8 deletions(-) (limited to 'users/app') 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 ## -- cgit v1.2.3 From c0527cc18788fc60180d9293a546c93e6a77b788 Mon Sep 17 00:00:00 2001 From: Azul 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/app') 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 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 ---------------------------- 5 files changed, 83 insertions(+), 59 deletions(-) create mode 100644 users/app/models/account_settings.rb create mode 100644 users/app/models/signup_service.rb (limited to 'users/app') 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 ## -- cgit v1.2.3 From 370be6caa8a366774fba15d09fb03ee4d923b861 Mon Sep 17 00:00:00 2001 From: Azul 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 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'users/app') 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 ## -- cgit v1.2.3 From c073099f0283492b30e702d833721206ab9986cc Mon Sep 17 00:00:00 2001 From: jessib 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 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'users/app') 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 -- cgit v1.2.3 From 115d96398246dcda23a51728dfafe1ea3c8ede88 Mon Sep 17 00:00:00 2001 From: jessib 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/app') 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 75db45671d432a0d81805ad50c6cc9f8f7eff7a7 Mon Sep 17 00:00:00 2001 From: Azul 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 +-------------- 3 files changed, 22 insertions(+), 18 deletions(-) create mode 100644 users/app/models/login_format_validation.rb (limited to 'users/app') 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, -- cgit v1.2.3 From e325de14dd0d9878a3392051dc98a14d9811d2c9 Mon Sep 17 00:00:00 2001 From: Azul 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/app') 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 53a8481e1b2307c772220293a9a4e1cc939b7e07 Mon Sep 17 00:00:00 2001 From: Azul 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/app') 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 Date: Mon, 26 Aug 2013 10:59:18 +0200 Subject: first steps towards enabling token based auth --- users/app/controllers/controller_extension/authentication.rb | 4 ++++ .../controllers/controller_extension/token_authentication.rb | 12 ++++++++++++ 2 files changed, 16 insertions(+) create mode 100644 users/app/controllers/controller_extension/token_authentication.rb (limited to 'users/app') 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 + -- cgit v1.2.3 From e60ee749cab0f80cf23ca57e28c7de6d1b3a395b Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 27 Aug 2013 11:14:30 +0200 Subject: basic testing for token based auth in tests --- users/app/controllers/controller_extension/token_authentication.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'users/app') 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 -- cgit v1.2.3 From 420bfb326f974eec14b04d6a170ed2d28c14180f Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 27 Aug 2013 14:36:27 +0200 Subject: clear token on logout with test --- .../controllers/controller_extension/token_authentication.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'users/app') 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 -- cgit v1.2.3 From 147ccec989672f9b1314aa6dcc5ce8578e841370 Mon Sep 17 00:00:00 2001 From: Azul 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/app') 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 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 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'users/app') 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(/^_*/, '') -- cgit v1.2.3