make use of the UnauthorizedUser

Null Pattern for current_user - use it to get rid of some conditionals

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