Skip to content

Commit

Permalink
Make REST authorizations finer-grained
Browse files Browse the repository at this point in the history 
While "rest-3#all" authorization still exists, it is no longer
required to use it when only a subset of REST methods is to be
accessed by particular client. Each method has now its own
authorization.

Related change: Due to the current authentication architecture
in midPoint, these authorizations are checked in the respective
methods' bodies. So, if an unauthorized method is called, the
"login success" is audited, but the operation immediately fails.
The failure is recorded in the termination (logout) event. And
this is the change: the logout event now contains the real status
of the whole operation. (Previously, it was always SUCCESS.)
  • Loading branch information
@mederly
mederly committed Feb 13, 2024
1 parent 9b70b2e commit da667ef
Show file tree
Hide file tree
Showing 14 changed files with 335 additions and 78 deletions.
38 changes: 26 additions & 12 deletions ...mpl/src/main/java/com/evolveum/midpoint/authentication/impl/util/EndPointsUrlMapping.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@

import static com.evolveum.midpoint.security.api.AuthorizationConstants.*;

import com.evolveum.midpoint.authentication.api.authorization.AuthorizationAction;
import com.evolveum.midpoint.authentication.impl.authorization.AuthorizationActionValue;
import com.evolveum.midpoint.authentication.api.util.AuthConstants;
import com.evolveum.midpoint.security.api.AuthorizationConstants;
import com.evolveum.midpoint.security.api.RestMethod;

import java.util.ArrayList;
import java.util.List;

/**
* @author lazyman
Expand Down Expand Up @@ -173,16 +175,9 @@ public enum EndPointsUrlMapping {
ACTUATOR_METRICS("/actuator/metrics/**",
new AuthorizationActionValue(AUTZ_ACTUATOR_METRICS_URL,
"ActuatorEndpoint.authActuator.metrics.label", "ActuatorEndpoint.authActuator.metrics.description")),
REST("/ws/**",
new AuthorizationActionValue(AUTZ_REST_ALL_URL,
"RestEndpoint.authRest.all.label", "RestEndpoint.authRest.all.description")),
REST2("/rest/**",
new AuthorizationActionValue(AUTZ_REST_ALL_URL,
"RestEndpoint.authRest.all.label", "RestEndpoint.authRest.all.description")),
REST3("/api/**",
new AuthorizationActionValue(AUTZ_REST_ALL_URL,
"RestEndpoint.authRest.all.label", "RestEndpoint.authRest.all.description")),

REST("/ws/**", createRestAuthorizationActionValues()),
REST2("/rest/**", createRestAuthorizationActionValues()),
REST3("/api/**", createRestAuthorizationActionValues()),

INSPECTOR("/inspector/**",
new AuthorizationActionValue(AUTZ_GUI_ALL_URL,
Expand All @@ -197,6 +192,25 @@ public enum EndPointsUrlMapping {
new AuthorizationActionValue(AUTZ_GUI_ALL_URL,
"WicketDebugInfo.authUri.guiAll.label", "WicketDebugInfo.authUri.guiAll.description"));

/**
* Besides rest-3#all, we also treat any action URI specific to a REST method as sufficient
* to allow access to the REST interface. Detailed authorization is then done in the implementation
* of individual REST methods.
*/
private static AuthorizationActionValue[] createRestAuthorizationActionValues() {
List<AuthorizationActionValue> actions = new ArrayList<>();
actions.add(
new AuthorizationActionValue(AUTZ_REST_ALL_URL,
"RestEndpoint.authRest.all.label", "RestEndpoint.authRest.all.description"));
for (var restMethod : RestMethod.values()) {
actions.add(
new AuthorizationActionValue(
restMethod.getActionUri(),
restMethod.getLabel(),
restMethod.getDescription()));
}
return actions.toArray(new AuthorizationActionValue[0]);
}

private final String url;

