Re: [PR] AMQ-9472 Add test for wildcard producer breaking authorization [activemq]

2024-04-08 Thread via GitHub


mattrpav commented on PR #1198:
URL: https://github.com/apache/activemq/pull/1198#issuecomment-2043874595

   I think that the behavior should match the destination policy. 
   
   1. Wildcard 
   2. Fully qualified name (authoritative)
   
   I agree, it should be a new flag since we would not want to change behavior 
on existing users. 


-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AMQ-9472 Add test for wildcard producer breaking authorization [activemq]

2024-04-08 Thread via GitHub


cshannon commented on PR #1198:
URL: https://github.com/apache/activemq/pull/1198#issuecomment-2043856614

   The only other thing i can think of is we could technically change the 
behavior of the authorization broker plugin because you could make an argument 
that any matching wildcard should implicitly be authorized if it matches a 
topic. In this case by publishing to a wildcard the intent is anything that 
matches gets it so should be authorized.
   
   However, the problem with changing that would be a change in the current 
authorization behavior which is not good for existing users not expecting it. 
So I think that we'd need to have a config option or an extension to prevent 
unintended consequences since it's been like this for a long time.
   
   So I would view any changes here more as an enhancement or improvement. If 
we made it configurable I think it would need to be something where the config 
would be described as "implicitly grant matching wildcard destinations" or 
something like that and probably a new feature and not a bug fix release. 
Subscribing to a wildcard destination would also need to be explored as well.


-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AMQ-9472 Add test for wildcard producer breaking authorization [activemq]

2024-04-08 Thread via GitHub


cshannon commented on PR #1198:
URL: https://github.com/apache/activemq/pull/1198#issuecomment-2043790765

   In terms of working around this, the options are pretty much:
   
   1. If you are going to create topics then you need to make sure you have 
proper ACLs set up for those topics (including matching subscriptions on 
wildcards) that consumers might subscribe to. In this case you could add read 
ACLs for A.> for the users group. 
   2. You could always customize the authorization logic by implementing your 
own plugin or overriding/extending the `AuthorizationBroker` and 
`AuthorizationDestinationInterceptor`


-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AMQ-9472 Add test for wildcard producer breaking authorization [activemq]

2024-04-08 Thread via GitHub


cshannon commented on PR #1198:
URL: https://github.com/apache/activemq/pull/1198#issuecomment-2043752900

   This is not really a bug to me, it's a side effect of how wild card 
subscriptions work in the broker. The issue here is not with the authorization 
plugin or logic but with how subscriptions currently work.
   
   When consumers are created, subscriptions get added for all matching 
destinations. In this case, there are 2 matching destinations because of the 
wildcard destination that was auto created. When creating a consumer on Topic 
A.B, first the new consumer is 
[authorized](https://github.com/apache/activemq/blob/e025e443e65d4bd3c2c27f11d6caa7bfbd2c9626/activemq-broker/src/main/java/org/apache/activemq/security/AuthorizationBroker.java#L148)
 on that destination and that passes. 
   
   But then later on the addSubscription() is called for both destinations to 
add two subscriptions for the new consumer...one on A.B which is properly 
[authorized](https://github.com/apache/activemq/blob/e025e443e65d4bd3c2c27f11d6caa7bfbd2c9626/activemq-broker/src/main/java/org/apache/activemq/security/AuthorizationDestinationFilter.java#L40)
 and then another on A.> which is not authorized because there's no proper ACLs 
for "users" when adding the subscription for A.>
   
   So this is just how the broker currently works to handle wildcard 
subscriptions, since you are publishing to a wildcard topic then any 
subscription that matches needs to also subscribe tot he wildcard to get that 
message.


-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] ARTEMIS-4498 - Expose internal queues for management and observability [activemq-artemis]

2024-04-08 Thread via GitHub


clebertsuconic commented on PR #4856:
URL: 
https://github.com/apache/activemq-artemis/pull/4856#issuecomment-2043625204

   @AntonRoskvist , @andytaylor can we settle with these changes?
   
   
   I added the "internal attribute" to the AddressControl as well.
   
   @andytaylor if you could take a look at the changes made to the web console 
here please?


-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] ARTEMIS-4510 - Add auto-create-destination logic to diverts [activemq-artemis]

2024-04-08 Thread via GitHub


clebertsuconic closed pull request #4681: ARTEMIS-4510 - Add 
auto-create-destination logic to diverts
URL: https://github.com/apache/activemq-artemis/pull/4681


-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] ARTEMIS-4717 Upgrade commons-configuration2 to 2.10.1 [activemq-artemis]

2024-04-08 Thread via GitHub


clebertsuconic merged PR #4877:
URL: https://github.com/apache/activemq-artemis/pull/4877


-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] ARTEMIS-4510 - Add auto-create-destination logic to diverts [activemq-artemis]

2024-04-08 Thread via GitHub


clebertsuconic commented on PR #4867:
URL: 
https://github.com/apache/activemq-artemis/pull/4867#issuecomment-2043460610

   actually.. I pushed a commit
   


-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] ARTEMIS-4510 - Add auto-create-destination logic to diverts [activemq-artemis]

2024-04-08 Thread via GitHub


clebertsuconic closed pull request #4867: ARTEMIS-4510 - Add 
auto-create-destination logic to diverts
URL: https://github.com/apache/activemq-artemis/pull/4867


-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[PR] ARTEMIS-4717 Upgrade commons-configuration2 to 2.10.1 [activemq-artemis]

2024-04-08 Thread via GitHub


brusdev opened a new pull request, #4877:
URL: https://github.com/apache/activemq-artemis/pull/4877

   (no comment)


-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] ARTEMIS-4510 - Add auto-create-destination logic to diverts [activemq-artemis]

2024-04-08 Thread via GitHub


clebertsuconic commented on PR #4867:
URL: 
https://github.com/apache/activemq-artemis/pull/4867#issuecomment-2042993128

   @AntonRoskvist I pulled your changes, removed all the docs and stuff and 
pushed on main (I pushed as you).
   
   now you have to give a JIRA to your other change.
   
   it's probably easier if you cherry-pick that issue on main than rebasing as 
you now have conflicts. I don't mind if you keep this very same PR but you will 
need to push -f with just that change now.
   
   
   if you can't do it I will do those things myself.. let me know if you want / 
can do it.
   
   
   Thank you.


-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AMQ-9472 Add test for wildcard producer breaking authorization [activemq]

2024-04-08 Thread via GitHub


mattrpav commented on PR #1198:
URL: https://github.com/apache/activemq/pull/1198#issuecomment-2042861290

   I was able to reproduce using a stock Apache v6.1.1 build. This appears to 
be a 'does not permit' issue and not a 'does not restrict' issue.


-- 
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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] ARTEMIS-4510 - Add auto-create-destination logic to diverts [activemq-artemis]

2024-04-08 Thread via GitHub


AntonRoskvist commented on PR #4867:
URL: 
https://github.com/apache/activemq-artemis/pull/4867#issuecomment-2042323454

   @clebertsuconic Sounds great, thanks!
   
   I went ahead and separated the changes.
   
   Let me know when/if you are happy with your testing (removing the config 
option and always reusing the session) and I'll go ahead and remove the config 
option as well as some documentation from 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: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org