New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Authorization implementation according latest plugin #1014
Conversation
@@ -48,6 +48,7 @@ public void setCreatedAt(final DateTime createdAt) { | |||
this.createdAt = createdAt; | |||
} | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kunal-Jha please address this comment as well
src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java
Outdated
Show resolved
Hide resolved
@@ -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()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you call newValue.asBaseResource()
- it will create a resource without a name. I think it's fine to create a resource without a name for a subscription that is not created yet, but in this user modifies subscription that already exists so I think the id should be used as a resource name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added in the commit
src/main/java/org/zalando/nakadi/service/AuthorizationValidator.java
Outdated
Show resolved
Hide resolved
@Kunal-Jha please, check the comments once more. I think we have only some minor things and it's missing a merge from master. |
@rcillo Apologies!! somehow I missed that :( . . Added the new commits. |
👍 |
👍 |
One-line summary
Description
Made Changes to Nakadi according to the new plugin api interfaces. All the
allAuthorizationAttributeValid
are changed except for admin authorization where permissions are used.Review