From ab1917c5fe0f03e7719863a5598ad575d9fef302 Mon Sep 17 00:00:00 2001
From: NavaL <ayoyo@thoughtworks.com>
Date: Thu, 14 Jul 2016 15:06:20 +0200
Subject: [feature] restrict is_admin in the user api, to only allow querying
 for him/herself

So that it we do not expose the is_admin property to anyone else
including other admins.
---
 app/controllers/api/users_controller.rb      | 10 +++++++++-
 app/models/user.rb                           |  7 +++++--
 test/functional/api/users_controller_test.rb | 12 ++++++++++--
 test/integration/api/signup_test.rb          |  2 +-
 test/unit/user_test.rb                       | 13 ++++++-------
 5 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb
index c79a729..709e076 100644
--- a/app/controllers/api/users_controller.rb
+++ b/app/controllers/api/users_controller.rb
@@ -28,12 +28,20 @@ module Api
         @user = User.find(params[:id])
       end
       if @user
-        respond_with @user
+        respond_with user_response
       else
         not_found
       end
     end
 
+    def user_response
+      @user.to_hash.tap do |user_hash|
+        if @user == current_user
+          user_hash['is_admin'] = @user.is_admin?
+        end
+      end
+    end
+
     def create
       if current_user.is_monitor?
         create_test_account
diff --git a/app/models/user.rb b/app/models/user.rb
index 6541305..93830cc 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -74,13 +74,16 @@ class User < CouchRest::Model::Base
   end
 
   def to_json(options={})
+    to_hash.to_json(options)
+  end
+
+  def to_hash()
     {
       :login => self.login,
       :ok => self.valid?,
       :id => self.id,
       :enabled => self.enabled?,
-      :is_admin => self.is_admin?
-    }.to_json(options)
+    }
   end
 
   def salt
diff --git a/test/functional/api/users_controller_test.rb b/test/functional/api/users_controller_test.rb
index 32afd0a..b69770d 100644
--- a/test/functional/api/users_controller_test.rb
+++ b/test/functional/api/users_controller_test.rb
@@ -86,14 +86,22 @@ class Api::UsersControllerTest < ApiControllerTest
     login :is_admin? => true
     api_get :show, :id => 0, :login => user.login, :format => :json
     assert_response :success
-    assert_json_response user
+    assert_json_response user.to_hash
     api_get :show, :id => user.id, :format => :json
     assert_response :success
-    assert_json_response user
+    assert_json_response user.to_hash
     api_get :show, :id => "0", :format => :json
     assert_response :not_found
   end
 
+  test "admin can show is_admin property" do
+    user = FactoryGirl.create :user, login: "admin2"
+    login user
+    api_get :show, :id => user.id, :format => :json
+    assert_response :success
+    assert_json_response user.to_hash.merge(:is_admin => true)
+  end
+
   test "normal users cannot show user" do
     user = find_record :user
     login
diff --git a/test/integration/api/signup_test.rb b/test/integration/api/signup_test.rb
index 2e515c1..dc24420 100644
--- a/test/integration/api/signup_test.rb
+++ b/test/integration/api/signup_test.rb
@@ -8,7 +8,7 @@ class SignupTest < SrpTest
   end
 
   test "signup response" do
-    assert_json_response :login => @login, :ok => true, :is_admin => false, :id => @user.id, :enabled => true
+    assert_json_response :login => @login, :ok => true, :id => @user.id, :enabled => true
     assert last_response.successful?
   end
 
diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb
index 55d0648..02e94df 100644
--- a/test/unit/user_test.rb
+++ b/test/unit/user_test.rb
@@ -71,13 +71,12 @@ class UserTest < ActiveSupport::TestCase
     assert_equal key, @user.public_key
   end
 
-  test "user to json includes id, login, valid, is_admin and enabled" do
-    json_content = JSON.parse @user.to_json
-    assert_equal @user.id, json_content["id"]
-    assert_equal @user.valid?, json_content["ok"]
-    assert_equal @user.login, json_content["login"]
-    assert_equal @user.enabled?, json_content["enabled"]
-    assert_equal @user.is_admin?, json_content["is_admin"]
+  test "user to hash includes id, login, valid and enabled" do
+    hash = @user.to_hash
+    assert_equal @user.id, hash[:id]
+    assert_equal @user.valid?, hash[:ok]
+    assert_equal @user.login, hash[:login]
+    assert_equal @user.enabled?, hash[:enabled]
   end
 
 
-- 
cgit v1.2.3