Hi Daniel,
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.
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:
class Member
Set<Role> roles;
Set<ProjectParticipant> participations;
class Role
String name
Set<Permission> permissions;
class ProjectParticipant
boolean owner;
boolean admin;
boolean scheduler;
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);
}
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.
Also, in my service layer methods, which are @Transactional via
Spring, I execute the following whenever a change to the data
structure will result in a permission change being required (realm has
been injected via Spring):
public void createProject(Project project) {
log.debug("Creating project: "+project);
dao.save(project);
log.debug("Adding project particaption to person for:
"+project);
ProjectParticipant p= new ProjectParticipant();
p.setCreated(new Date());
p.setMember(member);
p.setProject(project);
p.setOwner(true);
member.getPerson().addParticipation(p);
memberService.updateMember(member);
// Clear and refresh Shiro permissions
realm.clearCachedAuthorizationInfo(SecurityUtils.getSubject().getPrincipals());
}
In fact, I've also got things working so that I can use a
SchedulerParticipantDetails instead of a boolean for
ProjectParticipant.scheduler, and plan to do the same with owner and
admin:
class ProjectParticipantDetails
Certificate certificate
Rating rating
For my application, each project requires that the scheduler have a
certificate that qualifies them to be a scheduler. Different projects
have different certificate requirements, so the Project also defines
which type of Certification is required. In addition, the project
owner or admin can "rate" the scheduler on their performance. The
rating for a scheduler can be different from project to project, so
this seemed like a good place to keep it.
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.
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.
Tauren
On Thu, Jul 23, 2009 at 3:16 AM, Daniel J. Lauk<[email protected]> wrote:
> Hey Tauren!
>
> Sorry for the quite long delay. I've been out of country for a while.
> I'll keep my answers (inline) short, as I am 1) short on time and 2)
> not sure if you still need it (working through a lot of emails after
> my trip right now).
>
>> I'm not actually using that code, I just borrowed it as an example
>> from the getting started page on the shiro site to make my application
>> easier to describe. My User object (called Member) isn't combined
> [...snip...]
>> I agree that your implementation is more elegant than the sample from
>> the Shiro getting started page. Again, I do this differently than the
>> example I provided. For instance, I have a panel in my app that
> [...snip...]
>
> Oh, OK. I see.
> No offense meant in any way, but if you don't provide examples that
> actually are based on your code, how can the mailing list folks give
> you good advise? ;-)
>
> [----8<--------8<--------8<--------8<--------8<--------8<----]
>
>>> I see. The problem here is, that some of your application logic
>>> implies access privileges.
>>> I'd store e.g. the project manager in the application model, and take
>>> care for some work flow to always change the application model's data
>>> with the security realms' data in a transactional way.
>>
>> I'm not totally clear on what you mean.
>
> I think I have not expressed it as clear as I should have. I guess
> this would have spared some of the discussion below. I'll just skip
> most of it as I think it makes no sense to comment on it regarding you
> got me wrong in the first place.
>
> [...]
>>> To sum up my advices: Separate your application logic and model from
>>> the security management.
>>> Ask yourself: Does the application really need to know who has which
>>> roles/permissions? Isn't it enough, if it can rely that certain
>>> actions will be prohibited if the issuer of the action is not
>>> privileged to do so?
>>
>> The interaction between the application logic/hibernate queries and
>> shiro is really what I'm mainly confused about. The applications I've
>> built in the past did not require anything more than role based
>> security. I was able to encode into my hibernate queries all of the
>> logic to grab the exact datasets from the database that I wanted to
>> display.
> [...]
>> So even though my application domain model contains the role and
>> permission information, you are suggesting that I do not program the
>> application logic to directly query User.getRoles or
>> User.getPermissions. This makes sense now!
>>
>> However, I'm still confused on how to display paginated lists of data
>> from hibernate. What I suggested above (having hibernate queries
>> include User.permissions like("%value%") clauses) is making my
>> hibernate queries do just that. It is not using shiro for security,
>> but bypassing it and querying the domain model directly.
>
> OK, so I'll finally try my best to resolve all this confusion. Also, I
> think I understood your requirement for hibernate queries now. I'll
> get to that in a second.
>
> I'll state it very explicit again: Separate your application logic and
> model from the security management.
> If you store roles and permissions together with your application
> domain model, you obiously don't separate them. The point of issue is
> that your User class exposes the interfaces "getRoles" and
> "getPermissions". It doesn't matter that your business logic makes no
> use of them. This is why I wrote the part with "Ask yourself: Does the
> application really need to know who has which roles/permissions?". If
> your classes expose the interface, then subtly you actually want to
> answer this question with "yes".
>
> Now back to queries. To make it very plain (using pseudo SQL as I am
> too lazy to fetch the hibernate book from the shelf):
>
> Bad version (1):
> select p.id from projects p left join project_permission perm on
> perm.project_id = p.id where perm.action = "schedule" and perm.user_id
> = ?
>
> Good version (2):
> select p.id from projects p left join project_schedulers ps on
> ps.project_id = p.id where ps.user_id = ?
>
> The code in (2) does not refer to any permission or role or what ever.
> It stays in the business domain. To do that there is an additional
> relation needed -- I named it "project schedulers". But that's still
> business logic.
> What helps me separate the security stuff from my business logic is
> the following mind trick: I just imagine, all security information is
> stored outside the DB, e.g. in a directory accessed via LDAP.
> Consider: If you take the permissions out of the DB in (1) you'll need
> to change (1) because the code touches more than one concern. If you
> decide to separate the growing project_permissions table into multiple
> tables (e.g. schedule_project_permissions), you'd have to change code
> (1) as well.
> In (2) the code knows absolutely nothing about security and therefore
> if you swap or modify your realms your app still works. Also you can
> unit test this part now without the security infrastructure in place.
> Yeay! :-)
>
> "But" I hear you say, "I still need to assign a permission, right?"
> Yes. Absolutely. But how that works is up to the realm that you're
> using. The confusion arises, as your realm is also using the DB
> (accessed via hibernate). This brings me back to the comment from far
> above: "[You'll need] some work flow to always change the application
> model's data with the security realms' data in a transactional way".
> It's neither enough if you add a permission for the user, nor enough
> if you add an entry with the "project schedulers". You'll need to do
> both. And to support ACID you should wrap all of that in some kind of
> transaction.
>
> Don't let yourself be fooled by anyone counting LOC, classes, tables
> etc.. Good* (i.e. clean/maintainable) code may need well more LOC.
> That's why we don't call it "minimal", but "maintainable" ;-)
>
> *) Good is relative of course. With an embedded realtime system you'd
> apply different measures. In that case size usually does matter. A
> lot. :-)
>
> Cheers,
> DJ
>