You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2021/09/10 09:46:11 UTC
[GitHub] [qpid-jms] michaelandrepearce opened a new pull request #43: QPIDJMS-548: support taking principle and credentials from context
michaelandrepearce opened a new pull request #43:
URL: https://github.com/apache/qpid-jms/pull/43
Add mapping from jndi principle and credentials to username and password for the default connection factory when using provider url
Add 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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-jms] michaelandrepearce commented on pull request #43: QPIDJMS-548: support taking principle and credentials from context
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #43:
URL: https://github.com/apache/qpid-jms/pull/43#issuecomment-917989047
@gemmellr :dog: 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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-jms] michaelandrepearce commented on pull request #43: QPIDJMS-548: support taking principle and credentials from context
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #43:
URL: https://github.com/apache/qpid-jms/pull/43#issuecomment-921768245
Just for your benefit, we are actually trying to switch some Kafka Connect JMS Sinks to use AMQP to get everything over to AMQP instead CORE/OPENWIRE , we are using this vendors connector.
https://github.com/lensesio/stream-reactor/tree/master/kafka-connect-jms/src/main/scala/com/datamountaineer/streamreactor/connect/jms
Which is where the issue lays around the credentials. It seems to be not happy when we switch to QPID-JMS client and is relying on creating the connection and passing in the credentials using connectionFactory.createConnection(user,password) but as noted when using qpid its barthing out access issues to artemis which we dont get with openwire or artemis client.
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-jms] michaelandrepearce commented on a change in pull request #43: QPIDJMS-548: support taking principle and credentials from context
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #43:
URL: https://github.com/apache/qpid-jms/pull/43#discussion_r711019518
##########
File path: qpid-jms-client/src/main/java/org/apache/qpid/jms/jndi/JmsInitialContextFactory.java
##########
@@ -80,6 +80,14 @@ public Context getInitialContext(Hashtable<?, ?> environment) throws NamingExcep
String expanded = expand(location, environmentCopy);
if (ProviderFactory.findProviderFactory(new URI(expanded)) != null) {
environmentCopy.put(CONNECTION_FACTORY_DEFAULT_KEY_PREFIX + REMOTE_URI_PROP, expanded);
+ String principle = environmentCopy.containsKey(Context.SECURITY_PRINCIPAL) ? (String) environment.get(Context.SECURITY_PRINCIPAL) : System.getProperty(Context.SECURITY_PRINCIPAL);
Review comment:
fair enough can strip out the system and leave it just from jndi env.
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-jms] gemmellr commented on a change in pull request #43: QPIDJMS-548: support taking principle and credentials from context
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #43:
URL: https://github.com/apache/qpid-jms/pull/43#discussion_r706099787
##########
File path: qpid-jms-client/src/main/java/org/apache/qpid/jms/jndi/JmsInitialContextFactory.java
##########
@@ -80,6 +80,14 @@ public Context getInitialContext(Hashtable<?, ?> environment) throws NamingExcep
String expanded = expand(location, environmentCopy);
if (ProviderFactory.findProviderFactory(new URI(expanded)) != null) {
environmentCopy.put(CONNECTION_FACTORY_DEFAULT_KEY_PREFIX + REMOTE_URI_PROP, expanded);
+ String principle = environmentCopy.containsKey(Context.SECURITY_PRINCIPAL) ? (String) environment.get(Context.SECURITY_PRINCIPAL) : System.getProperty(Context.SECURITY_PRINCIPAL);
Review comment:
Context.SECURITY_PRINCIPAL and Context.SECURITY_CREDENTIALS aren't listed as being specifiable via system property the way the provider URL explicitly is. I think that makes sense given their intended use (for naming access) that it would likely be context-specific. So if we ever were to do this I would remove that.
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-jms] gemmellr commented on pull request #43: QPIDJMS-548: support taking principle and credentials from context
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #43:
URL: https://github.com/apache/qpid-jms/pull/43#issuecomment-923758890
I dont, as I say I cant really see a reason this would alter the client behaviour at all really, during connection the different routes are essentially the same and the details are passed into the client innards in the same way at the same point in both cases, one just calls a getter to retrieve the existing value from the factory first while the other is provided it, from then they use a common impl.
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-jms] michaelandrepearce edited a comment on pull request #43: QPIDJMS-548: support taking principle and credentials from context
Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #43:
URL: https://github.com/apache/qpid-jms/pull/43#issuecomment-921763750
@gemmellr the issue, we are getting is that whilst ActiveMQ (Openwire) and ActiveMQ Artemis (CORE) clients seem to be honouring the credentials when set via JMS as such its vendor agnostic and secrets can be present to app securely and separately
connectionFactory.createConnection(user,password)
where as with Qpid we are getting issue whereby connecting to secured artemis clusters we seem to be having to set the credentials on the CF itself for it to be happy with artemis when secured. And thus looking for options how to securly and in an agnostic way (by using JNDI properties that are explicitly meant for credentials) set that then on the CF if it is having issues not honouring the creds passed when we do connectionFactory.createConnection(user,password)
putting credentials in url, isnt the best as you are essentially mixing a config item with a security item, e.g. i want the URL to be visible to ops/dev teams theres no security concerns there, but credentials clearly more sensitive. Thus why to look to use the JNDI properties that are meant for credentials.
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-jms] michaelandrepearce edited a comment on pull request #43: QPIDJMS-548: support taking principle and credentials from context
Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #43:
URL: https://github.com/apache/qpid-jms/pull/43#issuecomment-921768245
Just for your benefit, we are actually trying to switch some Kafka Connect JMS Sinks to use AMQP to get everything over to AMQP instead CORE/OPENWIRE , we are using this vendors connector.
https://github.com/lensesio/stream-reactor/tree/master/kafka-connect-jms/src/main/scala/com/datamountaineer/streamreactor/connect/jms
Which is where the issue lays around the credentials. It seems to be not happy when we switch to QPID-JMS client and is relying on creating the connection and passing in the credentials using connectionFactory.createConnection(user,password)
https://github.com/lensesio/stream-reactor/blob/master/kafka-connect-jms/src/main/scala/com/datamountaineer/streamreactor/connect/jms/JMSSessionProvider.scala#L109
but as noted when using qpid its barthing out access issues to artemis which we dont get with openwire or artemis client.
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-jms] michaelandrepearce edited a comment on pull request #43: QPIDJMS-548: support taking principle and credentials from context
Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on pull request #43:
URL: https://github.com/apache/qpid-jms/pull/43#issuecomment-922887939
> If you are saying that the factory.createConnection(user, pass) method doesnt work and this does, then that would likely be a bug that should be investigated and fixed, but it is not a good reason to add this hack, which I still dont intend to.
>
Ok let me see if i can work out why this seems to not be working in the other project i referenced and see if we can make some kind of reproducer, agreed, i would also prefer to have this working as it should with the credentials just being set at connection creation time, just i struggled the first time to see why it wasn't working where as setting on the CF itself was. If you have any tips to where it might be causing issue for us to hunt down please share.
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-jms] michaelandrepearce commented on pull request #43: QPIDJMS-548: support taking principle and credentials from context
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #43:
URL: https://github.com/apache/qpid-jms/pull/43#issuecomment-922887939
> If you are saying that the factory.createConnection(user, pass) method doesnt work and this does, then that would likely be a bug that should be investigated and fixed, but it is not a good reason to add this hack, which I still dont intend to.
>
Ok let me see if i can work out why this seems to not be working in the other project i referenced and see if we can make some kind of reproducer, agreed, i would also prefer to have this working as it should, just i struggled the first time to see why it wasn't working where as setting on the CF itself was. If you have any tips to where it might be causing issue for us to hunt down please share.
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-jms] gemmellr commented on pull request #43: QPIDJMS-548: support taking principle and credentials from context
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #43:
URL: https://github.com/apache/qpid-jms/pull/43#issuecomment-921804323
Well again, there are ways that dont require putting them in the URL, including the very one you used to implement the change under the covers.
If you are saying that the factory.createConnection(user, pass) method doesnt work and this does, then that would likely be a bug that should be investigated and fixed, but it is not a good reason to add this hack, which I still dont intend to.
For what its worth, the createConnection(user, pass) method is tested, and it operates by passing the credentials to the client innards in the same field as setting them on the factory does, since both are transferred to an intermediate object from where they actually get used, its actually the more explicitly set of the two, so I dont immediately see how this could make any difference at all to what the client does.
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-jms] michaelandrepearce commented on pull request #43: QPIDJMS-548: support taking principle and credentials from context
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #43:
URL: https://github.com/apache/qpid-jms/pull/43#issuecomment-921763750
@gemmellr the issue, we are getting is that whilst ActiveMQ (Openwire) and ActiveMQ Artemis (CORE) clients seem to be honouring the credentials when set via JMS as such its vendor agnostic and secrets can be present to app securely and separately
connectionFactory.createConnection(user,password)
where as with Qpid we are getting issue whereby connecting to secured artemis clusters we seem to be having to set the credentials on the CF itself for it to be happy with artemis when secured. And thus looking for options how to securly and in an agnostic way (by using JNDI properties that are explicitly meant for credentials)
putting credentials in url, isnt the best as you are essentially mixing a config item with a security item, e.g. i want the URL to be visible to ops/dev teams theres no security concerns there, but credentials clearly more sensitive. Thus why to look to use the JNDI properties that are meant for credentials.
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org