Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Created AuthorizationHandler interface. #7

Closed
wants to merge 10 commits into from

Conversation

mohideen
Copy link
Contributor

No description provided.

@mohideen
Copy link
Contributor Author

I will update the return type of getACLforType after @whikloj updates #4 with the new interface name.

@whikloj
Copy link
Member

whikloj commented Aug 26, 2015

@mohideen, PR #4 is updated. cheers.

* @param objectType
* @return List of ACL objects for the objectType
*/
List<WebACAuthorization> getAuthorizationforType(final RDF objectType);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the WebACAuthorization class exist?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the interface I am building. So it will or some implementation of it at least.

@mohideen mohideen changed the title Created AccessToClassHandler interface. Created AuthorizationHandler interface. Aug 27, 2015
* @param aclPath Path of acl to get authorizations.
* @param resourcePath Path of the resource requested by user. (ACL: accessTo)
* @param objectTypes rdf:type values of the resource. (ACL: accessToClass)
* @param agent (user or group) (ACL: agent)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will you determine whether this is an agent (or agents) and therefore use acl:agent or a group (or groups) and therefore use acl:agentClass?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we decided to use acl:agent for both users and groups.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but we also probably need to keep the userId separate from the groupIds... in order to facilitate prioritization in determining the effective Authorization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would there be any distinction between an user and a group in the acl:Authorization object?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently they are retrieved by use of different methods getAgents and getAgentClasses.
_edit_
but that was assuming they would be stored in the Authorization differently, if they are all in the ACL with acl:agent I should probably update that.
The difference will be that an agent is a string username, and an agentClass is a URI that resolves to a list of agents

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@whikloj, acl:agentClass is very different from our String-based notion of "groups".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @awoods, I must have dozed off. So we are using Strings for both agents and agentClasses? Then I also am wondering how I can tell if a

<> acl:agent "Jim" 

is an username or group called "Jim"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@whikloj, I don't think it matters. What matters is if "Jim" is the requesting user's userId or one of their groupIds.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, then I am dumping the WebACAuthorization.getAgentClasses() as it will be irrelevant.

@peichman-umd
Copy link
Contributor

Just a point of clarification, are the add/replace/removeAuthorizations methods intended to be used for the editing of ACLs?

@mohideen
Copy link
Contributor Author

Yes, to manage acl:Authorization nodes in a given ex:Acl.

@peichman-umd
Copy link
Contributor

@mohideen I was under the impression that since we were making the acl:Authorization nodes LDP children under the ex:Acl container, that we would use the usual LDP methods for editing them (PUT to add and replace, DELETE to remove). In that case, do we need specialized methods in the AuthorizationHandler?

@mohideen
Copy link
Contributor Author

I was thinking about these in the context of UI. We might not need these then.

@awoods
Copy link
Member

awoods commented Aug 27, 2015

Agreed, @peichman-umd and @mohideen. We will likely be using straight LDP to update Authorizations, not object setters.

@peichman-umd
Copy link
Contributor

@mohideen, can you clarify the different purposes of the two getAuthorizations() methods in their Javadocs?

@mohideen
Copy link
Contributor Author

Wasn't sure if the delegate OR AuthorizationHandler will be implementing the logic for traversing up the tree to find if an ACL exists.

@mohideen
Copy link
Contributor Author

First method assumes it happens in delegate, second assume in AuthHandler. We will only need one.

@peichman-umd
Copy link
Contributor

My recollection from yesterday's discussion and from the Google doc work is that the is the delegate that would be doing the initial tree traversal to find the applicable ACL. @awoods, @acoburn, @whikloj, does that sound right?

@whikloj
Copy link
Member

whikloj commented Aug 27, 2015

I thought we just wanted the delegate to make the final decision with all relevant data, which puts it into the AuthHandler. But I don't know if we had a final decision on that.

@awoods
Copy link
Member

awoods commented Aug 27, 2015

Like @acoburn has mentioned, it would be ideal to keep jcr.Sessions (to the extent possible) from leaking outside of the Delegate.

@whikloj
Copy link
Member

whikloj commented Aug 27, 2015

Ok, so the Delegate checks for the ex:hasAcl and then passes that resource to the AuthHandler.

@awoods
Copy link
Member

awoods commented Aug 27, 2015

@whikloj, sounds good.

return null;
}

public Set<WebACAuthorization> getAuthorizations(FedoraResource resource, String agent) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method can be removed as well.



/**
* Get authorizations that have both accessTo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to understand this comment: "...that have both accessTo [and what?]"

@whikloj
Copy link
Member

whikloj commented Sep 4, 2015

I think this should be closed due to work being done @acoburn

@acoburn
Copy link
Contributor

acoburn commented Sep 4, 2015

We can close this in favor of #8

@acoburn acoburn closed this Sep 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants