Skip to content

Commit

Permalink
feat(api): Surface context around what resource was denied access (#256)
Browse files Browse the repository at this point in the history
In response to user feedback that seeing just a `403` error lacked
sufficient context to determine exactly what permission was lacking,
particularly when a pipeline or orchestration failed.

The chosen response messaging mimics what clouddriver is already
doing.

https://github.com/spinnaker/clouddriver/blob/master/clouddriver-core/src/main/groovy/com/netflix/spinnaker/clouddriver/deploy/DescriptionValidator.groovy#L29

```
{
  "error": "Forbidden",
  "message": "Access denied to application clouddriver",
  "status": 403,
  "timestamp": 1534222128232
}
```
  • Loading branch information
ajordens authored Aug 28, 2018
1 parent 397706a commit 3fc0363
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright 2018 Netflix, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.netflix.spinnaker.fiat.shared;

import org.springframework.http.HttpStatus;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;

@ControllerAdvice
public class FiatAccessDeniedExceptionHandler {
@ExceptionHandler(AccessDeniedException.class)
public void handleAccessDeniedException(AccessDeniedException e,
HttpServletResponse response,
HttpServletRequest request) throws IOException {
String errorMessage = FiatPermissionEvaluator.getAuthorizationFailure().map(a ->
"Access denied to " + a.getResourceType().modelClass.getSimpleName().toLowerCase() + " " + a.getResourceName()
).orElse("Access is denied");

response.sendError(HttpStatus.FORBIDDEN.value(), errorMessage);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
@Component
@Slf4j
public class FiatPermissionEvaluator implements PermissionEvaluator {
private final static ThreadLocal<AuthorizationFailure> authorizationFailure = new ThreadLocal<>();

private final Registry registry;
private final FiatService fiatService;
private final FiatStatus fiatStatus;
Expand All @@ -66,7 +68,6 @@ public class FiatPermissionEvaluator implements PermissionEvaluator {

private final RetryHandler retryHandler;


interface RetryHandler {
default <T> T retry(String description, Callable<T> callable) throws Exception {
return callable.call();
Expand Down Expand Up @@ -175,20 +176,13 @@ public boolean hasPermission(Authentication authentication,
}

UserPermission.View permission = getPermission(getUsername(authentication));
return permissionContains(permission, resourceName.toString(), r, a);
}
boolean hasPermission = permissionContains(permission, resourceName.toString(), r, a);

private String getUsername(Authentication authentication) {
String username = "anonymous";
if (authentication.isAuthenticated() && authentication.getPrincipal() != null) {
Object principal = authentication.getPrincipal();
if (principal instanceof User) {
username = ((User) principal).getUsername();
} else if (StringUtils.isNotEmpty(principal.toString())) {
username = principal.toString();
}
}
return username;
authorizationFailure.set(
hasPermission ? null : new AuthorizationFailure(a, r, resourceName.toString())
);

return hasPermission;
}

public void invalidatePermission(String username) {
Expand Down Expand Up @@ -275,6 +269,23 @@ public boolean storeWholePermission() {
return permission != null;
}

public static Optional<AuthorizationFailure> getAuthorizationFailure() {
return Optional.ofNullable(authorizationFailure.get());
}

private String getUsername(Authentication authentication) {
String username = "anonymous";
if (authentication.isAuthenticated() && authentication.getPrincipal() != null) {
Object principal = authentication.getPrincipal();
if (principal instanceof User) {
username = ((User) principal).getUsername();
} else if (StringUtils.isNotEmpty(principal.toString())) {
username = principal.toString();
}
}
return username;
}

private boolean permissionContains(UserPermission.View permission,
String resourceName,
ResourceType resourceType,
Expand Down Expand Up @@ -329,4 +340,28 @@ private boolean permissionContains(UserPermission.View permission,
public boolean isAdmin() {
return true; // TODO(ttomsu): Chosen by fair dice roll. Guaranteed to be random.
}

public class AuthorizationFailure {
private final Authorization authorization;
private final ResourceType resourceType;
private final String resourceName;

public AuthorizationFailure(Authorization authorization, ResourceType resourceType, String resourceName) {
this.authorization = authorization;
this.resourceType = resourceType;
this.resourceName = resourceName;
}

public Authorization getAuthorization() {
return authorization;
}

public ResourceType getResourceType() {
return resourceType;
}

public String getResourceName() {
return resourceName;
}
}
}

0 comments on commit 3fc0363

Please sign in to comment.