Merge pull request #1006 from ferbncode/security-chedchad

Get user from subject name in LoggingFilter

build.gradle

@@ -138,7 +138,7 @@ dependencies {
     compile "io.dropwizard.metrics:metrics-servlets:$dropwizardVersion"
     compile "io.dropwizard.metrics:metrics-jvm:$dropwizardVersion"
     compile 'org.apache.commons:commons-lang3:3.8.1'
-    compile 'org.zalando:nakadi-plugin-api:3.0.1'
+    compile 'org.zalando:nakadi-plugin-api:3.1.2'
     compile 'org.echocat.jomon:runtime:1.6.3'
 
     // kafka & zookeeper

src/main/java/org/zalando/nakadi/config/SecuritySettings.java

@@ -21,6 +21,7 @@ public enum AuthMode {
     private final String clientId;
     private final AuthMode authMode;
     private final String adminClientId;
+    public static final String UNAUTHENTICATED_CLIENT_ID = "unauthenticated";
 
     @Autowired
     public SecuritySettings(@Value("${nakadi.oauth2.tokenInfoUrl}") final String tokenInfoUrl,

src/main/java/org/zalando/nakadi/filters/LoggingFilter.java

@@ -9,6 +9,8 @@
 import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Component;
 import org.springframework.web.filter.OncePerRequestFilter;
+import org.zalando.nakadi.plugin.api.authz.AuthorizationService;
+import org.zalando.nakadi.plugin.api.authz.Subject;
 import org.zalando.nakadi.service.NakadiKpiPublisher;
 import org.zalando.nakadi.util.FlowIdUtils;
 
@@ -19,7 +21,6 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
-import java.security.Principal;
 import java.util.Optional;
 
 @Component
@@ -31,11 +32,15 @@ public class LoggingFilter extends OncePerRequestFilter {
     private final NakadiKpiPublisher nakadiKpiPublisher;
     private final String accessLogEventType;
 
+    private final AuthorizationService authorizationService;
+
     @Autowired
     public LoggingFilter(final NakadiKpiPublisher nakadiKpiPublisher,
+                         final AuthorizationService authorizationService,
                          @Value("${nakadi.kpi.event-types.nakadiAccessLog}") final String accessLogEventType) {
         this.nakadiKpiPublisher = nakadiKpiPublisher;
         this.accessLogEventType = accessLogEventType;
+        this.authorizationService = authorizationService;
     }
 
     private class RequestLogInfo {
@@ -52,7 +57,7 @@ private class RequestLogInfo {
 
         private RequestLogInfo(final HttpServletRequest request, final long requestTime) {
             this.userAgent = Optional.ofNullable(request.getHeader("User-Agent")).orElse("-");
-            this.user = Optional.ofNullable(request.getUserPrincipal()).map(Principal::getName).orElse("-");
+            this.user = authorizationService.getSubject().map(Subject::getName).orElse("-");
             this.method = request.getMethod();
             this.path = request.getRequestURI();
             this.query = Optional.ofNullable(request.getQueryString()).map(q -> "?" + q).orElse("");

src/main/java/org/zalando/nakadi/metrics/MonitoringRequestFilter.java

@@ -7,30 +7,34 @@
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.stereotype.Component;
 import org.springframework.web.filter.OncePerRequestFilter;
+import org.zalando.nakadi.config.SecuritySettings;
+import org.zalando.nakadi.plugin.api.authz.AuthorizationService;
+import org.zalando.nakadi.plugin.api.authz.Subject;
 
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
-import java.security.Principal;
-import java.util.Optional;
 
 @Component
 public class MonitoringRequestFilter extends OncePerRequestFilter {
 
     private final Timer httpConnectionsTimer;
     private final Counter openHttpConnectionsCounter;
     private final MetricRegistry perPathMetricRegistry;
+    private final AuthorizationService authorizationService;
 
     @Autowired
     public MonitoringRequestFilter(final MetricRegistry metricRegistry,
-                                   @Qualifier("perPathMetricRegistry") final MetricRegistry perPathMetricRegistry) {
+                                   @Qualifier("perPathMetricRegistry") final MetricRegistry perPathMetricRegistry,
+                                   final AuthorizationService authorizationService) {
         openHttpConnectionsCounter = metricRegistry.counter(MetricUtils.NAKADI_PREFIX
                 + "general.openSynchronousHttpConnections");
         httpConnectionsTimer = metricRegistry
                 .timer(MetricUtils.NAKADI_PREFIX + "general.synchronousHttpConnections");
         this.perPathMetricRegistry = perPathMetricRegistry;
+        this.authorizationService = authorizationService;
     }
 
     @Override
@@ -40,9 +44,8 @@ protected void doFilterInternal(final HttpServletRequest request,
         openHttpConnectionsCounter.inc();
         final Timer.Context timerContext = httpConnectionsTimer.time();
 
-        final String clientId = Optional.ofNullable(request.getUserPrincipal())
-                .map(Principal::getName)
-                .orElse("unauthenticated");
+        final String clientId = authorizationService.getSubject().map(Subject::getName)
+                .orElse(SecuritySettings.UNAUTHENTICATED_CLIENT_ID);
         final String perPathMetricKey = MetricRegistry.name(
                 clientId,
                 request.getMethod(),

src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java

@@ -4,8 +4,11 @@
 import org.zalando.nakadi.plugin.api.authz.AuthorizationAttribute;
 import org.zalando.nakadi.plugin.api.authz.AuthorizationService;
 import org.zalando.nakadi.plugin.api.authz.Resource;
+import org.zalando.nakadi.plugin.api.authz.Subject;
 
+import javax.annotation.Nullable;
 import java.util.List;
+import java.util.Optional;
 
 public class DefaultAuthorizationService implements AuthorizationService {
 
@@ -19,6 +22,12 @@ public boolean isAuthorizationAttributeValid(final AuthorizationAttribute author
         return true;
     }
 
+    @Override
+    @Nullable
+    public Optional<Subject> getSubject() {
+        return Optional.empty();
+    }
+
     @Override
     public List<Resource> filter(final List<Resource> input) throws PluginException {
         return input;

src/main/java/org/zalando/nakadi/security/ClientResolver.java

@@ -13,9 +13,9 @@
 import org.springframework.web.method.support.HandlerMethodArgumentResolver;
 import org.springframework.web.method.support.ModelAndViewContainer;
 import org.zalando.nakadi.config.SecuritySettings;
-import org.zalando.nakadi.service.FeatureToggleService;
+import org.zalando.nakadi.plugin.api.authz.AuthorizationService;
+import org.zalando.nakadi.plugin.api.authz.Subject;
 
-import java.security.Principal;
 import java.util.Map;
 import java.util.Optional;
 
@@ -26,10 +26,13 @@ public class ClientResolver implements HandlerMethodArgumentResolver {
 
     private static final String FULL_ACCESS_CLIENT_ID = "adminClientId";
     private final SecuritySettings settings;
+    private final AuthorizationService authorizationService;
 
     @Autowired
-    public ClientResolver(final SecuritySettings settings, final FeatureToggleService featureToggleService) {
+    public ClientResolver(final SecuritySettings settings,
+                          final AuthorizationService authorizationService) {
         this.settings = settings;
+        this.authorizationService = authorizationService;
     }
 
     @Override
@@ -41,15 +44,22 @@ public boolean supportsParameter(final MethodParameter parameter) {
     public Client resolveArgument(final MethodParameter parameter,
                                   final ModelAndViewContainer mavContainer,
                                   final NativeWebRequest request,
-                                  final WebDataBinderFactory binderFactory) throws Exception {
-        final Optional<String> clientId = Optional.ofNullable(request.getUserPrincipal()).map(Principal::getName);
-        if (clientId.filter(settings.getAdminClientId()::equals).isPresent()
-                || settings.getAuthMode() == OFF) {
-            return new FullAccessClient(clientId.orElse(FULL_ACCESS_CLIENT_ID));
-        }
+                                  final WebDataBinderFactory binderFactory) {
+
+        final String clientId = authorizationService.getSubject().map(Subject::getName)
+                .orElse(SecuritySettings.UNAUTHENTICATED_CLIENT_ID);
 
-        return clientId.map(client -> new NakadiClient(client, getRealm()))
-                .orElseThrow(() -> new UnauthorizedUserException("Client unauthorized"));
+        if(settings.getAuthMode() == OFF) {
+            return new FullAccessClient(clientId);
+        } else {
+            if (clientId.equals(settings.getAdminClientId())) {
+                return new FullAccessClient(FULL_ACCESS_CLIENT_ID);
+            } else if (!authorizationService.getSubject().isPresent()) {
+                throw new UnauthorizedUserException("Client unauthorized");
+            } else {
+                return new NakadiClient(clientId, getRealm());
+            }
+        }
     }
 
     public String getRealm() {

src/test/java/org/zalando/nakadi/controller/CursorsControllerTest.java

@@ -18,20 +18,21 @@
 import org.zalando.nakadi.exceptions.runtime.NoSuchEventTypeException;
 import org.zalando.nakadi.exceptions.runtime.NoSuchSubscriptionException;
 import org.zalando.nakadi.exceptions.runtime.ServiceTemporarilyUnavailableException;
+import org.zalando.nakadi.plugin.api.authz.AuthorizationService;
 import org.zalando.nakadi.repository.EventTypeRepository;
 import org.zalando.nakadi.repository.db.SubscriptionDbRepository;
 import org.zalando.nakadi.security.ClientResolver;
 import org.zalando.nakadi.service.CursorConverter;
 import org.zalando.nakadi.service.CursorTokenService;
 import org.zalando.nakadi.service.CursorsService;
-import org.zalando.nakadi.service.FeatureToggleService;
 import org.zalando.nakadi.utils.RandomSubscriptionBuilder;
 import org.zalando.nakadi.utils.TestUtils;
 import org.zalando.nakadi.view.CursorCommitResult;
 import org.zalando.nakadi.view.SubscriptionCursor;
 import org.zalando.problem.Problem;
 
 import java.util.List;
+import java.util.Optional;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
@@ -74,16 +75,15 @@ public class CursorsControllerTest {
 
     private final CursorsService cursorsService = mock(CursorsService.class);
     private final MockMvc mockMvc;
-    private final FeatureToggleService featureToggleService;
     private final SubscriptionDbRepository subscriptionRepository;
     private final CursorConverter cursorConverter;
+    private final AuthorizationService authorizationService;
 
     public CursorsControllerTest() throws Exception {
 
-        featureToggleService = mock(FeatureToggleService.class);
-        when(featureToggleService.isFeatureEnabled(any())).thenReturn(true);
-
         subscriptionRepository = mock(SubscriptionDbRepository.class);
+        authorizationService = mock(AuthorizationService.class);
+        when(authorizationService.getSubject()).thenReturn(Optional.empty());
         cursorConverter = mock(CursorConverter.class);
 
         IntStream.range(0, DUMMY_CURSORS.size()).forEach(idx ->
@@ -104,7 +104,7 @@ public CursorsControllerTest() throws Exception {
 
         mockMvc = standaloneSetup(controller)
                 .setMessageConverters(new StringHttpMessageConverter(), TestUtils.JACKSON_2_HTTP_MESSAGE_CONVERTER)
-                .setCustomArgumentResolvers(new ClientResolver(settings, featureToggleService))
+                .setCustomArgumentResolvers(new ClientResolver(settings, authorizationService))
                 .setControllerAdvice(new NakadiProblemExceptionHandler(), new CursorsExceptionHandler())
                 .build();
     }

src/test/java/org/zalando/nakadi/controller/EventPublishingControllerTest.java

@@ -23,15 +23,16 @@
 import org.zalando.nakadi.exceptions.runtime.EventTypeTimeoutException;
 import org.zalando.nakadi.metrics.EventTypeMetricRegistry;
 import org.zalando.nakadi.metrics.EventTypeMetrics;
+import org.zalando.nakadi.plugin.api.authz.AuthorizationService;
 import org.zalando.nakadi.security.ClientResolver;
 import org.zalando.nakadi.service.BlacklistService;
 import org.zalando.nakadi.service.EventPublisher;
-import org.zalando.nakadi.service.FeatureToggleService;
 import org.zalando.nakadi.service.NakadiKpiPublisher;
 import org.zalando.nakadi.utils.TestUtils;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 import java.util.function.Supplier;
 
 import static org.hamcrest.CoreMatchers.equalTo;
@@ -71,6 +72,7 @@ public class EventPublishingControllerTest {
     private EventTypeMetricRegistry eventTypeMetricRegistry;
     private NakadiKpiPublisher kpiPublisher;
     private BlacklistService blacklistService;
+    private AuthorizationService authorizationService;
 
     @Before
     public void setUp() {
@@ -79,21 +81,21 @@ public void setUp() {
         eventTypeMetricRegistry = new EventTypeMetricRegistry(metricRegistry);
         kpiPublisher = mock(NakadiKpiPublisher.class);
         settings = mock(SecuritySettings.class);
+        authorizationService = mock(AuthorizationService.class);
+        when(authorizationService.getSubject()).thenReturn(Optional.of(() ->  "adminClientId"));
         when(settings.getAuthMode()).thenReturn(OFF);
-        when(settings.getAdminClientId()).thenReturn("nakadi");
+        when(settings.getAdminClientId()).thenReturn("adminClientId");
 
         blacklistService = Mockito.mock(BlacklistService.class);
         when(blacklistService.isProductionBlocked(any(), any())).thenReturn(false);
 
-        final FeatureToggleService featureToggleService = Mockito.mock(FeatureToggleService.class);
-
         final EventPublishingController controller =
                 new EventPublishingController(publisher, eventTypeMetricRegistry, blacklistService, kpiPublisher,
                         "kpiEventTypeName");
 
         mockMvc = standaloneSetup(controller)
                 .setMessageConverters(new StringHttpMessageConverter(), TestUtils.JACKSON_2_HTTP_MESSAGE_CONVERTER)
-                .setCustomArgumentResolvers(new ClientResolver(settings, featureToggleService))
+                .setCustomArgumentResolvers(new ClientResolver(settings, authorizationService))
                 .setControllerAdvice(new NakadiProblemExceptionHandler(), new EventPublishingExceptionHandler())
                 .build();
     }

src/test/java/org/zalando/nakadi/controller/EventStreamControllerTest.java

@@ -42,7 +42,6 @@
 import org.zalando.nakadi.service.EventStreamConfig;
 import org.zalando.nakadi.service.EventStreamFactory;
 import org.zalando.nakadi.service.EventTypeChangeListener;
-import org.zalando.nakadi.service.FeatureToggleService;
 import org.zalando.nakadi.service.converter.CursorConverterImpl;
 import org.zalando.nakadi.service.timeline.TimelineService;
 import org.zalando.nakadi.utils.TestUtils;
@@ -59,6 +58,7 @@
 import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
@@ -107,7 +107,6 @@ public class EventStreamControllerTest {
     private EventStreamController controller;
     private MetricRegistry metricRegistry;
     private MetricRegistry streamMetrics;
-    private FeatureToggleService featureToggleService;
     private SecuritySettings settings;
     private BlacklistService blacklistService;
     private EventTypeCache eventTypeCache;
@@ -128,6 +127,7 @@ public void setup() throws UnknownHostException, InvalidCursorException {
         topicRepositoryMock = mock(TopicRepository.class);
         adminService = mock(AdminService.class);
         authorizationService = mock(AuthorizationService.class);
+        when(authorizationService.getSubject()).thenReturn(Optional.empty());
         when(topicRepositoryMock.topicExists(TEST_TOPIC)).thenReturn(true);
         eventStreamFactoryMock = mock(EventStreamFactory.class);
         eventTypeCache = mock(EventTypeCache.class);
@@ -149,7 +149,6 @@ public void setup() throws UnknownHostException, InvalidCursorException {
         blacklistService = Mockito.mock(BlacklistService.class);
         Mockito.when(blacklistService.isConsumptionBlocked(any(), any())).thenReturn(false);
 
-        featureToggleService = mock(FeatureToggleService.class);
         timelineService = mock(TimelineService.class);
         when(timelineService.getTopicRepository((Timeline) any())).thenReturn(topicRepositoryMock);
         when(timelineService.getTopicRepository((EventTypeBase) any())).thenReturn(topicRepositoryMock);
@@ -172,7 +171,7 @@ public void setup() throws UnknownHostException, InvalidCursorException {
 
         mockMvc = standaloneSetup(controller)
                 .setMessageConverters(new StringHttpMessageConverter(), TestUtils.JACKSON_2_HTTP_MESSAGE_CONVERTER)
-                .setCustomArgumentResolvers(new ClientResolver(settings, featureToggleService))
+                .setCustomArgumentResolvers(new ClientResolver(settings, authorizationService))
                 .build();
     }
 

src/test/java/org/zalando/nakadi/controller/EventTypeControllerTestCase.java

@@ -19,6 +19,7 @@
 import org.zalando.nakadi.enrichment.Enrichment;
 import org.zalando.nakadi.partitioning.PartitionResolver;
 import org.zalando.nakadi.plugin.api.ApplicationService;
+import org.zalando.nakadi.plugin.api.authz.AuthorizationService;
 import org.zalando.nakadi.repository.EventTypeRepository;
 import org.zalando.nakadi.repository.TopicRepository;
 import org.zalando.nakadi.repository.db.SubscriptionDbRepository;
@@ -41,6 +42,7 @@
 import uk.co.datumedge.hamcrest.json.SameJSONAs;
 
 import java.io.IOException;
+import java.util.Optional;
 import java.util.UUID;
 
 import static org.mockito.Matchers.any;
@@ -84,6 +86,7 @@ public class EventTypeControllerTestCase {
     protected final AuthorizationValidator authorizationValidator = mock(AuthorizationValidator.class);
     protected final AdminService adminService = mock(AdminService.class);
     protected final NakadiKpiPublisher nakadiKpiPublisher = mock(NakadiKpiPublisher.class);
+    protected final AuthorizationService authorizationService = mock(AuthorizationService.class);
     protected final NakadiAuditLogPublisher nakadiAuditLogPublisher = mock(NakadiAuditLogPublisher.class);
 
     protected MockMvc mockMvc;
@@ -102,6 +105,7 @@ public void init() throws Exception {
                 "t2.large", TestUtils.OBJECT_MAPPER, nakadiSettings);
         when(timelineService.getTopicRepository((Timeline) any())).thenReturn(topicRepository);
         when(timelineService.getTopicRepository((EventTypeBase) any())).thenReturn(topicRepository);
+        when(authorizationService.getSubject()).thenReturn(Optional.empty());
         when(transactionTemplate.execute(any())).thenAnswer(invocation -> {
             final TransactionCallback callback = (TransactionCallback) invocation.getArguments()[0];
             return callback.doInTransaction(null);
@@ -122,7 +126,7 @@ public void init() throws Exception {
 
         mockMvc = standaloneSetup(controller)
                 .setMessageConverters(new StringHttpMessageConverter(), TestUtils.JACKSON_2_HTTP_MESSAGE_CONVERTER)
-                .setCustomArgumentResolvers(new ClientResolver(settings, featureToggleService))
+                .setCustomArgumentResolvers(new ClientResolver(settings, authorizationService))
                 .setControllerAdvice(new NakadiProblemExceptionHandler(), new EventTypeExceptionHandler())
                 .build();
     }