[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13004430#comment-13004430 ] Thomas Mueller commented on JCR-2910: - .isAdmin() is not the correct way to enforce security. So please add a new issue: remove User.isAdmin(). Personally, I believe isAdmin() is a good concept, because it's simple and easy to understand. There is a good reason why all operating systems support it. if we later decided to extend backup rights Such statements are a clear sign over YAGNI: http://en.wikipedia.org/wiki/You_ain%27t_gonna_need_it The main problem with the what if statements is that they don't reflect reality. If nobody ever requested the feature so far, then there is most likely no need for it in the future. Adding features in advance just because somebody might (!) want it in the future is a well known time waster. If we really add the feature in the future, then the easiest solution is to change it at that time. I believe in make things as simple as possible. Artificially adding complexity doesn't help anybody. It's just wasting time. Please add JackrabbitSession.isAdmin() -- Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13004454#comment-13004454 ] Jukka Zitting commented on JCR-2910: OK, fair enough. Since the feature is already as User.isAdmin() and if I understood correctly this functionality is only needed for a fairly limited set of uses, would a utility method as suggested by Felix be more appropriate here? Adding a new public API method just for convenience is fine if it saves thousands of lines of repetitive client code, but I'd rather put it in a shared utility method if we're talking about just a few dozens of instances where this functionality is used. Please add JackrabbitSession.isAdmin() -- Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13004458#comment-13004458 ] angela commented on JCR-2910: - as stated above i don't see any benefit in having JackrabbitSession.getUser(). Please add JackrabbitSession.isAdmin() -- Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13004463#comment-13004463 ] Felix Meschberger commented on JCR-2910: So please add a new issue: remove User.isAdmin(). +1 The isAdmin method makes no sense: What is the semantics a user being admin anyway ? Sounds like some wrong (or old outdated) solution which can now be better handled with real access control and user management. Personally, I believe isAdmin() is a good concept, because it's simple and easy to understand. There is a good reason why all operating systems support it. That's not really true. Core Unix, which is a methusalem in computer counting, has the concept of the single root user and many things check for uid==0. But over time better systems have been developed and nowadays unix also sports a privilege system which allows assigning rights for regular system administration tasks to regular users. Ok internally it still reverts to using root (probably) but this is an implementation detail. This is why I don't like the isAdmin() method, neither on the [Jackrbabbit]Session nor on the User. Rather start defining real-world usable permissions, which is nowadays possible and can be used. Please add JackrabbitSession.isAdmin() -- Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13004479#comment-13004479 ] Bart van der Schans commented on JCR-2910: -- I agree with Felix. Many attempts have been made to get rid of or restrict the root concept in unix/linux like rbsac, selinux grsecurity or even with sudo (iirc the root user in netbsd is even just another regular user by default) A user should just have the privileges/permissions to do something or not. Also the User.isAdmin() method is not really clear in it's meaning. Are you the user admin? Or belong to a group of admins? Or do you have some kind of admin privileges? Imo you should never check whether a user is admin, just check if the user has the appropriate privileges. Please add JackrabbitSession.isAdmin() -- Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13003902#comment-13003902 ] Felix Meschberger commented on JCR-2910: I am not sure about such a method. This sounds like a utility method making it into the API which smells wrong. Please add JackrabbitSession.isAdmin Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13003915#comment-13003915 ] Jukka Zitting commented on JCR-2910: What's your use case for knowing whether the session is an admin session? Normally the admin status of a session should only be visible to a client as full access to all repository content. If we do need such a check for admin status, I'd rather implement it as something like session.checkPermission(/, admin) instead of introducing a new method for this. Please add JackrabbitSession.isAdmin Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13003921#comment-13003921 ] Thomas Mueller commented on JCR-2910: - It's a security issue. Only admin users are allowed to backup the repository, or change the configuration using the GUI. I expect to use this method in quite a few places. Creating a utility method is possible of course, but it would just make things more complicated. something like session.checkPermission(/, admin) Which would be very ugly, don't you agree? SessionImpl.isAdmin() is already implemented, why make things more complicated than really necessary? If you like complicated, please explain *why*. Please add JackrabbitSession.isAdmin Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13003926#comment-13003926 ] Felix Meschberger commented on JCR-2910: What about JackrabbitSession.getUser()? Perfect. In addition this is orthogonal to Session.getUserID() Please add JackrabbitSession.isAdmin Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13003923#comment-13003923 ] Thomas Mueller commented on JCR-2910: - What about JackrabbitSession.getUser()? Please add JackrabbitSession.isAdmin Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13003931#comment-13003931 ] Dominique Pfister commented on JCR-2910: What about JackrabbitSession.getUser()? +1 Please add JackrabbitSession.isAdmin Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13003933#comment-13003933 ] Jukka Zitting commented on JCR-2910: casting to an implementation If your code works purely on the JCR API level, then the API should already enforce any access controls you need. Why do you then need the isAdmin() method? If you go below the JCR API, then you're already accessing implementation details and using SessionImpl.isAdmin() shouldn't be a problem. Please add JackrabbitSession.isAdmin Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13003935#comment-13003935 ] Thomas Mueller commented on JCR-2910: - The API we expose for these operations should simply throw exceptions if the user doesn't have enough access rights. That's exactly what I want to do. However, for this to implement, I need to know whether the current user has admin rights. Session.hasCapability() That's boolean Session.hasCapability(java.lang.String methodName, java.lang.Object target, java.lang.Object[] arguments) throws RepositoryException - which is even worse than Session.checkPermission(/, admin), well checkPermission isn't all that great because the second is a string property, and defined actions are add_node, set_property, remove, and read. As for the use case, I guess it's similar to find out if a Linux user is root. I would guess that's fairly easy in any API. If your code works purely on the JCR API level As I already wrote, I want to find out if the current logged in user is admin, because I want to backup the file system (just as an example). SessionImpl.isAdmin() shouldn't be a problem I repeat myself: it *is* a problem. One reason is that it doesn't work with virtual repositories. Casting to an implementation class is never really a good idea. So let's go for JackrabbitSession.getUser(). Please add JackrabbitSession.isAdmin Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13003940#comment-13003940 ] Jukka Zitting commented on JCR-2910: backup the file system [...] doesn't work with virtual repositories So you want a backup feature that should both work with virtual repositories (that could be backed by anything), but still want to have the backup feature work on the file system level. I don't think that's a good idea. What we should do instead, is to have something like JackrabbitSession.backup (or RepositoryManager.backup) that takes care of all the required low-level functionality. Then a virtual repository can just decide whether it wants to direct the backup() call to just one main repository or try to combine the backups from multiple backend repositories. The Jackrabbit implementation of such a method would be in SessionImpl.backup(), where accessing the SessionImpl.isAdmin() method is no problem. Please add JackrabbitSession.isAdmin() -- Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13003942#comment-13003942 ] angela commented on JCR-2910: - What about JackrabbitSession.getUser()? while i would not object to move the SessionImpl.isAdmin to the API, that new suggestion doesn't make any sense to me. i added the former method recently to SessionImpl as we used to have plenty of duplicate code in jackrabbit-core that retrieved the subject from the session and tested if it contained a AdminPrincipal... thus SessionImpl.isAdmin basically doesn't rely on user management being implemented (in fact it is an optional feature that can be turned off by using a different security configuration). having this in mind Session#getUser really doesn't add any benefit over retrieving the User from the UserManager. Please add JackrabbitSession.isAdmin() -- Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13003972#comment-13003972 ] Thomas Mueller commented on JCR-2910: - So you want a backup feature that should both work with virtual repositories No. But it might still be a virtual repository. Plus, as I already wrote, it's not just backup. There is a lot of code (most of it no my code) that needs this admin check, and it would be a lot of work to convert everything. From what I know, it's cluster configuration, data store garbage collection, index configuration, managing storage, managing workspaces, some diagnostic functions, data store import, some testing and profiling, some LDAP features. All those features currently check for admin rights in some way. Most of then either use getUserID().equals(admin) or cast to an implementation class. If you want to convert all that code, please feel free :-) SessionImpl.isAdmin OK, that means the following code wouldn't always work? (((User) js.getUserManager().getAuthorizable(session.getUserID())) Well, if the only reliable way is SessionImpl.isAdmin(), then let's add JackrabbitSession.isAdmin(). I agree with Felix, getUser() would be nice, and I don't understand how there could be no user. But JackrabbitSession.isAdmin() is OK as well. What is definitely *not* ok is: - casting to implementation, - or Session.getUserID().equals(admin), - or requiring to change a lot of code. Please add JackrabbitSession.isAdmin() -- Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[
https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13003987#comment-13003987
]
Jukka Zitting commented on JCR-2910:
a lot of code
That's not a good reason to introduce flawed design. The issue description
already outlines two workarounds that currently work with three or just one
line of code. They have similarly problems as the proposed isAdmin() method,
but at least they don't require changes to public API. As a longer term
solution we should get rid of such sloppy administration features and make them
proper parts of the repository implementation.
What I'm trying to avoid here is encouraging client code like this:
if (session.isAdmin()) {
doSomethingThatOnlyAdminsAreAllowedToDo();
}
It's too easy to accidentally or on purpose forget the isAdmin() call from
above, in which case you have an instant security issue.
The above code is perfectly fine within the implementation (and is also how our
normal access controls fundamentally work), but should not be used at the
client level. To borrow the Unix example, no Unix system allows a backup
program to access all system data just on the assumption that the program
should first check the geteuid() return value.
Please add JackrabbitSession.isAdmin()
--
Key: JCR-2910
URL: https://issues.apache.org/jira/browse/JCR-2910
Project: Jackrabbit Content Repository
Issue Type: Improvement
Reporter: Thomas Mueller
Priority: Minor
Currently finding out if the session user is an admin requires:
JackrabbitSession js = (JackrabbitSession) session;
User user = ((User) js.getUserManager().getAuthorizable(session.getUserID()));
boolean isAdmin = user.isAdmin();
Or: ((SessionImpl) session).isAdmin(). However casting to an implementation
is problematic for several reasons.
I think it would make sense to add isAdmin() to the JackrabbitSession
interface, so the code above would be:
((JackrabbitSession) session).isAdmin()
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[
https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13004015#comment-13004015
]
Thomas Mueller commented on JCR-2910:
-
flawed design
isAdmin() simply is not flawed design. Within Unix systems, there is a very
simple way to find out if the user is root.
encouraging client code... if (session.isAdmin()) {
So, basically you admit that the only reason for you to be against this feature
is to make my work harder? :-)
already outlines two workarounds
I already gave you the reasons why the workarounds are not acceptable.
Acceptable solutions are JackrabbitSession.getUser() (actually I would prefer
that), or JackrabbitSession.isAdmin(). I fail to see any reason why this would
be flawed design.
Please add JackrabbitSession.isAdmin()
--
Key: JCR-2910
URL: https://issues.apache.org/jira/browse/JCR-2910
Project: Jackrabbit Content Repository
Issue Type: Improvement
Reporter: Thomas Mueller
Priority: Minor
Currently finding out if the session user is an admin requires:
JackrabbitSession js = (JackrabbitSession) session;
User user = ((User) js.getUserManager().getAuthorizable(session.getUserID()));
boolean isAdmin = user.isAdmin();
Or: ((SessionImpl) session).isAdmin(). However casting to an implementation
is problematic for several reasons.
I think it would make sense to add isAdmin() to the JackrabbitSession
interface, so the code above would be:
((JackrabbitSession) session).isAdmin()
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13004084#comment-13004084 ] Jukka Zitting commented on JCR-2910: Within Unix systems, there is a very simple way to find out if the user is root. That's just because Unix defines the special user id zero as root. The equivalent solution for us would be to define admin as the one and only administrator account. the only reason for you to be against this feature is to make my work harder? :-) :-) In the short term perhaps, but I strongly believe that a solution like the one proposed leads to more work in the long run. For example, what if we later decided to extend backup rights also to some special system account that only needs read access to everything but should never be able to write anything? This is fairly common in many environments, but would be impossible to implement if our backup tools relied on an isAdmin() method. However, a backup() API method could easily be modified to do this. Similarly, having administrative operations behind an API makes it possible to easily access that functionality through not just multiple different user interfaces (including JMX) but also from automated tests or things like scheduled backups. Please add JackrabbitSession.isAdmin() -- Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13004112#comment-13004112 ] Tobias Bocanegra commented on JCR-2910: --- i agree with jukka. Session.isAdmin() is not the correct way to enforce security since it's not extendable. basically, all resources and services need to be subject to access control. i'd rather add the respective policies like (rep:backup) and check against them than a vague isAdmin() check. Please add JackrabbitSession.isAdmin() -- Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (JCR-2910) Please add JackrabbitSession.isAdmin()
[ https://issues.apache.org/jira/browse/JCR-2910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13004208#comment-13004208 ] Alexander Klimetschek commented on JCR-2910: I agree with Jukka Tobi. isAdmin() is too much authentication only (just a user identification) and not much of an authorization privilege (only a very generic one, playing god). But the use case clearly is focused around authorization, and might need new privileges as the right solution. Please add JackrabbitSession.isAdmin() -- Key: JCR-2910 URL: https://issues.apache.org/jira/browse/JCR-2910 Project: Jackrabbit Content Repository Issue Type: Improvement Reporter: Thomas Mueller Priority: Minor Currently finding out if the session user is an admin requires: JackrabbitSession js = (JackrabbitSession) session; User user = ((User) js.getUserManager().getAuthorizable(session.getUserID())); boolean isAdmin = user.isAdmin(); Or: ((SessionImpl) session).isAdmin(). However casting to an implementation is problematic for several reasons. I think it would make sense to add isAdmin() to the JackrabbitSession interface, so the code above would be: ((JackrabbitSession) session).isAdmin() -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
