Add Serializable classes of model to be passed by GWT RPC

Bug #730086 reported by Scott Ritchie
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
MUGLE
Fix Released
Medium
Prageeth Silva

Bug Description

Write serializable versions of each of the model classes to be passed over by GWT RPC so they can be accessed by the client side of the platform; Should only have getters (and setters ?), and should only contain information the client side should be able to see (so there'll need to be 2 versions of some of the model classes according to the public/private nature denoted in the Platform View)

Related branches

Revision history for this message
Prageeth Silva (prageethsilva) wrote :

I've already started implementing these classes as part of the datastore wrapper.
This includes Services too. However, it may take a couple of days to get everything completely implemented and I'd commit all of it as soon as I'm done.

Changed in mugle:
assignee: nobody → Prageeth Silva (prageethsilva)
Changed in mugle:
milestone: none → 0.1.1
Revision history for this message
Matt Giuca (mgiuca) wrote :

I'm still a bit concerned about this approach. I talked to Prageeth about it yesterday.

Firstly, I am concerned that these "info" classes will duplicate a lot of code. You will need to specify all of the fields again in the info class as you do in the model classes, and if you need to change any fields, update both versions. As you outline in the bug description, you might need two versions of each class as well -- that implies a lot of duplication. Please try to avoid this if possible! Prageeth suggested using null to hide the data rather than a separate class.

My second concern is that there would be a security issue if any data was placed in an info class that shouldn't be accessed by the person that is receiving that class, even if the client code doesn't make it available. It doesn't seem tractable to manually write different versions of each class for different stakeholders -- e.g., a dev team may need to be visible to admins, lecturers, the team themselves, and other non-team members. That's four different access groups who may have four distinct sets of fields they can see. Any mistakes would be security vulnerabilities. A general solution might be required, where on the server side, the access level is used to construct the correct set of fields to put into the info class.

Revision history for this message
Prageeth Silva (prageethsilva) wrote :

I think I have taken care of most of the issues Matt is concerned about.

So here's a summary for my plan. First of all, i agree with the fact that there will be an exact copy of fields in both the model class and the info class. This means that when a new field is added or removed in the model class, it needs to be done in the info class as well.

About having different versions of classes; I was thinking of using null as Matt mentioned earlier. And this will all be done on the server side (by looking at the current login). I've already added the static methods that take in a model class instance and the role of the current logged in user; and it returns the the counterpart (info instance) with the desired fields been set to null. However, I have not completely implemented this yet. So I personally don't see a security issue since only the desired values are passed back to the client.

I hope this implementation would be better.

Revision history for this message
Matt Giuca (mgiuca) wrote :

> I personally don't see a security issue since only the desired values are passed back to the client.
Right. If only the desired values are passed back to the client, then there is no security *vulnerability*.

I say security "issue" since I would be concerned that if any one mistake was made in selecting which fields to set to null, then we would be leaking information to the client. In other words, it seems a bit too flimsy.

The main concern is just that the code for doing this will have to be written specially for each field of each class. I would feel better about it if there was some generic code which handles all the logic for selecting which fields to null out. Is there?

Revision history for this message
Prageeth Silva (prageethsilva) wrote :

Yes you were correct in saying that we would have to do it for every field manually. I can't seem to think of a way to get this done in a generic manner.

As I was suggesting yesterday, given the use of annotations work, this could still mean that we have to manually select which fields need to be hidden or not. This also takes us back to the same problem; any mistake in the selection could result in the same issue you mentioned above.

Anyhow, should I go ahead and implement what I already planned? Or should I wait till the decision is finalized?

Revision history for this message
Prageeth Silva (prageethsilva) wrote :

I was looking at creating custom annotations, that might come in handy to create this generic form.
The link below gives examples of how this can be used:

http://download.oracle.com/javase/1.5.0/docs/guide/language/annotations.html

I would think that we should have an annotation called the @UserLevel that takes in the Role enum (annotations are allowed to only have primitives, String, Class, enum and array of those types).

So depending on which Role it is, going from User -> Developer -> Admin, the info classes can be generated on the run, without us manually having to do this for every field.

If this seems to be a better approach, please let me know so that I can start implementing this.

Revision history for this message
Matt Giuca (mgiuca) wrote :

This should be marked fixed when Prageeth merges his data-wrapper branch.

Changed in mugle:
status: New → In Progress
Revision history for this message
Prageeth Silva (prageethsilva) wrote :

The branch has been merged with the trunk and everything compiles but GWT complains about java.io imports in shared or client packages. I've addressed that in https://bugs.launchpad.net/mugle/+bug/728148 and I'll look into this.

Changed in mugle:
status: In Progress → Fix Committed
Matt Giuca (mgiuca)
Changed in mugle:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.