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

Protect by id and type of entity #186

Merged
merged 3 commits into from
May 11, 2016

Conversation

buehner
Copy link
Member

@buehner buehner commented May 10, 2016

This PR introduces an implementation of the

public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, Object permission)

which is based on the ID and the type of a target entity.

The implementation provided here searches for a matching DAO, retrieves the entity from the database and simply calls the (existing) implementation of hasPermission with the object.

Two methods in the MapService and the LayerGroupService now make use of the new implementation.

Please review

@weskamm
Copy link
Member

weskamm commented May 10, 2016

Although the title seems to be missing something, this PR adds a nice new flexibility!
I like it, 👍

}

// if we could not find an exact match, we'll try to use the "next best"
// from the entity hierarchy
Copy link
Member

Choose a reason for hiding this comment

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

This will possibly give you true for a class that isn't the best choice, right?

Consider:

A.java (With a matching DAO)
⇧
B.java (With a matching DAO)
⇧
C.java (With a matching DAO)
⇧
D.java (No DAO)

If entityClass is an instance of D, we'll drop here, as we do not have a direct dao-match. The best bet for a dao would be the C-Dao, but when we iterate, isAssignableFrom will be true for any of the parent classes (btw. their daos). We might end up with the not-well-suited A-Dao, for example.

Did I understand this correctly? Amnd if so, can getSuperclass() help? We could then factor ot the doa finding into a small methdi that we could call reapeatedly unto i we actually have the best bet. What do you think?

@marcjansen
Copy link
Member

Pleas give this PR a better title and also please take the time to think about the longer remark I added to the diff.

Regardless of whether you take the suggestion, this is good to go IMO. Thanks @buehner. Please merge at will (taking or ignoring my suggestion).

@buehner buehner changed the title Protect by type and Protect by id and type of entity May 10, 2016
@buehner
Copy link
Member Author

buehner commented May 11, 2016

Yes. You're right @marcjansen

But this will probably never be a "real" problem as we only need a DAO to call findById (which will most likely always be the same implementation from the GenericHibernateDao).

So i will merge this PR now and open a new issue to adress your proposal.

@buehner
Copy link
Member Author

buehner commented May 11, 2016

I just opened #187

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

3 participants