Implement support to manage permissions for external authentication #194
Conversation
mbarto
commented
Oct 21, 2019
mbarto
commented
- changes to db security table to add explicit fields for username and groupname (to be used instead of ids when authentication is externalized)
- centralized security stuff in SecurityDAO, so that we can plug a new implementation for external authentication
- removed some dependencies from user / groups id where not needed
- introduced ExternalSecurityDAO implementation
…l authentication seamless support
| // SecurityRule security = new SecurityRule(); | ||
| // security.setCanRead(true); | ||
| // security.setCanWrite(true); | ||
| // security.setCategory(category); | ||
| // security.setResource(resource); | ||
| // | ||
| // // /////////////////////////////////////////////////////// | ||
| // // The Exception should be trapped because only one | ||
| // // between resource and category can be specified | ||
| // // /////////////////////////////////////////////////////// | ||
| // try{ | ||
| // securityDAO.persist(security); | ||
| // fail("Exception not trapped"); | ||
| // } catch(Exception exc) { | ||
| // if(LOGGER.isDebugEnabled()){ | ||
| // LOGGER.debug("OK: exception trapped", exc); | ||
| // } | ||
| // } |
There was a problem hiding this comment.
I think you can remove this
| @@ -906,14 +906,6 @@ public void updateSecurityRules(long id, List<SecurityRule> rules) | |||
| // insert new rules | |||
| for (SecurityRule rule : rules) { | |||
| rule.setResource(resource); | |||
| //Retrieve from db the entity usergroup, if the securityrule is related to a group | |||
There was a problem hiding this comment.
I see this rule is a little strange, so probably this is the reason why you deleted it.
I'm afraid this was made to allow this method to work when the rule comes from a ReST API request, so the group is not from the DB but from the client, therefore it has only the ID set.
About this I think (as far as I remember ) there was some tests made using client, that are not automated. There should be a way to execute them. Can you give it a try?
| @@ -82,6 +86,14 @@ protected void authenticate(HttpServletRequest req) { | |||
| } | |||
|
|
|||
| } | |||
| if (!everyoneFound && addEveryOneGroup) { | |||
There was a problem hiding this comment.
I think this is because headers may not contain the everyone group but the geostore model require it to share public resources, isn't it?
Does it work even for guest user?
Can you please explain a little bit this in a comment
… exception if it doesn't exist
