You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by sjhiggs <gi...@git.apache.org> on 2017/04/17 13:49:11 UTC
[GitHub] activemq-artemis pull request #1206: ARTEMIS-1116 - added role mapper for LD...
GitHub user sjhiggs opened a pull request:
https://github.com/apache/activemq-artemis/pull/1206
ARTEMIS-1116 - added role mapper for LDAP roles to local ActiveMQ roles
For ARTEMIS-1116, allows configuring LDAP role mapping to local ActiveMQ roles.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sjhiggs/activemq-artemis role_mapping
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1206.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1206
----
commit 1333024c659b6f1c3c37a3e2aa7b58c125973aa8
Author: Stephen Higgs <sh...@redhat.com>
Date: 2017-04-16T02:05:25Z
added role mapper
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #1206: ARTEMIS-1116 - added role mapper for LD...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1206
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1206: ARTEMIS-1116 - added role mapper for LDAP role...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1206
Thanks for fixing the commit message and checkstyle issues.
The more I think about this the more I think this should be a general feature which all login modules can use rather than just the LDAP login module. In general, this functionality would only be useful for login modules pulling data from an external system like LDAP which makes it a natural fit for the LDAP login module, but other users might implement their own login module where this functionality would be useful (e.g. integrating with a legacy auth system) and I wouldn't want to require a re-implementation. Also, I think moving the configuration into broker.xml from login.config will be provide a better user experience.
From a configuration perspective this would be a good fit in <security-settings> next to <security-setting> and <security-setting-plugin>. The name <role-mapping> seems appropriate. It could have a "from" and "to" attribute where "from" is delimited with e.g. a comma.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1206: ARTEMIS-1116 - added role mapper for LDAP role...
Posted by sjhiggs <gi...@git.apache.org>.
Github user sjhiggs commented on the issue:
https://github.com/apache/activemq-artemis/pull/1206
Proposed XML configuration structure:
~~~
<role-mappings>
<role-mapping>
<role>admin</role>
<mapped-roles>
<mapped-role>Administrator</mapped-role>
<mapped-role>amq</mapped-role>
</mapped-roles>
</role-mapping>
<role-mappings>
~~~
Thoughts?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1206: ARTEMIS-1116 - added role mapper for LDAP role...
Posted by sjhiggs <gi...@git.apache.org>.
Github user sjhiggs commented on the issue:
https://github.com/apache/activemq-artemis/pull/1206
That's fine, I was assuming "from" was the external/LDAP role and "to" is the internal AMQ role, do you see it the opposite?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1206: ARTEMIS-1116 - added role mapper for LDAP role...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1206
> I was assuming "from" was the external/LDAP role and "to" is the internal AMQ role, do you see it the opposite?
I see the "from" was the external role and "to" is the internal Artemis role as you said.
> ...the from will probably need a different delimiter because the role name may be something like a DN.
Whatever makes sense from an implementation perspective is fine with me. I was just going on the fact that the original commit used commas as a delimiter.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1206: ARTEMIS-1116 - added role mapper for LDAP role...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1206
Couple of things:
- The commit message should follow the format outlined in the [Hacking Guide](https://github.com/apache/activemq-artemis/blob/master/docs/hacking-guide/en/maintainers.md#commit-messages).
- You have a fair number of checkstyle violations. See more [here](https://builds.apache.org/blue/organizations/jenkins/ActiveMQ-Artemis-PR-Build/detail/ActiveMQ-Artemis-PR-Build/2607/pipeline). You run the checkstyle stuff locally by using 'mvn -Pdev install'.
- I wonder if this feature would be better if it was more general so that it could be used with any login module rather than isolated only to the LDAP login module. What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1206: ARTEMIS-1116 - added role mapper for LDAP role...
Posted by sjhiggs <gi...@git.apache.org>.
Github user sjhiggs commented on the issue:
https://github.com/apache/activemq-artemis/pull/1206
Updated - it works a little differently than I originally anticipated; a set of roles is calculated when parsing the broker.xml taking into account not only the original roles but the role mappings as well. This commit is quite a bit more intrusive than the previous one. Tests are included, but needs review all the same.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1206: ARTEMIS-1116 - added role mapper for LDAP role...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1206
I was thinking something a bit less verbose like...
<security-settings>
...
<role-mapping to="admin" from="Administrator,amq"/>
...
<security-settings>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---