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

Reply via email to