Closed wildtayne closed 6 years ago
That is a lot of views and functions @srrollo. Kudos on the orchestration. 👍
I love the trick in getUserActivity
(and elsewhere) to choose between one user and all users. 🥇
(I wonder if that trick has a security risk. Not saying I know there is, but that question just popped into my head.)
A few things (some minor):
NULL
: it is easy to miss this detail in the querygetStudentActivity
could invoke getUserActivity
and then apply a filter, as is done in function getStudentActivitySummary
. In general it is not efficient to do so, but it is OK in this application due to interactive use.getStudentActivityAnon
could reuse getStudentActivity
Looks pretty good apart from the improvements that @smurthys suggested.
Thanks for the review @smurthys. I've push a commit addressing most of the observations. A couple notes:
getStudentActivity/Anon
SELECT
on views owned by ClassDB for other roles to be able to access them, while functions do not require an explicit grant of EXECUTE
RETURNS
block, and the column names from the contained query itself are ignored. However, I think it's better to have any function's internal query schema match its output for clarity.I don't think the use of COALESCE
in these functions presents an issue, simply because unprivileged users can never provide an input to those functions other than the result of SESSION_USER
(unless their Postgres rolename was NULL
, somehow).
Thanks @srrollo for the changes. Eyeballing them, they look good.
We should discuss the LIKE
trick on campus. I've no particular security concern as of now.
This PR adds the new user views discussed for #184. A summary of the changes:
StudentActivityAll
andStudentActivityAnon
views have been removedClassDB.getUserXYZActivity()
functions now supplyNULL
for the default parameterNULL
is passed, the function returns activity records for all usersClassDB.getStudentActivitySummary()
: LikegetUserActivitySummary()
, but restricted to studentsClassDB.getStudentActivitySummaryAnon()
: Anonymized version ofgetStudentActivitySummary()
ClassDB.getStudentActivity()
: LikegetUserActivity()
but restricted to studentsClassDB.getStudentActivityAnon()
: Anonymized version ofgetStudentActivity
ClassDB.StudentActivitySummary
: View wrappinggetStudentActivitySummary()
ClassDB.StudentActivitySummaryAnon
: View wrappinggetStudentActivitySummaryAnon()
ClassDB.StudentActivity
: View wrappinggetStudentActivity()
ClassDB.StudentActivityAnon
: View wrappinggetStudentActivityAnon()
Similar functions and views limited to only connection or DDL activity have not been added, but could be easily added in the future, since all functions now take a
NULL
parameter.