From 59fcd5e77c5ee356e3fefc56c29d09f1c20f63c6 Mon Sep 17 00:00:00 2001 From: Kunal Jha Date: Tue, 29 Jan 2019 17:56:21 +0100 Subject: [PATCH 01/13] changed function to add the new plugin interface --- .../org/zalando/nakadi/domain/EventType.java | 1 + .../zalando/nakadi/domain/EventTypeBase.java | 17 +++++++++- .../zalando/nakadi/domain/ResourceImpl.java | 6 ++++ .../auth/DefaultAuthorizationService.java | 2 +- .../service/AuthorizationValidator.java | 31 ++++++++----------- .../nakadi/service/EventTypeService.java | 5 +-- 6 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/zalando/nakadi/domain/EventType.java b/src/main/java/org/zalando/nakadi/domain/EventType.java index 6f2f18830d..251819608a 100644 --- a/src/main/java/org/zalando/nakadi/domain/EventType.java +++ b/src/main/java/org/zalando/nakadi/domain/EventType.java @@ -48,6 +48,7 @@ public void setCreatedAt(final DateTime createdAt) { this.createdAt = createdAt; } + public Resource asResource() { return new ResourceImpl<>(getName(), ResourceImpl.EVENT_TYPE_RESOURCE, getAuthorization(), this); } diff --git a/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java b/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java index e552ee548e..8f329059e0 100644 --- a/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java +++ b/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java @@ -4,6 +4,8 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.google.common.collect.ImmutableList; import org.zalando.nakadi.partitioning.PartitionStrategy; +import org.zalando.nakadi.plugin.api.authz.EventTypeAuthz; +import org.zalando.nakadi.plugin.api.authz.Resource; import javax.annotation.Nullable; import javax.validation.Valid; @@ -15,7 +17,7 @@ import static java.util.Collections.unmodifiableList; -public class EventTypeBase { +public class EventTypeBase implements EventTypeAuthz { private static final List EMPTY_PARTITION_KEY_FIELDS = ImmutableList.of(); private static final List EMPTY_ORDERING_KEY_FIELDS = ImmutableList.of(); @@ -255,4 +257,17 @@ public void setAudience(@Nullable final Audience audience) { public void setAuthorization(final ResourceAuthorization authorization) { this.authorization = authorization; } + + @Override + public String getAuthCompatibilityMode() { + return this.compatibilityMode.toString(); + } + + @Override + public String getAuthCleanupPolicy() { + return this.cleanupPolicy.toString(); + } + public Resource asEventBaseResource() { + return new ResourceImpl<>(getName(), ResourceImpl.EVENT_TYPE_RESOURCE, getAuthorization(), this); + } } diff --git a/src/main/java/org/zalando/nakadi/domain/ResourceImpl.java b/src/main/java/org/zalando/nakadi/domain/ResourceImpl.java index 36474fe9c6..a59520215b 100644 --- a/src/main/java/org/zalando/nakadi/domain/ResourceImpl.java +++ b/src/main/java/org/zalando/nakadi/domain/ResourceImpl.java @@ -5,6 +5,7 @@ import org.zalando.nakadi.plugin.api.authz.Resource; import java.util.List; +import java.util.Map; import java.util.Optional; public class ResourceImpl implements Resource { @@ -43,6 +44,11 @@ public Optional> getAttributesForOperation( return authorization.getAttributesForOperation(operation); } + @Override + public Map> getAuthorization() { + return authorization.asMapValue(); + } + @Override public T get() { return resource; diff --git a/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java b/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java index c69d19ddc7..3255c46b20 100644 --- a/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java +++ b/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java @@ -18,7 +18,7 @@ public boolean isAuthorized(final Operation operation, final Resource resource) } @Override - public boolean isAuthorizationAttributeValid(final AuthorizationAttribute authorizationAttribute) { + public boolean isAuthorizationAttributeValid(final Resource resource) { return true; } diff --git a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java index 87fd9d7a94..05291011a6 100644 --- a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java +++ b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java @@ -44,12 +44,12 @@ public AuthorizationValidator( this.adminService = adminService; } - public void validateAuthorization(@Nullable final ValidatableAuthorization auth) throws UnableProcessException, + public void validateAuthorization(@Nullable final Resource resource) throws UnableProcessException, ServiceTemporarilyUnavailableException { - if (auth != null) { - final Map> authorization = auth.asMapValue(); - checkAuthAttributesAreValid(authorization); - checkAuthAttributesNoDuplicates(authorization); + + if (resource.getAuthorization() != null) { + checkAuthAttributesAreValid(resource); + checkAuthAttributesNoDuplicates(resource.getAuthorization()); } } @@ -86,18 +86,11 @@ private void checkAuthAttributesNoDuplicates(final Map> allAttributes) + private void checkAuthAttributesAreValid(Resource resource) throws UnableProcessException, ServiceTemporarilyUnavailableException { try { - final String errorMessage = allAttributes.values().stream() - .flatMap(Collection::stream) - .filter(attr -> !authorizationService.isAuthorizationAttributeValid(attr)) - .map(attr -> String.format("authorization attribute %s:%s is invalid", - attr.getDataType(), attr.getValue())) - .collect(Collectors.joining(", ")); - - if (!Strings.isNullOrEmpty(errorMessage)) { - throw new UnableProcessException(errorMessage); + if (authorizationService.isAuthorizationAttributeValid(resource)) { + throw new UnableProcessException("Authorization Attributes are not valid"); } } catch (final PluginException e) { throw new ServiceTemporarilyUnavailableException("Error calling authorization plugin", e); @@ -204,14 +197,16 @@ public void authorizeSubscriptionAdmin(final Subscription subscription) throws A authorizeResourceAdmin(subscription.asResource()); } - public void validateAuthorization(final ValidatableAuthorization oldValue, final ValidatableAuthorization newValue) + public void validateAuthorization(final Resource oldValue, final Resource newValue) throws UnableProcessException, ServiceTemporarilyUnavailableException { - if (oldValue != null && newValue == null) { + Map> oldAuth = oldValue.getAuthorization(); + Map> newAuth = newValue.getAuthorization(); + if (oldAuth != null && newAuth == null) { throw new UnableProcessException( "Changing authorization object to `null` is not possible due to existing one"); } - if (oldValue != null && oldValue.equals(newValue)) { + if (oldAuth != null && oldAuth.equals(newAuth)) { return; } diff --git a/src/main/java/org/zalando/nakadi/service/EventTypeService.java b/src/main/java/org/zalando/nakadi/service/EventTypeService.java index 30278450eb..acf668b12c 100644 --- a/src/main/java/org/zalando/nakadi/service/EventTypeService.java +++ b/src/main/java/org/zalando/nakadi/service/EventTypeService.java @@ -158,7 +158,7 @@ public void create(final EventTypeBase eventType, final Optional user) validateCompaction(eventType); enrichment.validate(eventType); partitionResolver.validate(eventType); - authorizationValidator.validateAuthorization(eventType.getAuthorization()); + authorizationValidator.validateAuthorization(eventType.asEventBaseResource()); eventTypeRepository.saveEventType(eventType); @@ -349,7 +349,8 @@ public void update(final String eventTypeName, eventTypeOptionsValidator.checkRetentionTime(eventTypeBase.getOptions()); authorizationValidator.authorizeEventTypeAdmin(original); } - authorizationValidator.validateAuthorization(original.getAuthorization(), eventTypeBase.getAuthorization()); + + authorizationValidator.validateAuthorization(original.asResource(), eventType.asResource()); validateName(eventTypeName, eventTypeBase); validateCompactionUpdate(original, eventTypeBase); validateSchema(eventTypeBase); From 384201ae7f87c8e7d56b1676311aa3fce83276c1 Mon Sep 17 00:00:00 2001 From: Kunal Jha Date: Tue, 29 Jan 2019 18:41:54 +0100 Subject: [PATCH 02/13] made changes to all event and subscription classes for new plugin interface --- .../zalando/nakadi/domain/EventTypeBase.java | 2 +- .../org/zalando/nakadi/domain/Subscription.java | 2 +- .../zalando/nakadi/domain/SubscriptionBase.java | 17 ++++++++++++++++- .../auth/DefaultAuthorizationService.java | 11 ++++++++++- .../nakadi/service/AuthorizationValidator.java | 10 ++++------ .../nakadi/service/EventTypeService.java | 5 ++--- .../SubscriptionValidationService.java | 4 ++-- .../service/AuthorizationValidatorTest.java | 13 ++++++++----- 8 files changed, 44 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java b/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java index 8f329059e0..d59cef4bdd 100644 --- a/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java +++ b/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java @@ -267,7 +267,7 @@ public String getAuthCompatibilityMode() { public String getAuthCleanupPolicy() { return this.cleanupPolicy.toString(); } - public Resource asEventBaseResource() { + public Resource asBaseResource() { return new ResourceImpl<>(getName(), ResourceImpl.EVENT_TYPE_RESOURCE, getAuthorization(), this); } } diff --git a/src/main/java/org/zalando/nakadi/domain/Subscription.java b/src/main/java/org/zalando/nakadi/domain/Subscription.java index 69bdf7443d..4c243bc10d 100644 --- a/src/main/java/org/zalando/nakadi/domain/Subscription.java +++ b/src/main/java/org/zalando/nakadi/domain/Subscription.java @@ -17,7 +17,7 @@ public Subscription() { public Subscription(final String id, final DateTime createdAt, final DateTime updatedAt, final SubscriptionBase subscriptionBase) { - super(subscriptionBase); + super(subscriptionBase, id); this.id = id; this.createdAt = createdAt; this.updatedAt = updatedAt; diff --git a/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java b/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java index 30498dd880..9d3c89f4ff 100644 --- a/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java +++ b/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java @@ -1,6 +1,7 @@ package org.zalando.nakadi.domain; import com.google.common.collect.ImmutableList; +import org.zalando.nakadi.plugin.api.authz.Resource; import org.zalando.nakadi.view.SubscriptionCursorWithoutToken; import javax.annotation.Nullable; @@ -23,6 +24,7 @@ public enum InitialPosition { CURSORS } + private String id; @NotNull @Size(min = 1, message = "must contain at least one character") private String owningApplication; @@ -48,7 +50,8 @@ public enum InitialPosition { public SubscriptionBase() { } - public SubscriptionBase(final SubscriptionBase subscriptionBase) { + public SubscriptionBase(final SubscriptionBase subscriptionBase, final String id) { + this.setId(id); this.setOwningApplication(subscriptionBase.getOwningApplication()); this.setEventTypes(subscriptionBase.getEventTypes()); this.setConsumerGroup(subscriptionBase.getConsumerGroup()); @@ -57,6 +60,14 @@ public SubscriptionBase(final SubscriptionBase subscriptionBase) { this.setAuthorization(subscriptionBase.getAuthorization()); } + public String getId() { + return id; + } + + public void setId(final String id) { + this.id = id; + } + public SubscriptionAuthorization getAuthorization() { return authorization; } @@ -126,4 +137,8 @@ public boolean equals(final Object o) { public int hashCode() { return Objects.hash(owningApplication, eventTypes, consumerGroup, readFrom, initialCursors); } + + public Resource asBaseResource() { + return new ResourceImpl<>(id, ResourceImpl.SUBSCRIPTION_RESOURCE, getAuthorization(), this); + } } diff --git a/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java b/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java index 3255c46b20..3a2d1693ff 100644 --- a/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java +++ b/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java @@ -18,10 +18,19 @@ public boolean isAuthorized(final Operation operation, final Resource resource) } @Override - public boolean isAuthorizationAttributeValid(final Resource resource) { + public boolean isAuthorizationAttributeValid(final AuthorizationAttribute attribute) { return true; } + @Override + public boolean isAuthorizationForResourceValid(final Resource resource) { + return true; + } + + public DefaultAuthorizationService() { + super(); + } + @Override @Nullable public Optional getSubject() { diff --git a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java index 05291011a6..d25d640c3f 100644 --- a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java +++ b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java @@ -5,7 +5,6 @@ import org.springframework.stereotype.Service; import org.zalando.nakadi.domain.EventType; import org.zalando.nakadi.domain.Subscription; -import org.zalando.nakadi.domain.ValidatableAuthorization; import org.zalando.nakadi.exceptions.runtime.AccessDeniedException; import org.zalando.nakadi.exceptions.runtime.InternalNakadiException; import org.zalando.nakadi.exceptions.runtime.ServiceTemporarilyUnavailableException; @@ -17,7 +16,6 @@ import org.zalando.nakadi.repository.EventTypeRepository; import javax.annotation.Nullable; -import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Optional; @@ -86,10 +84,10 @@ private void checkAuthAttributesNoDuplicates(final Map> oldAuth = oldValue.getAuthorization(); - Map> newAuth = newValue.getAuthorization(); + final Map> oldAuth = oldValue.getAuthorization(); + final Map> newAuth = newValue.getAuthorization(); if (oldAuth != null && newAuth == null) { throw new UnableProcessException( "Changing authorization object to `null` is not possible due to existing one"); diff --git a/src/main/java/org/zalando/nakadi/service/EventTypeService.java b/src/main/java/org/zalando/nakadi/service/EventTypeService.java index acf668b12c..b585b33b31 100644 --- a/src/main/java/org/zalando/nakadi/service/EventTypeService.java +++ b/src/main/java/org/zalando/nakadi/service/EventTypeService.java @@ -158,7 +158,7 @@ public void create(final EventTypeBase eventType, final Optional user) validateCompaction(eventType); enrichment.validate(eventType); partitionResolver.validate(eventType); - authorizationValidator.validateAuthorization(eventType.asEventBaseResource()); + authorizationValidator.validateAuthorization(eventType.asBaseResource()); eventTypeRepository.saveEventType(eventType); @@ -349,8 +349,7 @@ public void update(final String eventTypeName, eventTypeOptionsValidator.checkRetentionTime(eventTypeBase.getOptions()); authorizationValidator.authorizeEventTypeAdmin(original); } - - authorizationValidator.validateAuthorization(original.asResource(), eventType.asResource()); + authorizationValidator.validateAuthorization(original.asResource(), eventTypeBase.asBaseResource()); validateName(eventTypeName, eventTypeBase); validateCompactionUpdate(original, eventTypeBase); validateSchema(eventTypeBase); diff --git a/src/main/java/org/zalando/nakadi/service/subscription/SubscriptionValidationService.java b/src/main/java/org/zalando/nakadi/service/subscription/SubscriptionValidationService.java index 8adb08444d..7a04478dc8 100644 --- a/src/main/java/org/zalando/nakadi/service/subscription/SubscriptionValidationService.java +++ b/src/main/java/org/zalando/nakadi/service/subscription/SubscriptionValidationService.java @@ -80,7 +80,7 @@ public void validateSubscription(final SubscriptionBase subscription) validateInitialCursors(subscription, allPartitions); } // Verify that subscription authorization object is valid - authorizationValidator.validateAuthorization(subscription.getAuthorization()); + authorizationValidator.validateAuthorization(subscription.asBaseResource()); } public void validateSubscriptionChange(final Subscription old, final SubscriptionBase newValue) @@ -100,7 +100,7 @@ public void validateSubscriptionChange(final Subscription old, final Subscriptio if (!Objects.equals(newValue.getInitialCursors(), old.getInitialCursors())) { throw new SubscriptionUpdateConflictException("Not allowed to change initial cursors"); } - authorizationValidator.validateAuthorization(old.getAuthorization(), newValue.getAuthorization()); + authorizationValidator.validateAuthorization(old.asResource(), newValue.asBaseResource()); } public void validatePartitionsToStream(final Subscription subscription, final List partitions) { diff --git a/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java b/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java index c2f0d5f077..b6e2fe4786 100644 --- a/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java +++ b/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java @@ -4,12 +4,14 @@ import org.junit.Test; import org.zalando.nakadi.domain.ResourceAuthorization; import org.zalando.nakadi.domain.ResourceAuthorizationAttribute; +import org.zalando.nakadi.domain.ResourceImpl; import org.zalando.nakadi.exceptions.runtime.AccessDeniedException; import org.zalando.nakadi.exceptions.runtime.ServiceTemporarilyUnavailableException; import org.zalando.nakadi.exceptions.runtime.UnableProcessException; import org.zalando.nakadi.plugin.api.PluginException; 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.repository.EventTypeRepository; import org.zalando.nakadi.utils.EventTypeTestBuilder; @@ -49,9 +51,9 @@ public void whenInvalidAuthAttributesThenInvalidEventTypeException() { when(authorizationService.isAuthorizationAttributeValid(attr2)).thenReturn(true); when(authorizationService.isAuthorizationAttributeValid(attr3)).thenReturn(true); when(authorizationService.isAuthorizationAttributeValid(attr4)).thenReturn(false); - + final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); try { - validator.validateAuthorization(auth); + validator.validateAuthorization(resource); fail("Exception expected to be thrown"); } catch (final UnableProcessException e) { assertThat(e.getMessage(), equalTo("authorization attribute type1:value1 is invalid, " + @@ -68,9 +70,10 @@ public void whenDuplicatesThenInvalidEventTypeException() { ImmutableList.of(attr3, attr4)); when(authorizationService.isAuthorizationAttributeValid(any())).thenReturn(true); + final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); try { - validator.validateAuthorization(auth); + validator.validateAuthorization(resource); fail("Exception expected to be thrown"); } catch (final UnableProcessException e) { assertThat(e.getMessage(), equalTo( @@ -88,8 +91,8 @@ public void whenPluginExceptionInIsAuthorizationAttributeValidThenServiceUnavail ImmutableList.of(attr3)); when(authorizationService.isAuthorizationAttributeValid(any())).thenThrow(new PluginException("blah")); - - validator.validateAuthorization(auth); + final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); + validator.validateAuthorization(resource); } @Test From 4ea6e884f475138f3c77cdd14d55d290c853eaf9 Mon Sep 17 00:00:00 2001 From: Kunal Jha Date: Wed, 30 Jan 2019 13:06:57 +0100 Subject: [PATCH 03/13] fixed tests --- .../java/org/zalando/nakadi/domain/EventTypeBase.java | 3 +++ .../nakadi/service/AuthorizationValidatorTest.java | 10 +++------- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java b/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java index d59cef4bdd..03d2791bb0 100644 --- a/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java +++ b/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java @@ -258,15 +258,18 @@ public void setAuthorization(final ResourceAuthorization authorization) { this.authorization = authorization; } + @JsonIgnore @Override public String getAuthCompatibilityMode() { return this.compatibilityMode.toString(); } + @JsonIgnore @Override public String getAuthCleanupPolicy() { return this.cleanupPolicy.toString(); } + public Resource asBaseResource() { return new ResourceImpl<>(getName(), ResourceImpl.EVENT_TYPE_RESOURCE, getAuthorization(), this); } diff --git a/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java b/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java index b6e2fe4786..d330b6f232 100644 --- a/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java +++ b/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java @@ -47,17 +47,13 @@ public void whenInvalidAuthAttributesThenInvalidEventTypeException() { final ResourceAuthorization auth = new ResourceAuthorization( ImmutableList.of(attr1), ImmutableList.of(attr2), ImmutableList.of(attr3, attr4)); - when(authorizationService.isAuthorizationAttributeValid(attr1)).thenReturn(false); - when(authorizationService.isAuthorizationAttributeValid(attr2)).thenReturn(true); - when(authorizationService.isAuthorizationAttributeValid(attr3)).thenReturn(true); - when(authorizationService.isAuthorizationAttributeValid(attr4)).thenReturn(false); final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); + when(authorizationService.isAuthorizationForResourceValid(resource)).thenReturn(true); try { validator.validateAuthorization(resource); fail("Exception expected to be thrown"); } catch (final UnableProcessException e) { - assertThat(e.getMessage(), equalTo("authorization attribute type1:value1 is invalid, " + - "authorization attribute type4:value4 is invalid")); + assertThat(e.getMessage(), equalTo("Authorization Attributes are not valid")); } } @@ -90,7 +86,7 @@ public void whenPluginExceptionInIsAuthorizationAttributeValidThenServiceUnavail ImmutableList.of(attr2), ImmutableList.of(attr3)); - when(authorizationService.isAuthorizationAttributeValid(any())).thenThrow(new PluginException("blah")); + when(authorizationService.isAuthorizationForResourceValid(any())).thenThrow(new PluginException("blah")); final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); validator.validateAuthorization(resource); } From b71732d9935e083f2f0622e2ee87a50a26a61edc Mon Sep 17 00:00:00 2001 From: Kunal Jha Date: Fri, 1 Feb 2019 13:46:55 +0100 Subject: [PATCH 04/13] addressed reviews --- src/main/java/org/zalando/nakadi/domain/EventType.java | 1 - src/main/java/org/zalando/nakadi/domain/EventTypeBase.java | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/zalando/nakadi/domain/EventType.java b/src/main/java/org/zalando/nakadi/domain/EventType.java index 251819608a..c3ebc15ffc 100644 --- a/src/main/java/org/zalando/nakadi/domain/EventType.java +++ b/src/main/java/org/zalando/nakadi/domain/EventType.java @@ -15,7 +15,6 @@ public EventType(final EventTypeBase eventType, final String version, final Date super(eventType); this.updatedAt = updatedAt; this.createdAt = createdAt; - this.setSchema(new EventTypeSchema(eventType.getSchema(), version, updatedAt)); } diff --git a/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java b/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java index 03d2791bb0..2d12ce6169 100644 --- a/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java +++ b/src/main/java/org/zalando/nakadi/domain/EventTypeBase.java @@ -270,6 +270,7 @@ public String getAuthCleanupPolicy() { return this.cleanupPolicy.toString(); } + @JsonIgnore public Resource asBaseResource() { return new ResourceImpl<>(getName(), ResourceImpl.EVENT_TYPE_RESOURCE, getAuthorization(), this); } From 0f0dd57a8e6f0ce364b127c18a76a25c428a028d Mon Sep 17 00:00:00 2001 From: Kunal Jha Date: Mon, 4 Feb 2019 13:51:43 +0100 Subject: [PATCH 05/13] added error messages back to nakadi, removed id from Subscription base --- .../nakadi/domain/SubscriptionBase.java | 12 +--- .../runtime/AccessDeniedException.java | 5 ++ .../auth/DefaultAuthorizationService.java | 2 +- .../service/AuthorizationValidator.java | 55 +++++++++++-------- .../service/AuthorizationValidatorTest.java | 14 +++-- 5 files changed, 50 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java b/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java index 9d3c89f4ff..6e473fb310 100644 --- a/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java +++ b/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java @@ -24,7 +24,6 @@ public enum InitialPosition { CURSORS } - private String id; @NotNull @Size(min = 1, message = "must contain at least one character") private String owningApplication; @@ -51,7 +50,6 @@ public SubscriptionBase() { } public SubscriptionBase(final SubscriptionBase subscriptionBase, final String id) { - this.setId(id); this.setOwningApplication(subscriptionBase.getOwningApplication()); this.setEventTypes(subscriptionBase.getEventTypes()); this.setConsumerGroup(subscriptionBase.getConsumerGroup()); @@ -60,14 +58,6 @@ public SubscriptionBase(final SubscriptionBase subscriptionBase, final String id this.setAuthorization(subscriptionBase.getAuthorization()); } - public String getId() { - return id; - } - - public void setId(final String id) { - this.id = id; - } - public SubscriptionAuthorization getAuthorization() { return authorization; } @@ -139,6 +129,6 @@ public int hashCode() { } public Resource asBaseResource() { - return new ResourceImpl<>(id, ResourceImpl.SUBSCRIPTION_RESOURCE, getAuthorization(), this); + return new ResourceImpl<>(null, ResourceImpl.SUBSCRIPTION_RESOURCE, getAuthorization(), this); } } diff --git a/src/main/java/org/zalando/nakadi/exceptions/runtime/AccessDeniedException.java b/src/main/java/org/zalando/nakadi/exceptions/runtime/AccessDeniedException.java index c494b381f0..5f1baf7869 100644 --- a/src/main/java/org/zalando/nakadi/exceptions/runtime/AccessDeniedException.java +++ b/src/main/java/org/zalando/nakadi/exceptions/runtime/AccessDeniedException.java @@ -12,6 +12,11 @@ public AccessDeniedException(final AuthorizationService.Operation operation, fin this.operation = operation; } + public AccessDeniedException(final Resource resource) { + this.resource = resource; + this.operation = null; + } + public Resource getResource() { return resource; } diff --git a/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java b/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java index 3a2d1693ff..3fe3db1fd9 100644 --- a/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java +++ b/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java @@ -23,7 +23,7 @@ public boolean isAuthorizationAttributeValid(final AuthorizationAttribute attrib } @Override - public boolean isAuthorizationForResourceValid(final Resource resource) { + public boolean areAllAuthorizationsForResourceValid(final Resource resource) { return true; } diff --git a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java index d25d640c3f..4a07eaf462 100644 --- a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java +++ b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java @@ -16,6 +16,7 @@ import org.zalando.nakadi.repository.EventTypeRepository; import javax.annotation.Nullable; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Optional; @@ -44,9 +45,8 @@ public AuthorizationValidator( public void validateAuthorization(@Nullable final Resource resource) throws UnableProcessException, ServiceTemporarilyUnavailableException { - if (resource.getAuthorization() != null) { - checkAuthAttributesAreValid(resource); + checkAuthorisationForResourceAreValid(resource); checkAuthAttributesNoDuplicates(resource.getAuthorization()); } } @@ -84,11 +84,31 @@ private void checkAuthAttributesNoDuplicates(final Map> allAttributes) + throws UnableProcessException, ServiceTemporarilyUnavailableException { + try { + final String errorMessage = allAttributes.values().stream() + .flatMap(Collection::stream) + .filter(attr -> !authorizationService.isAuthorizationAttributeValid(attr)) + .map(attr -> String.format("authorization attribute %s:%s is invalid", + attr.getDataType(), attr.getValue())) + .collect(Collectors.joining(", ")); + + if (!Strings.isNullOrEmpty(errorMessage)) { + throw new UnableProcessException(errorMessage); } } catch (final PluginException e) { throw new ServiceTemporarilyUnavailableException("Error calling authorization plugin", e); @@ -139,28 +159,13 @@ public void authorizeStreamRead(final EventType eventType) throws AccessDeniedEx } final Resource resource = eventType.asResource(); - try { - if (!authorizationService.isAuthorized(AuthorizationService.Operation.READ, resource) - && !adminService.hasAllDataAccess(AuthorizationService.Operation.READ)) { - throw new AccessDeniedException(AuthorizationService.Operation.READ, resource); - } - } catch (final PluginException e) { - throw new ServiceTemporarilyUnavailableException("Error calling authorization plugin", e); - } + checkResourceAuthorization(resource); } public void authorizeSubscriptionRead(final Subscription subscription) throws AccessDeniedException { if (null != subscription.getAuthorization()) { final Resource resource = subscription.asResource(); - try { - // In this case operation for read will invoke - if (!authorizationService.isAuthorized(AuthorizationService.Operation.READ, resource) - && !adminService.hasAllDataAccess(AuthorizationService.Operation.READ)) { - throw new AccessDeniedException(AuthorizationService.Operation.READ, resource); - } - } catch (final PluginException e) { - throw new ServiceTemporarilyUnavailableException("Error calling authorization plugin", e); - } + checkResourceAuthorization(resource); } subscription.getEventTypes().forEach( (eventTypeName) -> { @@ -178,6 +183,12 @@ public void authorizeSubscriptionCommit(final Subscription subscription) throws return; } final Resource resource = subscription.asResource(); + checkResourceAuthorization(resource); + + } + + private void checkResourceAuthorization(final Resource resource) + throws ServiceTemporarilyUnavailableException, AccessDeniedException { try { if (!authorizationService.isAuthorized(AuthorizationService.Operation.READ, resource) && !adminService.hasAllDataAccess(AuthorizationService.Operation.READ)) { diff --git a/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java b/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java index d330b6f232..6ba5fd8088 100644 --- a/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java +++ b/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java @@ -48,12 +48,16 @@ public void whenInvalidAuthAttributesThenInvalidEventTypeException() { ImmutableList.of(attr1), ImmutableList.of(attr2), ImmutableList.of(attr3, attr4)); final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); - when(authorizationService.isAuthorizationForResourceValid(resource)).thenReturn(true); + when(authorizationService.isAuthorizationAttributeValid(attr1)).thenReturn(false); + when(authorizationService.isAuthorizationAttributeValid(attr2)).thenReturn(true); + when(authorizationService.isAuthorizationAttributeValid(attr3)).thenReturn(true); + when(authorizationService.isAuthorizationAttributeValid(attr4)).thenReturn(false); try { validator.validateAuthorization(resource); fail("Exception expected to be thrown"); } catch (final UnableProcessException e) { - assertThat(e.getMessage(), equalTo("Authorization Attributes are not valid")); + assertThat(e.getMessage(), equalTo("authorization attribute type1:value1 is invalid, " + + "authorization attribute type4:value4 is invalid")); } } @@ -66,10 +70,11 @@ public void whenDuplicatesThenInvalidEventTypeException() { ImmutableList.of(attr3, attr4)); when(authorizationService.isAuthorizationAttributeValid(any())).thenReturn(true); + when(authorizationService.areAllAuthorizationsForResourceValid(any())).thenReturn(true); final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); try { - validator.validateAuthorization(resource); + validator.validateAuthorization(resource); fail("Exception expected to be thrown"); } catch (final UnableProcessException e) { assertThat(e.getMessage(), equalTo( @@ -86,7 +91,8 @@ public void whenPluginExceptionInIsAuthorizationAttributeValidThenServiceUnavail ImmutableList.of(attr2), ImmutableList.of(attr3)); - when(authorizationService.isAuthorizationForResourceValid(any())).thenThrow(new PluginException("blah")); + when(authorizationService.areAllAuthorizationsForResourceValid(any())).thenThrow(new PluginException("blah")); + when(authorizationService.isAuthorizationAttributeValid(any())).thenThrow(new PluginException("blah")); final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); validator.validateAuthorization(resource); } From aee3eb17f5b11f65b60d66b68514c69ab033ad12 Mon Sep 17 00:00:00 2001 From: Kunal Jha Date: Mon, 4 Feb 2019 15:37:47 +0100 Subject: [PATCH 06/13] removed the id from subscription Base --- src/main/java/org/zalando/nakadi/domain/Subscription.java | 2 +- src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/zalando/nakadi/domain/Subscription.java b/src/main/java/org/zalando/nakadi/domain/Subscription.java index 4c243bc10d..69bdf7443d 100644 --- a/src/main/java/org/zalando/nakadi/domain/Subscription.java +++ b/src/main/java/org/zalando/nakadi/domain/Subscription.java @@ -17,7 +17,7 @@ public Subscription() { public Subscription(final String id, final DateTime createdAt, final DateTime updatedAt, final SubscriptionBase subscriptionBase) { - super(subscriptionBase, id); + super(subscriptionBase); this.id = id; this.createdAt = createdAt; this.updatedAt = updatedAt; diff --git a/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java b/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java index 6e473fb310..0731dc6be2 100644 --- a/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java +++ b/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java @@ -49,7 +49,7 @@ public enum InitialPosition { public SubscriptionBase() { } - public SubscriptionBase(final SubscriptionBase subscriptionBase, final String id) { + public SubscriptionBase(final SubscriptionBase subscriptionBase) { this.setOwningApplication(subscriptionBase.getOwningApplication()); this.setEventTypes(subscriptionBase.getEventTypes()); this.setConsumerGroup(subscriptionBase.getConsumerGroup()); From 99758f38aa8af4354c367784317883af3e5c6178 Mon Sep 17 00:00:00 2001 From: Kunal Jha Date: Tue, 5 Feb 2019 17:15:38 +0100 Subject: [PATCH 07/13] made changes to handle exception from authorization plugin --- .../auth/DefaultAuthorizationService.java | 3 +- .../service/AuthorizationValidator.java | 13 ++++--- .../service/AuthorizationValidatorTest.java | 34 +++++++++++++++++-- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java b/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java index 3fe3db1fd9..8c8fd37683 100644 --- a/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java +++ b/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java @@ -23,8 +23,7 @@ public boolean isAuthorizationAttributeValid(final AuthorizationAttribute attrib } @Override - public boolean areAllAuthorizationsForResourceValid(final Resource resource) { - return true; + public void areAllAuthorizationsForResourceValid(final Resource resource) { } public DefaultAuthorizationService() { diff --git a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java index 4a07eaf462..0cfaddd438 100644 --- a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java +++ b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java @@ -6,6 +6,7 @@ import org.zalando.nakadi.domain.EventType; import org.zalando.nakadi.domain.Subscription; import org.zalando.nakadi.exceptions.runtime.AccessDeniedException; +import org.zalando.nakadi.exceptions.runtime.ForbiddenOperationException; import org.zalando.nakadi.exceptions.runtime.InternalNakadiException; import org.zalando.nakadi.exceptions.runtime.ServiceTemporarilyUnavailableException; import org.zalando.nakadi.exceptions.runtime.UnableProcessException; @@ -13,6 +14,8 @@ 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.exceptions.AuthorizationInvalidException; +import org.zalando.nakadi.plugin.api.exceptions.OperationOnResourceNotPermitedException; import org.zalando.nakadi.repository.EventTypeRepository; import javax.annotation.Nullable; @@ -85,13 +88,15 @@ private void checkAuthAttributesNoDuplicates(final Map Date: Tue, 5 Feb 2019 17:44:18 +0100 Subject: [PATCH 08/13] fixed reviews regardinng subscription base --- .../java/org/zalando/nakadi/domain/SubscriptionBase.java | 4 ++-- .../service/subscription/SubscriptionValidationService.java | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java b/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java index 0731dc6be2..6794505e23 100644 --- a/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java +++ b/src/main/java/org/zalando/nakadi/domain/SubscriptionBase.java @@ -128,7 +128,7 @@ public int hashCode() { return Objects.hash(owningApplication, eventTypes, consumerGroup, readFrom, initialCursors); } - public Resource asBaseResource() { - return new ResourceImpl<>(null, ResourceImpl.SUBSCRIPTION_RESOURCE, getAuthorization(), this); + public Resource asBaseResource(final String id) { + return new ResourceImpl<>(id, ResourceImpl.SUBSCRIPTION_RESOURCE, getAuthorization(), this); } } diff --git a/src/main/java/org/zalando/nakadi/service/subscription/SubscriptionValidationService.java b/src/main/java/org/zalando/nakadi/service/subscription/SubscriptionValidationService.java index 7a04478dc8..718660619d 100644 --- a/src/main/java/org/zalando/nakadi/service/subscription/SubscriptionValidationService.java +++ b/src/main/java/org/zalando/nakadi/service/subscription/SubscriptionValidationService.java @@ -80,7 +80,7 @@ public void validateSubscription(final SubscriptionBase subscription) validateInitialCursors(subscription, allPartitions); } // Verify that subscription authorization object is valid - authorizationValidator.validateAuthorization(subscription.asBaseResource()); + authorizationValidator.validateAuthorization(subscription.asBaseResource("new-subscription")); } public void validateSubscriptionChange(final Subscription old, final SubscriptionBase newValue) @@ -100,7 +100,7 @@ public void validateSubscriptionChange(final Subscription old, final Subscriptio if (!Objects.equals(newValue.getInitialCursors(), old.getInitialCursors())) { throw new SubscriptionUpdateConflictException("Not allowed to change initial cursors"); } - authorizationValidator.validateAuthorization(old.asResource(), newValue.asBaseResource()); + authorizationValidator.validateAuthorization(old.asResource(), newValue.asBaseResource(old.getId())); } public void validatePartitionsToStream(final Subscription subscription, final List partitions) { @@ -193,7 +193,7 @@ private void checkEventTypesExist(final Map> eventTy .collect(Collectors.toList()); if (!missingEventTypes.isEmpty()) { throw new NoSuchEventTypeException(String.format("Failed to create subscription, event type(s) not " + - "found: '%s'", StringUtils.join(missingEventTypes, "', '"))); + "found: '%s'", StringUtils.join(missingEventTypes, "', '"))); } } } From ee84db95fec694d2b122f80ca0a2fb60f959c323 Mon Sep 17 00:00:00 2001 From: Kunal Jha Date: Thu, 7 Feb 2019 16:23:38 +0100 Subject: [PATCH 09/13] added support for permission as resource and removed isAuthorizationAttributeValid --- .../nakadi/controller/StoragesController.java | 2 +- .../zalando/nakadi/domain/ResourceImpl.java | 1 + .../auth/DefaultAuthorizationService.java | 16 +++++------ .../zalando/nakadi/service/AdminService.java | 19 ++++++------- .../service/AuthorizationValidator.java | 27 +++---------------- .../nakadi/service/AdminServiceTest.java | 2 +- .../service/AuthorizationValidatorTest.java | 26 +++++++----------- 7 files changed, 32 insertions(+), 61 deletions(-) diff --git a/src/main/java/org/zalando/nakadi/controller/StoragesController.java b/src/main/java/org/zalando/nakadi/controller/StoragesController.java index d639b0b3a9..1981a0884b 100644 --- a/src/main/java/org/zalando/nakadi/controller/StoragesController.java +++ b/src/main/java/org/zalando/nakadi/controller/StoragesController.java @@ -17,8 +17,8 @@ import org.zalando.nakadi.exceptions.runtime.NoSuchStorageException; import org.zalando.nakadi.exceptions.runtime.StorageIsUsedException; import org.zalando.nakadi.exceptions.runtime.UnknownStorageTypeException; -import org.zalando.nakadi.plugin.api.PluginException; import org.zalando.nakadi.plugin.api.authz.AuthorizationService; +import org.zalando.nakadi.plugin.api.exceptions.PluginException; import org.zalando.nakadi.service.AdminService; import org.zalando.nakadi.service.StorageService; diff --git a/src/main/java/org/zalando/nakadi/domain/ResourceImpl.java b/src/main/java/org/zalando/nakadi/domain/ResourceImpl.java index a59520215b..4d28e273a2 100644 --- a/src/main/java/org/zalando/nakadi/domain/ResourceImpl.java +++ b/src/main/java/org/zalando/nakadi/domain/ResourceImpl.java @@ -14,6 +14,7 @@ public class ResourceImpl implements Resource { public static final String ADMIN_RESOURCE = "nakadi"; public static final String EVENT_TYPE_RESOURCE = "event-type"; public static final String SUBSCRIPTION_RESOURCE = "subscription"; + public static final String PERMISSION_RESOURCE = "permission"; private final T resource; private final String name; diff --git a/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java b/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java index 8c8fd37683..904e6d87b1 100644 --- a/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java +++ b/src/main/java/org/zalando/nakadi/plugin/auth/DefaultAuthorizationService.java @@ -1,10 +1,12 @@ package org.zalando.nakadi.plugin.auth; -import org.zalando.nakadi.plugin.api.PluginException; -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 org.zalando.nakadi.plugin.api.exceptions.AuthorizationInvalidException; +import org.zalando.nakadi.plugin.api.exceptions.OperationOnResourceNotPermittedException; +import org.zalando.nakadi.plugin.api.exceptions.PluginException; import javax.annotation.Nullable; import java.util.List; @@ -13,17 +15,13 @@ public class DefaultAuthorizationService implements AuthorizationService { @Override - public boolean isAuthorized(final Operation operation, final Resource resource) { - return true; - } - - @Override - public boolean isAuthorizationAttributeValid(final AuthorizationAttribute attribute) { + public boolean isAuthorized(final Operation operation, final Resource resource) throws PluginException { return true; } @Override - public void areAllAuthorizationsForResourceValid(final Resource resource) { + public void isAuthorizationForResourceValid(final Resource resource) throws PluginException, + AuthorizationInvalidException, OperationOnResourceNotPermittedException { } public DefaultAuthorizationService() { diff --git a/src/main/java/org/zalando/nakadi/service/AdminService.java b/src/main/java/org/zalando/nakadi/service/AdminService.java index ef160f1aac..f1437cd2ff 100644 --- a/src/main/java/org/zalando/nakadi/service/AdminService.java +++ b/src/main/java/org/zalando/nakadi/service/AdminService.java @@ -13,9 +13,10 @@ import org.zalando.nakadi.domain.ResourceImpl; import org.zalando.nakadi.exceptions.runtime.DbWriteOperationsBlockedException; import org.zalando.nakadi.exceptions.runtime.UnableProcessException; -import org.zalando.nakadi.plugin.api.PluginException; import org.zalando.nakadi.plugin.api.authz.AuthorizationService; import org.zalando.nakadi.plugin.api.authz.Resource; +import org.zalando.nakadi.plugin.api.exceptions.AuthorizationInvalidException; +import org.zalando.nakadi.plugin.api.exceptions.PluginException; import org.zalando.nakadi.repository.db.AuthorizationDbRepository; import java.util.List; @@ -27,6 +28,7 @@ import static org.zalando.nakadi.domain.ResourceImpl.ADMIN_RESOURCE; import static org.zalando.nakadi.domain.ResourceImpl.ALL_DATA_ACCESS_RESOURCE; +import static org.zalando.nakadi.domain.ResourceImpl.PERMISSION_RESOURCE; @Service public class AdminService { @@ -116,16 +118,11 @@ private List removeDefaultAdmin(final List permissions) } private void validateAllAdmins(final List admins) throws UnableProcessException, PluginException { - final List invalid = admins.stream().filter(permission -> - !authorizationService.isAuthorizationAttributeValid(permission.getAuthorizationAttribute())) - .collect(Collectors.toList()); - if (!invalid.isEmpty()) { - final String message = invalid.stream() - .map(permission -> String.format("authorization attribute %s:%s is invalid", - permission.getAuthorizationAttribute().getDataType(), - permission.getAuthorizationAttribute().getValue())) - .collect(Collectors.joining(", ")); - throw new UnableProcessException(message); + try { + authorizationService.isAuthorizationForResourceValid(new ResourceImpl<>(PERMISSION_RESOURCE, + PERMISSION_RESOURCE, ResourceAuthorization.fromPermissionsList(admins), null)); + } catch (AuthorizationInvalidException e) { + throw new UnableProcessException(e.getMessage()); } } } diff --git a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java index 0cfaddd438..8192d1213d 100644 --- a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java +++ b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java @@ -10,16 +10,15 @@ import org.zalando.nakadi.exceptions.runtime.InternalNakadiException; import org.zalando.nakadi.exceptions.runtime.ServiceTemporarilyUnavailableException; import org.zalando.nakadi.exceptions.runtime.UnableProcessException; -import org.zalando.nakadi.plugin.api.PluginException; 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.exceptions.AuthorizationInvalidException; -import org.zalando.nakadi.plugin.api.exceptions.OperationOnResourceNotPermitedException; +import org.zalando.nakadi.plugin.api.exceptions.OperationOnResourceNotPermittedException; +import org.zalando.nakadi.plugin.api.exceptions.PluginException; import org.zalando.nakadi.repository.EventTypeRepository; import javax.annotation.Nullable; -import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Optional; @@ -90,10 +89,9 @@ private void checkAuthAttributesNoDuplicates(final Map> allAttributes) - throws UnableProcessException, ServiceTemporarilyUnavailableException { - try { - final String errorMessage = allAttributes.values().stream() - .flatMap(Collection::stream) - .filter(attr -> !authorizationService.isAuthorizationAttributeValid(attr)) - .map(attr -> String.format("authorization attribute %s:%s is invalid", - attr.getDataType(), attr.getValue())) - .collect(Collectors.joining(", ")); - - if (!Strings.isNullOrEmpty(errorMessage)) { - throw new UnableProcessException(errorMessage); - } - } catch (final PluginException e) { - throw new ServiceTemporarilyUnavailableException("Error calling authorization plugin", e); - } - } public void authorizeEventTypeWrite(final EventType eventType) throws AccessDeniedException, ServiceTemporarilyUnavailableException { diff --git a/src/test/java/org/zalando/nakadi/service/AdminServiceTest.java b/src/test/java/org/zalando/nakadi/service/AdminServiceTest.java index 08383af94c..feca5787ce 100644 --- a/src/test/java/org/zalando/nakadi/service/AdminServiceTest.java +++ b/src/test/java/org/zalando/nakadi/service/AdminServiceTest.java @@ -73,7 +73,7 @@ public AdminServiceTest() { for (final AuthorizationService.Operation operation : AuthorizationService.Operation.values()) { defaultAdminPermissions.add(new Permission("nakadi", operation, defaultAdmin)); } - when(authorizationService.isAuthorizationAttributeValid(any())).thenReturn(true); + doNothing().when(authorizationService).isAuthorizationForResourceValid(any()); when(featureToggleService.isFeatureEnabled(FeatureToggleService.Feature.DISABLE_DB_WRITE_OPERATIONS)) .thenReturn(false); } diff --git a/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java b/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java index d7cf888803..a62ec944ac 100644 --- a/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java +++ b/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java @@ -9,12 +9,12 @@ import org.zalando.nakadi.exceptions.runtime.ForbiddenOperationException; import org.zalando.nakadi.exceptions.runtime.ServiceTemporarilyUnavailableException; import org.zalando.nakadi.exceptions.runtime.UnableProcessException; -import org.zalando.nakadi.plugin.api.PluginException; 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.exceptions.AuthorizationInvalidException; -import org.zalando.nakadi.plugin.api.exceptions.OperationOnResourceNotPermitedException; +import org.zalando.nakadi.plugin.api.exceptions.OperationOnResourceNotPermittedException; +import org.zalando.nakadi.plugin.api.exceptions.PluginException; import org.zalando.nakadi.repository.EventTypeRepository; import org.zalando.nakadi.utils.EventTypeTestBuilder; @@ -52,16 +52,13 @@ public void whenInvalidAuthAttributesThenInvalidEventTypeException() { ImmutableList.of(attr1), ImmutableList.of(attr2), ImmutableList.of(attr3, attr4)); final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); - when(authorizationService.isAuthorizationAttributeValid(attr1)).thenReturn(false); - when(authorizationService.isAuthorizationAttributeValid(attr2)).thenReturn(true); - when(authorizationService.isAuthorizationAttributeValid(attr3)).thenReturn(true); - when(authorizationService.isAuthorizationAttributeValid(attr4)).thenReturn(false); + doThrow(new AuthorizationInvalidException("some attributes are not ok")) + .when(authorizationService).isAuthorizationForResourceValid(any()); try { validator.validateAuthorization(resource); fail("Exception expected to be thrown"); } catch (final UnableProcessException e) { - assertThat(e.getMessage(), equalTo("authorization attribute type1:value1 is invalid, " + - "authorization attribute type4:value4 is invalid")); + assertThat(e.getMessage(), equalTo("some attributes are not ok")); } } @@ -73,7 +70,6 @@ public void whenDuplicatesThenInvalidEventTypeException() { ImmutableList.of(attr3, attr2, attr2), ImmutableList.of(attr3, attr4)); - when(authorizationService.isAuthorizationAttributeValid(any())).thenReturn(true); final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); try { @@ -93,9 +89,8 @@ public void whenOperationOnResourceNotPermittedExceptionThenForbiddenOperationEx ImmutableList.of(attr1), ImmutableList.of(attr2), ImmutableList.of(attr3)); - when(authorizationService.isAuthorizationAttributeValid(any())).thenReturn(true); - doThrow(new OperationOnResourceNotPermitedException("blah")) - .when(authorizationService).areAllAuthorizationsForResourceValid(any()); + doThrow(new OperationOnResourceNotPermittedException("blah")) + .when(authorizationService).isAuthorizationForResourceValid(any()); final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); validator.validateAuthorization(resource); } @@ -107,9 +102,8 @@ public void whenAuthorizationInvalidExceptionThenUnableProcessException() { ImmutableList.of(attr1), ImmutableList.of(attr2), ImmutableList.of(attr3)); - when(authorizationService.isAuthorizationAttributeValid(any())).thenReturn(true); doThrow(new AuthorizationInvalidException("blah")) - .when(authorizationService).areAllAuthorizationsForResourceValid(any()); + .when(authorizationService).isAuthorizationForResourceValid(any()); final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); validator.validateAuthorization(resource); } @@ -121,8 +115,8 @@ public void whenPluginExceptionInIsAuthorizationAttributeValidThenServiceUnavail ImmutableList.of(attr1), ImmutableList.of(attr2), ImmutableList.of(attr3)); - - when(authorizationService.isAuthorizationAttributeValid(any())).thenThrow(new PluginException("blah")); + doThrow(new PluginException("blah")) + .when(authorizationService).isAuthorizationForResourceValid(any()); final Resource resource = new ResourceImpl("myResource1", "event-type", auth, null); validator.validateAuthorization(resource); } From f2a8f4e4a76fbc3f36fc293910ccf2bf19764219 Mon Sep 17 00:00:00 2001 From: Kunal Jha Date: Thu, 7 Feb 2019 18:44:15 +0100 Subject: [PATCH 10/13] added check for null authorization --- src/main/java/org/zalando/nakadi/domain/ResourceImpl.java | 8 +++++++- .../zalando/nakadi/service/AuthorizationValidator.java | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/zalando/nakadi/domain/ResourceImpl.java b/src/main/java/org/zalando/nakadi/domain/ResourceImpl.java index 4d28e273a2..03ba6b2ee7 100644 --- a/src/main/java/org/zalando/nakadi/domain/ResourceImpl.java +++ b/src/main/java/org/zalando/nakadi/domain/ResourceImpl.java @@ -42,12 +42,18 @@ public String getType() { @Override public Optional> getAttributesForOperation( final AuthorizationService.Operation operation) { + if (null == authorization) { + return Optional.empty(); + } return authorization.getAttributesForOperation(operation); } @Override public Map> getAuthorization() { - return authorization.asMapValue(); + if (authorization != null) { + return authorization.asMapValue(); + } + return null; } @Override diff --git a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java index 8192d1213d..a068348d68 100644 --- a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java +++ b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java @@ -45,7 +45,7 @@ public AuthorizationValidator( this.adminService = adminService; } - public void validateAuthorization(@Nullable final Resource resource) throws UnableProcessException, + public void validateAuthorization(final Resource resource) throws UnableProcessException, ServiceTemporarilyUnavailableException { if (resource.getAuthorization() != null) { checkAuthorisationForResourceAreValid(resource); From cf553f040556c00db2446eb0d5a82070665de962 Mon Sep 17 00:00:00 2001 From: Kunal Jha Date: Fri, 8 Feb 2019 18:15:26 +0100 Subject: [PATCH 11/13] null authorization support and return error fixes --- .../org/zalando/nakadi/domain/ResourceAuthorization.java | 7 +++---- .../zalando/nakadi/domain/SubscriptionAuthorization.java | 4 ++-- .../java/org/zalando/nakadi/service/AdminService.java | 3 ++- .../zalando/nakadi/service/AuthorizationValidator.java | 6 +++--- .../nakadi/service/AuthorizationValidatorTest.java | 9 +++++---- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/zalando/nakadi/domain/ResourceAuthorization.java b/src/main/java/org/zalando/nakadi/domain/ResourceAuthorization.java index 7e3c817391..5db528ff84 100644 --- a/src/main/java/org/zalando/nakadi/domain/ResourceAuthorization.java +++ b/src/main/java/org/zalando/nakadi/domain/ResourceAuthorization.java @@ -104,13 +104,12 @@ public Optional> getAttributesForOperation( } } - @Override public Map> asMapValue() { return ImmutableMap.of( - "admins", getAdmins(), - "readers", getReaders(), - "writers", getWriters()); + AuthorizationService.Operation.ADMIN.toString(), getAdmins(), + AuthorizationService.Operation.READ.toString(), getReaders(), + AuthorizationService.Operation.WRITE.toString(), getWriters()); } @Override diff --git a/src/main/java/org/zalando/nakadi/domain/SubscriptionAuthorization.java b/src/main/java/org/zalando/nakadi/domain/SubscriptionAuthorization.java index 4c345b879d..c8d1b7472c 100644 --- a/src/main/java/org/zalando/nakadi/domain/SubscriptionAuthorization.java +++ b/src/main/java/org/zalando/nakadi/domain/SubscriptionAuthorization.java @@ -64,8 +64,8 @@ public int hashCode() { @Override public Map> asMapValue() { return ImmutableMap.of( - "admins", getAdmins(), - "readers", getReaders() + AuthorizationService.Operation.ADMIN.toString(), getAdmins(), + AuthorizationService.Operation.READ.toString(), getReaders() ); } diff --git a/src/main/java/org/zalando/nakadi/service/AdminService.java b/src/main/java/org/zalando/nakadi/service/AdminService.java index f1437cd2ff..b7470dcae2 100644 --- a/src/main/java/org/zalando/nakadi/service/AdminService.java +++ b/src/main/java/org/zalando/nakadi/service/AdminService.java @@ -13,6 +13,7 @@ import org.zalando.nakadi.domain.ResourceImpl; import org.zalando.nakadi.exceptions.runtime.DbWriteOperationsBlockedException; import org.zalando.nakadi.exceptions.runtime.UnableProcessException; +import org.zalando.nakadi.exceptions.runtime.UnprocessableEntityException; import org.zalando.nakadi.plugin.api.authz.AuthorizationService; import org.zalando.nakadi.plugin.api.authz.Resource; import org.zalando.nakadi.plugin.api.exceptions.AuthorizationInvalidException; @@ -122,7 +123,7 @@ private void validateAllAdmins(final List admins) throws UnableProce authorizationService.isAuthorizationForResourceValid(new ResourceImpl<>(PERMISSION_RESOURCE, PERMISSION_RESOURCE, ResourceAuthorization.fromPermissionsList(admins), null)); } catch (AuthorizationInvalidException e) { - throw new UnableProcessException(e.getMessage()); + throw new UnprocessableEntityException(e.getMessage()); } } } diff --git a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java index a068348d68..3f41d95a75 100644 --- a/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java +++ b/src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java @@ -10,6 +10,7 @@ import org.zalando.nakadi.exceptions.runtime.InternalNakadiException; import org.zalando.nakadi.exceptions.runtime.ServiceTemporarilyUnavailableException; import org.zalando.nakadi.exceptions.runtime.UnableProcessException; +import org.zalando.nakadi.exceptions.runtime.UnprocessableEntityException; import org.zalando.nakadi.plugin.api.authz.AuthorizationAttribute; import org.zalando.nakadi.plugin.api.authz.AuthorizationService; import org.zalando.nakadi.plugin.api.authz.Resource; @@ -18,7 +19,6 @@ import org.zalando.nakadi.plugin.api.exceptions.PluginException; import org.zalando.nakadi.repository.EventTypeRepository; -import javax.annotation.Nullable; import java.util.List; import java.util.Map; import java.util.Optional; @@ -47,8 +47,8 @@ public AuthorizationValidator( public void validateAuthorization(final Resource resource) throws UnableProcessException, ServiceTemporarilyUnavailableException { + checkAuthorisationForResourceAreValid(resource); if (resource.getAuthorization() != null) { - checkAuthorisationForResourceAreValid(resource); checkAuthAttributesNoDuplicates(resource.getAuthorization()); } } @@ -94,7 +94,7 @@ private void checkAuthorisationForResourceAreValid(final Resource resource) } catch (OperationOnResourceNotPermittedException e) { throw new ForbiddenOperationException(e.getMessage()); } catch (AuthorizationInvalidException e) { - throw new UnableProcessException(e.getMessage()); + throw new UnprocessableEntityException(e.getMessage()); } catch (PluginException e) { throw new ServiceTemporarilyUnavailableException("Error calling authorization plugin", e); } diff --git a/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java b/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java index a62ec944ac..6bddc43d30 100644 --- a/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java +++ b/src/test/java/org/zalando/nakadi/service/AuthorizationValidatorTest.java @@ -9,6 +9,7 @@ import org.zalando.nakadi.exceptions.runtime.ForbiddenOperationException; import org.zalando.nakadi.exceptions.runtime.ServiceTemporarilyUnavailableException; import org.zalando.nakadi.exceptions.runtime.UnableProcessException; +import org.zalando.nakadi.exceptions.runtime.UnprocessableEntityException; import org.zalando.nakadi.plugin.api.authz.AuthorizationAttribute; import org.zalando.nakadi.plugin.api.authz.AuthorizationService; import org.zalando.nakadi.plugin.api.authz.Resource; @@ -57,7 +58,7 @@ public void whenInvalidAuthAttributesThenInvalidEventTypeException() { try { validator.validateAuthorization(resource); fail("Exception expected to be thrown"); - } catch (final UnableProcessException e) { + } catch (final UnprocessableEntityException e) { assertThat(e.getMessage(), equalTo("some attributes are not ok")); } } @@ -77,8 +78,8 @@ public void whenDuplicatesThenInvalidEventTypeException() { fail("Exception expected to be thrown"); } catch (final UnableProcessException e) { assertThat(e.getMessage(), equalTo( - "authorization property 'admins' contains duplicated attribute(s): type1:value1, type3:value3; " + - "authorization property 'readers' contains duplicated attribute(s): type2:value2")); + "authorization property 'ADMIN' contains duplicated attribute(s): type1:value1, type3:value3; " + + "authorization property 'READ' contains duplicated attribute(s): type2:value2")); } } @@ -95,7 +96,7 @@ public void whenOperationOnResourceNotPermittedExceptionThenForbiddenOperationEx validator.validateAuthorization(resource); } - @Test(expected = UnableProcessException.class) + @Test(expected = UnprocessableEntityException.class) public void whenAuthorizationInvalidExceptionThenUnableProcessException() { final ResourceAuthorization auth = new ResourceAuthorization( From ccd22e17ae1ad376f1d5844edf67d43b89200163 Mon Sep 17 00:00:00 2001 From: Kunal jha Date: Tue, 12 Feb 2019 09:45:07 +0100 Subject: [PATCH 12/13] Addressed Comments --- src/main/java/org/zalando/nakadi/domain/EventType.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/zalando/nakadi/domain/EventType.java b/src/main/java/org/zalando/nakadi/domain/EventType.java index c3ebc15ffc..9e71003b24 100644 --- a/src/main/java/org/zalando/nakadi/domain/EventType.java +++ b/src/main/java/org/zalando/nakadi/domain/EventType.java @@ -47,7 +47,6 @@ public void setCreatedAt(final DateTime createdAt) { this.createdAt = createdAt; } - public Resource asResource() { return new ResourceImpl<>(getName(), ResourceImpl.EVENT_TYPE_RESOURCE, getAuthorization(), this); } From 22379f4c8a1c29adc35a51bf9bd7c708bd304ca7 Mon Sep 17 00:00:00 2001 From: Kunal Jha Date: Tue, 12 Feb 2019 12:27:37 +0100 Subject: [PATCH 13/13] updated inteface version --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 72fb1876f6..14fefd5522 100644 --- a/build.gradle +++ b/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.1.2' + compile 'org.zalando:nakadi-plugin-api:3.2.1' compile 'org.echocat.jomon:runtime:1.6.3' // kafka & zookeeper