Skip to content

Commit 6eca5a0

Browse files
authored
Merge pull request cdapio#16036 from cdapio/CDAP-21203_dev
CDAP-21203 : Handle unauthorized exception case for remote calls for Audit logs and proper error message
2 parents e0d0682 + 3906708 commit 6eca5a0

File tree

3 files changed

+54
-26
lines changed

3 files changed

+54
-26
lines changed

cdap-app-fabric/src/main/java/io/cdap/cdap/gateway/handlers/meta/RemotePrivilegesHandler.java

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import io.cdap.cdap.security.spi.authorization.AuditLogContext;
3535
import io.cdap.cdap.security.spi.authorization.AuthorizationResponse;
3636
import io.cdap.cdap.security.spi.authorization.PermissionManager;
37+
import io.cdap.cdap.security.spi.authorization.UnauthorizedException;
3738
import io.cdap.http.HttpResponder;
3839
import io.netty.handler.codec.http.FullHttpRequest;
3940
import io.netty.handler.codec.http.HttpResponseStatus;
@@ -83,23 +84,30 @@ public void enforce(FullHttpRequest request, HttpResponder responder) throws Exc
8384
AuthorizationPrivilege.class);
8485
LOG.debug("Enforcing for {}", authorizationPrivilege);
8586
Set<Permission> permissions = authorizationPrivilege.getPermissions();
86-
if (authorizationPrivilege.getChildEntityType() != null) {
87-
//It's expected that we'll always have one, but let's handle generic case
88-
for (Permission permission : permissions) {
89-
accessEnforcer.enforceOnParent(authorizationPrivilege.getChildEntityType(),
90-
authorizationPrivilege.getEntity(),
91-
authorizationPrivilege.getPrincipal(), permission);
87+
HttpResponseStatus responseStatus = HttpResponseStatus.OK;
88+
try {
89+
if (authorizationPrivilege.getChildEntityType() != null) {
90+
//It's expected that we'll always have one, but let's handle generic case
91+
for (Permission permission : permissions) {
92+
accessEnforcer.enforceOnParent(authorizationPrivilege.getChildEntityType(),
93+
authorizationPrivilege.getEntity(),
94+
authorizationPrivilege.getPrincipal(), permission);
95+
}
96+
} else {
97+
accessEnforcer.enforce(authorizationPrivilege.getEntity(),
98+
authorizationPrivilege.getPrincipal(),
99+
permissions);
92100
}
93-
} else {
94-
accessEnforcer.enforce(authorizationPrivilege.getEntity(),
95-
authorizationPrivilege.getPrincipal(),
96-
permissions);
101+
} catch (UnauthorizedException e) {
102+
// In case of UnauthorizedException, we have the audit logs. So, propagating them with proper response instead of
103+
// throwing it here as we will loose the audit logs. The caller should handle the logs and throw as needed.
104+
responseStatus = HttpResponseStatus.valueOf(e.getStatusCode());
97105
}
98106
Queue<AuditLogContext> auditLogContextQueue = SecurityRequestContext.getAuditLogQueue();
99107
//Clearing this so it doesn't get double write to messaging queue
100108
//This should be written by the Client who is calling this service.
101109
SecurityRequestContext.clearAuditLogQueue();
102-
responder.sendJson(HttpResponseStatus.OK, GSON.toJson(auditLogContextQueue));
110+
responder.sendJson(responseStatus, GSON.toJson(auditLogContextQueue));
103111
}
104112

105113
@POST

cdap-security-spi/src/main/java/io/cdap/cdap/security/spi/authorization/UnauthorizedException.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,15 @@ public UnauthorizedException(Principal principal, EntityId entityId) {
7777
}
7878

7979
public UnauthorizedException(Principal principal, Set<? extends ActionOrPermission> actions,
80-
EntityId entityId,
81-
boolean mustHaveAllPermissions) {
80+
EntityId entityId, boolean mustHaveAllPermissions) {
81+
this(principal, actions, entityId, mustHaveAllPermissions, true);
82+
}
83+
84+
public UnauthorizedException(Principal principal, Set<? extends ActionOrPermission> actions,
85+
EntityId entityId, boolean mustHaveAllPermissions, boolean includePrincipal) {
8286
this(principal.toString(), actions.stream().map(action -> action.toString())
83-
.collect(Collectors.toCollection(LinkedHashSet::new)), getEntityLabel(entityId),
84-
null, mustHaveAllPermissions, true, null);
87+
.collect(Collectors.toCollection(LinkedHashSet::new)), getEntityLabel(entityId),
88+
null, mustHaveAllPermissions, includePrincipal, null);
8589
}
8690

8791
public UnauthorizedException(@Nullable String principal, Set<String> missingPermissions,

cdap-security/src/main/java/io/cdap/cdap/security/authorization/RemoteAccessEnforcer.java

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ public class RemoteAccessEnforcer extends AbstractAccessEnforcer {
8686
.enableComplexMapKeySerialization()
8787
.create();
8888

89+
private static final Type AUDIT_LOG_QUEUE_TYPE = new TypeToken<LinkedBlockingDeque<AuditLogContext>>(){}.getType();
90+
8991
private static final Type MAP_ENTITY_TYPE = new TypeToken<Map<EntityId, AuthorizationResponse>>() {
9092
}.getType();
9193

@@ -305,19 +307,33 @@ private EnforcementResponse doEnforce(AuthorizationPrivilege authorizationPrivil
305307
LOG.trace("Remotely enforcing on authorization privilege {}", authorizationPrivilege);
306308
try {
307309
HttpResponse response = remoteClient.execute(request);
308-
309-
Type queueType = new TypeToken<LinkedBlockingDeque<AuditLogContext>>(){}.getType();
310-
Queue<AuditLogContext> deserializedQueue = GSON.fromJson(response.getResponseBodyAsString(), queueType);
311-
312-
if (response.getResponseCode() == HttpURLConnection.HTTP_OK) {
313-
return new EnforcementResponse(true, deserializedQueue, null);
310+
Queue<AuditLogContext> deserializedQueue = GSON.fromJson(response.getResponseBodyAsString(),
311+
AUDIT_LOG_QUEUE_TYPE);
312+
switch (response.getResponseCode()) {
313+
case HttpURLConnection.HTTP_OK:
314+
return new EnforcementResponse(true, deserializedQueue, null);
315+
316+
default:
317+
return new EnforcementResponse(false,
318+
deserializedQueue,
319+
new IOException(String.format("Failed to enforce with code %d: %s",
320+
response.getResponseCode(),
321+
response.getResponseBodyAsString())));
314322
}
315-
return new EnforcementResponse(false,
316-
deserializedQueue,
317-
new IOException(String.format("Failed to enforce with code %d: %s",
318-
response.getResponseCode(),
319-
response.getResponseBodyAsString())));
320323
} catch (UnauthorizedException e) {
324+
try {
325+
Queue<AuditLogContext> deserializedQueue = GSON.fromJson(e.getMessage(), AUDIT_LOG_QUEUE_TYPE);
326+
return new EnforcementResponse(false,
327+
deserializedQueue,
328+
new UnauthorizedException(
329+
authorizationPrivilege.getPrincipal(),
330+
authorizationPrivilege.getPermissions(),
331+
authorizationPrivilege.getEntity(),
332+
true, false));
333+
334+
} catch (Exception ex) {
335+
// no-op
336+
}
321337
return new EnforcementResponse(false, null, e);
322338
}
323339
}

0 commit comments

Comments
 (0)