Skip to content

Commit 6039944

Browse files
Add warning logging to 5 silent catch blocks in SubjectContext
Replace empty catch blocks in security-critical authorization code with LOG.warn calls that include exception context and stack traces: - isTeamAsset(): log team asset ownership lookup failures - isInTeam(): log team hierarchy traversal failures - getRolesForTeams(): log role resolution failures - hasRole(): log role check failures via team chain - UserPolicyIterator: log resource owner policy load failures No behavioral changes - fail-closed pattern preserved. Logging enables diagnosis of intermittent auth failures caused by transient DB errors or data inconsistencies.
1 parent 9da7bea commit 6039944

File tree

1 file changed

+27
-7
lines changed
  • openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator

1 file changed

+27
-7
lines changed

openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/SubjectContext.java

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,12 @@ public boolean isTeamAsset(String parentTeam, List<EntityReference> owners) {
169169
Entity.getEntity(Entity.TEAM, owner.getId(), TEAM_FIELDS, Include.NON_DELETED);
170170
return isInTeam(parentTeam, team.getEntityReference());
171171
} catch (Exception ex) {
172-
// Ignore and return false
172+
LOG.warn(
173+
"Failed to check team asset ownership for team [{}] with owner [{}]: {}",
174+
parentTeam,
175+
owner.getId(),
176+
ex.getMessage(),
177+
ex);
173178
}
174179
}
175180
}
@@ -182,8 +187,8 @@ public static boolean isInTeam(String parentTeam, EntityReference team) {
182187
Set<UUID> visitedTeams = new HashSet<>();
183188
stack.push(team); // Start with team and see if the parent matches
184189
while (!stack.isEmpty()) {
190+
EntityReference currentTeamRef = stack.pop();
185191
try {
186-
EntityReference currentTeamRef = stack.pop();
187192
// Skip if we've already visited this team to prevent circular dependencies
188193
if (visitedTeams.contains(currentTeamRef.getId())) {
189194
LOG.warn(
@@ -199,7 +204,12 @@ public static boolean isInTeam(String parentTeam, EntityReference team) {
199204
listOrEmpty(parent.getParents())
200205
.forEach(stack::push); // Continue to go up the chain of parents
201206
} catch (Exception ex) {
202-
// Ignore and return false
207+
LOG.warn(
208+
"Failed to traverse team hierarchy for parent [{}] at team [{}]: {}",
209+
parentTeam,
210+
currentTeamRef.getName(),
211+
ex.getMessage(),
212+
ex);
203213
}
204214
}
205215
return false;
@@ -226,7 +236,8 @@ private static List<EntityReference> getRolesForTeams(
226236
roles.addAll(team.getDefaultRoles());
227237
roles.addAll(getRolesForTeams(team.getParents(), visitedTeams));
228238
} catch (Exception ex) {
229-
// Ignore and continue
239+
LOG.warn(
240+
"Failed to resolve roles for team [{}]: {}", teamRef.getName(), ex.getMessage(), ex);
230241
}
231242
}
232243
return roles.stream().distinct().collect(Collectors.toList());
@@ -280,8 +291,8 @@ public static boolean hasRole(User user, String role) {
280291
}
281292
listOrEmpty(user.getTeams()).forEach(stack::push); // Continue to go up the chain of parents
282293
while (!stack.isEmpty()) {
294+
EntityReference currentTeamRef = stack.pop();
283295
try {
284-
EntityReference currentTeamRef = stack.pop();
285296
// Skip if we've already visited this team to prevent circular dependencies
286297
if (visitedTeams.contains(currentTeamRef.getId())) {
287298
LOG.warn(
@@ -298,7 +309,12 @@ public static boolean hasRole(User user, String role) {
298309
listOrEmpty(parent.getParents())
299310
.forEach(stack::push); // Continue to go up the chain of parents
300311
} catch (Exception ex) {
301-
// Ignore the exception and return false
312+
LOG.warn(
313+
"Failed to check role [{}] for team [{}]: {}",
314+
role,
315+
currentTeamRef.getName(),
316+
ex.getMessage(),
317+
ex);
302318
}
303319
}
304320
return false;
@@ -471,7 +487,11 @@ static class UserPolicyIterator implements Iterator<PolicyContext> {
471487
Entity.TEAM, resourceOwner.getId(), TEAM_FIELDS, Include.NON_DELETED);
472488
iterators.add(new TeamPolicyIterator(team.getId(), teamsVisited, true));
473489
} catch (Exception ex) {
474-
// Ignore
490+
LOG.warn(
491+
"Failed to load policies for resource owner team [{}]: {}",
492+
resourceOwner.getId(),
493+
ex.getMessage(),
494+
ex);
475495
}
476496
}
477497
}

0 commit comments

Comments
 (0)