massbay-cs / cs225-proj4

https://gitter.im/massbay-cs
Other
0 stars 2 forks source link

Add checkPermission to getters/setters for users table #78

Closed Zenexer closed 8 years ago

Zenexer commented 8 years ago

We need to ensure that the current user has permission to view/modify/create/delete before reading/writing from the database. This will be enforced with auth.Permissions#checkPermission. A call to checkPermission is added to each method; if the user doesn't have permission for the operation, checkPermission throws an auth.AuthorizationException.

There are descriptive Javadoc comments in the auth.Permissions class. Be sure to read them before starting.

Important: This needs to be done on the authorization branch.

Here are a few examples:

Deleting a user

public class UserData_Table extends InitDB implements Interface_UserData {
    ...
    @Override
    // AuthorizationException has been added to the "throws" list
    public void removeUser(int uid) throws DoesNotExistException, AuthorizationException {
        String table = "USERS";
        // The field argument is null because we're operating on an entire row.
        // We pass uid as the last argument because this operation affects another user.
        Permissions.get().checkPermission(table, null, Operation.DELETE, uid);
        ...
    }
    ...
}
public interface Interface_UserData {
    ...
    // AuthorizationException has been added to the "throws" list
    public void removeUser(int uid) throws DoesNotExistException, AuthorizationException;
    ...
}

Setting a task description

public class Tasks_Table extends InitDB implements Interface_TaskData {
    ...
    // AuthorizationException has been added to the "throws" list
    public void setDescription(int uid, String description) throws DoesNotExistException, AuthorizationException {
        // Note that we DON'T pass uid; uid is just the ID of the current user in this case.
        Permissions.get().checkPermission(tableName, "DESCRIPTION", Operation.MODIFY);
        setDBString("DESCRIPTION", tableName, uid, description);
    }
    ...
}
public interface Interface_TaskData {
    ...
    // AuthorizationException has been added to the "throws" list
    public void setDescription(int uid, String description) throws DoesNotExistException, AuthorizationException;
    ...
}

Viewing a task description

public class Tasks_Table extends InitDB implements Interface_TaskData {
    ...
    // AuthorizationException has been added to the "throws" list
    public String getDescription(int uid) throws DoesNotExistException, AuthorizationException {
        // Note that we DON'T pass uid; uid is just the ID of the current user in this case.
        Permissions.get().checkPermission(tableName, "DESCRIPTION", Operation.VIEW);
        return getDBString("DESCRIPTION", tableName, uid, description);
    }
    ...
}
public interface Interface_TaskData {
    ...
    // AuthorizationException has been added to the "throws" list
    public String getDescription(int uid) throws DoesNotExistException, AuthorizationException;
    ...
}
wconner commented 8 years ago

Couple of questions:

There is a setAddress method which takes in an Address Object as its parameter. Should I set the privilege the same as creating a user?

Same thing for queryEntireTable method which returns a String list of all users and their info (I removed password from this list).

Should we remove the get and set password methods

Zenexer commented 8 years ago

Does that method create a user? If not, then you shouldn't use the same method invocation. All it does is call other setters, all of which will have their own permission checks, so you don't need a permission check for that method.

queryEntireTable should run checkPermission with Operation.VIEW for every field that it reads, excluding UID. To make matters more complicated, it's also going to need to call each of those methods for every UID that it reads. It will then have to catch any exceptions and exclude the affected row from the result. For example: try { /* Run permission checks */ } catch (AuthorizationException ex) { continue; } Before you invest any time in this, make sure the method is actually used. It's quite likely that it's just a debug method that has no use in the current application, especially since it calls System.exit(1) if it fails.

How are users going to set their passwords if you remove setPassword? How will admins reset passwords for users who forgot their passwords? As for getPassword, the whole password system needs to be replaced with a proper hashing system. That's a separate task. For now, code it the same as you would any other method.

wconner commented 8 years ago

queryEntireTable has 2 uses, 1 in a test, and the other in a class called printInfo which has no methods or constructors just a public static void main(String args[]) were they call queryEntireTable on all of the different tables, so like you said I think they're using this for debugging purposes only.

I've put PWD back into the queryEntireTable method and have set password authorizations, thanks for the help