Skip to content

Commit

Permalink
[#11357] Allow access to submission/results page for unregistered ins…
Browse files Browse the repository at this point in the history 
…tructors (#11488)

* Modify access controls of necessary APIs to cater for unregistered instructors

* Extend front-end access for submission/result pages to unregistered instructors

* Remove unnecessary studentemail parameter for session submission/result links

* Remove the now unnecessary join instruction for instructor submission/result reminders

* Replace instructor submission/result links with generic ones

* Add course join notes for instructor

* Include list of session links for regenerate email

* Display session links for instructor search result
  • Loading branch information
@wkurniawan07
wkurniawan07 authored Feb 6, 2022
1 parent 30ac04e commit 4767707
Show file tree
Hide file tree
Showing 52 changed files with 616 additions and 375 deletions.
2 changes: 0 additions & 2 deletions src/main/java/teammates/common/util/Templates.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ public static class EmailTemplates {
FileHelper.readResourceFile("studentEmailFragment-registrationKeyReset.html");
public static final String FRAGMENT_INSTRUCTOR_COURSE_JOIN =
FileHelper.readResourceFile("instructorEmailFragment-courseJoin.html");
public static final String FRAGMENT_INSTRUCTOR_COURSE_JOIN_REMINDER =
FileHelper.readResourceFile("instructorEmailFragment-reminderToJoinCourseBeforeSubmittingResponse.html");
public static final String FRAGMENT_INSTRUCTOR_COURSE_REJOIN_AFTER_GOOGLE_ID_RESET =
FileHelper.readResourceFile("instructorEmailFragment-googleIdReset.html");
public static final String FRAGMENT_INSTRUCTOR_COURSE_REJOIN_AFTER_REGKEY_RESET =
Expand Down
48 changes: 9 additions & 39 deletions src/main/java/teammates/logic/api/EmailGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,20 +114,18 @@ public List<EmailWrapper> generateFeedbackSessionOpeningSoonEmails(FeedbackSessi
.withSessionName(session.getFeedbackSessionName())
.toAbsoluteString();

emails.add(generateFeedbackSessionOpeningSoonEmailBase(course, session, coOwner,
EmailType.FEEDBACK_OPENING_SOON, editUrl));
emails.add(generateFeedbackSessionOpeningSoonEmailBase(course, session, coOwner, editUrl));
}

return emails;
}

/**
* Creates an email for a co-owner, reminding them that a session is opening soon.
* @return
*/
private EmailWrapper generateFeedbackSessionOpeningSoonEmailBase(
CourseAttributes course, FeedbackSessionAttributes session,
InstructorAttributes coOwner, EmailType type, String editUrl) {
InstructorAttributes coOwner, String editUrl) {

String additionalNotes;

Expand Down Expand Up @@ -155,10 +153,11 @@ private EmailWrapper generateFeedbackSessionOpeningSoonEmailBase(
"${startTime}", SanitizationHelper.sanitizeForHtml(
TimeHelper.formatInstant(startTime, session.getTimeZone(), DATETIME_DISPLAY_FORMAT)),
"${additionalNotes}", additionalNotes,
"${sessionEditUrl}", editUrl,
"${additionalContactInformation}", "");

EmailWrapper email = getEmptyEmailAddressedToEmail(coOwner.getEmail());
email.setType(type);
email.setType(EmailType.FEEDBACK_OPENING_SOON);
email.setSubjectFromType(course.getName(), session.getFeedbackSessionName());
email.setContent(emailBody);
return email;
Expand Down Expand Up @@ -271,12 +270,6 @@ public EmailWrapper generateFeedbackSessionSummaryOfCourse(
: "";

for (FeedbackSessionAttributes fsa : sessions) {
if (isInstructor) {
// Currently, it is pointless to list down session links for instructor
// as instructor needs to register before submitting/viewing session responses.
continue;
}

String submitUrlHtml = "(Feedback session is not yet opened)";
String reportUrlHtml = "(Feedback session is not yet published)";

Expand All @@ -287,7 +280,7 @@ public EmailWrapper generateFeedbackSessionSummaryOfCourse(
.withCourseId(course.getId())
.withSessionName(fsa.getFeedbackSessionName())
.withRegistrationKey(userKey)
.withStudentEmail(userEmail)
.withEntityType(isInstructor ? Const.EntityType.INSTRUCTOR : "")
.toAbsoluteString();
submitUrlHtml = "<a href=\"" + submitUrl + "\">" + submitUrl + "</a>";
}
Expand All @@ -297,7 +290,7 @@ public EmailWrapper generateFeedbackSessionSummaryOfCourse(
.withCourseId(course.getId())
.withSessionName(fsa.getFeedbackSessionName())
.withRegistrationKey(userKey)
.withStudentEmail(userEmail)
.withEntityType(isInstructor ? Const.EntityType.INSTRUCTOR : "")
.toAbsoluteString();
reportUrlHtml = "<a href=\"" + reportUrl + "\">" + reportUrl + "</a>";
}
Expand Down Expand Up @@ -420,7 +413,6 @@ private EmailWrapper generateSessionLinksRecoveryEmailForExistingStudent(String
.withCourseId(course.getId())
.withSessionName(session.getFeedbackSessionName())
.withRegistrationKey(student.getKey())
.withStudentEmail(student.getEmail())
.toAbsoluteString();
submitUrlHtml = "[<a href=\"" + submitUrl + "\">submission link</a>]";
}
Expand All @@ -430,7 +422,6 @@ private EmailWrapper generateSessionLinksRecoveryEmailForExistingStudent(String
.withCourseId(course.getId())
.withSessionName(session.getFeedbackSessionName())
.withRegistrationKey(student.getKey())
.withStudentEmail(student.getEmail())
.toAbsoluteString();
reportUrlHtml = "[<a href=\"" + reportUrl + "\">result link</a>]";
}
Expand Down Expand Up @@ -482,22 +473,13 @@ private EmailWrapper generateFeedbackSessionEmailBaseForInstructorReminders(
CourseAttributes course, FeedbackSessionAttributes session, InstructorAttributes instructor,
String template, String additionalContactInformation) {

String submitUrl = Config.getFrontEndAppUrl(Const.WebPageURIs.INSTRUCTOR_SESSION_SUBMISSION_PAGE)
.withCourseId(course.getId())
.withSessionName(session.getFeedbackSessionName())
.toAbsoluteString();

String reportUrl = Config.getFrontEndAppUrl(Const.WebPageURIs.INSTRUCTOR_SESSION_RESULTS_PAGE)
String submitUrl = Config.getFrontEndAppUrl(Const.WebPageURIs.SESSION_SUBMISSION_PAGE)
.withCourseId(course.getId())
.withSessionName(session.getFeedbackSessionName())
.withRegistrationKey(instructor.getKey())
.withEntityType(Const.EntityType.INSTRUCTOR)
.toAbsoluteString();

// if instructor hasn't joined yet, remind them to join before submitting feedback
String instructorJoinReminderFragment =
instructor.isRegistered()
? ""
: generateInstructorJoinReminderFragment(instructor);

Instant endTime = TimeHelper.getMidnightAdjustedInstantBasedOnZone(
session.getEndTime(), session.getTimeZone(), false);
String emailBody = Templates.populateTemplate(template,
Expand All @@ -510,9 +492,7 @@ private EmailWrapper generateFeedbackSessionEmailBaseForInstructorReminders(
"${instructorFragment}", "",
"${sessionInstructions}", session.getInstructionsString(),
"${submitUrl}", submitUrl,
"${reportUrl}", reportUrl,
"${feedbackAction}", FEEDBACK_ACTION_SUBMIT_EDIT_OR_VIEW,
"${additionalNotes}", instructorJoinReminderFragment,
"${additionalContactInformation}", additionalContactInformation);

EmailWrapper email = getEmptyEmailAddressedToEmail(instructor.getEmail());
Expand All @@ -522,12 +502,6 @@ private EmailWrapper generateFeedbackSessionEmailBaseForInstructorReminders(
return email;
}

private String generateInstructorJoinReminderFragment(InstructorAttributes instructor) {
return Templates.populateTemplate(EmailTemplates.FRAGMENT_INSTRUCTOR_COURSE_JOIN_REMINDER,
"${feedbackAction}", FEEDBACK_ACTION_SUBMIT_EDIT_OR_VIEW,
"${joinUrl}", getInstructorCourseJoinUrl(instructor));
}

/**
* Generates the feedback session closing emails for the given {@code session}.
*/
Expand Down Expand Up @@ -691,14 +665,12 @@ private EmailWrapper generateFeedbackSessionEmailBaseForStudents(
.withCourseId(course.getId())
.withSessionName(session.getFeedbackSessionName())
.withRegistrationKey(student.getKey())
.withStudentEmail(student.getEmail())
.toAbsoluteString();

String reportUrl = Config.getFrontEndAppUrl(Const.WebPageURIs.SESSION_RESULTS_PAGE)
.withCourseId(course.getId())
.withSessionName(session.getFeedbackSessionName())
.withRegistrationKey(student.getKey())
.withStudentEmail(student.getEmail())
.toAbsoluteString();

Instant endTime = TimeHelper.getMidnightAdjustedInstantBasedOnZone(
Expand All @@ -715,7 +687,6 @@ private EmailWrapper generateFeedbackSessionEmailBaseForStudents(
"${submitUrl}", submitUrl,
"${reportUrl}", reportUrl,
"${feedbackAction}", feedbackAction,
"${additionalNotes}", "",
"${additionalContactInformation}", additionalContactInformation);

EmailWrapper email = getEmptyEmailAddressedToEmail(student.getEmail());
Expand Down Expand Up @@ -760,7 +731,6 @@ private EmailWrapper generateFeedbackSessionEmailBaseForInstructors(
"${submitUrl}", "{in the actual email sent to the students, this will be the unique link}",
"${reportUrl}", "{in the actual email sent to the students, this will be the unique link}",
"${feedbackAction}", feedbackAction,
"${additionalNotes}", "",
"${additionalContactInformation}", additionalContactInformation);

EmailWrapper email = getEmptyEmailAddressedToEmail(instructor.getEmail());
Expand Down
24 changes: 22 additions & 2 deletions src/main/java/teammates/ui/webapi/Action.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public abstract class Action {
UserInfo userInfo;
AuthType authType;
private StudentAttributes unregisteredStudent;
private InstructorAttributes unregisteredInstructor;

// buffer to store the request body
private String requestBody;
Expand Down Expand Up @@ -118,11 +119,14 @@ public RequestLogUser getUserInfoForLogging() {
String googleId = userInfo == null ? null : userInfo.getId();

user.setGoogleId(googleId);
if (unregisteredStudent == null) {
if (unregisteredStudent == null && unregisteredInstructor == null) {
user.setRegkey(getRequestParamValue(Const.ParamsNames.REGKEY));
} else {
} else if (unregisteredStudent != null) {
user.setRegkey(unregisteredStudent.getKey());
user.setEmail(unregisteredStudent.getEmail());
} else {
user.setRegkey(unregisteredInstructor.getKey());
user.setEmail(unregisteredInstructor.getEmail());
}
return user;
}
Expand Down Expand Up @@ -254,6 +258,22 @@ Optional<StudentAttributes> getUnregisteredStudent() {
return Optional.empty();
}

/**
* Gets the unregistered instructor by the HTTP param.
*/
Optional<InstructorAttributes> getUnregisteredInstructor() {
String key = getRequestParamValue(Const.ParamsNames.REGKEY);
if (!StringHelper.isEmpty(key)) {
InstructorAttributes instructorAttributes = logic.getInstructorForRegistrationKey(key);
if (instructorAttributes == null) {
return Optional.empty();
}
unregisteredInstructor = instructorAttributes;
return Optional.of(instructorAttributes);
}
return Optional.empty();
}

InstructorPermissionSet constructInstructorPrivileges(InstructorAttributes instructor, String feedbackSessionName) {
InstructorPermissionSet privilege = instructor.getPrivileges().getCourseLevelPrivileges();
if (feedbackSessionName != null) {
Expand Down
24 changes: 21 additions & 3 deletions src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,17 +108,25 @@ InstructorAttributes getInstructorOfCourseFromRequest(String courseId) {
return logic.getInstructorForEmail(courseId, moderatedPerson);
} else if (!StringHelper.isEmpty(previewAsPerson)) {
return logic.getInstructorForEmail(courseId, previewAsPerson);
} else if (userInfo != null) {
return logic.getInstructorForGoogleId(courseId, userInfo.getId());
} else {
return getUnregisteredInstructor().orElseGet(() -> {
if (userInfo == null) {
return null;
}
return logic.getInstructorForGoogleId(courseId, userInfo.getId());
});
}
return null;
}

/**
* Checks the access control for instructor feedback submission.
*/
void checkAccessControlForInstructorFeedbackSubmission(
InstructorAttributes instructor, FeedbackSessionAttributes feedbackSession) throws UnauthorizedAccessException {
if (instructor == null) {
throw new UnauthorizedAccessException("Trying to access system using a non-existent instructor entity");
}

String moderatedPerson = getRequestParamValue(Const.ParamsNames.FEEDBACK_SESSION_MODERATED_PERSON);
String previewAsPerson = getRequestParamValue(Const.ParamsNames.PREVIEWAS);

Expand All @@ -131,6 +139,16 @@ void checkAccessControlForInstructorFeedbackSubmission(
gateKeeper.verifyAccessible(logic.getInstructorForGoogleId(feedbackSession.getCourseId(), userInfo.getId()),
feedbackSession, Const.InstructorPermissions.CAN_MODIFY_SESSION);
} else {
if (!StringHelper.isEmpty(instructor.getGoogleId())) {
if (userInfo == null) {
// Instructor is associated to a google ID; even if registration key is passed, do not allow access
throw new UnauthorizedAccessException("Login is required to access this feedback session");
} else if (!userInfo.id.equals(instructor.getGoogleId())) {
// Logged in instructor is not the same as the instructor registered for the given key,
// do not allow access
throw new UnauthorizedAccessException("You are not authorized to access this feedback session");
}
}
gateKeeper.verifySessionSubmissionPrivilegeForInstructor(feedbackSession, instructor);
}
}
Expand Down
8 changes: 2 additions & 6 deletions src/main/java/teammates/ui/webapi/GetFeedbackSessionAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,12 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException {
StudentAttributes studentAttributes = getStudentOfCourseFromRequest(courseId);
checkAccessControlForStudentFeedbackSubmission(studentAttributes, feedbackSession);
break;
case FULL_DETAIL:
gateKeeper.verifyLoggedInUserPrivileges(userInfo);
gateKeeper.verifyAccessible(logic.getInstructorForGoogleId(courseId, userInfo.getId()),
feedbackSession, Const.InstructorPermissions.CAN_MODIFY_SESSION);
break;
case INSTRUCTOR_SUBMISSION:
case INSTRUCTOR_RESULT:
InstructorAttributes instructorAttributes = getInstructorOfCourseFromRequest(courseId);
checkAccessControlForInstructorFeedbackSubmission(instructorAttributes, feedbackSession);
break;
case INSTRUCTOR_RESULT:
case FULL_DETAIL:
gateKeeper.verifyLoggedInUserPrivileges(userInfo);
gateKeeper.verifyAccessible(logic.getInstructorForGoogleId(courseId, userInfo.getId()),
feedbackSession, Const.InstructorPermissions.CAN_VIEW_SESSION_IN_SECTIONS);
Expand Down
16 changes: 9 additions & 7 deletions src/main/java/teammates/ui/webapi/GetInstructorAction.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package teammates.ui.webapi;

import teammates.common.datatransfer.attributes.FeedbackSessionAttributes;
import teammates.common.datatransfer.attributes.InstructorAttributes;
import teammates.common.util.Const;
import teammates.ui.output.InstructorData;
Expand All @@ -11,6 +10,8 @@
*/
class GetInstructorAction extends BasicFeedbackSubmissionAction {

private static final String UNAUTHORIZED_ACCESS = "You are not allowed to view this resource!";

@Override
AuthType getMinAuthLevel() {
return AuthType.PUBLIC;
Expand All @@ -21,13 +22,13 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException {
Intent intent = Intent.valueOf(getNonNullRequestParamValue(Const.ParamsNames.INTENT));
switch (intent) {
case INSTRUCTOR_SUBMISSION:
case INSTRUCTOR_RESULT:
String courseId = getNonNullRequestParamValue(Const.ParamsNames.COURSE_ID);
String feedbackSessionName = getNonNullRequestParamValue(Const.ParamsNames.FEEDBACK_SESSION_NAME);
FeedbackSessionAttributes feedbackSession = getNonNullFeedbackSession(feedbackSessionName, courseId);
InstructorAttributes instructorAttributes = getInstructorOfCourseFromRequest(feedbackSession.getCourseId());
checkAccessControlForInstructorFeedbackSubmission(instructorAttributes, feedbackSession);
InstructorAttributes instructorAttributes = getInstructorOfCourseFromRequest(courseId);
if (instructorAttributes == null) {
throw new UnauthorizedAccessException(UNAUTHORIZED_ACCESS);
}
break;
case INSTRUCTOR_RESULT:
case FULL_DETAIL:
gateKeeper.verifyLoggedInUserPrivileges(userInfo);
break;
Expand All @@ -51,9 +52,9 @@ public JsonResult execute() {

switch (intent) {
case INSTRUCTOR_SUBMISSION:
case INSTRUCTOR_RESULT:
instructorAttributes = getInstructorOfCourseFromRequest(courseId);
break;
case INSTRUCTOR_RESULT:
case FULL_DETAIL:
instructorAttributes = logic.getInstructorForGoogleId(courseId, userInfo.getId());
break;
Expand All @@ -66,6 +67,7 @@ public JsonResult execute() {
}

InstructorData instructorData = new InstructorData(instructorAttributes);
instructorData.setInstitute(logic.getCourseInstitute(courseId));
if (intent == Intent.FULL_DETAIL) {
instructorData.setGoogleId(instructorAttributes.getGoogleId());
}
Expand Down
Loading

0 comments on commit 4767707

Please sign in to comment.