From 7a9ece43bd61246b450471ed6bb1089570321e38 Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 17 Apr 2014 19:27:47 +0200 Subject: make use of the UnauthorizedUser Null Pattern for current_user - use it to get rid of some conditionals --- app/controllers/v1/certs_controller.rb | 44 +++--------------- app/controllers/v1/messages_controller.rb | 5 +- app/models/service_level.rb | 14 +++++- app/models/unauthenticated_user.rb | 20 ++++++-- config/defaults.yml | 13 ++++-- .../support/app/controllers/tickets_controller.rb | 36 +++++++++------ engines/support/app/views/tickets/new.html.haml | 18 ++------ engines/support/app/views/tickets/show.html.haml | 4 +- test/functional/v1/certs_controller_test.rb | 54 +++++++++++----------- 9 files changed, 104 insertions(+), 104 deletions(-) diff --git a/app/controllers/v1/certs_controller.rb b/app/controllers/v1/certs_controller.rb index 64cfa7f..580c90c 100644 --- a/app/controllers/v1/certs_controller.rb +++ b/app/controllers/v1/certs_controller.rb @@ -1,50 +1,20 @@ class V1::CertsController < ApplicationController - before_filter :require_login, :unless => :anonymous_certs_allowed? + before_filter :require_eip_access # GET /cert def show - @cert = ClientCertificate.new(:prefix => certificate_prefix) + @cert = ClientCertificate.new(:prefix => service_level.cert_prefix) render text: @cert.to_s, content_type: 'text/plain' end protected - def anonymous_certs_allowed? - APP_CONFIG[:allow_anonymous_certs] + def require_eip_access + access_denied unless service_level.provides?(:eip) end - # - # this is some temporary logic until we store the service level in the user db. - # - # better logic might look like this: - # - # if logged_in? - # service_level = user.service_level - # elsif allow_anonymous? - # service_level = service_levels[:anonymous] - # else - # service_level = nil - # end - # - # if service_level.bandwidth == 'limited' && allow_limited? - # prefix = limited - # elsif allow_unlimited? - # prefix = unlimited - # else - # prefix = nil - # end - # - def certificate_prefix - if logged_in? - if APP_CONFIG[:allow_unlimited_certs] - APP_CONFIG[:unlimited_cert_prefix] - elsif APP_CONFIG[:allow_limited_certs] - APP_CONFIG[:limited_cert_prefix] - end - elsif !APP_CONFIG[:allow_limited_certs] - APP_CONFIG[:unlimited_cert_prefix] - else - APP_CONFIG[:limited_cert_prefix] - end + + def service_level + current_user.effective_service_level end end diff --git a/app/controllers/v1/messages_controller.rb b/app/controllers/v1/messages_controller.rb index f71d0f1..85156b7 100644 --- a/app/controllers/v1/messages_controller.rb +++ b/app/controllers/v1/messages_controller.rb @@ -7,12 +7,11 @@ module V1 respond_to :json def index - render json: (current_user ? current_user.messages : [] ) + render json: current_user.messages end def update - message = Message.find(params[:id]) - if (message and current_user) + if message = Message.find(params[:id]) message.mark_as_read_by(current_user) message.save render json: true diff --git a/app/models/service_level.rb b/app/models/service_level.rb index 31a713b..d0bd9b3 100644 --- a/app/models/service_level.rb +++ b/app/models/service_level.rb @@ -13,8 +13,20 @@ class ServiceLevel end def config_hash - APP_CONFIG[:service_levels][@id] + @config_hash || APP_CONFIG[:service_levels][@id].with_indifferent_access end delegate :to_json, to: :config_hash + + def provides?(service) + services.include? service.to_s + end + + def services + config_hash[:services] || [] + end + + def cert_prefix + config_hash[:cert_prefix] + end end diff --git a/app/models/unauthenticated_user.rb b/app/models/unauthenticated_user.rb index ba6470a..7845a6f 100644 --- a/app/models/unauthenticated_user.rb +++ b/app/models/unauthenticated_user.rb @@ -1,13 +1,27 @@ # 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. + def effective_service_level + ServiceLevel.new id: APP_CONFIG[:unauthenticated_service_level] + end def is_admin? false end - def effective_service_level - ServiceLevel.new id: APP_CONFIG[:unauthenticated_service_level] + def id + nil + end + + def email_address + nil + end + + def login + nil + end + + def messages + [] end end diff --git a/config/defaults.yml b/config/defaults.yml index 47c3ad7..383aa1c 100644 --- a/config/defaults.yml +++ b/config/defaults.yml @@ -7,11 +7,6 @@ cert_options: &cert_options client_cert_lifespan: 2 client_cert_bit_size: 2024 client_cert_hash: "SHA256" - allow_limited_certs: false - allow_unlimited_certs: true - allow_anonymous_certs: false - limited_cert_prefix: "LIMITED" - unlimited_cert_prefix: "UNLIMITED" downloads: &downloads client_download_domain: https://dl.bitmask.net @@ -53,12 +48,17 @@ service_levels: &service_levels name: anonymous cert_prefix: "LIMITED" description: "anonymous account, with rate limited VPN" + services: + - eip 1: name: free cert_prefix: "LIMITED" description: "free account, with rate limited VPN" cost: 0 quota: 100 + services: + - eip + - email 2: name: premium cert_prefix: "UNLIMITED" @@ -66,6 +66,9 @@ service_levels: &service_levels cost: USD: 10 EUR: 10 + services: + - eip + - email default_service_level: 1 unauthenticated_service_level: 0 diff --git a/engines/support/app/controllers/tickets_controller.rb b/engines/support/app/controllers/tickets_controller.rb index d65ee43..cf8743a 100644 --- a/engines/support/app/controllers/tickets_controller.rb +++ b/engines/support/app/controllers/tickets_controller.rb @@ -5,7 +5,8 @@ class TicketsController < ApplicationController #has_scope :open, :type => boolean before_filter :require_login, :only => [:index] - before_filter :fetch_ticket, :only => [:show, :update, :destroy] # don't now have an edit method + before_filter :fetch_ticket, :only => [:show, :update, :destroy] + before_filter :require_ticket_access, :only => [:show, :update, :destroy] before_filter :fetch_user before_filter :set_title @@ -17,11 +18,11 @@ class TicketsController < ApplicationController def create @ticket = Ticket.new(params[:ticket]) - @ticket.comments.last.posted_by = (logged_in? ? current_user.id : nil) #protecting posted_by isn't working, so this should protect it. + #protecting posted_by isn't working, so this should protect it: + @ticket.comments.last.posted_by = current_user.id @ticket.comments.last.private = false unless admin? - @ticket.created_by = current_user.id if logged_in? - @ticket.email = current_user.email_address if logged_in? and current_user.email_address - + @ticket.created_by = current_user.id + @ticket.email = current_user.email_address if current_user.email_address if @ticket.save flash[:notice] = t(:thing_was_successfully_created, :thing => t(:ticket)) end @@ -58,7 +59,7 @@ class TicketsController < ApplicationController end if @ticket.comments_changed? - @ticket.comments.last.posted_by = (current_user ? current_user.id : nil) + @ticket.comments.last.posted_by = current_user.id @ticket.comments.last.private = false unless admin? end @@ -120,19 +121,28 @@ class TicketsController < ApplicationController return ticket end - def ticket_access? - @ticket and (admin? or !@ticket.created_by or (current_user and current_user.id == @ticket.created_by)) - end - def fetch_ticket @ticket = Ticket.find(params[:id]) - if !@ticket and admin? - redirect_to auto_tickets_path, :alert => t(:no_such_thing, :thing => 'ticket') - return + if !@ticket + if admin? + redirect_to auto_tickets_path, + alert: t(:no_such_thing, thing: 'ticket') + else + access_denied + end end + end + + def require_ticket_access access_denied unless ticket_access? end + def ticket_access? + admin? or + @ticket.created_by.blank? or + current_user.id == @ticket.created_by + end + def fetch_user if params[:user_id] @user = User.find(params[:user_id]) diff --git a/engines/support/app/views/tickets/new.html.haml b/engines/support/app/views/tickets/new.html.haml index 8f217a5..e391499 100644 --- a/engines/support/app/views/tickets/new.html.haml +++ b/engines/support/app/views/tickets/new.html.haml @@ -2,22 +2,14 @@ = render 'tickets/tabs' -- if admin? && @user - - email = @user.email_address - - regarding = @user.login -- elsif logged_in? - - email = current_user.email_address - - regarding = current_user.login +- user = @user if admin? +- user ||= current_user = simple_form_for @ticket, :validate => true, :html => {:class => 'form-horizontal'} do |f| = hidden_ticket_fields = f.input :subject - - if logged_in? - = f.input :email, input_html: {value: email} - = f.input :regarding_user, input_html: {value: regarding} - - else - = f.input :email - = f.input :regarding_user + = f.input :email, input_html: {value: user.email} + = f.input :regarding_user, input_html: {value: user.login} = f.simple_fields_for :comments, @comment do |c| = c.input :body, :label => t(:description), :as => :text, :input_html => {:class => "full-width", :rows=> 5} - if admin? @@ -27,4 +19,4 @@ - if logged_in? = link_to t(:cancel), auto_tickets_path, :class => :btn - else - = link_to t(:cancel), home_path, :class => 'btn' \ No newline at end of file + = link_to t(:cancel), home_path, :class => 'btn' diff --git a/engines/support/app/views/tickets/show.html.haml b/engines/support/app/views/tickets/show.html.haml index bfdb773..edb6e6f 100644 --- a/engines/support/app/views/tickets/show.html.haml +++ b/engines/support/app/views/tickets/show.html.haml @@ -7,6 +7,6 @@ = render :partial => 'tickets/comment', :collection => @ticket.comments %tr %td.user - = logged_in? ? current_user.login : t(:anonymous) + = current_user.login || t(:anonymous) %td.comment - = render 'tickets/new_comment_form' \ No newline at end of file + = render 'tickets/new_comment_form' diff --git a/test/functional/v1/certs_controller_test.rb b/test/functional/v1/certs_controller_test.rb index 2c70e52..3631947 100644 --- a/test/functional/v1/certs_controller_test.rb +++ b/test/functional/v1/certs_controller_test.rb @@ -3,42 +3,42 @@ require 'test_helper' class V1::CertsControllerTest < ActionController::TestCase test "send limited cert without login" do - with_config allow_limited_certs: true, allow_anonymous_certs: true do - cert = stub :to_s => "limited cert" - ClientCertificate.expects(:new).with(:prefix => APP_CONFIG[:limited_cert_prefix]).returns(cert) - get :show - assert_response :success - assert_equal cert.to_s, @response.body - end + cert = expect_cert('LIMITED') + get :show + assert_response :success + assert_equal cert.to_s, @response.body + end + + test "send limited cert" do + login + cert = expect_cert('LIMITED') + get :show + assert_response :success + assert_equal cert.to_s, @response.body end test "send unlimited cert" do - with_config allow_unlimited_certs: true do - login - cert = stub :to_s => "unlimited cert" - ClientCertificate.expects(:new).with(:prefix => APP_CONFIG[:unlimited_cert_prefix]).returns(cert) - get :show - assert_response :success - assert_equal cert.to_s, @response.body - end + login effective_service_level: ServiceLevel.new(id: 2) + cert = expect_cert('UNLIMITED') + get :show + assert_response :success + assert_equal cert.to_s, @response.body end - test "login required if anonymous certs disabled" do - with_config allow_anonymous_certs: false do + test "redirect if no eip service offered" do + with_config({service_levels: {0 => {services: []}}}) do get :show assert_response :redirect end end - test "send limited cert" do - with_config allow_limited_certs: true, allow_unlimited_certs: false do - login - cert = stub :to_s => "real cert" - ClientCertificate.expects(:new).with(:prefix => APP_CONFIG[:limited_cert_prefix]).returns(cert) - get :show - assert_response :success - assert_equal cert.to_s, @response.body - end - end + protected + def expect_cert(prefix) + cert = stub :to_s => "#{prefix.downcase} cert" + ClientCertificate.expects(:new). + with(:prefix => prefix). + returns(cert) + return cert + end end -- cgit v1.2.3