Clean up the lib/WeBWorK/Authz.pm file.

[drgrice1]

This is primarily just code clean up, but there are a few of notable changes to the code.

First, the setCachedUser method no longer attempts to look up the database permission record twice in the event that an exception occurs on the first lookup. That was rather stupid code. If an exception was thrown the first time, it is most likely going to be thrown again the second time. I think the reason this was done was because it used to be the case that when the database getPermissionLevel method was called and a record in the database did not exist, the record would be created. However, that is no longer the case, and even if it were that would not throw an exception and would return the permission record on the first call. So just plain stupid code here.

Second, the setCachedUser method no longer refuses to cache the user if it detects that the request is a LTI 1.1 authentication request. That should not have been done, and results in extra database calls for any LTI 1.1 authentication request. In addition the method does not croak if the oauth_nonce parameter is detected and the lis_person_sourced_id parameter (or one of its misspellings) or the lis_person_contact_email_primary parameter is not present.

Third, the hasPermissions method no longer returns 1 for a user that does not have a permission level defined in the database if the lis_person_sourcedid parameter is present. It just returns 0 as it does for any other case in which the permission level is not defined in the database. This is actually a potential security vulnerability. If the LTIAdvantage code is not set up right, this could result in a user without sufficient permissions gaining access to things the user does not have permission for.

[somiaj]

Wouldn't if defined $permissionLevel && $permissionLevel->permission do the same thing as if defined $permissionLevel && defined $permissionLevel->permission && $permissionLevel->permission ne ''?

[drgrice1]

Not quite. It would do different things if $permissionLevel->permission is 0 versus it being '' which is what it would be if the permission column is NULL in the database.

[drgrice1]

Actually, the defined $permissionLevel->permission check is probably not needed, since the webwork2 database layer does not actually return undefined.

[somiaj]

I missed that $permissionLevel->permission was an integer. Thanks for the clarification.