Skip to content

Feature: Support for authoriztion of connection fields - #2461

Merged
mattrpav merged 3 commits into
apache:mainfrom
mattrpav:amq-authenticate-clientid
Oct 15, 2026
Merged

mattrpav merged 3 commits into
apache:mainfrom
mattrpav:amq-authenticate-clientid

Conversation

@mattrpav

@mattrpav mattrpav commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor
  • Add an isSsl() helper getter to Connector
  • Add ConnectionCallback for processing clientId authz
  • Include support for authorizing network connections

This feature provides hooks for authorization connections based on clientId, including specifying if a specific user may register their connection as a network connection.

Network Connection authorization is optional, and detected automatically by the broker-side JAAS handling when a clientId authorization is configured. If clientId authorization is not configure, the authorization check for network connections is skipped matching current behavior.

@mattrpav
mattrpav requested a review from cshannon August 14, 2026 19:27
@mattrpav mattrpav self-assigned this Aug 14, 2026
@mattrpav mattrpav changed the title [#] Add support for authorizing clientId values based on userId Add support for authorizing clientId values based on userId Aug 20, 2026
@mattrpav mattrpav moved this from Backlog to In progress in Apache ActiveMQ v6.4.0 Sep 7, 2026
@mattrpav mattrpav moved this from In progress to In review in Apache ActiveMQ v6.4.0 Sep 7, 2026
@mattrpav
mattrpav force-pushed the amq-authenticate-clientid branch from e79af2f to 18e2734 Compare September 7, 2026 20:40
@mattrpav mattrpav changed the title Add support for authorizing clientId values based on userId Feature: Support for authorizing clientId values based on userId Sep 7, 2026
@mattrpav
mattrpav force-pushed the amq-authenticate-clientid branch 2 times, most recently from 083533e to da71780 Compare September 7, 2026 21:07
@mattrpav mattrpav added this to the 2026 Security hardening milestone Sep 21, 2026
// itself a network connection must have been permitted to do so. Bridges normally
// send BrokerInfo first, in which case authentication already saw the flag and
// there are no connection states here yet.
var connectionStates = listConnectionStates();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general it doesn't really seem like we should be doing any kind of authentication checks here and I think it makes sense to move this and treat each connection attempt independently.

The proper place to do the checking is here:

After the addConnection() executes to handle the jaas authenticaiton logic there is that line which process addNetworkConnection(). These checks should be done either before the addNetworkConnection() call or just moved into that method first so that we throw the exception and cleanup before continuing.

I don't think it makes sense to loop over all states, I think we can just allow it as long as the current connection attempt is ok (there's usually only 1 state anyways)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. The original intent was authz only the network conn info part. Since the feature expanded to allow authz on a number of connection related infos, the place you pointed out is not just subjectively better -- but required for those other authz points to be functional.

@mattrpav
mattrpav force-pushed the amq-authenticate-clientid branch from da71780 to 6f0320f Compare October 14, 2026 16:47
@mattrpav
mattrpav requested a review from cshannon October 14, 2026 17:02

@cshannon cshannon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the design of the auth decision based on network/client id being made entirely inside of the jaas module makes sense and aligns with existing authentication. I could see there being tweaks to this in the future (maybe other ways to configure or enhancements) but it's a new feature so it's fine to go into 6.4.0 and it could always be improved if stuff comes up.

@mattrpav mattrpav changed the title Feature: Support for authorizing clientId values based on userId Feature: Support for authoriztion of connection fields Oct 15, 2026
@mattrpav
mattrpav merged commit b67c5e0 into apache:main Oct 15, 2026
19 of 20 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in Apache ActiveMQ v6.4.0 Oct 15, 2026
@mattrpav
mattrpav deleted the amq-authenticate-clientid branch October 15, 2026 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants