You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/06/26 13:49:27 UTC
[GitHub] [pulsar] wuzhanpeng opened a new pull request #11113: fix authorization not working when subscription permission is empty
wuzhanpeng opened a new pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113
### Motivation
This PR fixed the problem that `org.apache.pulsar.broker.authorization.AuthorizationProvider#canConsumeAsync` returned `true` when `org.apache.pulsar.common.policies.data.AuthPolicies#getSubscriptionAuthentication` was empty (i.e. the set of `subscription_auth_roles` in admin policies is empty).
### Modifications
- `canConsumeAsync` of `PulsarAuthorizationProvider` and its relative test case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #11113: fix authorization not working when subscription permission is empty
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113#issuecomment-974749038
@wuzhanpeng:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wuzhanpeng commented on pull request #11113: fix authorization not working when subscription permission is empty
Posted by GitBox <gi...@apache.org>.
wuzhanpeng commented on pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113#issuecomment-869558072
> This looks like a behaviour change.
> You we advertise this in the release notes ?
> Will existing users be affected by this change ?
1) for regular users who enable authorization, they will grant topic and subscription permissions before actually consuming messages, thus these users would not be affected.
2) for those noncompliant users who never grant subscription permissions but still want authorization service, the subscriber would not be able to establish after this behavior takes effect.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wuzhanpeng commented on pull request #11113: fix authorization not working when subscription permission is empty
Posted by GitBox <gi...@apache.org>.
wuzhanpeng commented on pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113#issuecomment-869522692
@rdhabalia @hangc0276
Could you help review these changes ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] 315157973 commented on pull request #11113: fix authorization not working when subscription permission is empty
Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113#issuecomment-956100190
@wuzhanpeng Hello, is there an email address for discussion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #11113: fix authorization not working when subscription permission is empty
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113#issuecomment-1036503376
I am going to remove the 2.8 label for now. @wuzhanpeng do you plan to continue with this work, or should we close this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #11113: fix authorization not working when subscription permission is empty
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113#issuecomment-977728903
@wuzhanpeng is this a bug fix (no need to update docs?)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wuzhanpeng commented on pull request #11113: fix authorization not working when subscription permission is empty
Posted by GitBox <gi...@apache.org>.
wuzhanpeng commented on pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113#issuecomment-875412415
@hangc0276 @eolivelli @codelipenghui Could you help review these changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on a change in pull request #11113: fix authorization not working when subscription permission is empty
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on a change in pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113#discussion_r697542442
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java
##########
@@ -120,7 +120,7 @@ public void initialize(ServiceConfiguration conf, ConfigurationCacheService conf
// list is empty)
Review comment:
It appears that the original intent of this code was to consider an empty list as a special case that had special logic. I don't have enough context to know why, but it might be worth reviewing https://github.com/apache/pulsar/pull/2981, where this code was first introduced. If I had to guess, it's because it doesn't make much sense to have a subscription that cannot be consumed.
If this change is accepted, the comment on this line and the one above need to be updated: https://github.com/apache/pulsar/blob/879ab3a174b6cc78c15199f8c6fe6aefb1959116/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java#L119-L120
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] hangc0276 commented on pull request #11113: fix authorization not working when subscription permission is empty
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113#issuecomment-894137536
> @hangc0276 @eolivelli @codelipenghui Could you help review these changes?
@wuzhanpeng This PR will break the existing subscription behavior although the existing behavior not expected. It will cause the existing applications which doesn't grant role and subscription permission, subscribe failed after upgrading Pulsar to the version including this PR. We'd better start a discussion in dev@pulsar.apache.org first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #11113: fix authorization not working when subscription permission is empty
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113#issuecomment-974749038
@wuzhanpeng:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #11113: fix authorization not working when subscription permission is empty
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113#issuecomment-869544440
This looks like a behaviour change.
You we advertise this in the release notes ?
Will existing users be affected by this change ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] hangc0276 commented on pull request #11113: fix authorization not working when subscription permission is empty
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #11113:
URL: https://github.com/apache/pulsar/pull/11113#issuecomment-894141407
move to 2.8.2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org