search

icfnext / aem-groovy-console

The AEM Groovy Console provides an interface for running Groovy scripts in the AEM container. Scripts can be created to manipulate content in the JCR, call OSGi services, or execute arbitrary code using the CQ, Sling, or JCR APIs.
Other
159 stars 94 forks source link

After #90, always allow admin change is broken #98

Closed paul-bjorkstrand closed 4 years ago

paul-bjorkstrand commented 4 years ago

The change in #90 making it secure by default breaks the change always allowing admin.

hasPermission = allowedGroups ? user.admin || memberOfGroupIds.intersect(allowedGroups as Iterable) : false

needs to change to

hasPermission = user.admin || (allowedGroups ? memberOfGroupIds.intersect(allowedGroups as Iterable) : false)

PR coming soon.

markdaugherty commented 4 years ago

Hi, this change was intentional to make the tool secure by default.

paul-bjorkstrand commented 4 years ago

So, it was intentional to remove the ability for even the admin user (not an adminstrator, but the admin account itself) from being able to access when the groups are empty? That isn't security, IMO.

If you are blocking the admin user (aka the one who can do anything on the server, regardless how it is restricted by permissions) then you are removing an avenue to allow only the admin access without creating a special group for the admin account.

That sounds overkill to me. The admin, in every other case within AEM, can do anything, and has special privileges to access anything. even within jackrabbit itself (examples 1 and 2).

Ostensibly, the groovy console is the only important/useful thing this add-on installs. Forcing a user (who has admin privileges to install it in the first place) to go through such a boilerplate hoop to get it working OOTB is silly.

Please note; I am not advocating for the removal of the configuration that secures it, and leaving the default false for all other users (aka not the admin) makes perfect sense. All I am advocating is "secure by default, which means only the admin can do it without configuration".

paul-bjorkstrand commented 4 years ago

Additional note:

If you decide to not change the behavior of the console, then you may want to at least consider documenting the fact that is is entirely unusable unless at least one group name is set in the config.

markdaugherty commented 4 years ago

Agreed, thanks for your input on this.