refactor(Addressed PR feedback): Update to introduce accept key

src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java

@@ -24,7 +24,7 @@
 import java.util.regex.Pattern;
 
 import static io.github.manusant.ss.descriptor.MethodDescriptor.path;
-import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.REQUESTING_USER_ID_PARAM;
+import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.ACCEPT_KEY;
 import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.manageAcceptDependentEmail;
 import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt;
 
@@ -63,20 +63,24 @@ OtpUser preCreateHook(OtpUser user, Request req) {
             Auth0Connection.ensureApiUserHasApiKey(req);
             user.applicationId = requestingUser.apiUser.id;
         }
-        if (Objects.nonNull(user.mobilityProfile)) {
-            user.mobilityProfile.updateMobilityMode();
-        }
-        manageAcceptDependentEmail(user);
+        preliminaryTasks(user);
         return super.preCreateHook(user, req);
     }
 
     @Override
     OtpUser preUpdateHook(OtpUser user, OtpUser preExistingUser, Request req) {
+        preliminaryTasks(user);
+        return super.preUpdateHook(user, preExistingUser, req);
+    }
+
+    /**
+     * Tasks to be carried out before creating or updating a user.
+     */
+    private void preliminaryTasks(OtpUser user) {
         if (Objects.nonNull(user.mobilityProfile)) {
             user.mobilityProfile.updateMobilityMode();
         }
         manageAcceptDependentEmail(user);
-        return super.preUpdateHook(user, preExistingUser, req);
     }
 
     @Override
@@ -88,8 +92,7 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) {
             .get(path("/acceptdependent")
                     .withDescription("Accept a dependent request.")
                     .withResponses(SwaggerUtils.createStandardResponses(OtpUser.class))
-                    .withPathParam().withName(USER_ID_PARAM).withRequired(true).withDescription("The dependent user id.").and()
-                    .withPathParam().withName(REQUESTING_USER_ID_PARAM).withRequired(true).withDescription("The requesting user id.").and()
+                    .withPathParam().withName(ACCEPT_KEY).withRequired(true).withDescription("The accept dependent unique key.").and()
                     .withResponseType(OtpUser.class),
                 TrustedCompanion::acceptDependent
             )

src/main/java/org/opentripplanner/middleware/i18n/Message.java

@@ -18,6 +18,7 @@ public enum Message {
     ACCEPT_DEPENDENT_EMAIL_LINK_TEXT,
     ACCEPT_DEPENDENT_EMAIL_SUBJECT,
     ACCEPT_DEPENDENT_EMAIL_MANAGE,
+    ACCEPT_DEPENDENT_ERROR,
     LABEL_AND_CONTENT,
     SMS_STOP_NOTIFICATIONS,
     TRIP_EMAIL_SUBJECT,

src/main/java/org/opentripplanner/middleware/models/OtpUser.java

@@ -93,6 +93,9 @@ public enum Notification {
     /** Users that are dependent on this user. */
     public List<String> dependents = new ArrayList<>();
 
+    /** This user's name */
+    public String name;
+
     @Override
     public boolean delete() {
         return delete(true);

src/main/java/org/opentripplanner/middleware/models/RelatedUser.java

@@ -8,7 +8,7 @@ public enum RelatedUserStatus {
 
     public String email;
     public RelatedUserStatus status = RelatedUserStatus.PENDING;
-    public boolean acceptDependentEmailSent;
+    public String acceptKey;
     public String nickName;
 
     public RelatedUser() {
@@ -20,5 +20,10 @@ public RelatedUser(String email, RelatedUserStatus status, String nickName) {
         this.status = status;
         this.nickName = nickName;
     }
+
+    public RelatedUser(String email, RelatedUserStatus status, String nickName, String acceptKey) {
+        this (email, status, nickName);
+        this.acceptKey = acceptKey;
+    }
 }
 

src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java

@@ -1,5 +1,6 @@
 package org.opentripplanner.middleware.tripmonitor;
 
+import com.mongodb.client.model.Filters;
 import org.eclipse.jetty.http.HttpStatus;
 import org.opentripplanner.middleware.OtpMiddlewareMain;
 import org.opentripplanner.middleware.i18n.Message;
@@ -16,9 +17,10 @@
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Optional;
+import java.util.UUID;
 
 import static com.mongodb.client.model.Filters.eq;
-import static org.opentripplanner.middleware.controllers.api.ApiController.USER_ID_PARAM;
 import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.SETTINGS_PATH;
 import static org.opentripplanner.middleware.utils.I18nUtils.label;
 import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt;
@@ -34,7 +36,7 @@ private TrustedCompanion() {
     private static final String OTP_UI_URL = ConfigUtils.getConfigPropertyAsText("OTP_UI_URL");
     private static final String TRUSTED_COMPANION_CONFIRMATION_PAGE_URL = ConfigUtils.getConfigPropertyAsText("TRUSTED_COMPANION_CONFIRMATION_PAGE_URL");
     private static final String TRUSTED_COMPANION_ERROR_PAGE_URL = ConfigUtils.getConfigPropertyAsText("TRUSTED_COMPANION_ERROR_PAGE_URL");
-    public static final String REQUESTING_USER_ID_PARAM = "requestingUserId";
+    public static final String ACCEPT_KEY = "acceptKey";
 
     /** Note: This path is excluded from security checks, see {@link OtpMiddlewareMain#initializeHttpEndpoints()}. */
     public static final String ACCEPT_DEPENDENT_PATH = "api/secure/user/acceptdependent";
@@ -44,54 +46,68 @@ private TrustedCompanion() {
      * successful redirect the user to the confirmation page, else redirect to the error page with related error.
      */
     public static OtpUser acceptDependent(Request request, Response response) {
-        boolean accepted = false;
-        String error = "";
-        OtpUser relatedUser = getUserFromRequest(request, REQUESTING_USER_ID_PARAM);
-        OtpUser dependentUser = getUserFromRequest(request, USER_ID_PARAM);
-        if (relatedUser != null && dependentUser != null) {
-            boolean isRelated = dependentUser.relatedUsers
+        String acceptKey = getAcceptKeyFromRequest(request);
+        OtpUser dependentUser = getUserFromAcceptKey(request, acceptKey);
+        OtpUser relatedUser = getRelatedUserFromEmail(dependentUser, acceptKey);
+
+        if (dependentUser != null && relatedUser != null) {
+            Optional<RelatedUser> relatedUserToUpdate = dependentUser.relatedUsers
                 .stream()
                 .filter(related -> related.email.equals(relatedUser.email))
-                // Update related user status. This assumes a related user with "pending" status was previously added.
-                .peek(related -> related.status = RelatedUser.RelatedUserStatus.CONFIRMED)
-                .findFirst()
-                .isPresent();
-
-            if (isRelated) {
-                // Maintain a list of dependents.
-                relatedUser.dependents.add(dependentUser.id);
-                Persistence.otpUsers.replace(relatedUser.id, relatedUser);
-                // Update list of related users.
-                Persistence.otpUsers.replace(dependentUser.id, dependentUser);
-                accepted = true;
-            } else {
-                error = "Dependent did not request you as a trusted companion!";
-            }
-        } else {
-            error = "Required users do not exist!";
-        }
+                .findFirst();
+            relatedUserToUpdate.ifPresent(value -> value.status = RelatedUser.RelatedUserStatus.CONFIRMED);
+
+            // Maintain a list of dependents.
+            relatedUser.dependents.add(dependentUser.id);
+            Persistence.otpUsers.replace(relatedUser.id, relatedUser);
+            // Update list of related users.
+            Persistence.otpUsers.replace(dependentUser.id, dependentUser);
 
-        if (accepted) {
             // Redirect to confirmation page and provide dependent user information.
             response.redirect(TRUSTED_COMPANION_CONFIRMATION_PAGE_URL);
             return dependentUser;
-        } else {
-            response.redirect(String.format("%s?error=%s", TRUSTED_COMPANION_ERROR_PAGE_URL, error));
+        }
+
+        response.redirect(
+            String.format("%s?error=%s", TRUSTED_COMPANION_ERROR_PAGE_URL, Message.ACCEPT_DEPENDENT_ERROR.get(null))
+        );
+        return null;
+    }
+
+    /**
+     * Using the accept key, find the matching related user's email and from that return the related user.
+     */
+    private static OtpUser getRelatedUserFromEmail(OtpUser dependentUser, String acceptKey) {
+        if (dependentUser == null || acceptKey == null) {
             return null;
         }
+        Optional<RelatedUser> relatedUser = dependentUser.relatedUsers
+            .stream()
+            .filter(user -> user.acceptKey.equalsIgnoreCase(acceptKey))
+            .findFirst();
+        return relatedUser.map(user -> Persistence.otpUsers.getOneFiltered(eq("email", user.email))).orElse(null);
     }
 
     /**
-     * Extract the user id from the request parameters and in-turn retrieve the matching user.
+     * Extract the accept key from the request parameters.
      */
-    private static OtpUser getUserFromRequest(Request request, String parameterName) {
-        String userId = HttpUtils.getQueryParamFromRequest(request, parameterName, false);
-        if (userId.isEmpty()) {
-            logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "User id not provided.");
+    private static String getAcceptKeyFromRequest(Request request) {
+        String acceptKey = HttpUtils.getQueryParamFromRequest(request, ACCEPT_KEY, false);
+        if (acceptKey.isEmpty()) {
+            logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Accept key not provided.");
             return null;
         }
+        return acceptKey;
+    }
 
-        OtpUser user = Persistence.otpUsers.getById(userId);
+    /**
+     * Retrieve the dependent user matching the accept key.
+     */
+    private static OtpUser getUserFromAcceptKey(Request request, String acceptKey) {
+        if (acceptKey == null) {
+            return null;
+        }
+        OtpUser user = getUserForAcceptKey(acceptKey);
         if (user == null) {
             logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Otp user unknown.");
             return null;
@@ -116,11 +132,12 @@ public static void manageAcceptDependentEmail(OtpUser dependentUser, boolean isT
 
         dependentUser.relatedUsers
             .stream()
-            .filter(relatedUser -> !relatedUser.acceptDependentEmailSent)
+            .filter(relatedUser -> relatedUser.acceptKey == null)
             .forEach(relatedUser -> {
+                String acceptKey = UUID.randomUUID().toString();
                 OtpUser userToReceiveEmail = Persistence.otpUsers.getOneFiltered(eq("email", relatedUser.email));
-                if (userToReceiveEmail != null && (isTest || sendAcceptDependentEmail(dependentUser, userToReceiveEmail))) {
-                    relatedUser.acceptDependentEmailSent = true;
+                if (userToReceiveEmail != null && (isTest || sendAcceptDependentEmail(dependentUser, userToReceiveEmail, acceptKey))) {
+                    relatedUser.acceptKey = acceptKey;
                 }
             });
 
@@ -131,22 +148,21 @@ public static void manageAcceptDependentEmail(OtpUser dependentUser, boolean isT
     /**
      * Send 'accept dependent' email.
      */
-    private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser relatedUser) {
+    private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser relatedUser, String acceptKey) {
         Locale locale = I18nUtils.getOtpUserLocale(relatedUser);
 
         String acceptDependentLinkLabel = Message.ACCEPT_DEPENDENT_EMAIL_LINK_TEXT.get(locale);
-        String acceptDependentUrl = getAcceptDependentUrl(dependentUser);
+        String acceptDependentUrl = getAcceptDependentUrl(acceptKey);
+        String addressee = (dependentUser.name != null) ? dependentUser.name : dependentUser.email;
 
         // A HashMap is needed instead of a Map for template data to be serialized to the template renderer.
         Map<String, Object> templateData = new HashMap<>(
             Map.of(
                 "acceptDependentLinkAnchorLabel", acceptDependentLinkLabel,
                 "acceptDependentLinkLabelAndUrl", label(acceptDependentLinkLabel, acceptDependentUrl, locale),
-                "acceptDependentUrl", getAcceptDependentUrl(dependentUser),
+                "acceptDependentUrl", acceptDependentUrl,
                 "emailFooter", Message.ACCEPT_DEPENDENT_EMAIL_FOOTER.get(locale),
-                // TODO: The user's email address isn't very personal, but that is all I have to work with! Suggetions?
-                "emailGreeting", String.format("%s %s", dependentUser.email, Message.ACCEPT_DEPENDENT_EMAIL_GREETING.get(locale)),
-                // TODO: This is required in the `OtpUserContainer.ftl` template. Not sure what to provide. Suggestions?
+                "emailGreeting", String.format("%s %s", addressee, Message.ACCEPT_DEPENDENT_EMAIL_GREETING.get(locale)),
                 "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH),
                 "manageLinkText", Message.ACCEPT_DEPENDENT_EMAIL_MANAGE.get(locale)
             )
@@ -161,7 +177,19 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r
         );
     }
 
-    private static String getAcceptDependentUrl(OtpUser dependentUser) {
-        return String.format("%s/%s/%s?userId=%s", AWS_API_SERVER, AWS_API_STAGE, ACCEPT_DEPENDENT_PATH, dependentUser.id);
+    private static String getAcceptDependentUrl(String acceptKey) {
+        return String.format("%s/%s%s", AWS_API_SERVER, AWS_API_STAGE, getAcceptDependentEndPoint(acceptKey));
+    }
+
+    public static String getAcceptDependentEndPoint(String acceptKey) {
+        return String.format("/%s?%s=%s", ACCEPT_DEPENDENT_PATH, ACCEPT_KEY, acceptKey);
+    }
+
+    /**
+     * @return the {@link OtpUser} found with a {@link RelatedUser#acceptKey} in {@link OtpUser#relatedUsers} that
+     * matches the provided acceptKey.
+     */
+    private static OtpUser getUserForAcceptKey(String acceptKey) {
+        return Persistence.otpUsers.getOneFiltered(Filters.elemMatch("relatedUsers", Filters.eq(ACCEPT_KEY, acceptKey)));
     }
 }

src/main/resources/Message.properties

@@ -3,6 +3,7 @@ ACCEPT_DEPENDENT_EMAIL_GREETING = would like you to be a trusted companion.
 ACCEPT_DEPENDENT_EMAIL_LINK_TEXT = Accept trusted companion
 ACCEPT_DEPENDENT_EMAIL_SUBJECT = Trusted companion request
 ACCEPT_DEPENDENT_EMAIL_MANAGE = Manage settings
+ACCEPT_DEPENDENT_ERROR = Unable to accept trusted companion.
 LABEL_AND_CONTENT = %s: %s
 SMS_STOP_NOTIFICATIONS = To stop receiving notifications, reply STOP.
 TRIP_EMAIL_SUBJECT = %s Notification

src/main/resources/Message_fr.properties

@@ -3,6 +3,7 @@ ACCEPT_DEPENDENT_EMAIL_GREETING = TODO
 ACCEPT_DEPENDENT_EMAIL_LINK_TEXT = TODO
 ACCEPT_DEPENDENT_EMAIL_SUBJECT = TODO
 ACCEPT_DEPENDENT_EMAIL_MANAGE = TODO
+ACCEPT_DEPENDENT_ERROR = TODO
 LABEL_AND_CONTENT = %s\u00A0: %s
 SMS_STOP_NOTIFICATIONS = Pour arrêter ces notifications, envoyez STOP.
 TRIP_EMAIL_SUBJECT = Notification pour %s

src/main/resources/latest-spark-swagger-output.yaml

@@ -2885,8 +2885,8 @@ definitions:
         - "PENDING"
         - "CONFIRMED"
         - "INVALID"
-      acceptDependentEmailSent:
-        type: "boolean"
+      acceptKey:
+        type: "string"
       nickName:
         type: "string"
   FareComponent:
@@ -3183,6 +3183,8 @@ definitions:
         type: "array"
         items:
           type: "string"
+      nickName:
+        type: "string"
   MobilityProfile:
     type: "object"
     properties:

src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java

@@ -15,20 +15,21 @@
 import org.opentripplanner.middleware.testutils.ApiTestUtils;
 import org.opentripplanner.middleware.testutils.OtpMiddlewareTestEnvironment;
 import org.opentripplanner.middleware.testutils.PersistenceTestUtils;
+import org.opentripplanner.middleware.tripmonitor.TrustedCompanion;
 import org.opentripplanner.middleware.utils.HttpResponseValues;
 import org.opentripplanner.middleware.utils.JsonUtils;
 
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
+import java.util.UUID;
 import java.util.stream.Stream;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
-import static org.opentripplanner.middleware.controllers.api.ApiController.USER_ID_PARAM;
 import static org.opentripplanner.middleware.testutils.ApiTestUtils.createAndAssignAuth0User;
 import static org.opentripplanner.middleware.testutils.ApiTestUtils.getMockHeaders;
 import static org.opentripplanner.middleware.testutils.ApiTestUtils.makeGetRequest;
@@ -37,8 +38,6 @@
 import static org.opentripplanner.middleware.auth.Auth0Connection.restoreDefaultAuthDisabled;
 import static org.opentripplanner.middleware.auth.Auth0Connection.setAuthDisabled;
 import static org.opentripplanner.middleware.testutils.PersistenceTestUtils.deleteOtpUser;
-import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.ACCEPT_DEPENDENT_PATH;
-import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.REQUESTING_USER_ID_PARAM;
 
 public class OtpUserControllerTest extends OtpMiddlewareTestEnvironment {
     private static final String INITIAL_PHONE_NUMBER = "+15555550222"; // Fake US 555 number.
@@ -177,23 +176,17 @@ void canPreserveSmsConsentDate() throws Exception {
 
     @Test
     void canAcceptDependentRequest() {
+        String acceptKey = UUID.randomUUID().toString();
         dependentUserOne.relatedUsers.add(new RelatedUser(
             relatedUserOne.email,
             RelatedUser.RelatedUserStatus.PENDING,
-            nickName
+            nickName,
+            acceptKey
         ));
         Persistence.otpUsers.replace(dependentUserOne.id, dependentUserOne);
 
-        String path = String.format(
-            "%s?%s=%s&%s=%s",
-            ACCEPT_DEPENDENT_PATH,
-            REQUESTING_USER_ID_PARAM,
-            relatedUserOne.id,
-            USER_ID_PARAM,
-            dependentUserOne.id
-        );
-        HttpResponseValues response = makeGetRequest(path, null);
-        System.out.println(response.uri);
+        String path = TrustedCompanion.getAcceptDependentEndPoint(acceptKey);
+        makeGetRequest(path, null);
 
         relatedUserOne = Persistence.otpUsers.getById(relatedUserOne.id);
         assertTrue(relatedUserOne.dependents.contains(dependentUserOne.id));