Merge pull request #107 from gate-sso/refactoring_nss_host

Fixing issue with 500 thrown on nss/host - Added name constraint on h…

app/controllers/application_controller.rb

@@ -16,6 +16,13 @@ def render_404
   end
 
   def authenticate_access_token!
-    head :unauthorized if !(AccessToken.valid_token params[:token])
+    unless AccessToken.valid_token(params[:token])
+      render_error(['Unauthorized'], :unauthorized)
+    end
+  end
+
+  def render_error(errors, status = 400)
+    render 'common/errors', locals: { errors: errors }, status: status
+    return
   end
 end

app/controllers/nss_controller.rb

@@ -1,5 +1,6 @@
 class NssController < ApplicationController
   skip_before_filter :verify_authenticity_token, only: [ :add_host, :add_user_to_group ]
+  before_filter :authenticate_access_token!, only: %i[add_host]
 
   def host
     token =  AccessToken.valid_token params[:token]
@@ -22,16 +23,18 @@ def host
   end
 
   def add_host
-    token =  AccessToken.valid_token params[:token]
-    if token
-      @response = HostMachine.find_or_create_by(name: params[:name]) if params[:name].present?
-      @group = Group.find_or_create_by(name: (params[:name] + "_host_group").downcase.squish ) if params[:group_name].present?
-      @response.groups << @group  if @response.present? and @group.present? and @response.groups.find_by_id(@group.id).blank?
-      @group = Group.find_or_create_by(name: params[:group_name] ) if params[:group_name].present?
-      @response.groups << @group  if @response.present? and @group.present? and @response.groups.find_by_id(@group.id).blank?
-      @response.save!
+    if params[:name].present?
+      host = HostMachine.find_or_create_by(name: params[:name])
+      host.add_host_group(params[:name])
+      host.add_group(params[:group_name])
+      render 'add_host', locals: { host: host }, format: :json
+    else
+      errors = ['Name can\'t be blank']
+      if params.key?(:group_name) && params[:group_name].blank?
+        errors << 'Group Name can\'t be blank'
+      end
+      render_error(errors)
     end
-    render json: @response
   end
 
   def group

app/models/group.rb

@@ -13,6 +13,7 @@ class Group < ActiveRecord::Base
   belongs_to :vpn
 
   validates_uniqueness_of :name, case_sensitive: false
+  validates :name, presence: true
 
   before_create :set_lower_case_name
   acts_as_paranoid

app/models/host_machine.rb

@@ -4,13 +4,14 @@ class HostMachine < ActiveRecord::Base
   has_many :host_access_groups
   has_many :groups, through: :host_access_groups
   validates_uniqueness_of :name, case_sensitive: false
+  validates :name, presence: true
 
   before_create :set_lower_case_name
   before_save :set_host_access_key
   before_create :set_host_access_key
 
   def set_host_access_key
-    self.access_key = ROTP::Base32.random_base32 
+    self.access_key = ROTP::Base32.random_base32
   end
 
   def set_lower_case_name
@@ -34,4 +35,17 @@ def sysadmins
     users.uniq
   end
 
+  def add_host_group(name)
+    if name.squish.present?
+      name = "#{name.squish}_host_group"
+      self.add_group(name.squish.downcase)
+    end
+  end
+
+  def add_group(name)
+    if name.squish.present?
+      self.groups << Group.find_or_initialize_by(name: name.squish.downcase)
+      self.save
+    end
+  end
 end

app/views/common/errors.json.jbuilder

@@ -0,0 +1,2 @@
+json.success false
+json.errors errors

app/views/nss/add_host.json.jbuilder

@@ -0,0 +1,4 @@
+json.success true
+json.access_key host.access_key
+json.host host.name
+json.groups host.groups.map(&:name)

spec/controllers/nss_controller_spec.rb

@@ -30,11 +30,16 @@
     expect(data["success"]).to eq(true)
   end
 
-  it "should return sysadmins for that host" do 
+  it 'it shouldn\'t return sysadmins for invalid token' do
+    json = { token: '', name: 'random_host', group_name: '', format: :json }
+    post 'add_host', json
+    body = response.body
+    expect(JSON.parse(body)['success']).to eq(false)
+  end
 
+  it "should return sysadmins for that host" do
     sign_in user
     access_token = create(:access_token)
-
     json =  { token: access_token.token, name: "random_host_01" }
     post "add_host", { token: access_token.token, name: "random_host_01", group_name: "random_group_01", format: :json}
     body = response.body
@@ -151,7 +156,7 @@
     group.burst_host_cache
 
     cache_count_aft = REDIS_CACHE.keys("*").count
-    expect(cache_count_aft).to eq cache_count_bfr 
+    expect(cache_count_aft).to eq cache_count_bfr
 
   end
 

spec/models/host_machine_spec.rb

@@ -79,4 +79,50 @@
 
     end
   end
+
+  context 'add_host_group' do
+    let(:host_machine) { HostMachine.find_or_create_by(name: 'machine')  }
+    it 'should create host group given valid name' do
+      host_machine.add_host_group(host_machine.name)
+      groups = host_machine.groups.map(&:name)
+      expect(groups.include?("#{host_machine.name}_host_group")).to eq(true)
+      expect(host_machine.valid?).to eq(true)
+    end
+
+    it 'should create the group with all downcase' do
+      host_machine.add_host_group(host_machine.name.upcase)
+      groups = host_machine.groups.map(&:name)
+      expect(groups.include?("#{host_machine.name.downcase}_host_group")).to eq(true)
+    end
+
+    it 'shouldn\'t add the group if the name is invalid' do
+      host_machine.add_host_group('')
+      groups = host_machine.groups.map(&:name)
+      expect(groups.include?("")).to eq(false)
+      expect(groups.include?("_host_group")).to eq(false)
+    end
+  end
+
+  context 'add_group' do
+    let(:host_machine) { HostMachine.find_or_create_by(name: 'machine')  }
+    let(:group_name) { 'machine_group'  }
+    it 'should create host group given valid name' do
+      host_machine.add_group(group_name)
+      groups = host_machine.groups.map(&:name)
+      expect(groups.include?(group_name)).to eq(true)
+      expect(host_machine.valid?).to eq(true)
+    end
+
+    it 'should create the group with all downcase' do
+      host_machine.add_group(group_name.upcase)
+      groups = host_machine.groups.map(&:name)
+      expect(groups.include?(group_name.downcase)).to eq(true)
+    end
+
+    it 'shouldn\'t add the group if the name is invalid' do
+      host_machine.add_group('')
+      groups = host_machine.groups.map(&:name)
+      expect(groups.include?("")).to eq(false)
+    end
+  end
 end

spec/rails_helper.rb

@@ -35,6 +35,7 @@
   config.include Devise::Test::ControllerHelpers, type: :controller
   config.include Devise::Test::ControllerHelpers, type: :view
   config.include ControllerHelpers, type: :controller
+  config.render_views = true
   # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures
   config.fixture_path = "#{::Rails.root}/spec/fixtures"