[
https://jira.duraspace.org/browse/DS-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=24893#comment-24893
]
Mark Diggory commented on DS-658:
---------------------------------
Still reviewing this, feedback for the whole team.
Authorization shouldn't be in the Domain Model, it should be part of the Domain
itself meaning that the application should be responsible for checking if the
user can execute and action before doing it, not the data model. In this
regard, exposing these methods on the data model is poor form in the first
place.
We would not be adding these methods in the "UI" as suggested above, they would
be part of the Common Application Business Tier codebase. This would be
utilized to evaluate cases where the "Subject" has "Rights" to edit and would
give a much more powerful set of tools for evaluating rights to do specific
actions in dspace than would be present in these methods.
I think we need to get back to the "who" is executing the above activity and
you'll see what I'm getting at.
EPerson.canEdit(item) vs Item.canEdit()
now you know the "who" and we do not need this crazy complexity of embedding
authorization into the middle of dspace object, it can be placed in the
application tier and not embedded in the Domain Model. For instance, lets take
the cases where authorization "needs to be turned off" to accomplish a task
inside the DSpace object.
https://github.com/DSpace/DSpace/blob/master/dspace-api/src/main/java/org/dspace/content/Item.java#L184
static Item create(Context context) throws SQLException, AuthorizeException
{
TableRow row = DatabaseManager.create(context, "item");
Item i = new Item(context, row);
// Call update to give the item a last modified date. OK this isn't
// amazingly efficient but creates don't happen that often.
context.turnOffAuthorisationSystem();
i.update();
context.restoreAuthSystemState();
context.addEvent(new Event(Event.CREATE, Constants.ITEM, i.getID(),
null));
log.info(LogManager.getHeader(context, "create_item", "item_id="
+ row.getIntColumn("item_id")));
return i;
}
And then:
https://github.com/DSpace/DSpace/blob/master/dspace-api/src/main/java/org/dspace/content/Item.java#L1475
public void update() throws SQLException, AuthorizeException
{
// Check authorisation
// only do write authorization if user is not an editor
if (!canEdit())
{
AuthorizeManager.authorizeAction(ourContext, this, Constants.WRITE);
}
log.info(LogManager.getHeader(ourContext, "update_item", "item_id="
+ getID()));
// Set sequence IDs for bitstreams in item
int sequence = 0;
Bundle[] bunds = getBundles();
Instead if this had stayed up in the domain and not embedded in the model there
would be no reason for doing silly things like the hacks we created to turn
on/off the authorization system. If you wanted it in the domain model, the
EPerson is probably the most appropriate. but even then, we still want to see
the AuthorizationManager as the Domain tool used to do this.
if(AuthorizeManager.authorizeAction(ourContext, item, Constants.WRITE))
{
item.update();
}
or more the convenience your seeking in a domain model object...
if(eperson.can(Constants.WRITE, item))
{
item.update();
}
or maybe even
if(context.can(Constants.WRITE, item))
{
item.update();
}
and
public void update() throws SQLException, AuthorizeException
{
// Check authorisation
// only do write authorization if user is not an editor
// NOT NEEDED if (!canEdit())
// NOT NEEDED {
// NOT NEEDED AuthorizeManager.authorizeAction(ourContext, this,
Constants.WRITE);
// NOT NEEDED }
log.info(LogManager.getHeader(ourContext, "update_item", "item_id="
+ getID()));
// Set sequence IDs for bitstreams in item
int sequence = 0;
Bundle[] bunds = getBundles();
and
static Item create(Context context) throws SQLException, AuthorizeException
{
TableRow row = DatabaseManager.create(context, "item");
Item i = new Item(context, row);
// Call update to give the item a last modified date. OK this isn't
// amazingly efficient but creates don't happen that often.
// NOT NEEDED context.turnOffAuthorisationSystem();
i.update();
// NOT NEEDED context.restoreAuthSystemState();
context.addEvent(new Event(Event.CREATE, Constants.ITEM, i.getID(),
null));
log.info(LogManager.getHeader(context, "create_item", "item_id="
+ row.getIntColumn("item_id")));
return i;
}
Sorry, using the Item object for these conveniences was probably wrong from the
get go... I understand the intent and the need. But its not a good
encapsulation.
> Provide a "nonAnon" attribute to XMLUI theme
> --------------------------------------------
>
> Key: DS-658
> URL: https://jira.duraspace.org/browse/DS-658
> Project: DSpace
> Issue Type: New Feature
> Components: XMLUI
> Affects Versions: 1.6.2
> Reporter: S Ottenhoff
> Priority: Major
> Attachments: DS-658.patch
>
>
> Use case: institution has decided that all information about student theses
> must be hidden from anonymous users. This includes author name, title, etc.
> This is fairly easy to implement in the XMLUI. The XMLUI theme needs one
> additional variable called "nonAnon" (boolean on item.canView()).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://jira.duraspace.org/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Dspace-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/dspace-devel
