[#11352] Logging in does not preserve query parameters other than the…

… first one (#11353) * Encode & when forming nextUrl * Remove unnecessary studentEmail parameter when querying unregistered student
TEAMMATES · Aug 10, 2021 · 37f1cb4 · 37f1cb4
1 parent 2b8b524
commit 37f1cb4
Show file tree

Hide file tree

Showing 9 changed files with 51 additions and 42 deletions.
diff --git a/src/client/java/teammates/client/scripts/DataMigrationForTextQuestionDetailsFormat.java b/src/client/java/teammates/client/scripts/DataMigrationForTextQuestionDetailsFormat.java
@@ -1,7 +1,6 @@
 package teammates.client.scripts;
 
 import java.io.IOException;
-import java.lang.reflect.Field;
 
 import com.google.gson.JsonParseException;
 import com.googlecode.objectify.cmd.Query;
@@ -25,13 +24,6 @@ public static void main(String[] args) throws IOException {
 
     @Override
     protected Query<FeedbackQuestion> getFilterQuery() {
-        // Version 1: question has createdAt field
-        // Instant earliestDate = TimeHelper.parseInstant("2015-11-30T16:00:00.00Z");
-        // return ofy().load().type(FeedbackQuestion.class)
-        //         .filter("createdAt <=", earliestDate)
-        //         .order("-createdAt");
-
-        // Version 2: question does not have createdAt field
         return ofy().load().type(FeedbackQuestion.class)
                 .filter("questionType =", FeedbackQuestionType.TEXT);
     }
@@ -43,22 +35,6 @@ protected boolean isPreview() {
 
     @Override
     protected boolean isMigrationNeeded(FeedbackQuestion question) {
-        // Version 1: question has createdAt field
-        // if (question.getQuestionType() != FeedbackQuestionType.TEXT) {
-        //     return false;
-        // }
-
-        // Version 2: question does not have createdAt field
-        try {
-            Field createdAt = question.getClass().getDeclaredField("createdAt");
-            createdAt.setAccessible(true);
-            if (createdAt.get(question) != null) {
-                return false;
-            }
-        } catch (ReflectiveOperationException e) {
-            // continue
-        }
-
         try {
             // Old non-JSON format will encounter exception when GSON tries to parse it.
             JsonUtils.fromJson(question.getQuestionText(), FeedbackQuestionType.TEXT.getQuestionDetailsClass());

diff --git a/src/client/java/teammates/client/scripts/IndexFeedbackSessionFields.java b/src/client/java/teammates/client/scripts/IndexFeedbackSessionFields.java
@@ -0,0 +1,36 @@
+package teammates.client.scripts;
+
+import com.googlecode.objectify.cmd.Query;
+
+import teammates.storage.entity.FeedbackSession;
+
+/**
+ * Index the newly-indexable fields of feedback sessions.
+ */
+public class IndexFeedbackSessionFields extends DataMigrationEntitiesBaseScript<FeedbackSession> {
+
+    public static void main(String[] args) {
+        new IndexFeedbackSessionFields().doOperationRemotely();
+    }
+
+    @Override
+    protected Query<FeedbackSession> getFilterQuery() {
+        return ofy().load().type(FeedbackSession.class);
+    }
+
+    @Override
+    protected boolean isPreview() {
+        return true;
+    }
+
+    @Override
+    protected boolean isMigrationNeeded(FeedbackSession session) {
+        return true;
+    }
+
+    @Override
+    protected void migrateEntity(FeedbackSession session) {
+        // Save without any update; this will build the previously non-existing indexes
+        saveEntityDeferred(session);
+    }
+}
diff --git a/src/main/java/teammates/storage/entity/FeedbackSession.java b/src/main/java/teammates/storage/entity/FeedbackSession.java
@@ -35,7 +35,6 @@ public class FeedbackSession extends BaseEntity {
     @Unindex
     private String instructions;
 
-    @Unindex
     @Translate(InstantTranslatorFactory.class)
     private Instant createdTime;
 
@@ -48,7 +47,6 @@ public class FeedbackSession extends BaseEntity {
     @Translate(InstantTranslatorFactory.class)
     private Instant endTime;
 
-    @Unindex
     @Translate(InstantTranslatorFactory.class)
     private Instant sessionVisibleFromTime;
 

diff --git a/src/main/java/teammates/ui/servlets/DevServerLoginServlet.java b/src/main/java/teammates/ui/servlets/DevServerLoginServlet.java
@@ -22,19 +22,19 @@ public class DevServerLoginServlet extends AuthServlet {
 
     @Override
     public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String nextUrl = req.getParameter("nextUrl");
+        if (nextUrl == null) {
+            nextUrl = "/";
+        }
         if (!Config.isDevServer()) {
             resp.setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY);
-            resp.setHeader("Location", Const.WebPageURIs.LOGIN);
+            resp.setHeader("Location", Const.WebPageURIs.LOGIN + "?nextUrl=" + nextUrl.replace("&", "%26"));
             return;
         }
 
         String cookie = HttpRequestHelper.getCookieValueFromRequest(req, Const.SecurityConfig.AUTH_COOKIE_NAME);
         UserInfoCookie uic = UserInfoCookie.fromCookie(cookie);
         boolean isLoginNeeded = uic == null || !uic.isValid();
-        String nextUrl = req.getParameter("nextUrl");
-        if (nextUrl == null) {
-            nextUrl = "/";
-        }
         if (!isLoginNeeded) {
             resp.sendRedirect(nextUrl);
             return;

diff --git a/src/main/java/teammates/ui/servlets/LoginServlet.java b/src/main/java/teammates/ui/servlets/LoginServlet.java
@@ -23,19 +23,19 @@ public class LoginServlet extends AuthServlet {
 
     @Override
     public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String nextUrl = req.getParameter("nextUrl");
+        if (nextUrl == null) {
+            nextUrl = "/";
+        }
         if (Config.isDevServer()) {
             resp.setStatus(HttpStatus.SC_MOVED_PERMANENTLY);
-            resp.setHeader("Location", "/devServerLogin");
+            resp.setHeader("Location", "/devServerLogin?nextUrl=" + nextUrl.replace("&", "%26"));
             return;
         }
 
         String cookie = HttpRequestHelper.getCookieValueFromRequest(req, Const.SecurityConfig.AUTH_COOKIE_NAME);
         UserInfoCookie uic = UserInfoCookie.fromCookie(cookie);
         boolean isLoginNeeded = uic == null || !uic.isValid();
-        String nextUrl = req.getParameter("nextUrl");
-        if (nextUrl == null) {
-            nextUrl = "/";
-        }
         if (!isLoginNeeded) {
             resp.sendRedirect(nextUrl);
             return;

diff --git a/src/web/app/pages-session/session-result-page/session-result-page.component.ts b/src/web/app/pages-session/session-result-page/session-result-page.component.ts
@@ -85,7 +85,7 @@ export class SessionResultPageComponent implements OnInit {
       this.feedbackSessionName = queryParams.fsname;
       this.regKey = queryParams.key || '';
 
-      const nextUrl: string = `${window.location.pathname}${window.location.search}`;
+      const nextUrl: string = `${window.location.pathname}${window.location.search.replace(/&/g, '%26')}`;
       this.authService.getAuthUser(undefined, nextUrl).subscribe((auth: AuthInfo) => {
         if (auth.user) {
           this.loggedInUser = auth.user.id;

diff --git a/src/web/app/pages-session/session-submission-page/session-submission-page.component.ts b/src/web/app/pages-session/session-submission-page/session-submission-page.component.ts
@@ -149,7 +149,7 @@ export class SessionSubmissionPageComponent implements OnInit, AfterViewInit {
         this.isSubmissionFormsDisabled = true;
       }
 
-      const nextUrl: string = `${window.location.pathname}${window.location.search}`;
+      const nextUrl: string = `${window.location.pathname}${window.location.search.replace(/&/g, '%26')}`;
       this.authService.getAuthUser(undefined, nextUrl).subscribe((auth: AuthInfo) => {
         const isPreviewOrModeration: boolean = !!(auth.user && (this.moderatedPerson || this.previewAsPerson));
         if (auth.user) {

diff --git a/src/web/app/public-page.component.ts b/src/web/app/public-page.component.ts
@@ -28,9 +28,8 @@ export class PublicPageComponent {
         // Loads institute for student session submission and result
         const courseId: string = queryParams.courseid;
         const regKey: string = queryParams.key;
-        const studentEmail: string = queryParams.studentemail;
-        if (courseId && regKey && studentEmail) {
-          this.studentService.getStudent(courseId, studentEmail, regKey).subscribe((student: Student) => {
+        if (courseId && regKey) {
+          this.studentService.getStudent(courseId, '', regKey).subscribe((student: Student) => {
             this.institute = student.institute || '';
           });
         }

diff --git a/src/web/app/user-join-page.component.ts b/src/web/app/user-join-page.component.ts
@@ -62,7 +62,7 @@ export class UserJoinPageComponent implements OnInit {
       }, (resp: ErrorMessageOutput) => {
         if (resp.status === 403) {
           this.isLoading = false;
-          const nextUrl: string = `${window.location.pathname}${window.location.search}`;
+          const nextUrl: string = `${window.location.pathname}${window.location.search.replace(/&/g, '%26')}`;
           this.authService.getAuthUser(undefined, nextUrl).subscribe((auth: AuthInfo) => {
             if (!auth.user) {
               window.location.href = `${this.backendUrl}${auth.studentLoginUrl}`;