sefe · smurugan-sefe · Jan 27, 2026
diff --git a/src/Dorc.Api/Controllers/PropertyValuesController.cs b/src/Dorc.Api/Controllers/PropertyValuesController.cs
@@ -3,6 +3,7 @@
 using Dorc.ApiModel;
 using Dorc.ApiModel.MonitorRunnerApi;
 using Dorc.Core;
+using Dorc.Core.Interfaces;
 using Dorc.Core.VariableResolution;
 using Dorc.PersistentData.Sources.Interfaces;
 using Microsoft.AspNetCore.Authorization;
@@ -22,17 +23,21 @@ public class PropertyValuesController : ControllerBase
         private readonly IVariableResolver _variableResolver;
         private readonly IEnvironmentsPersistentSource _environmentsPersistentSource;
         private readonly IVariableScopeOptionsResolver _variableScopeOptionsResolver;
+        private readonly ISecurityPrivilegesChecker _securityPrivilegesChecker;
 
         public PropertyValuesController(IPropertyValuesService propertyValuesService,
             IPropertyValuesPersistentSource propertyValuesPersistentSource,
             [FromKeyedServices("VariableResolver")] IVariableResolver variableResolver,
-            IEnvironmentsPersistentSource environmentsPersistentSource, IVariableScopeOptionsResolver variableScopeOptionsResolver)
+            IEnvironmentsPersistentSource environmentsPersistentSource,
+            IVariableScopeOptionsResolver variableScopeOptionsResolver,
+            ISecurityPrivilegesChecker securityPrivilegesChecker)
         {
             _variableScopeOptionsResolver = variableScopeOptionsResolver;
             _environmentsPersistentSource = environmentsPersistentSource;
             _variableResolver = variableResolver;
             _propertyValuesPersistentSource = propertyValuesPersistentSource;
             _propertyValuesService = propertyValuesService;
+            _securityPrivilegesChecker = securityPrivilegesChecker;
         }
 
         /// <summary>
@@ -94,25 +99,50 @@ public IActionResult GetPropertyValues(string propertyName = null, string enviro
         {
             try
             {
+                // Compute environment-scoped privilege once
+                var canReadSecrets = false;
+                if (!string.IsNullOrWhiteSpace(environmentName))
+                {
+                    if (!_environmentsPersistentSource.EnvironmentExists(environmentName))
+                        return StatusCode(StatusCodes.Status400BadRequest,
+                            $"'environmentName' must include a valid environment name, '{environmentName}' not located");
+
+                    canReadSecrets = _securityPrivilegesChecker.CanReadSecrets(User, environmentName);
+                }
+
                 var propertyValues = _propertyValuesService.GetPropertyValues(propertyName, environmentName,
                     User).ToArray();
                 if (!propertyValues.Any())
                 {
                     return NotFound();
                 }
 
+                if (!canReadSecrets)
+                {
+                    foreach (var pv in propertyValues)
+                    {
+                        if (pv?.Property?.Secure == true)
+                        {
+                            pv.Value = null;
+                        }
+                    }
+                }
+
                 return Ok(propertyValues);
             }
+
             catch (NonEnoughRightsException e)
             {
                 return StatusCode(StatusCodes.Status403Forbidden, e);
             }
+
             catch (Exception e)
             {
                 return StatusCode(StatusCodes.Status500InternalServerError, e);
             }
         }
 
+
         /// <summary>
         /// Edit property values
         /// </summary>
@@ -149,4 +179,4 @@ public IActionResult DeletePropertyValue(IEnumerable<PropertyValueDto> propertyV
             return Ok(_propertyValuesService.DeletePropertyValues(propertyValuesToDelete, User));
         }
     }
-}
+}
diff --git a/src/Dorc.Api/Controllers/RefDataScopedPropertyValuesController.cs b/src/Dorc.Api/Controllers/RefDataScopedPropertyValuesController.cs
@@ -1,4 +1,5 @@
 using Dorc.ApiModel;
+using Dorc.Core.Interfaces;
 using Dorc.PersistentData;
 using Dorc.PersistentData.Sources.Interfaces;
 using Microsoft.AspNetCore.Authorization;
