search

DASSL / ClassDB

An open-source system to let students experiment with relational data
https://dassl.github.io/ClassDB/
Other
7 stars 2 forks source link

Unnecessary test for login role before giving login capability (E) #215

Closed smurthys closed 6 years ago

smurthys commented 6 years ago

Function createRole permits a team role to login, but that does not seem to be a wise choice. The justification in the comment there notwithstanding, it is important for all activity to be traced to a team member.

EDIT: See the following comment for a revised statement of the issue.

smurthys commented 6 years ago

Reviewing the code more carefully, it seems the function does the right thing. The point of the referenced code is to forcibly give LOGIN to an existing user role and leave the LOGIN role as is for an existing team. (If the instructor has given a team LOGIN for some reason, that is not disturbed.)

However, it seems the check for ClassDB.canLogin($1) seems unnecessary given that the code proceeds to grant LOGIN if the user already has that ability. So, on the surface it looks like the OR condition is unnecessary.

IF NOT($3 OR ClassDB.canLogin($1)) THEN
  EXECUTE FORMAT('ALTER ROLE %s LOGIN', $1);
END IF;
smurthys commented 6 years ago

Closing this issue because the code is correctly granting login to users (not teams) who don't already have login.