MGM: Refactoring - replace hardcoded user "adm" UID and GID

ccaffy · ccaffy · commit c461dad76e82 · 2025-05-05T11:59:09.000+02:00
Created ADM_UID and ADM_GID in Constants.hh and replaced hardcoded
integers (3 and 4) with those two constants.

Fixes EOS-6399
diff --git a/common/Constants.hh b/common/Constants.hh
@@ -24,6 +24,7 @@
 
 #pragma once
 #include "common/Namespace.hh"
+#include <sys/types.h>
 
 EOSCOMMONNAMESPACE_BEGIN
 
@@ -83,5 +84,7 @@ static constexpr auto EOS_VTRACE_ATTR = "sys.vtrace";
 static constexpr auto EOS_UTRACE_ATTR = "sys.utrace";
 //! FST heartbeat key marker, the "stat." prefix makes it transient
 static constexpr auto FST_HEARTBEAT_KEY = "stat.heartbeat";
-
+//! ADM uid and gid
+static constexpr uid_t ADM_UID = 3;
+static constexpr gid_t ADM_GID = 4;
 EOSCOMMONNAMESPACE_END
diff --git a/mgm/Recycle.cc b/mgm/Recycle.cc
@@ -471,8 +471,8 @@ Recycle::Print(std::string& std_out, std::string& std_err,
   }
 
   if (global && ((!vid.uid) ||
-                 (vid.hasUid(3)) ||
-                 (vid.hasGid(4)))) {
+                 (vid.hasUid(eos::common::ADM_UID)) ||
+                 (vid.hasGid(eos::common::ADM_GID)))) {
     // add everything found in the recycle directory structure to the printmap
     std::string subdirs;
     XrdMgmOfsDirectory dirl;
@@ -1031,8 +1031,8 @@ Recycle::Purge(std::string& std_out, std::string& std_err,
   }
 
   if (vid.uid && !vid.sudoer &&
-      !(vid.hasUid(3)) &&
-      !(vid.hasGid(4))) {
+      !(vid.hasUid(eos::common::ADM_UID)) &&
+      !(vid.hasGid(eos::common::ADM_GID))) {
     std_err = "error: you cannot purge your recycle bin without being a sudor "
       "or having an admin role";
     return EPERM;
diff --git a/mgm/XrdMgmOfs/Chmod.cc b/mgm/XrdMgmOfs/Chmod.cc
@@ -148,8 +148,8 @@ XrdMgmOfs::_chmod(const char* path,
         if (((fmd && (fmd->getCUid() == vid.uid)) && (!acl.CanNotChmod())) ||
             ((cmd && (cmd->getCUid() == vid.uid)) && (!acl.CanNotChmod())) ||
             (!vid.uid) || // the root user
-            (vid.uid == 3) || // the admin user
-            (vid.gid == 4) || // the admin group
+            (vid.uid == eos::common::ADM_UID) || // the admin user
+            (vid.gid == eos::common::ADM_GID) || // the admin group
             (acl.CanChmod())
            ) { // a pre-defined mask to apply to the desired modbits
           // the chmod ACL entry
diff --git a/mgm/XrdMgmOfs/Chown.cc b/mgm/XrdMgmOfs/Chown.cc
@@ -86,7 +86,7 @@ XrdMgmOfs::_chown(const char* path,
     eos_static_debug("sys.acl %s acl.CanChown() %d", attrmap["sys.acl"].c_str(),
                      acl.CanChown());
 
-    if (((vid.uid) && (!vid.hasUid(3) && !vid.hasGid(4)) &&
+    if (((vid.uid) && (!vid.hasUid(eos::common::ADM_UID) && !vid.hasGid(eos::common::ADM_GID)) &&
          !acl.CanChown()) ||
         ((vid.uid) && !acl.IsMutable())) {
       errno = EPERM;
@@ -96,7 +96,7 @@ XrdMgmOfs::_chown(const char* path,
         cmd->setCUid(uid);
       }
 
-      if (((!vid.uid) || (vid.uid == 3) || (vid.gid == 4)) &&
+      if (((!vid.uid) || (vid.uid == eos::common::ADM_UID) || (vid.gid == eos::common::ADM_GID)) &&
           ((unsigned int)gid != 0xffffffff)) {
         // Change the group
         cmd->setCGid(gid);
@@ -141,7 +141,7 @@ XrdMgmOfs::_chown(const char* path,
       eos_static_debug("sys.acl %s acl.CanChown() %d", attrmap["sys.acl"].c_str(),
                        acl.CanChown());
 
-      if ((vid.uid) && (!vid.sudoer) && (vid.uid != 3) && (vid.gid != 4) &&
+      if ((vid.uid) && (!vid.sudoer) && (vid.uid != eos::common::ADM_UID) && (vid.gid != eos::common::ADM_GID) &&
           !acl.CanChown()) {
         errno = EPERM;
       } else {
diff --git a/mgm/XrdMgmOfs/Find.cc b/mgm/XrdMgmOfs/Find.cc
@@ -705,7 +705,7 @@ XrdMgmOfs::_find(const char* path, XrdOucErrInfo& out_error,
   bool limited = false;
   bool fail_if_limited = (out_error.getErrInfo() == E2BIG);
 
-  if ((vid.uid != 0) && (!vid.hasUid(3)) && (!vid.hasGid(4)) && (!vid.sudoer)) {
+  if ((vid.uid != 0) && (!vid.hasUid(eos::common::ADM_UID)) && (!vid.hasGid(eos::common::ADM_GID)) && (!vid.sudoer)) {
     limitresult = true;
   }
 
diff --git a/mgm/proc/ProcInterface.cc b/mgm/proc/ProcInterface.cc
@@ -23,6 +23,7 @@
 
 #include "ProcInterface.hh"
 #include <XrdOuc/XrdOucEnv.hh>
+#include "common/Constants.hh"
 #include "mgm/proc/admin/AccessCmd.hh"
 #include "mgm/proc/admin/ConfigCmd.hh"
 #include "mgm/proc/admin/ConvertCmd.hh"
@@ -588,8 +589,8 @@ ProcInterface::Authorize(const char* path, const char* info,
 
     // One has to be part of the virtual users 2(daemon)/3(adm)/4(adm)
     return ((vid.hasUid(DAEMONUID)) ||
-            (vid.hasUid(3)) ||
-            (vid.hasGid(4)));
+            (vid.hasUid(eos::common::ADM_UID)) ||
+            (vid.hasGid(eos::common::ADM_GID)));
   }
 
   // User access
diff --git a/mgm/proc/admin/AccessCmd.cc b/mgm/proc/admin/AccessCmd.cc
@@ -28,6 +28,7 @@
 #include "mgm/Access.hh"
 #include "mgm/Stat.hh"
 #include "common/StringUtils.hh"
+#include "common/Constants.hh"
 #include <XrdOuc/XrdOucEnv.hh>
 #include <cctype>
 
@@ -85,7 +86,7 @@ AccessCmd::ProcessRequest() noexcept
   eos::console::ReplyProto reply;
   eos::console::AccessProto access = mReqProto.access();
 
-  if ((mVid.uid != 0) && (!mVid.hasUid(3)) && (!mVid.hasGid(4)) &&
+  if ((mVid.uid != 0) && (!mVid.hasUid(eos::common::ADM_UID)) && (!mVid.hasGid(eos::common::ADM_GID)) &&
       (!mVid.sudoer)) {
     // root and admins only
     reply.set_std_out("");
diff --git a/mgm/proc/admin/ConvertCmd.cc b/mgm/proc/admin/ConvertCmd.cc
@@ -30,6 +30,7 @@
 #include "namespace/interface/IFileMD.hh"
 #include "namespace/interface/IContainerMD.hh"
 #include "common/table_formatter/TableFormatterBase.hh"
+#include "common/Constants.hh"
 #include <json/json.h>
 
 EOSMGMNAMESPACE_BEGIN
@@ -77,7 +78,7 @@ ConvertCmd::ProcessRequest() noexcept
     ListSubcmd(convert.list(), reply, jsonOutput);
   } else if (subcmd == eos::console::ConvertProto::kClear) {
     // check if vid has admin permissions (root, sudoer, admin user, admin group)
-    if (!vid.uid || vid.sudoer || vid.hasUid(3) || vid.hasGid(4)) {
+    if (!vid.uid || vid.sudoer || vid.hasUid(eos::common::ADM_UID) || vid.hasGid(eos::common::ADM_GID)) {
       ClearSubcmd(convert.clear(), reply);
     } else {
       reply.set_retc(EPERM);
diff --git a/mgm/proc/admin/QuotaCmd.cc b/mgm/proc/admin/QuotaCmd.cc
@@ -29,6 +29,7 @@
 #include "mgm/Quota.hh"
 #include "mgm/Recycle.hh"
 #include "common/Path.hh"
+#include "common/Constants.hh"
 
 EOSMGMNAMESPACE_BEGIN
 
@@ -209,8 +210,8 @@ void QuotaCmd::LsSubcmd(const eos::console::QuotaProto_LsProto& ls,
     }
   }
 
-  if ((!mVid.uid) || mVid.hasUid(3) ||
-      mVid.hasGid(4)) { // @note no.03 before 'vid' was used, using 'mVid' now
+  if ((!mVid.uid) || mVid.hasUid(eos::common::ADM_UID) ||
+      mVid.hasGid(eos::common::ADM_GID)) { // @note no.03 before 'vid' was used, using 'mVid' now
     // root and admin can set quota
     canQuota = true;
   } else {
@@ -327,7 +328,7 @@ void QuotaCmd::SetSubcmd(const eos::console::QuotaProto_SetProto& set,
 
   bool canQuota;
 
-  if ((!mVid.uid) || mVid.hasUid(3) || mVid.hasGid(4)) { // @note no.03
+  if ((!mVid.uid) || mVid.hasUid(eos::common::ADM_UID) || mVid.hasGid(eos::common::ADM_GID)) { // @note no.03
     // root and admin can set quota
     canQuota = true;
   } else {
@@ -519,7 +520,7 @@ void QuotaCmd::RmSubcmd(const eos::console::QuotaProto_RmProto& rm,
 
   bool canQuota;
 
-  if ((!mVid.uid) || mVid.hasUid(3) || mVid.hasGid(4)) { // @note no.02
+  if ((!mVid.uid) || mVid.hasUid(eos::common::ADM_UID) || mVid.hasGid(eos::common::ADM_GID)) { // @note no.02
     // root and admin can set quota
     canQuota = true;
   } else {
diff --git a/mgm/proc/user/Map.cc b/mgm/proc/user/Map.cc
@@ -24,6 +24,7 @@
 #include "mgm/proc/ProcInterface.hh"
 #include "mgm/XrdMgmOfs.hh"
 #include "mgm/config/IConfigEngine.hh"
+#include "common/Constants.hh"
 
 EOSMGMNAMESPACE_BEGIN
 
@@ -45,7 +46,7 @@ ProcCommand::Map()
   }
 
   if (mSubCmd == "link") {
-    if ((!pVid->uid) || vid.hasUid(3) || vid.hasGid(4)) {
+    if ((!pVid->uid) || vid.hasUid(eos::common::ADM_UID) || vid.hasGid(eos::common::ADM_GID)) {
       XrdOucString srcpath = pOpaque->Get("mgm.map.src");
       XrdOucString dstpath = pOpaque->Get("mgm.map.dest");
 
@@ -94,8 +95,8 @@ ProcCommand::Map()
     XrdOucString path = pOpaque->Get("mgm.map.src");
 
     if ((!pVid->uid) ||
-        vid.hasUid(3) ||
-        vid.hasGid(4)) {
+        vid.hasUid(eos::common::ADM_UID) ||
+        vid.hasGid(eos::common::ADM_GID)) {
       eos::common::RWMutexWriteLock lock(gOFS->PathMapMutex);
 
       if ((!path.length()) || (!gOFS->PathMap.count(path.c_str()))) {
diff --git a/mgm/proc/user/Motd.cc b/mgm/proc/user/Motd.cc
@@ -24,6 +24,7 @@
 #include "mgm/proc/ProcInterface.hh"
 #include "mgm/XrdMgmOfs.hh"
 #include "mgm/Stat.hh"
+#include "common/Constants.hh"
 #include <fcntl.h>
 
 EOSMGMNAMESPACE_BEGIN
@@ -39,7 +40,7 @@ ProcCommand::Motd()
   motdfile += "/motd";
 
   if (motdupload.length() &&
-      ((!pVid->uid) || vid.hasUid(3) || vid.hasGid(4))) {
+      ((!pVid->uid) || vid.hasUid(eos::common::ADM_UID) || vid.hasGid(eos::common::ADM_GID))) {
     // root + admins can set the MOTD
     ssize_t motdlen = 0;
     char* motdout = 0;
diff --git a/mgm/proc/user/NewfindCmd.cc b/mgm/proc/user/NewfindCmd.cc
@@ -27,6 +27,7 @@
 #include "common/LayoutId.hh"
 #include "common/Path.hh"
 #include "common/Timing.hh"
+#include "common/Constants.hh"
 #include "mgm/Acl.hh"
 #include "mgm/FsView.hh"
 #include "mgm/Stat.hh"
@@ -1263,8 +1264,8 @@ NewfindCmd::ProcessRequest() noexcept
   // For general users, cannot return more than 50k dirs and 100k files with one find,
   // unless there is an access rule allowing deeper queries.
   // Special users (like root) have the limit lifted by default.
-  const bool limit_result = ((mVid.uid != 0) && (!mVid.hasUid(3)) &&
-                             (!mVid.hasGid(4)) && (!mVid.sudoer));
+  const bool limit_result = ((mVid.uid != 0) && (!mVid.hasUid(eos::common::ADM_UID)) &&
+                             (!mVid.hasGid(eos::common::ADM_GID)) && (!mVid.sudoer));
   static uint64_t dir_limit = 50000;
   static uint64_t file_limit = 100000;
   Access::GetFindLimits(mVid, dir_limit, file_limit);
@@ -1623,8 +1624,8 @@ NewfindCmd::ProcessRequest(grpc::ServerWriter<eos::console::ReplyProto>* writer)
   // For general users, cannot return more than 50k dirs and 100k files with one find,
   // unless there is an access rule allowing deeper queries.
   // Special users (like root) have the limit lifted by default.
-  const bool limit_result = ((mVid.uid != 0) && (!mVid.hasUid(3)) &&
-                             (!mVid.hasGid(4)) && (!mVid.sudoer));
+  const bool limit_result = ((mVid.uid != 0) && (!mVid.hasUid(eos::common::ADM_UID)) &&
+                             (!mVid.hasGid(eos::common::ADM_GID)) && (!mVid.sudoer));
   static uint64_t dir_limit = 50000;
   static uint64_t file_limit = 100000;
   Access::GetFindLimits(mVid, dir_limit, file_limit);
diff --git a/mgm/proc/user/Quota.cc b/mgm/proc/user/Quota.cc
@@ -26,6 +26,7 @@
 #include "mgm/Acl.hh"
 #include "mgm/Quota.hh"
 #include "mgm/Stat.hh"
+#include "common/Constants.hh"
 
 EOSMGMNAMESPACE_BEGIN
 
@@ -109,7 +110,7 @@ ProcCommand::UserQuota()
 
   bool canQuota = false;
 
-  if ((!vid.uid) || vid.hasUid(3) || vid.hasGid(4)) {
+  if ((!vid.uid) || vid.hasUid(eos::common::ADM_UID) || vid.hasGid(eos::common::ADM_GID)) {
     // root and admin can set quota
     canQuota = true;
   } else {
diff --git a/mgm/proc/user/RouteCmd.cc b/mgm/proc/user/RouteCmd.cc
@@ -25,6 +25,7 @@
 #include "mgm/config/IConfigEngine.hh"
 #include "mgm/RouteEndpoint.hh"
 #include "mgm/PathRouting.hh"
+#include "common/Constants.hh"
 #include <sstream>
 
 EOSMGMNAMESPACE_BEGIN
@@ -77,7 +78,7 @@ void
 RouteCmd::LinkSubcmd(const eos::console::RouteProto_LinkProto& link,
                      eos::console::ReplyProto& reply)
 {
-  if ((mVid.uid != 0) && !mVid.hasUid(3) && !mVid.hasGid(4)) {
+  if ((mVid.uid != 0) && !mVid.hasUid(eos::common::ADM_UID) && !mVid.hasGid(eos::common::ADM_GID)) {
     reply.set_retc(EPERM);
     reply.set_std_err("error: you don't have the required priviledges to "
                       "execute this command");
@@ -107,7 +108,7 @@ void
 RouteCmd::UnlinkSubcmd(const eos::console::RouteProto_UnlinkProto& unlink,
                        eos::console::ReplyProto& reply)
 {
-  if ((mVid.uid != 0) && !mVid.hasUid(3) && !mVid.hasGid(4)) {
+  if ((mVid.uid != 0) && !mVid.hasUid(eos::common::ADM_UID) && !mVid.hasGid(eos::common::ADM_GID)) {
     reply.set_retc(EPERM);
     reply.set_std_err("error: you don't have the required priviledges to "
                       "execute this command");

Original file line number	Diff line number	Diff line change
`@@ -705,7 +705,7 @@ XrdMgmOfs::_find(const char* path, XrdOucErrInfo& out_error,`
`705`	`705`	`bool limited = false;`
`706`	`706`	`bool fail_if_limited = (out_error.getErrInfo() == E2BIG);`
`707`	`707`
`708`		`- if ((vid.uid != 0) && (!vid.hasUid(3)) && (!vid.hasGid(4)) && (!vid.sudoer)) {`
	`708`	`+ if ((vid.uid != 0) && (!vid.hasUid(eos::common::ADM_UID)) && (!vid.hasGid(eos::common::ADM_GID)) && (!vid.sudoer)) {`
`709`	`709`	`limitresult = true;`
`710`	`710`	`}`
`711`	`711`