diff options
author | jessib <jessib@riseup.net> | 2013-12-17 12:02:07 -0800 |
---|---|---|
committer | jessib <jessib@riseup.net> | 2013-12-17 12:02:07 -0800 |
commit | 98a247acb4d1be484c2982d48ec038eed3de3d55 (patch) | |
tree | f176092fa43145b9d8ce00a180fab70cf62da456 /users | |
parent | 634db9875cc8f6f6b9a4a83dfc6b8d53728eb2b5 (diff) | |
parent | 83cd3d95b78b6df9ac33a8ee169e570f4d3e2eeb (diff) |
Merge branch 'develop' into feature/billing-no-authenticated-payments
Conflicts:
billing/config/locales/en.yml
Diffstat (limited to 'users')
40 files changed, 754 insertions, 224 deletions
diff --git a/users/Gemfile b/users/Gemfile index e30033a..4101ead 100644 --- a/users/Gemfile +++ b/users/Gemfile @@ -1,4 +1,4 @@ -source "http://rubygems.org" +source "https://rubygems.org" eval(File.read(File.dirname(__FILE__) + '/../common_dependencies.rb')) eval(File.read(File.dirname(__FILE__) + '/../ui_dependencies.rb')) diff --git a/users/app/assets/javascripts/srp b/users/app/assets/javascripts/srp -Subproject d22bf3b9fe2fd31192e1e1b358e97e5a0f3f90b +Subproject 8f33d32d40b1e21ae7fb9a92c78a275422af421 diff --git a/users/app/assets/javascripts/users.js b/users/app/assets/javascripts/users.js index aaeba6e..8486756 100644 --- a/users/app/assets/javascripts/users.js +++ b/users/app/assets/javascripts/users.js @@ -46,6 +46,13 @@ $(form).find('input[type="submit"]').button('loading'); }; + resetButtons = function(submitEvent) { + var form = $('form.submitted') + // bootstrap loading state: + $(form).find('input[type="submit"]').button('reset'); + $(form).removeClass('submitted') + }; + // // PUBLIC FUNCTIONS // @@ -70,24 +77,36 @@ // srp.error = function(message) { clear_errors(); - var element, error, field; + var errors = extractErrors(message); + displayErrors(errors); + resetButtons(); + } + + function extractErrors(message) { if ($.isPlainObject(message) && message.errors) { - for (field in message.errors) { - if (field == 'base') { - alert_message(message.errors[field]); - continue; - } - error = message.errors[field]; - element = $('form input[name$="[' + field + ']"]'); - if (!element) { - continue; - } - element.trigger('element:validate:fail.ClientSideValidations', error).data('valid', false); - } - } else if (message.error) { - alert_message(message.error); + return message.errors; } else { - alert_message(JSON.stringify(message)); + return { + base: (message.error || JSON.stringify(message)) + }; + } + } + + function displayErrors(errors) { + for (var field in errors) { + var error = errors[field]; + if (field === 'base') { + alert_message(error); + } else { + displayFieldError(field, error); + } + } + } + + function displayFieldError(field, error) { + var element = $('form input[name$="[' + field + ']"]'); + if (element) { + element.trigger('element:validate:fail.ClientSideValidations', error).data('valid', false); } }; diff --git a/users/app/controllers/keys_controller.rb b/users/app/controllers/keys_controller.rb new file mode 100644 index 0000000..fb28901 --- /dev/null +++ b/users/app/controllers/keys_controller.rb @@ -0,0 +1,18 @@ +class KeysController < ApplicationController + + # + # Render the user's key as plain text, without a layout. + # + # We will show blank page if user doesn't have key (which shouldn't generally occur) + # and a 404 error if user doesn't exist + # + def show + user = User.find_by_login(params[:login]) + if user + render text: user.public_key, content_type: 'text/text' + else + raise ActionController::RoutingError.new('Not Found') + end + end + +end diff --git a/users/app/controllers/overviews_controller.rb b/users/app/controllers/overviews_controller.rb deleted file mode 100644 index 52ce267..0000000 --- a/users/app/controllers/overviews_controller.rb +++ /dev/null @@ -1,9 +0,0 @@ -class OverviewsController < UsersBaseController - - before_filter :authorize - before_filter :fetch_user - - def show - end - -end diff --git a/users/app/controllers/sessions_controller.rb b/users/app/controllers/sessions_controller.rb index 0494b51..ca228c2 100644 --- a/users/app/controllers/sessions_controller.rb +++ b/users/app/controllers/sessions_controller.rb @@ -1,6 +1,7 @@ class SessionsController < ApplicationController def new + redirect_to root_path if logged_in? @session = Session.new if authentication_errors @errors = authentication_errors @@ -14,12 +15,12 @@ class SessionsController < ApplicationController end # - # this is a bad hack, but user_overview_url(user) is not available + # this is a bad hack, but user_url(user) is not available # also, this doesn't work because the redirect happens as a PUT. no idea why. # #Warden::Manager.after_authentication do |user, auth, opts| # response = Rack::Response.new - # response.redirect "/users/#{user.id}/overview" + # response.redirect "/users/#{user.id}" # throw :warden, response.finish #end diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index f66277d..0b32ec7 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -13,7 +13,7 @@ class UsersController < UsersBaseController def index if params[:query] if @user = User.find_by_login(params[:query]) - redirect_to user_overview_url(@user) + redirect_to @user return else @users = User.by_login.startkey(params[:query]).endkey(params[:query].succ) @@ -34,6 +34,12 @@ class UsersController < UsersBaseController def edit end + ## added so updating service level works, but not sure we will actually want this. also not sure that this is place to prevent user from updating own effective service level, but here as placeholder: + def update + @user.update_attributes(params[:user]) unless (!admin? and params[:user][:effective_service_level]) + respond_with @user + end + def deactivate @user.enabled = false @user.save @@ -47,8 +53,16 @@ class UsersController < UsersBaseController end def destroy - @user.destroy - redirect_to admin? ? users_url : root_url + @user.account.destroy + flash[:notice] = I18n.t(:account_destroyed) + # admins can destroy other users + if @user != current_user + redirect_to users_url + else + # let's remove the invalid session + logout + redirect_to root_url + end end end diff --git a/users/app/controllers/v1/users_controller.rb b/users/app/controllers/v1/users_controller.rb index 03a5a62..0903888 100644 --- a/users/app/controllers/v1/users_controller.rb +++ b/users/app/controllers/v1/users_controller.rb @@ -24,15 +24,9 @@ module V1 end def update - account.update params[:user] + @user.account.update params[:user] respond_with @user end - protected - - def account - @user.account - end - end end diff --git a/users/app/models/account.rb b/users/app/models/account.rb index 5368a1b..cf998e4 100644 --- a/users/app/models/account.rb +++ b/users/app/models/account.rb @@ -1,5 +1,10 @@ # -# A Composition of a User record and it's identity records. +# The Account model takes care of the livecycle of a user. +# It composes a User record and it's identity records. +# It also allows for other engines to hook into the livecycle by +# monkeypatching the create, update and destroy methods. +# There's an ActiveSupport load_hook at the end of this file to +# make this more easy. # class Account @@ -22,16 +27,15 @@ class Account @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 + key = update_pgp_key(attrs[:public_key]) + @user.errors.set :public_key, key.errors.full_messages @user.save && save_identities @user.refresh_identity end def destroy return unless @user - Identity.by_user_id.key(@user.id).each do |identity| - identity.destroy - end + Identity.disable_all_for(@user) @user.destroy end @@ -46,12 +50,19 @@ class Account end def update_pgp_key(key) - @new_identity ||= Identity.for(@user) - @new_identity.set_key(:pgp, key) + PgpKey.new(key).tap do |key| + if key.present? && key.valid? + @new_identity ||= Identity.for(@user) + @new_identity.set_key(:pgp, key) + end + end end def save_identities @new_identity.try(:save) && @old_identity.try(:save) end + # You can hook into the account lifecycle from different engines using + # ActiveSupport.on_load(:account) do ... + ActiveSupport.run_load_hooks(:account, self) end diff --git a/users/app/models/identity.rb b/users/app/models/identity.rb index e0a24e9..cbb540e 100644 --- a/users/app/models/identity.rb +++ b/users/app/models/identity.rb @@ -27,6 +27,17 @@ class Identity < CouchRest::Model::Base emit(doc.address, doc.keys["pgp"]); } EOJS + view :disabled, + map: <<-EOJS + function(doc) { + if (doc.type != 'Identity') { + return; + } + if (typeof doc.user_id === "undefined") { + emit(doc._id, 1); + } + } + EOJS end @@ -50,6 +61,19 @@ class Identity < CouchRest::Model::Base identity end + def self.disable_all_for(user) + Identity.by_user_id.key(user.id).each do |identity| + identity.disable + identity.save + end + end + + def self.destroy_all_disabled + Identity.disabled.each do |identity| + identity.destroy + end + end + def self.attributes_from_user(user) { user_id: user.id, address: user.email_address, @@ -57,13 +81,22 @@ class Identity < CouchRest::Model::Base } end + def enabled? + self.destination && self.user_id + end + + def disable + self.destination = nil + self.user_id = nil + 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)) + def set_key(type, key) + return if keys[type] == key.to_s + write_attribute('keys', keys.merge(type => key.to_s)) end # for LoginFormatValidation @@ -93,7 +126,8 @@ class Identity < CouchRest::Model::Base end def destination_email - return if destination.valid? #this ensures it is Email + return if destination.nil? # this identity is disabled + return if destination.valid? # this ensures it is Email self.errors.add(:destination, destination.errors.messages[:email].first) #assumes only one error #TODO end diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb index 6303bb6..2b4c65e 100644 --- a/users/app/models/local_email.rb +++ b/users/app/models/local_email.rb @@ -1,5 +1,10 @@ class LocalEmail < Email + BLACKLIST_FROM_RFC2142 = [ + 'postmaster', 'hostmaster', 'domainadmin', 'webmaster', 'www', + 'abuse', 'noc', 'security', 'usenet', 'news', 'uucp', + 'ftp', 'sales', 'marketing', 'support', 'info' + ] def self.domain APP_CONFIG[:domain] @@ -11,6 +16,8 @@ class LocalEmail < Email :message => "needs to end in @#{domain}" } + validate :handle_allowed + def initialize(s) super append_domain_if_needed @@ -32,4 +39,30 @@ class LocalEmail < Email end end + def handle_allowed + errors.add(:handle, "is reserved.") if handle_reserved? + end + + def handle_reserved? + # *ARRAY in a case statement tests if ARRAY includes the handle. + case handle + when *APP_CONFIG[:handle_blacklist] + true + when *APP_CONFIG[:handle_whitelist] + false + when *BLACKLIST_FROM_RFC2142 + true + else + handle_in_passwd? + end + end + + def handle_in_passwd? + begin + !!Etc.getpwnam(handle) + rescue ArgumentError + # handle was not found + return false + end + end end diff --git a/users/app/models/pgp_key.rb b/users/app/models/pgp_key.rb new file mode 100644 index 0000000..66f8660 --- /dev/null +++ b/users/app/models/pgp_key.rb @@ -0,0 +1,48 @@ +class PgpKey + include ActiveModel::Validations + + KEYBLOCK_IDENTIFIERS = [ + '-----BEGIN PGP PUBLIC KEY BLOCK-----', + '-----END PGP PUBLIC KEY BLOCK-----', + ] + + # mostly for testing. + attr_accessor :keyblock + + validate :validate_keyblock_format + + def initialize(keyblock = nil) + @keyblock = keyblock + end + + def to_s + @keyblock + end + + def present? + @keyblock.present? + end + + # allow comparison with plain keyblock strings. + def ==(other) + self.equal?(other) or + # relax the comparison on line ends. + self.to_s.tr_s("\n\r", '') == other.tr_s("\r\n", '') + end + + protected + + def validate_keyblock_format + if keyblock_identifier_missing? + errors.add :public_key_block, + "does not look like an armored pgp public key block" + end + end + + def keyblock_identifier_missing? + KEYBLOCK_IDENTIFIERS.find do |identify| + !@keyblock.include?(identify) + end + end + +end diff --git a/users/app/models/service_level.rb b/users/app/models/service_level.rb new file mode 100644 index 0000000..299aaf1 --- /dev/null +++ b/users/app/models/service_level.rb @@ -0,0 +1,19 @@ +class ServiceLevel + + def initialize(attributes = {}) + @id = attributes[:id] || APP_CONFIG[:default_service_level] + end + + def self.authenticated_select_options + APP_CONFIG[:service_levels].map { |id,config_hash| [config_hash[:description], id] if config_hash[:name] != 'anonymous'}.compact + end + + def id + @id + end + + def config_hash + APP_CONFIG[:service_levels][@id] + end + +end diff --git a/users/app/models/token.rb b/users/app/models/token.rb index dd87344..001eb40 100644 --- a/users/app/models/token.rb +++ b/users/app/models/token.rb @@ -11,6 +11,25 @@ class Token < CouchRest::Model::Base validates :user_id, presence: true + design do + view :by_last_seen_at + end + + def self.expires_after + APP_CONFIG[:auth] && APP_CONFIG[:auth][:token_expires_after] + end + + def self.expired + return [] unless expires_after + by_last_seen_at.endkey(expires_after.minutes.ago) + end + + def self.destroy_all_expired + self.expired.each do |token| + token.destroy + end + end + def authenticate if expired? destroy @@ -27,21 +46,16 @@ class Token < CouchRest::Model::Base end def expired? - expires_after and - last_seen_at + expires_after.minutes < Time.now - end - - def expires_after - APP_CONFIG[:auth] && APP_CONFIG[:auth][:token_expires_after] + Token.expires_after and + last_seen_at < Token.expires_after.minutes.ago end def initialize(*args) super - self.id = SecureRandom.urlsafe_base64(32).gsub(/^_*/, '') - self.last_seen_at = Time.now - end - - design do + if new_record? + self.id = SecureRandom.urlsafe_base64(32).gsub(/^_*/, '') + self.last_seen_at = Time.now + end end end diff --git a/users/app/models/unauthenticated_user.rb b/users/app/models/unauthenticated_user.rb index 99a6874..0fc17d2 100644 --- a/users/app/models/unauthenticated_user.rb +++ b/users/app/models/unauthenticated_user.rb @@ -1,4 +1,6 @@ # The nil object for the user class class UnauthenticatedUser < Object + # will probably want something here to return service level as APP_CONFIG[:service_levels][0] but not sure how will be accessing. + end diff --git a/users/app/models/user.rb b/users/app/models/user.rb index a14fcb5..720f5a9 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -9,6 +9,12 @@ class User < CouchRest::Model::Base property :enabled, TrueClass, :default => true + # these will be null by default but we shouldn't ever pull them directly, but only via the methods that will return the full ServiceLevel + property :desired_service_level_code, Integer, :accessible => true + property :effective_service_level_code, Integer, :accessible => true + + before_save :update_effective_service_level + validates :login, :password_salt, :password_verifier, :presence => true @@ -94,6 +100,16 @@ class User < CouchRest::Model::Base @identity = Identity.for(self) end + def desired_service_level + code = self.desired_service_level_code || APP_CONFIG[:default_service_level] + ServiceLevel.new({id: code}) + end + + def effective_service_level + code = self.effective_service_level_code || self.desired_service_level.id + ServiceLevel.new({id: code}) + end + protected ## @@ -116,4 +132,12 @@ class User < CouchRest::Model::Base def serverside? true end + + def update_effective_service_level + # TODO: Is this always the case? Might there be a situation where the admin has set the effective service level and we don't want it changed to match the desired one? + if self.desired_service_level_code_changed? + self.effective_service_level_code = self.desired_service_level_code + end + end + end diff --git a/users/app/views/overviews/show.html.haml b/users/app/views/overviews/show.html.haml deleted file mode 100644 index d3409df..0000000 --- a/users/app/views/overviews/show.html.haml +++ /dev/null @@ -1,22 +0,0 @@ -.overview - - %h2.first= t(:overview_welcome, :username => @user.login) - - - if admin? - %p - = t(:created) - = @user.created_at - %br - = t(:updated) - = @user.updated_at - %br - = t(:enabled) - = @user.enabled? - - %p= t(:overview_intro) - - %ul.unstyled - %li= icon('user') + link_to(t(:overview_account), edit_user_path(@user)) - - # %li= icon('envelope') + link_to(t(:overview_email), {insert path for user identities, presuambly} - %li= icon('question-sign') + link_to(t(:overview_tickets), user_tickets_path(@user)) - %li= icon('shopping-cart') + link_to(t(:overview_billing), show_or_new_customer_link(@user)) if APP_CONFIG[:payment].present? diff --git a/users/app/views/users/_change_password.html.haml b/users/app/views/users/_change_password.html.haml new file mode 100644 index 0000000..425e3ee --- /dev/null +++ b/users/app/views/users/_change_password.html.haml @@ -0,0 +1,21 @@ +-# +-# CHANGE PASSWORD +-# +-# * everything about this form is handled with javascript. So take care when changing any ids. +-# * the login is required when changing the password because it is used as part of the salt when calculating the password verifier. +-# however, we don't want the user to change their login without generating a new key, so we hide the ui for this +-# (although it works perfectly fine to change username if the field was visible). +-# + +- form_options = {:url => '/not-used', :html => {:class => user_form_class('form-horizontal'), :id => 'update_login_and_password', :data => {token: session[:token]}}, :validate => true} += simple_form_for @user, form_options do |f| + %legend= t(:change_password) + = hidden_field_tag 'user_param', @user.to_param + .hidden + = f.input :login, :label => t(:username), :required => false, :input_html => {:id => :srp_username} + = f.input :password, :required => false, :validate => true, :input_html => { :id => :srp_password } + = f.input :password_confirmation, :required => false, :input_html => { :id => :srp_password_confirmation } + .control-group + .controls + = f.submit t(:save), :class => 'btn btn-primary' + diff --git a/users/app/views/users/_change_pgp_key.html.haml b/users/app/views/users/_change_pgp_key.html.haml new file mode 100644 index 0000000..e465125 --- /dev/null +++ b/users/app/views/users/_change_pgp_key.html.haml @@ -0,0 +1,13 @@ +-# +-# 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', :data => {token: session[:token]}}, :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', :data => {"loading-text" => "Saving..."} diff --git a/users/app/views/users/_change_service_level.html.haml b/users/app/views/users/_change_service_level.html.haml new file mode 100644 index 0000000..61e67d9 --- /dev/null +++ b/users/app/views/users/_change_service_level.html.haml @@ -0,0 +1,18 @@ +-# TODO: probably won't want here, but here for now. Also, we will need way to ensure payment if they pick a non-free plan. +-# +-# SERVICE LEVEL +-# +- if APP_CONFIG[:service_levels] + - form_options = {:html => {:class => user_form_class('form-horizontal'), :id => 'update_service_level', :data => {token: session[:token]}}, :validate => true} + = simple_form_for @user, form_options do |f| + %legend= t(:service_level) + - if @user != current_user + = t(:desired_service_level) + = f.select :desired_service_level_code, ServiceLevel.authenticated_select_options, :selected => @user.desired_service_level.id + - if @user != current_user + %p + = t(:effective_service_level) + = f.select :effective_service_level_code, ServiceLevel.authenticated_select_options, :selected => @user.effective_service_level.id + .control-group + .controls + = f.submit t(:save), :class => 'btn', :data => {"loading-text" => "Saving..."} diff --git a/users/app/views/users/_destroy_account.html.haml b/users/app/views/users/_destroy_account.html.haml new file mode 100644 index 0000000..445f3c4 --- /dev/null +++ b/users/app/views/users/_destroy_account.html.haml @@ -0,0 +1,27 @@ +-# +-# DESTROY ACCOUNT +-# + +%legend + - if @user == current_user + = t(:destroy_my_account) + - else + = t(:admin_destroy_account, :username => @user.login) +%p= t(:destroy_account_info) += link_to user_path(@user), :method => :delete, :confirm => t(:are_you_sure), :class => "btn btn-danger" do + %i.icon-remove.icon-white + = t(:destroy_my_account) +- if @user != current_user and @user.enabled? + %legend + = t(:deactivate_account, :username => @user.login) + %p= t(:deactivate_description) + = link_to deactivate_user_path(@user), :method => :post, :class => "btn btn-warning" do + %i.icon-pause.icon-white + = t(:deactivate) +- elsif @user != current_user and !@user.enabled? + %legend + = t(:enable_account, :username => @user.login) + %p= t(:enable_description) + = link_to enable_user_path(@user), :method => :post, :class => "btn btn-warning" do + %i.icon-ok.icon-white + = t(:enable) diff --git a/users/app/views/users/_edit.html.haml b/users/app/views/users/_edit.html.haml index 9d2473b..1d2b68a 100644 --- a/users/app/views/users/_edit.html.haml +++ b/users/app/views/users/_edit.html.haml @@ -1,66 +1,14 @@ -# -# edit user form, used by both show and edit actions. -# - --# --# CHANGE PASSWORD --# --# * everything about this form is handled with javascript. So take care when changing any ids. --# * the login is required when changing the password because it is used as part of the salt when calculating the password verifier. --# however, we don't want the user to change their login without generating a new key, so we hide the ui for this --# (although it works perfectly fine to change username if the field was visible). --# - -- form_options = {:url => '/not-used', :html => {:class => user_form_class('form-horizontal'), :id => 'update_login_and_password', :data => {token: session[:token]}}, :validate => true} -= simple_form_for @user, form_options do |f| - %legend= t(:change_password) - = hidden_field_tag 'user_param', @user.to_param - .hidden - = f.input :login, :label => t(:username), :required => false, :input_html => {:id => :srp_username} - = f.input :password, :required => false, :validate => true, :input_html => { :id => :srp_password } - = f.input :password_confirmation, :required => false, :input_html => { :id => :srp_password_confirmation } - .control-group - .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', :data => {token: session[:token]}}, :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', :data => {"loading-text" => "Saving..."} - --# --# DESTROY ACCOUNT --# - -%legend - - if @user == current_user - = t(:destroy_my_account) - - else - = t(:admin_destroy_account, :username => @user.login) -%p= t(:destroy_account_info) -= link_to user_path(@user), :method => :delete, :confirm => t(:are_you_sure), :class => "btn btn-danger" do - %i.icon-remove.icon-white - = t(:destroy_my_account) -- if @user != current_user and @user.enabled? - %legend - = t(:deactivate_account, :username => @user.login) - %p= t(:deactivate_description) - = link_to deactivate_user_path(@user), :method => :post, :class => "btn btn-warning" do - %i.icon-pause.icon-white - = t(:deactivate) -- elsif @user != current_user and !@user.enabled? - %legend - = t(:enable_account, :username => @user.login) - %p= t(:enable_description) - = link_to enable_user_path(@user), :method => :post, :class => "btn btn-warning" do - %i.icon-ok.icon-white - = t(:enable) +-# We render a bunch of forms here. Which we use depends upon config settings +-# user_actions and admin_actions. They both include an array of actions +-# allowed to users and admins. +-# Possible forms are: +-# 'change_password' +-# 'change_pgp_key' +-# 'change_service_level' +-# 'destroy_account' +- actions = APP_CONFIG[admin? ? :admin_actions : :user_actions] || [] +- actions.each do |action| + = render action diff --git a/users/app/views/users/_user.html.haml b/users/app/views/users/_user.html.haml index 990d9cf..583d22f 100644 --- a/users/app/views/users/_user.html.haml +++ b/users/app/views/users/_user.html.haml @@ -1,4 +1,4 @@ %tr - %td= link_to user.login, user_overview_path(user) + %td= link_to user.login, user %td= l(user.created_at, :format => :short) %td= l(user.updated_at, :format => :short) diff --git a/users/app/views/users/show.html.haml b/users/app/views/users/show.html.haml index 434c025..7bea370 100644 --- a/users/app/views/users/show.html.haml +++ b/users/app/views/users/show.html.haml @@ -1 +1,22 @@ -= render 'edit' +.overview + + %h2.first= t(:overview_welcome, :username => @user.login) + + - if admin? + %p + = t(:created) + = @user.created_at + %br + = t(:updated) + = @user.updated_at + %br + = t(:enabled) + = @user.enabled? + + %p= t(:overview_intro) + + %ul.unstyled + %li= icon('user') + link_to(t(:overview_account), edit_user_path(@user)) + - # %li= icon('envelope') + link_to(t(:overview_email), {insert path for user identities, presuambly} + %li= icon('question-sign') + link_to(t(:overview_tickets), user_tickets_path(@user)) + %li= icon('shopping-cart') + link_to(t(:overview_billing), billing_top_link(@user)) if APP_CONFIG[:payment].present? diff --git a/users/config/locales/en.yml b/users/config/locales/en.yml index b69f7f4..1b5dd5e 100644 --- a/users/config/locales/en.yml +++ b/users/config/locales/en.yml @@ -17,6 +17,7 @@ en: destroy_my_account: "Destroy my account" destroy_account_info: "This will permanently destroy your account and all the data associated with it. Proceed with caution!" admin_destroy_account: "Destroy the account %{username}" + account_destroyed: "The account has been destroyed successfully." set_email_address: "Set email address" forward_email: "Forward Email" email_aliases: "Email Aliases" diff --git a/users/config/routes.rb b/users/config/routes.rb index ccecfd5..de2ff37 100644 --- a/users/config/routes.rb +++ b/users/config/routes.rb @@ -13,13 +13,14 @@ Rails.application.routes.draw do get "signup" => "users#new", :as => "signup" resources :users, :except => [:create, :update] do - resource :overview, :only => [:show] # resource :email_settings, :only => [:edit, :update] - resources :email_aliases, :only => [:destroy], :id => /.*/ + # resources :email_aliases, :only => [:destroy], :id => /.*/ post 'deactivate', on: :member post 'enable', on: :member end get "/.well-known/host-meta" => 'webfinger#host_meta' get "/webfinger" => 'webfinger#search' + get "/key/:login" => 'keys#show' + end diff --git a/users/test/factories.rb b/users/test/factories.rb index c87e290..ae00d43 100644 --- a/users/test/factories.rb +++ b/users/test/factories.rb @@ -19,6 +19,16 @@ FactoryGirl.define do end end - factory :token + factory :token do + user + end + factory :pgp_key do + keyblock <<-EOPGP +-----BEGIN PGP PUBLIC KEY BLOCK----- ++Dummy+PGP+KEY+++Dummy+PGP+KEY+++Dummy+PGP+KEY+++Dummy+PGP+KEY+ +#{SecureRandom.base64(4032)} +-----END PGP PUBLIC KEY BLOCK----- + EOPGP + end end diff --git a/users/test/functional/keys_controller_test.rb b/users/test/functional/keys_controller_test.rb new file mode 100644 index 0000000..863be93 --- /dev/null +++ b/users/test/functional/keys_controller_test.rb @@ -0,0 +1,32 @@ +require 'test_helper' + +class KeysControllerTest < ActionController::TestCase + + test "get existing public key" do + public_key = 'my public key' + @user = stub_record :user, :public_key => public_key + User.stubs(:find_by_login).with(@user.login).returns(@user) + get :show, :login => @user.login + assert_response :success + assert_equal "text/text", response.content_type + assert_equal public_key, response.body + end + + test "get non-existing public key for user" do + # this isn't a scenerio that should generally occur. + @user = stub_record :user + User.stubs(:find_by_login).with(@user.login).returns(@user) + get :show, :login => @user.login + assert_response :success + assert_equal "text/text", response.content_type + assert_equal '', response.body.strip + end + + test "get public key for non-existing user" do + # raise 404 error if user doesn't exist (doesn't need to be this routing error, but seems fine to assume for now): + assert_raise(ActionController::RoutingError) { + get :show, :login => 'asdkljslksjfdlskfj' + } + end + +end diff --git a/users/test/functional/sessions_controller_test.rb b/users/test/functional/sessions_controller_test.rb index a630e6e..8b49005 100644 --- a/users/test/functional/sessions_controller_test.rb +++ b/users/test/functional/sessions_controller_test.rb @@ -17,6 +17,13 @@ class SessionsControllerTest < ActionController::TestCase assert_template "sessions/new" end + test "redirect to root_url if logged in" do + login + get :new + assert_response :redirect + assert_redirected_to root_url + end + test "renders json" do get :new, :format => :json assert_response :success @@ -41,20 +48,12 @@ class SessionsControllerTest < ActionController::TestCase assert_json_error :login => I18n.t(:all_strategies_failed) end - test "logout should reset warden user" do - expect_warden_logout + test "destory should logout" do + login + expect_logout delete :destroy assert_response :redirect assert_redirected_to root_url end - def expect_warden_logout - raw = mock('raw session') do - expects(:inspect) - end - request.env['warden'].expects(:raw_session).returns(raw) - request.env['warden'].expects(:logout) - end - - end diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb index 052de04..9c5f8d9 100644 --- a/users/test/functional/users_controller_test.rb +++ b/users/test/functional/users_controller_test.rb @@ -77,7 +77,11 @@ class UsersControllerTest < ActionController::TestCase test "admin can destroy user" do user = find_record :user + + # we destroy the user record and the associated data... user.expects(:destroy) + Identity.expects(:disable_all_for).with(user) + Ticket.expects(:destroy_all_from).with(user) login :is_admin? => true delete :destroy, :id => user.id @@ -88,9 +92,14 @@ class UsersControllerTest < ActionController::TestCase test "user can cancel account" do user = find_record :user + + # we destroy the user record and the associated data... user.expects(:destroy) + Identity.expects(:disable_all_for).with(user) + Ticket.expects(:destroy_all_from).with(user) login user + expect_logout delete :destroy, :id => @current_user.id assert_response :redirect diff --git a/users/test/functional/v1/sessions_controller_test.rb b/users/test/functional/v1/sessions_controller_test.rb index ff9fca1..4200e8f 100644 --- a/users/test/functional/v1/sessions_controller_test.rb +++ b/users/test/functional/v1/sessions_controller_test.rb @@ -52,26 +52,11 @@ class V1::SessionsControllerTest < ActionController::TestCase assert_equal @user.id, token.user_id end - test "logout should reset session" do - expect_warden_logout - delete :destroy - assert_response 204 - end - - test "logout should destroy token" do + test "destroy should logout" do login - expect_warden_logout - @token.expects(:destroy) + expect_logout delete :destroy assert_response 204 end - def expect_warden_logout - raw = mock('raw session') do - expects(:inspect) - end - request.env['warden'].expects(:raw_session).returns(raw) - request.env['warden'].expects(:logout) - end - end diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb index e41befa..edd0859 100644 --- a/users/test/integration/api/account_flow_test.rb +++ b/users/test/integration/api/account_flow_test.rb @@ -96,27 +96,41 @@ class AccountFlowTest < RackTest assert server_auth["M2"] end - test "update user" do + test "prevent changing login without changing password_verifier" do server_auth = @srp.authenticate(self) - 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 + put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:login => new_login}, :format => :json assert last_response.successful? - 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 - 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 + end + + test "upload pgp key" do + server_auth = @srp.authenticate(self) + key = FactoryGirl.build :pgp_key + put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => key}, :format => :json + assert_equal key, Identity.for(@user).keys[:pgp] + end + + # eventually probably want to remove most of this into a non-integration + # functional test + test "prevent uploading invalid key" do + server_auth = @srp.authenticate(self) + put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => :blah}, :format => :json assert_nil Identity.for(@user).keys[:pgp] end + test "prevent emptying public key" do + server_auth = @srp.authenticate(self) + key = FactoryGirl.build :pgp_key + put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => key}, :format => :json + put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => ""}, :format => :json + assert_equal key, Identity.for(@user).keys[:pgp] + end + end diff --git a/users/test/integration/api/python/umlauts.py b/users/test/integration/api/python/umlauts.py new file mode 100755 index 0000000..96fecbf --- /dev/null +++ b/users/test/integration/api/python/umlauts.py @@ -0,0 +1,79 @@ +#!/usr/bin/env python +# coding: utf-8 + +# under development + +import requests +import json +import string +import random +import srp._pysrp as srp +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() + + +# 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)) + +# log the server communication +def print_and_parse(response): + request = response.request + print request.method + ': ' + response.url + if hasattr(request, 'data'): + print " " + json.dumps(response.request.data) + print " -> " + response.text + try: + return json.loads(response.text) + except ValueError: + return None + +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) + } + print json.dumps(user_params) + return requests.post(server + '/users.json', data = user_params, verify = False) + +def authenticate(usr): + session = requests.session() + uname, A = usr.start_authentication() + params = { + 'login': uname, + 'A': binascii.hexlify(A) + } + 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/' + uname, verify = False, + data = {'client_auth': binascii.hexlify(M)}) + +def verify_or_debug(auth, usr): + 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"]) ) + +run_tests() diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb index 1deda45..4cefe35 100644 --- a/users/test/integration/browser/account_test.rb +++ b/users/test/integration/browser/account_test.rb @@ -6,6 +6,10 @@ class AccountTest < BrowserIntegrationTest Capybara.current_driver = Capybara.javascript_driver end + teardown do + Identity.destroy_all_disabled + end + test "normal account workflow" do username, password = submit_signup assert page.has_content?("Welcome #{username}") @@ -19,46 +23,85 @@ class AccountTest < BrowserIntegrationTest test "successful login" do username, password = submit_signup click_on 'Logout' - click_on 'Log In' - fill_in 'Username', with: username - fill_in 'Password', with: password - click_on 'Log In' + attempt_login(username, password) assert page.has_content?("Welcome #{username}") User.find_by_login(username).account.destroy end - test "change password" do + test "failed login" do + visit '/' + attempt_login("username", "wrong password") + assert_invalid_login(page) + end + + test "account destruction" do + username, password = submit_signup + click_on I18n.t('account_settings') + click_on I18n.t('destroy_my_account') + assert page.has_content?(I18n.t('account_destroyed')) + attempt_login(username, password) + assert_invalid_login(page) + end + + test "handle blocked after account destruction" do + username, password = submit_signup + click_on I18n.t('account_settings') + click_on I18n.t('destroy_my_account') + submit_signup(username) + assert page.has_content?('has already been taken') + end + + test "default user actions" do username, password = submit_signup click_on "Account Settings" - within('#update_login_and_password') do - fill_in 'Password', with: "other password" - fill_in 'Password confirmation', with: "other password" - click_on 'Save' + assert page.has_content? I18n.t('destroy_my_account') + assert page.has_no_css? '#update_login_and_password' + assert page.has_no_css? '#update_pgp_key' + end + + test "default admin actions" do + username, password = submit_signup + with_config admins: [username] do + click_on "Account Settings" + assert page.has_content? I18n.t('destroy_my_account') + assert page.has_no_css? '#update_login_and_password' + assert page.has_css? '#update_pgp_key' + end + end + + test "change password" do + with_config user_actions: ['change_password'] do + username, password = submit_signup + click_on "Account Settings" + within('#update_login_and_password') do + fill_in 'Password', with: "other password" + fill_in 'Password confirmation', with: "other password" + click_on 'Save' + end + click_on 'Logout' + attempt_login(username, "other password") + assert page.has_content?("Welcome #{username}") + User.find_by_login(username).account.destroy end - click_on 'Logout' - click_on 'Log In' - fill_in 'Username', with: username - fill_in 'Password', with: "other password" - click_on 'Log In' - assert page.has_content?("Welcome #{username}") - User.find_by_login(username).account.destroy end test "change pgp key" do - pgp_key = "My PGP Key Stub" - username, password = submit_signup - click_on "Account Settings" - within('#update_pgp_key') do - fill_in 'Public key', with: pgp_key - click_on 'Save' + with_config user_actions: ['change_pgp_key'] do + pgp_key = FactoryGirl.build :pgp_key + username, password = submit_signup + click_on "Account Settings" + within('#update_pgp_key') do + fill_in 'Public key', with: pgp_key + click_on 'Save' + end + page.assert_selector 'input[value="Saving..."]' + # at some point we're done: + page.assert_no_selector 'input[value="Saving..."]' + assert page.has_field? 'Public key', with: pgp_key.to_s + user = User.find_by_login(username) + assert_equal pgp_key, user.public_key + user.account.destroy end - page.assert_selector 'input[value="Saving..."]' - # at some point we're done: - page.assert_no_selector 'input[value="Saving..."]' - assert page.has_field? 'Public key', with: pgp_key - user = User.find_by_login(username) - assert_equal pgp_key, user.public_key - user.account.destroy end @@ -81,6 +124,19 @@ class AccountTest < BrowserIntegrationTest assert page.has_content?("server failed") end + def attempt_login(username, password) + click_on 'Log In' + fill_in 'Username', with: username + fill_in 'Password', with: password + click_on 'Log In' + end + + def assert_invalid_login(page) + assert page.has_selector? 'input.btn-primary.disabled' + assert page.has_content? I18n.t(:invalid_user_pass) + assert page.has_no_selector? 'input.btn-primary.disabled' + end + def inject_malicious_js page.execute_script <<-EOJS var calc = new srp.Calculate(); diff --git a/users/test/support/auth_test_helper.rb b/users/test/support/auth_test_helper.rb index 609f115..50e9453 100644 --- a/users/test/support/auth_test_helper.rb +++ b/users/test/support/auth_test_helper.rb @@ -38,12 +38,26 @@ module AuthTestHelper end end + def expect_logout + expect_warden_logout + @token.expects(:destroy) if @token + end + protected def header_for_token_auth @token = find_record(:token, :authenticate => @current_user) ActionController::HttpAuthentication::Token.encode_credentials @token.id end + + def expect_warden_logout + raw = mock('raw session') do + expects(:inspect) + end + request.env['warden'].expects(:raw_session).returns(raw) + request.env['warden'].expects(:logout) + end + end class ActionController::TestCase diff --git a/users/test/support/integration_test_helper.rb b/users/test/support/integration_test_helper.rb index cfe72cf..51e47c6 100644 --- a/users/test/support/integration_test_helper.rb +++ b/users/test/support/integration_test_helper.rb @@ -1,7 +1,7 @@ module IntegrationTestHelper - def submit_signup - username = "test_#{SecureRandom.urlsafe_base64}".downcase - password = SecureRandom.base64 + def submit_signup(username = nil, password = nil) + username ||= "test_#{SecureRandom.urlsafe_base64}".downcase + password ||= SecureRandom.base64 visit '/users/new' fill_in 'Username', with: username fill_in 'Password', with: password diff --git a/users/test/unit/account_test.rb b/users/test/unit/account_test.rb index 94a9980..4fb3c3d 100644 --- a/users/test/unit/account_test.rb +++ b/users/test/unit/account_test.rb @@ -2,6 +2,10 @@ require 'test_helper' class AccountTest < ActiveSupport::TestCase + teardown do + Identity.destroy_all_disabled + end + test "create a new account" do user = Account.create(FactoryGirl.attributes_for(:user)) assert user.valid? @@ -13,7 +17,8 @@ class AccountTest < ActiveSupport::TestCase end test "create and remove a user account" do - assert_no_difference "Identity.count" do + # We keep an identity that will block the handle from being reused. + assert_difference "Identity.count" do assert_no_difference "User.count" do user = Account.create(FactoryGirl.attributes_for(:user)) user.account.destroy diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb index 0842a77..eca104f 100644 --- a/users/test/unit/identity_test.rb +++ b/users/test/unit/identity_test.rb @@ -90,6 +90,35 @@ class IdentityTest < ActiveSupport::TestCase assert id.errors.messages[:destination].include? "needs to be a valid email address" end + test "disabled identity" do + id = Identity.for(@user) + id.disable + assert_equal @user.email_address, id.address + assert_equal nil, id.destination + assert_equal nil, id.user + assert !id.enabled? + assert id.valid? + end + + test "disabled identity blocks handle" do + id = Identity.for(@user) + id.disable + id.save + other_user = find_record :user + taken = Identity.build_for other_user, address: id.address + assert !taken.valid? + Identity.destroy_all_disabled + end + + test "destroy all disabled identities" do + id = Identity.for(@user) + id.disable + id.save + assert Identity.count > 0 + Identity.destroy_all_disabled + assert_equal 0, Identity.disabled.count + end + def alias_name @alias_name ||= Faker::Internet.user_name end diff --git a/users/test/unit/local_email_test.rb b/users/test/unit/local_email_test.rb index b25f46f..20ee7f1 100644 --- a/users/test/unit/local_email_test.rb +++ b/users/test/unit/local_email_test.rb @@ -24,6 +24,37 @@ class LocalEmailTest < ActiveSupport::TestCase assert_equal ["needs to end in @#{LocalEmail.domain}"], local.errors[:email] end + test "blacklists rfc2142" do + black_listed = LocalEmail.new('hostmaster') + assert !black_listed.valid? + end + + test "blacklists etc passwd" do + black_listed = LocalEmail.new('nobody') + assert !black_listed.valid? + end + + test "whitelist overwrites automatic blacklists" do + with_config handle_whitelist: ['nobody', 'hostmaster'] do + white_listed = LocalEmail.new('nobody') + assert white_listed.valid? + white_listed = LocalEmail.new('hostmaster') + assert white_listed.valid? + end + end + + test "blacklists from config" do + black_listed = LocalEmail.new('www-data') + assert !black_listed.valid? + end + + test "blacklist from config overwrites whitelist" do + with_config handle_whitelist: ['www-data'] do + black_listed = LocalEmail.new('www-data') + assert !black_listed.valid? + end + end + def handle @handle ||= Faker::Internet.user_name end diff --git a/users/test/unit/token_test.rb b/users/test/unit/token_test.rb index f56c576..6c9f209 100644 --- a/users/test/unit/token_test.rb +++ b/users/test/unit/token_test.rb @@ -7,9 +7,6 @@ class ClientCertificateTest < ActiveSupport::TestCase @user = find_record :user end - teardown do - end - test "new token for user" do sample = Token.new(:user_id => @user.id) assert sample.valid? @@ -61,6 +58,26 @@ class ClientCertificateTest < ActiveSupport::TestCase end end + test "Token.destroy_all_expired is noop if no expiry is set" do + expired = FactoryGirl.create :token, last_seen_at: 2.hours.ago + with_config auth: {} do + Token.destroy_all_expired + end + assert_equal expired, Token.find(expired.id) + end + + test "Token.destroy_all_expired cleans up expired tokens only" do + expired = FactoryGirl.create :token, last_seen_at: 2.hours.ago + fresh = FactoryGirl.create :token + with_config auth: {token_expires_after: 60} do + Token.destroy_all_expired + end + assert_nil Token.find(expired.id) + assert_equal fresh, Token.find(fresh.id) + fresh.destroy + end + + end |