Codebase inconsistencies with handling of ADMIN_ROLE and GROUP_ADMIN_ROLE
Description
The codebase has some inconsistencies from an earlier GeoServer 1.x migration around the handling of ADMIN_ROLE and GROUP_ADMIN_ROLE.
GeoServerRole.java defines POJO GeoServerRole with constants for the built-in ADMIN_ROLE, GROUP_ADMIN_ROLE, AUTHENTICATED_ROLE, ANY_ROLE, ANONYMOUS_ROLE. However the String values associated with these constants do not not match their name; making use error prone and confusing.
rest.properties example is written with its own set of constants ADMIN, IS_AUTHENTICATED_ANONYMOUSLY,
XML Role Service allows assigning a built-in role to an external role; so that an LDAP Manager can be recognized as having GeoServerRole GROUP_ADMIN_ROLE
Ideas for cleanup:
Many places in the code use either one or the other role (some smart ones, both)
Centralize the check that verifies both in a prominent place
Deprecate one of the two constants, make sure every admin check goes for the new method checking both
Remove the old one from the default geoserver data directory
Lowest
The codebase has some inconsistencies from an earlier GeoServer 1.x migration around the handling of ADMIN_ROLE and GROUP_ADMIN_ROLE.
GeoServerRole.java defines POJO GeoServerRole with constants for the built-in ADMIN_ROLE, GROUP_ADMIN_ROLE, AUTHENTICATED_ROLE, ANY_ROLE, ANONYMOUS_ROLE. However the String values associated with these constants do not not match their name; making use error prone and confusing.
rest.properties example is written with its own set of constants ADMIN, IS_AUTHENTICATED_ANONYMOUSLY,
AbstractGeoServerSecurityService.java defines string constants default, ADMIN, GROUP_ADMIN
XML Role Service allows assigning a built-in role to an external role; so that an LDAP Manager can be recognized as having GeoServerRole GROUP_ADMIN_ROLE
Ideas for cleanup:
Many places in the code use either one or the other role (some smart ones, both)
Centralize the check that verifies both in a prominent place
Deprecate one of the two constants, make sure every admin check goes for the new method checking both
Remove the old one from the default geoserver data directory