From a3a53d299f60531a1a8837f7fd54006feea379c9 Mon Sep 17 00:00:00 2001
From: Madhura Bhave <mbhave@pivotal.io>
Date: Tue, 19 Nov 2019 11:18:07 -0800
Subject: [PATCH] Check authorities when exposing health details

Fixes gh-18998
---
 .../HealthWebEndpointResponseMapper.java      | 21 +++++++++++
 .../HealthWebEndpointResponseMapperTests.java | 35 +++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/HealthWebEndpointResponseMapper.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/HealthWebEndpointResponseMapper.java
index 868d8280235..d7799d2923b 100644
--- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/HealthWebEndpointResponseMapper.java
+++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/HealthWebEndpointResponseMapper.java
@@ -16,11 +16,15 @@
 
 package org.springframework.boot.actuate.health;
 
+import java.security.Principal;
 import java.util.Set;
 import java.util.function.Supplier;
 
 import org.springframework.boot.actuate.endpoint.SecurityContext;
 import org.springframework.boot.actuate.endpoint.web.WebEndpointResponse;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.GrantedAuthority;
+import org.springframework.util.ClassUtils;
 import org.springframework.util.CollectionUtils;
 
 /**
@@ -108,12 +112,29 @@ public class HealthWebEndpointResponseMapper {
 		if (CollectionUtils.isEmpty(this.authorizedRoles)) {
 			return true;
 		}
+		Principal principal = securityContext.getPrincipal();
+		boolean checkAuthorities = isSpringSecurityAuthentication(principal);
 		for (String role : this.authorizedRoles) {
 			if (securityContext.isUserInRole(role)) {
 				return true;
 			}
+			if (checkAuthorities) {
+				Authentication authentication = (Authentication) principal;
+				for (GrantedAuthority authority : authentication.getAuthorities()) {
+					String name = authority.getAuthority();
+					if (role.equals(name)) {
+						return true;
+					}
+				}
+			}
 		}
+
 		return false;
 	}
 
+	private boolean isSpringSecurityAuthentication(Principal principal) {
+		return ClassUtils.isPresent("org.springframework.security.core.Authentication", null)
+				&& (principal instanceof Authentication);
+	}
+
 }
diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/HealthWebEndpointResponseMapperTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/HealthWebEndpointResponseMapperTests.java
index 739c6bd73fc..37d5b10819d 100644
--- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/HealthWebEndpointResponseMapperTests.java
+++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/HealthWebEndpointResponseMapperTests.java
@@ -29,6 +29,8 @@ import org.mockito.stubbing.Answer;
 import org.springframework.boot.actuate.endpoint.SecurityContext;
 import org.springframework.boot.actuate.endpoint.web.WebEndpointResponse;
 import org.springframework.http.HttpStatus;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.anyString;
@@ -84,6 +86,39 @@ public class HealthWebEndpointResponseMapperTests {
 		verify(securityContext).isUserInRole("ACTUATOR");
 	}
 
+	@Test
+	public void mapDetailsWithRightAuthoritiesInvokesSupplier() {
+		HealthWebEndpointResponseMapper mapper = createMapper(ShowDetails.WHEN_AUTHORIZED);
+		Supplier<Health> supplier = mockSupplier();
+		given(supplier.get()).willReturn(Health.down().build());
+		SecurityContext securityContext = getSecurityContext("ACTUATOR");
+		WebEndpointResponse<Health> response = mapper.mapDetails(supplier, securityContext);
+		assertThat(response.getStatus()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE.value());
+		assertThat(response.getBody().getStatus()).isEqualTo(Status.DOWN);
+		verify(supplier).get();
+	}
+
+	@Test
+	public void mapDetailsWithOtherAuthoritiesShouldNotInvokeSupplier() {
+		HealthWebEndpointResponseMapper mapper = createMapper(ShowDetails.WHEN_AUTHORIZED);
+		Supplier<Health> supplier = mockSupplier();
+		given(supplier.get()).willReturn(Health.down().build());
+		SecurityContext securityContext = getSecurityContext("OTHER");
+		WebEndpointResponse<Health> response = mapper.mapDetails(supplier, securityContext);
+		assertThat(response.getStatus()).isEqualTo(HttpStatus.NOT_FOUND.value());
+		assertThat(response.getBody()).isNull();
+		verifyZeroInteractions(supplier);
+	}
+
+	private SecurityContext getSecurityContext(String other) {
+		SecurityContext securityContext = mock(SecurityContext.class);
+		Authentication principal = mock(Authentication.class);
+		given(securityContext.getPrincipal()).willReturn(principal);
+		given(principal.getAuthorities())
+				.willAnswer((invocation) -> Collections.singleton(new SimpleGrantedAuthority(other)));
+		return securityContext;
+	}
+
 	@Test
 	public void mapDetailsWithUnavailableHealth() {
 		HealthWebEndpointResponseMapper mapper = createMapper(ShowDetails.ALWAYS);