From 699668abe415da7e936b3ea019188bfd7598dfc2 Mon Sep 17 00:00:00 2001
From: sneaky-patriki <n.patrikeos@student.unsw.edu.au>
Date: Sat, 16 Apr 2022 11:41:49 +0800
Subject: [PATCH] security: expose encoded hashid for unit entity and decode
 for /units/:id Modify unit_entity.rb to expose the id as the hashid rather
 than actual id, this will affect all endpoints returni ng units Modify
 units_api.rb to decode the hashed id for the /units/:id endpoint

---
 app/api/entities/unit_entity.rb |  5 ++++-
 app/api/units_api.rb            | 15 ++++++++-------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/app/api/entities/unit_entity.rb b/app/api/entities/unit_entity.rb
index 434e26be33..287c222588 100644
--- a/app/api/entities/unit_entity.rb
+++ b/app/api/entities/unit_entity.rb
@@ -9,7 +9,10 @@ def is_staff?(user, unit)
     end
 
     expose :code
-    expose :id
+    expose :id do |unit|
+        hashid = Hashids.new("unit_salt", 8)
+        hashid.encode(unit.id)
+    end
     expose :name
     expose :my_role do |unit, options|
       role = unit.role_for(options[:user])
diff --git a/app/api/units_api.rb b/app/api/units_api.rb
index 79ea3c55ec..f5d71301b2 100644
--- a/app/api/units_api.rb
+++ b/app/api/units_api.rb
@@ -22,6 +22,8 @@ class UnitsApi < Grape::API
     end
   end
 
+  hashid = Hashids.new("unit_salt", 8)
+
   desc 'Get units related to the current user for admin purposes'
   params do
     optional :include_in_active, type: Boolean, desc: 'Include units that are not active'
@@ -36,17 +38,16 @@ class UnitsApi < Grape::API
 
     units = units.where('active = true') unless params[:include_in_active]
 
-    hashid = Hashids.new("unit_salt")
-
-    units.each do |unit|
-        unit.id = hashid.encode(unit.id)
-    end
-
     present units, with: Entities::UnitEntity, user: current_user, summary_only: true
   end
 
   desc "Get a unit's details"
   get '/units/:id' do
+    id = params[:id]
+    puts id.class
+    unit_id = hashid.decode(id)[0]
+    puts unit_id
+    puts unit_id.class
     unit = Unit.includes(
       {unit_roles: [:role, :user]},
       {task_definitions: :tutorial_stream},
@@ -58,7 +59,7 @@ class UnitsApi < Grape::API
       :group_sets,
       :groups,
       :group_memberships
-    ).find(params[:id])
+  ).find(unit_id)
 
     unless (authorise? current_user, unit, :get_unit) || (authorise? current_user, User, :admin_units)
       error!({ error: "Couldn't find Unit with id=#{params[:id]}" }, 403)