Expand Down
1 change: 0 additions & 1 deletion model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/security/SecurityHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ public class SecurityHelper implements ModelAuditRecorder {
@Autowired private SystemObjectCache systemObjectCache;
@Autowired private ArchetypeManager archetypeManager;


@Override
public void auditLoginSuccess(@NotNull ObjectType object, @NotNull ConnectionEnvironment connEnv) {
FocusType focus = null;
Expand Down
44 changes: 38 additions & 6 deletions model/rest-impl/src/main/java/com/evolveum/midpoint/rest/impl/AbstractRestController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,24 @@
*/
package com.evolveum.midpoint.rest.impl;

import static com.evolveum.midpoint.security.api.AuthorizationConstants.AUTZ_REST_ALL_URL;

import static org.springframework.http.ResponseEntity.status;

import java.net.URI;
import java.util.Set;

import com.evolveum.midpoint.schema.AccessDecision;
import com.evolveum.midpoint.security.api.RestMethod;
import com.evolveum.midpoint.security.enforcer.api.AuthorizationParameters;

import com.evolveum.midpoint.security.enforcer.api.SecurityEnforcer;

import jakarta.servlet.http.HttpServletRequest;

import com.evolveum.midpoint.authentication.api.config.MidpointAuthentication;

import org.jetbrains.annotations.Nullable;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
Expand Down Expand Up @@ -60,6 +71,7 @@ public class AbstractRestController {
@Autowired protected TaskManager taskManager;
@Autowired protected PrismContext prismContext;
@Autowired private SystemObjectCache systemObjectCache;
@Autowired private SecurityEnforcer securityEnforcer;

protected Task initRequest() {
// No need to audit login. it was already audited during authentication
Expand Down Expand Up @@ -127,17 +139,26 @@ protected <T> ResponseEntity<?> createBody(ResponseEntity.BodyBuilder builder,
return builder.body(result);
}

/** Records the exception into the operation result (if it's empty!), logs it and creates the response. */
protected ResponseEntity<?> handleException(OperationResult result, Throwable t) {
LoggingUtils.logUnexpectedException(logger, "Got exception while servicing REST request: {}", t,
result != null ? result.getOperation() : "(null)");
return handleExceptionNoLog(result, t);
}

protected ResponseEntity<?> handleExceptionNoLog(OperationResult result, Throwable t) {
if (result.isEmpty()) {
result.recordFatalError("Unknown exception occurred", t);
} else {
result.computeStatus();
/** The version without operation result handling. */
protected ResponseEntity<?> handleException(Throwable t) {
LoggingUtils.logUnexpectedException(logger, "Got exception while servicing REST request", t);
return handleExceptionNoLog(null, t);
}

protected ResponseEntity<?> handleExceptionNoLog(@Nullable OperationResult result, Throwable t) {
if (result != null) {
if (result.isEmpty()) {
result.recordFatalError("Unknown exception occurred", t);
} else {
result.computeStatus();
}
}

return createErrorResponseBuilder(result, t);
Expand Down Expand Up @@ -220,7 +241,9 @@ private void auditLogout(Task task, OperationResult result) {

record.setChannel(SchemaConstants.CHANNEL_REST_URI);
record.setTimestamp(System.currentTimeMillis());
record.setOutcome(OperationResultStatus.SUCCESS);
// To be discussed: should we record SUCCESS here (as the logout is successful), or the real result of the operation?
record.setOutcome(result.getStatus());
record.setMessage(result.getMessage());
if (authentication instanceof MidpointAuthentication) {
record.setSessionIdentifier(((MidpointAuthentication) authentication).getSessionId());
}
Expand Down Expand Up @@ -259,4 +282,13 @@ protected String controllerBasePath() {

throw new NullPointerException("Base controller URL could not be determined.");
}

/** Makes sure that the current principal is authorized to call the specified REST method. */
protected void authorize(RestMethod method, Task task, OperationResult result) throws CommonException {
MidPointPrincipal principal = securityEnforcer.getMidPointPrincipal();
String methodUri = method.getActionUri();
if (securityEnforcer.decideAccess(principal, Set.of(methodUri, AUTZ_REST_ALL_URL), task, result) != AccessDecision.ALLOW) {
securityEnforcer.failAuthorization(methodUri, null, AuthorizationParameters.EMPTY, result);
}
}
}
15 changes: 9 additions & 6 deletions model/rest-impl/src/main/java/com/evolveum/midpoint/rest/impl/ClusterRestController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
package com.evolveum.midpoint.rest.impl;

import java.io.File;
import java.io.FileInputStream;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;
import java.util.List;

import com.evolveum.midpoint.authentication.api.config.MidpointAuthentication;
import com.evolveum.midpoint.authentication.api.util.AuthUtil;
import com.evolveum.midpoint.security.api.RestMethod;
import org.apache.commons.io.FileUtils;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
Expand Down Expand Up @@ -51,11 +51,14 @@

/**
* REST service used for inter-cluster communication.
* <p>
*
* These methods are NOT to be called generally by clients.
* They are to be called internally by midPoint running on other cluster nodes.
* <p>
*
* So the usual form of authentication will be CLUSTER (a.k.a. node authentication).
* This is also the reason why these do not need to be protected by special REST authorization action URIs.
* (See {@link RestMethod}.)
*
* However, for diagnostic purposes we might allow also administrator access sometimes in the future.
*/
@RestController
Expand Down Expand Up @@ -262,7 +265,7 @@ public ResponseEntity<?> getReportFile(
}
result.computeStatus();
} catch (Throwable t) {
response = handleException(null, t); // we don't return the operation result
response = handleException(t); // we don't return the operation result
}
finishRequest(task, result);
return response;
Expand All @@ -288,7 +291,7 @@ public ResponseEntity<?> deleteReportFile(
}
result.computeStatus();
} catch (Throwable t) {
response = handleException(null, t); // we don't return the operation result
response = handleException(t); // we don't return the operation result
}
finishRequest(task, result);
return response;
Expand All @@ -307,7 +310,7 @@ public ResponseEntity<?> getTask(@PathVariable("oid") String oid, @RequestParam(
response = ResponseEntity.ok(taskType);
result.computeStatus();
} catch (Throwable t) {
response = handleException(null, t); // we don't return the operation result
response = handleException(t); // we don't return the operation result
}
finishRequest(task, result);
return response;
Expand Down
12 changes: 11 additions & 1 deletion ...est-impl/src/main/java/com/evolveum/midpoint/rest/impl/ExtensionSchemaRestController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import com.evolveum.midpoint.prism.schema.SchemaDescription;
import com.evolveum.midpoint.prism.schema.SchemaRegistry;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.security.enforcer.api.AuthorizationParameters;
import com.evolveum.midpoint.security.api.RestMethod;
import com.evolveum.midpoint.security.enforcer.api.SecurityEnforcer;
import com.evolveum.midpoint.task.api.Task;
import com.evolveum.midpoint.xml.ns._public.common.common_3.SchemaFileType;
Expand All @@ -30,6 +30,14 @@
import java.nio.charset.StandardCharsets;
import java.util.Collection;

/**
* Special REST methods to access external schemas.
*
* Note about authorizations: These methods were covered by {@link ModelAuthorizationAction#GET_EXTENSION_SCHEMA} authorization.
* It is sufficient. However, to avoid the need of having `rest-3#all` authorization to use these methods, we also added
* special (much more specific, i.e. weaker) replacement for it: the one limited to {@link RestMethod#GET_EXTENSION_SCHEMA}.
* So, a user accessing these methods need just the above two (rather weak) authorizations.
*/
@RestController
@RequestMapping({ "/ws/schema", "/rest/schema", "/api/schema" })
public class ExtensionSchemaRestController extends AbstractRestController {
Expand All @@ -43,6 +51,7 @@ public ResponseEntity<?> listSchemas() {

ResponseEntity<?> response;
try {
authorize(RestMethod.GET_EXTENSION_SCHEMA, task, result);
securityEnforcer.authorize(
ModelAuthorizationAction.GET_EXTENSION_SCHEMA.getUrl(), task, result);

Expand Down Expand Up @@ -100,6 +109,7 @@ public ResponseEntity<?> getSchema(

ResponseEntity<?> response;
try {
authorize(RestMethod.GET_EXTENSION_SCHEMA, task, result);
securityEnforcer.authorize(
ModelAuthorizationAction.GET_EXTENSION_SCHEMA.getUrl(), task, result);

Expand Down

0 comments on commit da667ef

Please sign in to comment.