[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r312022902
##
File path: java/org/apache/catalina/realm/RealmBase.java
##
@@ -928,6 +928,15 @@ public boolean hasRole(Wrapper wrapper, Principal
principal, String role) {
}
}
+// Check for a role alias/mapping defined on context level
+if (getContainer() instanceof Context) {
+Context context = (Context) getContainer();
+String realRole = context.findRoleMapping(role);
Review comment:
I am not really skipping it, I have pushed into the check loop. You previous
check ignored a valid 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311972092
##
File path: java/org/apache/catalina/realm/RealmBase.java
##
@@ -928,6 +928,15 @@ public boolean hasRole(Wrapper wrapper, Principal
principal, String role) {
}
}
+// Check for a role alias/mapping defined on context level
+if (getContainer() instanceof Context) {
+Context context = (Context) getContainer();
+String realRole = context.findRoleMapping(role);
Review comment:
@markt-asf I have added another commit with the requested change. If that
meets your expectations, I will modify the tests accordingly.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311972092
##
File path: java/org/apache/catalina/realm/RealmBase.java
##
@@ -928,6 +928,15 @@ public boolean hasRole(Wrapper wrapper, Principal
principal, String role) {
}
}
+// Check for a role alias/mapping defined on context level
+if (getContainer() instanceof Context) {
+Context context = (Context) getContainer();
+String realRole = context.findRoleMapping(role);
Review comment:
@markt-asf I have added another commit with the requested change. It that
meets your expectations, I will modify the tests accordingly.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole() URL: https://github.com/apache/tomcat/pull/186#discussion_r311971810 ## File path: webapps/docs/changelog.xml ## @@ -47,6 +47,10 @@ + +63636: Context.findRoleMapping() never called +in RealmBase#hasRole(). (michaelo) + Review comment: Done. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311748751
##
File path: test/org/apache/catalina/realm/TestRealmBase.java
##
@@ -789,4 +791,45 @@ public void testHttpConstraint() throws IOException {
Assert.assertFalse(mapRealm.hasResourcePermission(
request, response, constraintsDelete, null));
}
+
+@Test
+public void testRoleMapping() throws Exception {
+Context context = new TesterContext() {
+private Map roleMapping = new HashMap<>();
+
+public void addRoleMapping(String role, String link) {
+roleMapping.put(role, link);
+}
+
+@Override
+public String findRoleMapping(String role) {
+return roleMapping.get(role);
+}
+};
+
+context.addRoleMapping(ROLE2, "very-complex-role-name");
Review comment:
I will do the changes tomorrow and see how I can properly modify tests.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311748567
##
File path: test/org/apache/catalina/realm/TestRealmBase.java
##
@@ -789,4 +791,45 @@ public void testHttpConstraint() throws IOException {
Assert.assertFalse(mapRealm.hasResourcePermission(
request, response, constraintsDelete, null));
}
+
+@Test
+public void testRoleMapping() throws Exception {
+Context context = new TesterContext() {
+private Map roleMapping = new HashMap<>();
+
+public void addRoleMapping(String role, String link) {
+roleMapping.put(role, link);
+}
+
+@Override
+public String findRoleMapping(String role) {
+return roleMapping.get(role);
+}
+};
+
+context.addRoleMapping(ROLE2, "very-complex-role-name");
Review comment:
I consider this way too inflexible, but yes -- let's discussed as soon as
this one has been resolved.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311748751
##
File path: test/org/apache/catalina/realm/TestRealmBase.java
##
@@ -789,4 +791,45 @@ public void testHttpConstraint() throws IOException {
Assert.assertFalse(mapRealm.hasResourcePermission(
request, response, constraintsDelete, null));
}
+
+@Test
+public void testRoleMapping() throws Exception {
+Context context = new TesterContext() {
+private Map roleMapping = new HashMap<>();
+
+public void addRoleMapping(String role, String link) {
+roleMapping.put(role, link);
+}
+
+@Override
+public String findRoleMapping(String role) {
+return roleMapping.get(role);
+}
+};
+
+context.addRoleMapping(ROLE2, "very-complex-role-name");
Review comment:
I will do the changes tomorrow and see how I can properly modifiy tests.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311748303
##
File path: java/org/apache/catalina/realm/RealmBase.java
##
@@ -928,6 +928,15 @@ public boolean hasRole(Wrapper wrapper, Principal
principal, String role) {
}
}
+// Check for a role alias/mapping defined on context level
+if (getContainer() instanceof Context) {
+Context context = (Context) getContainer();
+String realRole = context.findRoleMapping(role);
Review comment:
Wouldn't it be confusing when `findSecurityReference()` can map a role which
`findSecurityReferences()` does not contain?
Why the null check? Wouldn't it be possible for a servlet security role ref
to be also mapped to a technical role? My current does this.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole() URL: https://github.com/apache/tomcat/pull/186#discussion_r311742634 ## File path: webapps/docs/changelog.xml ## @@ -47,6 +47,10 @@ + +63636: Context.findRoleMapping() never called +in RealmBase#hasRole(). (michaelo) + Review comment: Alright, will do. I will probably need to fix the rest I have merged already. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole() URL: https://github.com/apache/tomcat/pull/186#discussion_r311742634 ## File path: webapps/docs/changelog.xml ## @@ -47,6 +47,10 @@ + +63636: Context.findRoleMapping() never called +in RealmBase#hasRole(). (michaelo) + Review comment: Alright, will do. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311703087
##
File path: java/org/apache/catalina/realm/RealmBase.java
##
@@ -928,6 +928,15 @@ public boolean hasRole(Wrapper wrapper, Principal
principal, String role) {
}
}
+// Check for a role alias/mapping defined on context level
+if (getContainer() instanceof Context) {
+Context context = (Context) getContainer();
+String realRole = context.findRoleMapping(role);
Review comment:
Just take your time to describe how you image that, I do not fully
understand it.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311702448
##
File path: test/org/apache/catalina/realm/TestRealmBase.java
##
@@ -789,4 +791,45 @@ public void testHttpConstraint() throws IOException {
Assert.assertFalse(mapRealm.hasResourcePermission(
request, response, constraintsDelete, null));
}
+
+@Test
+public void testRoleMapping() throws Exception {
+Context context = new TesterContext() {
+private Map roleMapping = new HashMap<>();
+
+public void addRoleMapping(String role, String link) {
+roleMapping.put(role, link);
+}
+
+@Override
+public String findRoleMapping(String role) {
+return roleMapping.get(role);
+}
+};
+
+context.addRoleMapping(ROLE2, "very-complex-role-name");
Review comment:
You probably want a `` with custom attributes. What
makes it different to a listener?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311490843
##
File path: java/org/apache/catalina/realm/RealmBase.java
##
@@ -928,6 +928,15 @@ public boolean hasRole(Wrapper wrapper, Principal
principal, String role) {
}
}
+// Check for a role alias/mapping defined on context level
+if (getContainer() instanceof Context) {
+Context context = (Context) getContainer();
+String realRole = context.findRoleMapping(role);
Review comment:
So, if I understand your request properly, this should not be part of
`RealmBase` at all, but has to be implemented in `StandardWrapper`, namely
`findSecurityReference()` whre it queries `getParent()#findRoleMapping()` if it
is not found in `references`?.
So `findSecurityReferences()` have to be changed as well? But not
`removeSecurityReference()`?
though, `getParent()` must be still of type `Context` (`instanceof` check)?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311513232
##
File path: java/org/apache/catalina/realm/RealmBase.java
##
@@ -928,6 +928,15 @@ public boolean hasRole(Wrapper wrapper, Principal
principal, String role) {
}
}
+// Check for a role alias/mapping defined on context level
+if (getContainer() instanceof Context) {
+Context context = (Context) getContainer();
+String realRole = context.findRoleMapping(role);
Review comment:
Both
> Consider moving this to Wrapper.findSecurityReference
and
> findSecurityReferences() should be unchanged.
seem to be a contradiction. Do you want me to change the `Wrapper` interface
and all of its implementors? That would still mean that I need to change
`RealmBase` anyway?!
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311490843
##
File path: java/org/apache/catalina/realm/RealmBase.java
##
@@ -928,6 +928,15 @@ public boolean hasRole(Wrapper wrapper, Principal
principal, String role) {
}
}
+// Check for a role alias/mapping defined on context level
+if (getContainer() instanceof Context) {
+Context context = (Context) getContainer();
+String realRole = context.findRoleMapping(role);
Review comment:
So, if I understand your request properly, this should not be part of
`RealmBase` at all, but has to be implemented in `StandardWrapper`, namely
`findSecurityReference()` whre it queries `getParent()#findRoleMapping() if it
is not found in `references`?.
So `findSecurityReferences()` have to be changed as well? But not
`removeSecurityReference()`?
though, `getParent()` must be still of type `Context` (`instanceof` check)?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311491516
##
File path: test/org/apache/catalina/realm/TestRealmBase.java
##
@@ -789,4 +791,45 @@ public void testHttpConstraint() throws IOException {
Assert.assertFalse(mapRealm.hasResourcePermission(
request, response, constraintsDelete, null));
}
+
+@Test
+public void testRoleMapping() throws Exception {
+Context context = new TesterContext() {
+private Map roleMapping = new HashMap<>();
+
+public void addRoleMapping(String role, String link) {
+roleMapping.put(role, link);
+}
+
+@Override
+public String findRoleMapping(String role) {
+return roleMapping.get(role);
+}
+};
+
+context.addRoleMapping(ROLE2, "very-complex-role-name");
Review comment:
That would OK, as long as it is an interface. I don't want to hardcore those
into my `context.xml`, but have them in a properties file. It could also be
potentionally a database or something else.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311490843
##
File path: java/org/apache/catalina/realm/RealmBase.java
##
@@ -928,6 +928,15 @@ public boolean hasRole(Wrapper wrapper, Principal
principal, String role) {
}
}
+// Check for a role alias/mapping defined on context level
+if (getContainer() instanceof Context) {
+Context context = (Context) getContainer();
+String realRole = context.findRoleMapping(role);
Review comment:
So, if I understand your request properly, this should not be part of
`RealmBase` at all, but has to be implemented in `StandardWrapper`, namely
`findSecurityReference()` whre it queries `getParent()#findRoleMapping() if it
is not found in `references`?.
So `findSecurityReferences()` have to be changed as well? But not
`removeSecurityReference()`?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole() URL: https://github.com/apache/tomcat/pull/186#discussion_r311483465 ## File path: webapps/docs/changelog.xml ## @@ -47,6 +47,10 @@ + +63636: Context.findRoleMapping() never called +in RealmBase#hasRole(). (michaelo) + Review comment: Pardon, I did not notice this. Will change, no issue. So this one should be after 63550? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole() URL: https://github.com/apache/tomcat/pull/186#discussion_r311483465 ## File path: webapps/docs/changelog.xml ## @@ -47,6 +47,10 @@ + +63636: Context.findRoleMapping() never called +in RealmBase#hasRole(). (michaelo) + Review comment: Pardon, I did not notice this. Will change, no 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole() URL: https://github.com/apache/tomcat/pull/186#discussion_r311483465 ## File path: webapps/docs/changelog.xml ## @@ -47,6 +47,10 @@ + +63636: Context.findRoleMapping() never called +in RealmBase#hasRole(). (michaelo) + Review comment: Pardo, I did not notice this. Will change, no 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #186: BZ 63636: Context#findRoleMapping() never called in RealmBase#hasRole()
michael-o commented on a change in pull request #186: BZ 63636:
Context#findRoleMapping() never called in RealmBase#hasRole()
URL: https://github.com/apache/tomcat/pull/186#discussion_r311466031
##
File path: test/org/apache/catalina/realm/TestRealmBase.java
##
@@ -789,4 +791,45 @@ public void testHttpConstraint() throws IOException {
Assert.assertFalse(mapRealm.hasResourcePermission(
request, response, constraintsDelete, null));
}
+
+@Test
+public void testRoleMapping() throws Exception {
+Context context = new TesterContext() {
+private Map roleMapping = new HashMap<>();
+
+public void addRoleMapping(String role, String link) {
+roleMapping.put(role, link);
+}
+
+@Override
+public String findRoleMapping(String role) {
+return roleMapping.get(role);
+}
+};
+
+context.addRoleMapping(ROLE2, "very-complex-role-name");
Review comment:
I use my fabulous
[`PropertiesRoleMappingListener`](http://mo-tomcat-ext.sourceforge.net/xref/net/sf/michaelo/tomcat/extras/listeners/PropertiesRoleMappingListener.html#PropertiesRoleMappingListener)
for this. I absolutely don't mind to amalgamate it into upstream somehow.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
