refactor(PR feedback): Addressed issues with auth and dependent user …

…reference

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

@@ -26,8 +26,7 @@
 
 import static io.github.manusant.ss.descriptor.MethodDescriptor.path;
 import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.ACCEPT_KEY;
-import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.EMAIL_PARAM;
-import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.GET_DEPENDENT_MOBILITY_PROFILE_PATH;
+import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.DEPENDENT_USER_ID;
 import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.USER_LOCALE;
 import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.ensureRelatedUserIntegrity;
 import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.manageAcceptDependentEmail;
@@ -103,13 +102,12 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) {
                     .withResponseType(OtpUser.class),
                 TrustedCompanion::acceptDependent
             )
-            .get(path(ROOT_ROUTE + String.format(GET_DEPENDENT_MOBILITY_PROFILE_PATH, ID_PARAM))
+            .get(path(ROOT_ROUTE + "/getdependentmobilityprofile")
                     .withDescription("Retrieve a dependent's mobility profile.")
                     .withResponses(SwaggerUtils.createStandardResponses(MobilityProfile.class))
-                    .withPathParam().withName(ID_PARAM).withRequired(true).withDescription("The id of the related user.").and()
-                    .withPathParam().withName(EMAIL_PARAM).withRequired(true).withDescription("The dependent's email address.").and()
+                    .withPathParam().withName(DEPENDENT_USER_ID).withRequired(true).withDescription("A dependent's user id.").and()
                     .withResponseType(MobilityProfile.class),
-                this::getDependentMobilityProfile, JsonUtils::toJson
+                TrustedCompanion::getDependentMobilityProfile, JsonUtils::toJson
             )
             .get(path(ROOT_ROUTE + String.format(VERIFY_ROUTE_TEMPLATE, ID_PARAM, VERIFY_PATH, PHONE_PARAM))
                     .withDescription("Request an SMS verification to be sent to an OtpUser's phone number.")
@@ -139,14 +137,6 @@ protected OtpUser getUserProfile(RequestingUser profile) {
         return otpUser;
     }
 
-    /**
-     * Extract the related user from the request and return a dependent's mobility profile if the right conditions
-     * are met.
-     */
-    private MobilityProfile getDependentMobilityProfile(Request request, Response response) {
-        return TrustedCompanion.getDependentMobilityProfile(request, response, getEntityForId(request, response));
-    }
-
     /**
      * HTTP endpoint to send an SMS text to an {@link OtpUser}'s phone number with a verification code. This is used
      * during user signup (or if a user wishes to change their notification preferences to use a new un-verified phone

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

@@ -90,7 +90,7 @@ public enum Notification {
     /** Companions and observers of this user. */
     public List<RelatedUser> relatedUsers = new ArrayList<>();
 
-    /** Users that are dependent on this user. */
+    /** A list of users (their ids only) that are dependent on this user. */
     public List<String> dependents = new ArrayList<>();
 
     /** This user's name */

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

@@ -4,6 +4,7 @@
 import org.apache.logging.log4j.util.Strings;
 import org.eclipse.jetty.http.HttpStatus;
 import org.opentripplanner.middleware.OtpMiddlewareMain;
+import org.opentripplanner.middleware.auth.Auth0Connection;
 import org.opentripplanner.middleware.i18n.Message;
 import org.opentripplanner.middleware.models.MobilityProfile;
 import org.opentripplanner.middleware.models.OtpUser;
@@ -46,10 +47,10 @@ private TrustedCompanion() {
     public static final String ACCEPT_KEY = "acceptKey";
     public static final String USER_LOCALE = "userLocale";
     public static final String EMAIL_PARAM = "email";
+    public static final String DEPENDENT_USER_ID = "dependentuserid";
 
     /** Note: This path is excluded from security checks, see {@link OtpMiddlewareMain#initializeHttpEndpoints()}. */
     public static final String ACCEPT_DEPENDENT_PATH = "api/secure/user/acceptdependent";
-    public static final String GET_DEPENDENT_MOBILITY_PROFILE_PATH = "/:%s/getdependentmobilityprofile";
 
     /**
      * Accept a request from another user to be their dependent. This will include both companions and observers. If
@@ -243,17 +244,19 @@ public static void removeDependent(OtpUser dependent, RelatedUser relatedUser) {
     /**
      * Retrieve the mobility profile for a dependent providing the requesting user is a trusted companion.
      */
-    public static MobilityProfile getDependentMobilityProfile(Request request, Response response, OtpUser relatedUser) {
+    public static MobilityProfile getDependentMobilityProfile(Request request, Response response) {
+        OtpUser relatedUser = Auth0Connection.getUserFromRequest(request).otpUser;
+
         if (relatedUser == null) {
             logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Related user not provided or unknown.");
         }
 
-        var dependentEmail = HttpUtils.getQueryParamFromRequest(request, EMAIL_PARAM, false);
-        if (dependentEmail == null) {
-            logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Required dependent email address not provided.");
+        var dependentUserId = HttpUtils.getQueryParamFromRequest(request, DEPENDENT_USER_ID, false);
+        if (dependentUserId == null) {
+            logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Required dependent id not provided.");
         }
 
-        var dependentUser = Persistence.otpUsers.getOneFiltered(eq(EMAIL_PARAM, dependentEmail));
+        var dependentUser = Persistence.otpUsers.getById(dependentUserId);
         if (dependentUser == null) {
             logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user unknown.");
         }
@@ -263,7 +266,7 @@ public static MobilityProfile getDependentMobilityProfile(Request request, Respo
                 logMessageAndHalt(
                     request,
                     HttpStatus.FORBIDDEN_403,
-                    String.format("Related user is not a trusted companion for %s!", dependentEmail)
+                    String.format("Related user is not a trusted companion for dependent with id: %s!", dependentUserId)
                 );
             } else {
                 return dependentUser.mobilityProfile;

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

@@ -1675,17 +1675,12 @@ paths:
           description: "An error occurred while performing the request. Contact an\
             \ API administrator for more information."
           examples: {}
-  /api/secure/user/{id}/getdependentmobilityprofile:
+  /api/secure/user/getdependentmobilityprofile:
     get:
       tags:
       - "api/secure/user"
       description: "Retrieve a dependent's mobility profile."
-      parameters:
-      - name: "id"
-        in: "path"
-        description: "The id of the related user."
-        required: true
-        type: "string"
+      parameters: []
       responses:
         "200":
           description: "Successful operation"

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

@@ -1,5 +1,6 @@
 package org.opentripplanner.middleware.controllers.api;
 
+import com.auth0.json.mgmt.users.User;
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpStatus;
 import org.junit.jupiter.api.AfterAll;
@@ -33,11 +34,14 @@
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 import static org.opentripplanner.middleware.auth.Auth0Connection.restoreDefaultAuthDisabled;
 import static org.opentripplanner.middleware.auth.Auth0Connection.setAuthDisabled;
+import static org.opentripplanner.middleware.auth.Auth0Users.createAuth0UserForEmail;
+import static org.opentripplanner.middleware.testutils.ApiTestUtils.TEMP_AUTH0_USER_PASSWORD;
 import static org.opentripplanner.middleware.testutils.ApiTestUtils.getMockHeaders;
 import static org.opentripplanner.middleware.testutils.ApiTestUtils.makeGetRequest;
 import static org.opentripplanner.middleware.testutils.ApiTestUtils.makeRequest;
 import static org.opentripplanner.middleware.testutils.ApiTestUtils.mockAuthenticatedGet;
 import static org.opentripplanner.middleware.testutils.PersistenceTestUtils.deleteOtpUser;
+import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.DEPENDENT_USER_ID;
 
 public class OtpUserControllerTest extends OtpMiddlewareTestEnvironment {
     private static final String INITIAL_PHONE_NUMBER = "+15555550222"; // Fake US 555 number.
@@ -73,7 +77,13 @@ public static void setUp() throws Exception {
         dependentUserTwo = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-two"));
         relatedUserThree = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("related-user-three"));
         dependentUserThree = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-three"));
+
         relatedUserFour = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("related-user-four"));
+
+        User auth0User = createAuth0UserForEmail(relatedUserFour.email, TEMP_AUTH0_USER_PASSWORD);
+        relatedUserFour.auth0UserId = auth0User.getId();
+        Persistence.otpUsers.replace(relatedUserFour.id, relatedUserFour);
+
         dependentUserFour = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-four"));
     }
 
@@ -273,12 +283,10 @@ void canRemoveUserFromRelatedUsersList() throws Exception {
 
     @Test
     void canGetDependentMobilityProfile() throws Exception {
-        setAuthDisabled(true);
-
         String path = String.format(
-            "api/secure/user/%s/getdependentmobilityprofile?email=%s",
-            relatedUserFour.id,
-            dependentUserFour.email
+            "api/secure/user/getdependentmobilityprofile?%s=%s",
+            DEPENDENT_USER_ID,
+            dependentUserFour.id
         );
 
         HttpResponseValues responseValues = makeGetRequest(path, getMockHeaders(relatedUserFour));
@@ -295,7 +303,6 @@ void canGetDependentMobilityProfile() throws Exception {
         assertEquals(HttpStatus.OK_200, responseValues.status);
         assertEquals(JsonUtils.toJson(mobilityProfile), responseValues.responseBody);
 
-        setAuthDisabled(false);
     }
 
     /**