Hey Tauren,
> Thanks for your response. I hope you had a good trip. I appreciate
> your additional feedback, as it does help to clarify things some more.
> I think I'm more on track now, but I'll give you some more concrete
> examples of what I'm doing in case you have any final suggestions for
> me.
the trip was fine, thank you. I read through your mail, and I have
only little advice left. Most of it are just thoughts on how I'd
change (not neccessarily improve) things.
> I'm still far from what you are suggesting, but I've actually already
> moved toward using business logic vs. directly keeping roles and
> permissions in the Member object. For instance, I now have these
> entities:
I don't think you're far away.
> class Member
> Set<Role> roles;
> Set<ProjectParticipant> participations;
>
> class Role
> String name
> Set<Permission> permissions;
>
> class ProjectParticipant
> boolean owner;
> boolean admin;
> boolean scheduler;
Maybe those classes need a little more polish. Depends on what they
are supposed to do. For my taste they are a little too close to the
physical design, i.e. how they are stored in the DB. But if you only
use them as DTOs that's absolutely OK, because that's the purpose of
the DTO.
(The fact that you referred to those classes as "entities" makes me
believe they are very close to the physical layout what is stored in
the DB.)
> And the following code in my Realm sets up Shiro permissions based on
> the business data:
>
> protected AuthorizationInfo
> doGetAuthorizationInfo(PrincipalCollection principals) {
> Long memberId = (Long)
> principals.fromRealm(getName()).iterator().next();
> Member member = memberService.getMember(memberId);
> if (member != null) {
> SimpleAuthorizationInfo info = new SimpleAuthorizationInfo();
> for (Role role : member.getRoles()) {
> info.addRole(role.getName());
> for (Permission perm : role.getPermissions()) {
> info.addStringPermission(perm.getPermissionString());
> }
> }
> for (ProjectParticipant pp : member.getParticipations()) {
> if (pp.isOwner()) {
>
> info.addStringPermission("project:delete:"+pp.getProject().getId());
> }
> if (pp.isOwner() || pp.isAdmin()) {
>
> info.addStringPermission("project:edit:"+pp.getProject().getId());
> }
> if (pp.isScheduler()) {
>
> info.addStringPermission("project:schedule:"+pp.getProject().getId());
> }
> }
> return info;
> } else {
> return null;
> }
> }
> �...@override
> public void clearCachedAuthorizationInfo(PrincipalCollection principals) {
> super.clearCachedAuthorizationInfo(principals);
> }
This actually looks very good to me. This way in your business logic
you can check for
currentSubject.hasPermission("project:edit:17") // [1]
instead of (I make this call up)
member.getParticipationForProject(project).isAdmin() // [2]
Brilliant! I haven't thought about that at all. Kudos to you!
So, as in call [1] your logic is nicely decoupled from the (more)
physical design, the classes from above are absolutely fine.
A last note: You surely know that with Shiro you can use several
realms simultaneously. Usually you have one realm per media/technology
(e.g. DB, LDAP...). An idea just popped up in my head. I don't know if
that is neccessarily a good idea (some more experienced developer
wants to chime in here?), but you could separate the "project
participation" related authorization into one realm and the general
role/permission based authorization into another realm.
The "project participation" implied permissions are tightly coupled to
business logic anyway. It will have to change if your security
implications governed by business logic change, e.g. if some day
admins are allowed to schedule, too. But if you separate this into its
own realm, you'd only have to touch this realm's code.
> Of course, there is still more to be done. For instance, I can see
> how I could remove the Role entity altogether as well and let the
> Shiro roles be determined by the business logic.
I think what you mean is what I tried to explain with the two parts
above. I am quite sure you're on the right track. You might want to
add an additional layer above those classes you presented earlier, to
remove the last bits of physical design that you don't need for your
business logic.
[...snip...]
> I'm thinking I'm not that far off from what you are suggesting, but
> perhaps you could confirm this. You mention that security permissions
> could be stored externally (LDAP, etc.). I suppose I could do this,
> but I'm not see the advantage. In my service layer, I when a change is
> made that requires a permission update, the permission information
> could be persisted to an LDAP server. Then my Realm would load the
> permissions from LDAP instead of from memberService. But for my needs
> at this time, that seems like adding an extra layer that I don't need,
> and just a way for data to get out-of-sync. So obviously I probably
> am missing a core concept in this regard.
You're not far off from my suggestions. And of course you are right:
If you have a single stand alone application, that does not need to
integrate with other IT infrastructure -- which is what I assume from
your explanations -- LDAP would mean that you have another
(independant) system to maintain and keep in sync etc. No gain at all.
I referred to LDAP only as an example. It helps me separating security
and business logic so I end up with loosely coupled subsystems. As I
said, it's a mind trick. If the application's design allowed for
switching to LDAP easily, without touching the classes making up the
business logic, that would be ideal. (Your code actually almost
fulfills that.)
> Thanks again for all your help! I'm much happier with the way I have
> things now than before, and much of it is due to your suggestions.
I'm glad I could help you out.
Cheers,
DJ