Skip to content

Commit

Permalink
feat(extensibility): Add UserMessageService in FiatAccessDeniedExcept…
Browse files Browse the repository at this point in the history
…ionHandler to provide optional custom messages to end-users on access denied exceptions (#781)
  • Loading branch information
jonsie committed Sep 24, 2020
1 parent eb541ff commit ea55b70
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 12 deletions.
2 changes: 2 additions & 0 deletions fiat-api/fiat-api.gradle
Expand Up @@ -15,6 +15,8 @@
*/

dependencies {
api "com.netflix.spinnaker.kork:kork-api"

implementation project(":fiat-core")

implementation "org.springframework:spring-aop"
Expand Down
Expand Up @@ -16,6 +16,8 @@

package com.netflix.spinnaker.fiat.shared;

import com.netflix.spinnaker.kork.api.exceptions.AccessDeniedDetails;
import com.netflix.spinnaker.kork.web.exceptions.ExceptionMessageDecorator;
import java.io.IOException;
import java.util.Enumeration;
import java.util.HashMap;
Expand All @@ -37,6 +39,12 @@ public class FiatAccessDeniedExceptionHandler {

private final DefaultErrorAttributes defaultErrorAttributes = new DefaultErrorAttributes();

private final ExceptionMessageDecorator exceptionMessageDecorator;

public FiatAccessDeniedExceptionHandler(ExceptionMessageDecorator exceptionMessageDecorator) {
this.exceptionMessageDecorator = exceptionMessageDecorator;
}

@ExceptionHandler(AccessDeniedException.class)
public void handleAccessDeniedException(
AccessDeniedException e, HttpServletResponse response, HttpServletRequest request)
Expand All @@ -54,29 +62,47 @@ public void handleAccessDeniedException(

String errorMessage =
FiatPermissionEvaluator.getAuthorizationFailure()
.map(this::authorizationFailureMessage)
.map(authorizationFailure -> authorizationFailureMessage(authorizationFailure, e))
.orElse("Access is denied");

response.sendError(HttpStatus.FORBIDDEN.value(), errorMessage);
}

private String authorizationFailureMessage(
FiatPermissionEvaluator.AuthorizationFailure authorizationFailure) {
FiatPermissionEvaluator.AuthorizationFailure authorizationFailure, AccessDeniedException e) {
StringJoiner sj = new StringJoiner(" ");

defaultErrorDecoration(sj, authorizationFailure);

// TODO(jonsie): Once we have migrated the current fiat-api code to kork-authz we can produce a
// proper fiat-api module that will allow us to export a class like the one below (instead
// of the current hack which is to pull this in from kork-api).
AccessDeniedDetails accessDeniedDetails =
new AccessDeniedDetails(
authorizationFailure.getResourceType().toString(),
authorizationFailure.getResourceName(),
authorizationFailure.hasAuthorization()
? authorizationFailure.getAuthorization().toString()
: null);

return exceptionMessageDecorator.decorate(e, sj.toString(), accessDeniedDetails);
}

/**
* Default access denied error decoration - decorates the error with the resource type and
* resource name that authorization was denied to.
*/
private void defaultErrorDecoration(
StringJoiner sj, FiatPermissionEvaluator.AuthorizationFailure authorizationFailure) {
// Make the resource type readable (ie, "service account" instead of "serviceaccount")
String resourceType =
authorizationFailure.getResourceType().toString().replace("_", " ").toLowerCase();

StringJoiner sj =
new StringJoiner(" ")
.add("Access denied to")
.add(resourceType)
.add(authorizationFailure.getResourceName());
sj.add("Access denied to").add(resourceType).add(authorizationFailure.getResourceName());

if (authorizationFailure.hasAuthorization()) {
sj.add("- required authorization:").add(authorizationFailure.getAuthorization().toString());
}

return sj.toString();
}

private Map<String, String> requestHeaders(HttpServletRequest request) {
Expand Down
Expand Up @@ -20,6 +20,12 @@ import com.netflix.spinnaker.fiat.model.Authorization
import com.netflix.spinnaker.fiat.model.UserPermission
import com.netflix.spinnaker.fiat.model.resources.Application
import com.netflix.spinnaker.fiat.model.resources.ServiceAccount
import com.netflix.spinnaker.kork.api.exceptions.ExceptionDetails
import com.netflix.spinnaker.kork.api.exceptions.ExceptionMessage
import com.netflix.spinnaker.kork.web.exceptions.ExceptionMessageDecorator
import org.jetbrains.annotations.Nullable
import org.springframework.beans.BeansException
import org.springframework.beans.factory.ObjectProvider
import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken
import spock.lang.Shared
import spock.lang.Subject
Expand All @@ -30,8 +36,44 @@ import javax.servlet.http.HttpServletRequest
import javax.servlet.http.HttpServletResponse

class FiatAccessDeniedExceptionHandlerSpec extends FiatSharedSpecification {

String additionalInformation = "Additional information"

ExceptionMessageDecorator exceptionMessageDecorator = new ExceptionMessageDecorator(new ObjectProvider<List<ExceptionMessage>>() {
@Override
List<ExceptionMessage> getObject(Object... args) throws BeansException {
return null
}

@Override
List<ExceptionMessage> getIfAvailable() throws BeansException {
return [new ExceptionMessage() {

@Override
boolean supports(Class<? extends Throwable> throwable) {
return throwable == AccessDeniedException.class
}

@Override
String message(Throwable throwable, @Nullable ExceptionDetails exceptionDetails) {
return additionalInformation
}
}]
}

@Override
List<ExceptionMessage> getIfUnique() throws BeansException {
return null
}

@Override
List<ExceptionMessage> getObject() throws BeansException {
return null
}
})

@Subject
def fiatAccessDeniedExceptionHandler = new FiatAccessDeniedExceptionHandler()
def fiatAccessDeniedExceptionHandler = new FiatAccessDeniedExceptionHandler(exceptionMessageDecorator)

def request = Mock(HttpServletRequest)
def response = Mock(HttpServletResponse)
Expand Down Expand Up @@ -63,7 +105,7 @@ class FiatAccessDeniedExceptionHandlerSpec extends FiatSharedSpecification {
fiatAccessDeniedExceptionHandler.handleAccessDeniedException(new AccessDeniedException("Forbidden"), response, request)

then:
1 * response.sendError(403, "Access denied to application service - required authorization: " + authorizationTypeRequired)
1 * response.sendError(403, "Access denied to application service - required authorization: " + authorizationTypeRequired + "\n" + additionalInformation)

where:
userAuthorizationType | authorizationTypeRequired
Expand All @@ -89,6 +131,6 @@ class FiatAccessDeniedExceptionHandlerSpec extends FiatSharedSpecification {
fiatAccessDeniedExceptionHandler.handleAccessDeniedException(new AccessDeniedException("Forbidden"), response, request)

then:
1 * response.sendError(403, "Access denied to service account readable")
1 * response.sendError(403, "Access denied to service account readable" + "\n" + additionalInformation)
}
}

0 comments on commit ea55b70

Please sign in to comment.