bhor-sanket commented on code in PR #753:
URL: https://github.com/apache/atlas/pull/753#discussion_r4193825506
##########
authorization/src/main/java/org/apache/atlas/authorize/AtlasAuthorizationUtils.java:
##########
@@ -231,6 +217,34 @@ public static boolean
isAccessAllowed(AtlasRelationshipAccessRequest request) {
return ret;
}
+ public static boolean isAccessAllowed(AtlasNotificationRequest request) {
+ MetricRecorder metric =
RequestContext.get().startMetricRecord("isAccessAllowed");
+
+ boolean ret = false;
+ String userName = getCurrentUserName();
+
+ if (StringUtils.isNotEmpty(userName)) {
+ try {
+ AtlasAuthorizer authorizer =
AtlasAuthorizerFactory.getAtlasAuthorizer();
+
+ request.setUser(userName, getCurrentUserGroups());
+
request.setClientIPAddress(RequestContext.get().getClientIPAddress());
+
request.setForwardedAddresses(RequestContext.get().getForwardedAddresses());
+
request.setRemoteIPAddress(RequestContext.get().getClientIPAddress());
+
+ ret = authorizer.isAccessAllowed(request);
+ } catch (AtlasAuthorizationException e) {
+ LOG.error("Unable to obtain AtlasAuthorizer", e);
+ }
+ } else {
+ ret = true;
Review Comment:
For a security-sensitive notification ingress, the empty-username branch
should return false, not true. Or at minimum, document why unauthenticated
access is intentionally allowed here.
##########
authorization/src/main/java/org/apache/atlas/authorize/simple/AtlasSimpleAuthorizer.java:
##########
@@ -253,6 +255,39 @@ public boolean
isAccessAllowed(AtlasRelationshipAccessRequest request) {
return hasEnd1EntityAccess && hasEnd2EntityAccess;
}
+ @Override
+ public boolean isAccessAllowed(AtlasNotificationRequest request) {
+ LOG.debug("==> SimpleAtlasAuthorizer.isAccessAllowed({})", request);
+
+ boolean ret = false;
+ Set<String> roles = getRoles(request.getUser(),
request.getUserGroups());
+
+ for (String role : roles) {
+ List<AtlasNotificationPermission> permissions =
getNotificationPermissionsForRole(role);
+
+ if (permissions != null) {
+ final String action = request.getAction() != null ?
request.getAction().getType() : null;
+ final String topicName = request.getTopicName();
+
+ for (AtlasNotificationPermission permission : permissions) {
+ if (isMatch(action, permission.getPrivileges()) &&
isMatch(topicName, permission.getTopicNames())) {
Review Comment:
The isMatch(String value, List<String> patterns) method at line 273 returns
true when value == null. So a null topicName would match any permission,
effectively bypassing topic scoping.
is it expected ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