@@ -15,14 +16,17 @@ public sealed class RefDataScopedPropertyValuesController : ControllerBase
         private readonly IPropertyValuesPersistentSource _propertyValuesPersistentSource;
         private readonly IEnvironmentsPersistentSource _environmentsPersistentSource;
         private readonly IRolePrivilegesChecker _rolePrivilegesChecker;
+        private readonly ISecurityPrivilegesChecker _securityPrivilegesChecker;
 
         public RefDataScopedPropertyValuesController(IPropertyValuesPersistentSource propertyValuesPersistentSource,
             IEnvironmentsPersistentSource environmentsPersistentSource,
-            IRolePrivilegesChecker rolePrivilegesChecker)
+            IRolePrivilegesChecker rolePrivilegesChecker,
+            ISecurityPrivilegesChecker securityPrivilegesChecker)
         {
             _rolePrivilegesChecker = rolePrivilegesChecker;
             _environmentsPersistentSource = environmentsPersistentSource;
             _propertyValuesPersistentSource = propertyValuesPersistentSource;
+            _securityPrivilegesChecker = securityPrivilegesChecker;
         }
 
         /// <summary>
@@ -49,16 +53,24 @@ public IActionResult Put([FromBody] PagedDataOperators operators, string scope,
             var getScopedPropertyValuesResponseDto = _propertyValuesPersistentSource.GetPropertyValuesForScopeByPage(limit,
                 page, operators, env, User);
 
-            var isAdmin = _rolePrivilegesChecker.IsAdmin(User);
-            if (!isAdmin)
-                return StatusCode(StatusCodes.Status200OK, getScopedPropertyValuesResponseDto);
 
+            var canReadSecrets = _securityPrivilegesChecker.CanReadSecrets(User, scope);
             foreach (var prop in getScopedPropertyValuesResponseDto.Items)
             {
-                prop.UserEditable = true;
+                if (prop.Secure == true && !canReadSecrets)
+                {
+                    prop.PropertyValue = null;
+                }
+            }
+
+            var isAdmin = _rolePrivilegesChecker.IsAdmin(User);
+            if (isAdmin && getScopedPropertyValuesResponseDto.Items != null)
+            {
+                foreach (var prop in getScopedPropertyValuesResponseDto.Items)
+                    prop.UserEditable = true;
             }
 
             return StatusCode(StatusCodes.Status200OK, getScopedPropertyValuesResponseDto);
         }
     }
-}
+}
diff --git a/src/Dorc.Api/Controllers/RefDataSearchPropertyValuesController.cs b/src/Dorc.Api/Controllers/RefDataSearchPropertyValuesController.cs
@@ -1,4 +1,5 @@
 using Dorc.ApiModel;
+using Dorc.Core.Interfaces;
 using Dorc.PersistentData;
 using Dorc.PersistentData.Sources.Interfaces;
 using Microsoft.AspNetCore.Authorization;
@@ -14,12 +15,14 @@ public sealed class RefDataSearchPropertyValuesController : ControllerBase
     {
         private readonly IPropertyValuesPersistentSource _propertyValuesPersistentSource;
         private readonly IRolePrivilegesChecker _rolePrivilegesChecker;
+        private readonly ISecurityPrivilegesChecker _securityPrivilegesChecker;
 
         public RefDataSearchPropertyValuesController(IPropertyValuesPersistentSource propertyValuesPersistentSource,
-            IRolePrivilegesChecker rolePrivilegesChecker)
+            IRolePrivilegesChecker rolePrivilegesChecker, ISecurityPrivilegesChecker securityPrivilegesChecker)
         {
             _rolePrivilegesChecker = rolePrivilegesChecker;
             _propertyValuesPersistentSource = propertyValuesPersistentSource;
+            _securityPrivilegesChecker = securityPrivilegesChecker;
         }
 
         /// <summary>
@@ -37,16 +40,38 @@ public IActionResult Put([FromBody] PagedDataOperators operators, int page = 1,
             var getScopedPropertyValuesResponseDto = _propertyValuesPersistentSource.GetPropertyValuesForSearchValueByPage(limit,
                 page, operators, User);
 
-            var isAdmin = _rolePrivilegesChecker.IsAdmin(User);
-            if (!isAdmin)
-                return StatusCode(StatusCodes.Status200OK, getScopedPropertyValuesResponseDto);
+            if (getScopedPropertyValuesResponseDto?.Items == null || getScopedPropertyValuesResponseDto.Items.Count == 0)
+                return Ok(getScopedPropertyValuesResponseDto);
 
-            foreach (var prop in getScopedPropertyValuesResponseDto.Items)
+            var canReadCache = new Dictionary<string, bool>(System.StringComparer.OrdinalIgnoreCase);
+
+            foreach (var item in getScopedPropertyValuesResponseDto.Items)
+            {
+                var scopeName = item.PropertyValueScope ?? string.Empty;
+
+                bool canReadSecrets = false;
+                if (!string.IsNullOrWhiteSpace(scopeName))
+                {
+                    if (!canReadCache.TryGetValue(scopeName, out canReadSecrets))
+                    {
+                        canReadSecrets = _securityPrivilegesChecker.CanReadSecrets(User, scopeName);
+                        canReadCache[scopeName] = canReadSecrets;
+                    }
+                }
+
+                if (item.Secure == true && !canReadSecrets)
+                {
+                    item.PropertyValue = null;
+                }
+            }
+
+            if (_rolePrivilegesChecker.IsAdmin(User))
             {
-                prop.UserEditable = true;
+                foreach (var item in getScopedPropertyValuesResponseDto.Items)
+                    item.UserEditable = true;
             }
 
-            return StatusCode(StatusCodes.Status200OK, getScopedPropertyValuesResponseDto);
+            return Ok(getScopedPropertyValuesResponseDto);
         }
     }
 }
diff --git a/src/Dorc.Core/SecurityPrivilegesChecker.cs b/src/Dorc.Core/SecurityPrivilegesChecker.cs
@@ -61,7 +61,8 @@ public bool IsEnvironmentOwnerOrDelegate(ClaimsPrincipal user, string environmen
         public bool CanReadSecrets(ClaimsPrincipal user, string environmentName)
         {
             var env = _environmentsPersistentSource.GetSecurityObject(environmentName);
-            return env != null && _securityObjectFilter.HasPrivilege(env, user, AccessLevel.ReadSecrets | AccessLevel.Owner);
+            // Use HasPrivilegeStrict to ensure admins don't bypass - they need explicit "Read Secrets" permission
+            return env != null && _securityObjectFilter.HasPrivilegeStrict(env, user, AccessLevel.ReadSecrets | AccessLevel.Owner);
         }
 
         public bool CanModifyProject(ClaimsPrincipal user, string projectName)

diff --git a/src/Dorc.PersistentData/ISecurityObjectFilter.cs b/src/Dorc.PersistentData/ISecurityObjectFilter.cs
@@ -15,5 +15,11 @@ public enum AccessLevel
     public interface ISecurityObjectFilter
     {
         bool HasPrivilege<T>(T securityObject, IPrincipal user, AccessLevel accessLevel) where T : SecurityObject;
+
+        /// <summary>
+        /// Checks if user has the specified privilege WITHOUT admin bypass.
+        /// Use this for permissions that should apply equally to all users including admins.
+        /// </summary>
+        bool HasPrivilegeStrict<T>(T securityObject, IPrincipal user, AccessLevel accessLevel) where T : SecurityObject;
     }
 }
diff --git a/src/Dorc.PersistentData/SecurityObjectFilter.cs b/src/Dorc.PersistentData/SecurityObjectFilter.cs
@@ -28,6 +28,11 @@ public bool HasPrivilege<T>(T securityObject, IPrincipal user, AccessLevel acces
                 return true;
             }
 
+            return HasPrivilegeStrict(securityObject, user, accessLevel);
+        }
+
+        public bool HasPrivilegeStrict<T>(T securityObject, IPrincipal user, AccessLevel accessLevel) where T : SecurityObject
+        {
             var userAccessControls = GetUserAccessControls(securityObject, user);
 
             var allowed = 0;