From 1ef574870986c700e87fe9e69b0cadaccccb34df Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 24 Sep 2024 15:02:13 +0100 Subject: [PATCH 01/17] refactor(Initial work to accept dependent requests): --- .../api/MonitoredTripController.java | 57 ++++ .../middleware/models/GuardianUser.java | 23 ++ .../middleware/models/MonitoredTrip.java | 5 + .../middleware/models/OtpUser.java | 37 ++- .../latest-spark-swagger-output.yaml | 289 +++++++++++------- .../api/GetMonitoredTripsTest.java | 112 ++++++- 6 files changed, 391 insertions(+), 132 deletions(-) create mode 100644 src/main/java/org/opentripplanner/middleware/models/GuardianUser.java diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java index d6ca475ec..d61b5e1d4 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java @@ -5,11 +5,16 @@ import com.mongodb.client.model.Filters; import org.bson.conversions.Bson; import org.eclipse.jetty.http.HttpStatus; +import org.opentripplanner.middleware.auth.Auth0Connection; +import org.opentripplanner.middleware.auth.RequestingUser; +import org.opentripplanner.middleware.models.GuardianUser; import org.opentripplanner.middleware.models.ItineraryExistence; import org.opentripplanner.middleware.models.MonitoredTrip; +import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip; import org.opentripplanner.middleware.tripmonitor.jobs.MonitoredTripLocks; +import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.InvalidItineraryReason; import org.opentripplanner.middleware.utils.JsonUtils; import org.opentripplanner.middleware.utils.SwaggerUtils; @@ -43,6 +48,12 @@ public MonitoredTripController(String apiPrefix) { protected void buildEndpoint(ApiEndpoint baseEndpoint) { // Add the api key route BEFORE the regular CRUD methods ApiEndpoint modifiedEndpoint = 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(), + MonitoredTripController::acceptDependent + ) .post(path("/checkitinerary") .withDescription("Returns the itinerary existence check results for a monitored trip.") .withRequestType(MonitoredTrip.class) @@ -185,6 +196,52 @@ boolean preDeleteHook(MonitoredTrip monitoredTrip, Request req) { return true; } + /** + * Accept a request from another user to be their dependent. This will include both companions and observers. + */ + private static OtpUser acceptDependent(Request request, Response response) { + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(request); + OtpUser guardianUser = requestingUser.otpUser; + if (guardianUser == null) { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Otp user unknown."); + return null; + } + + String dependentUserId = HttpUtils.getQueryParamFromRequest(request, USER_ID_PARAM, false); + if (dependentUserId.isEmpty()) { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user id not provided."); + return null; + } + + OtpUser dependentUser = Persistence.otpUsers.getById(dependentUserId); + if (dependentUser == null) { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user unknown!"); + return null; + } + + boolean isGuardian = dependentUser.guardians + .stream() + .filter(guardian -> guardian.userId.equals(guardianUser.id)) + // Update guardian status. This assumes the guardian with "pending" status was previously added. + .peek(guardian -> guardian.status = GuardianUser.GuardianUserStatus.CONFIRMED) + .findFirst() + .isPresent(); + + if (isGuardian) { + // Maintain a list of dependents. + guardianUser.dependents.add(dependentUserId); + Persistence.otpUsers.replace(guardianUser.id, guardianUser); + // Update list of guardians. + Persistence.otpUsers.replace(dependentUser.id, dependentUser); + } else { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent did not request user to be a guardian!"); + return null; + } + + // TODO: Not sure what is required in the response. For now, returning the updated guardian user. + return guardianUser; + } + /** * Check itinerary existence by making OTP requests on all days of the week. * @return The results of the itinerary existence check. diff --git a/src/main/java/org/opentripplanner/middleware/models/GuardianUser.java b/src/main/java/org/opentripplanner/middleware/models/GuardianUser.java new file mode 100644 index 000000000..aa013949c --- /dev/null +++ b/src/main/java/org/opentripplanner/middleware/models/GuardianUser.java @@ -0,0 +1,23 @@ +package org.opentripplanner.middleware.models; + +/** A guardian user is a companion or observer requested by a dependent. */ +public class GuardianUser { + public enum GuardianUserStatus { + PENDING, CONFIRMED, INVALID + } + + public String userId; + public String email; + public GuardianUserStatus status; + + public GuardianUser() { + // Required for JSON deserialization. + } + + public GuardianUser(String userId, String email, GuardianUserStatus status) { + this.userId = userId; + this.email = email; + this.status = status; + } +} + diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java index 290c90c8b..e1768ef54 100644 --- a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java @@ -28,6 +28,7 @@ import java.time.LocalDate; import java.time.LocalTime; import java.time.ZonedDateTime; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; @@ -179,6 +180,10 @@ public class MonitoredTrip extends Model { */ public boolean notifyAtLeadingInterval = true; + public GuardianUser primary; + public GuardianUser companion; + public List observers = new ArrayList<>(); + public MonitoredTrip() { } diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index 495d35954..556419027 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -9,7 +9,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.*; + +import java.util.ArrayList; +import java.util.Date; +import java.util.EnumSet; +import java.util.List; +import java.util.Objects; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -43,7 +48,7 @@ public enum Notification { * Notification preferences for this user * (EMAIL and/or SMS and/or PUSH). */ - public EnumSet notificationChannel = EnumSet.noneOf(OtpUser.Notification.class); + public EnumSet notificationChannel = EnumSet.noneOf(OtpUser.Notification.class); /** * Verified phone number for SMS notifications, in +15551234 format (E.164 format, includes country code, no spaces). @@ -80,6 +85,12 @@ public enum Notification { /** If this user was created by an {@link ApiUser}, this parameter will match the {@link ApiUser}'s id */ public String applicationId; + /** Companions and observers of this user. */ + public List guardians = new ArrayList<>(); + + /** Users that are dependent on this user. */ + public List dependents = new ArrayList<>(); + @Override public boolean delete() { return delete(true); @@ -113,6 +124,28 @@ public boolean delete(boolean deleteAuth0User) { } } + // If a guardian, invalidate relationship with all dependents. + for (String userId: dependents) { + OtpUser dependent = Persistence.otpUsers.getById(userId); + if (dependent != null) { + for (GuardianUser guardianUser : dependent.guardians) { + if (guardianUser.userId.equals(this.id)) { + guardianUser.status = GuardianUser.GuardianUserStatus.INVALID; + } + } + Persistence.otpUsers.replace(dependent.id, dependent); + } + } + + // If a dependent, remove relationship with all guardians. + for (GuardianUser guardianUser : guardians) { + OtpUser guardian = Persistence.otpUsers.getById(guardianUser.userId); + if (guardian != null) { + guardian.dependents.remove(this.id); + Persistence.otpUsers.replace(guardian.id, guardian); + } + } + return Persistence.otpUsers.removeById(this.id); } diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index 6b97e4c5f..50cdd5a1a 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -56,10 +56,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/AdminUser" schema: $ref: "#/definitions/AdminUser" + responseSchema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -90,10 +90,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/AdminUser" schema: $ref: "#/definitions/AdminUser" + responseSchema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -123,10 +123,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/Job" schema: $ref: "#/definitions/Job" + responseSchema: + $ref: "#/definitions/Job" /api/admin/user: get: tags: @@ -155,10 +155,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/ResponseList" schema: $ref: "#/definitions/ResponseList" + responseSchema: + $ref: "#/definitions/ResponseList" post: tags: - "api/admin/user" @@ -178,10 +178,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/AdminUser" schema: $ref: "#/definitions/AdminUser" + responseSchema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -220,10 +220,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/AdminUser" schema: $ref: "#/definitions/AdminUser" + responseSchema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -269,10 +269,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/AdminUser" schema: $ref: "#/definitions/AdminUser" + responseSchema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -309,10 +309,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/AdminUser" schema: $ref: "#/definitions/AdminUser" + responseSchema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -356,10 +356,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -402,10 +402,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -447,10 +447,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/TokenHolder" schema: $ref: "#/definitions/TokenHolder" + responseSchema: + $ref: "#/definitions/TokenHolder" /api/secure/application/fromtoken: get: tags: @@ -464,10 +464,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -498,10 +498,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -531,10 +531,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/Job" schema: $ref: "#/definitions/Job" + responseSchema: + $ref: "#/definitions/Job" /api/secure/application: get: tags: @@ -563,10 +563,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/ResponseList" schema: $ref: "#/definitions/ResponseList" + responseSchema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/application" @@ -586,10 +586,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -628,10 +628,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -677,10 +677,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -717,10 +717,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -754,10 +754,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/CDPUser" schema: $ref: "#/definitions/CDPUser" + responseSchema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -788,10 +788,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/CDPUser" schema: $ref: "#/definitions/CDPUser" + responseSchema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -821,10 +821,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/Job" schema: $ref: "#/definitions/Job" + responseSchema: + $ref: "#/definitions/Job" /api/secure/cdp: get: tags: @@ -853,10 +853,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/ResponseList" schema: $ref: "#/definitions/ResponseList" + responseSchema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/cdp" @@ -876,10 +876,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/CDPUser" schema: $ref: "#/definitions/CDPUser" + responseSchema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -918,10 +918,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/CDPUser" schema: $ref: "#/definitions/CDPUser" + responseSchema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -967,10 +967,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/CDPUser" schema: $ref: "#/definitions/CDPUser" + responseSchema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1007,10 +1007,44 @@ paths: "200": description: "Successful operation" examples: {} + schema: + $ref: "#/definitions/CDPUser" responseSchema: $ref: "#/definitions/CDPUser" + "400": + description: "The request was not formed properly (e.g., some required parameters\ + \ may be missing). See the details of the returned response to determine\ + \ the exact issue." + examples: {} + "401": + description: "The server was not able to authenticate the request. This\ + \ can happen if authentication headers are missing or malformed, or the\ + \ authentication server cannot be reached." + examples: {} + "403": + description: "The requesting user is not allowed to perform the request." + examples: {} + "404": + description: "The requested item was not found." + examples: {} + "500": + description: "An error occurred while performing the request. Contact an\ + \ API administrator for more information." + examples: {} + /api/secure/monitoredtrip/acceptdependent: + get: + tags: + - "api/secure/monitoredtrip" + description: "Accept a dependent request." + parameters: [] + responses: + "200": + description: "Successful operation" + examples: {} schema: - $ref: "#/definitions/CDPUser" + $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1050,10 +1084,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ItineraryExistence" schema: $ref: "#/definitions/ItineraryExistence" + responseSchema: + $ref: "#/definitions/ItineraryExistence" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1102,10 +1136,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/ResponseList" schema: $ref: "#/definitions/ResponseList" + responseSchema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/monitoredtrip" @@ -1125,10 +1159,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredTrip" schema: $ref: "#/definitions/MonitoredTrip" + responseSchema: + $ref: "#/definitions/MonitoredTrip" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1167,10 +1201,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredTrip" schema: $ref: "#/definitions/MonitoredTrip" + responseSchema: + $ref: "#/definitions/MonitoredTrip" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1216,10 +1250,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredTrip" schema: $ref: "#/definitions/MonitoredTrip" + responseSchema: + $ref: "#/definitions/MonitoredTrip" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1257,10 +1291,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredTrip" schema: $ref: "#/definitions/MonitoredTrip" + responseSchema: + $ref: "#/definitions/MonitoredTrip" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1298,10 +1332,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/TrackingResponse" schema: $ref: "#/definitions/TrackingResponse" + responseSchema: + $ref: "#/definitions/TrackingResponse" /api/secure/monitoredtrip/updatetracking: post: tags: @@ -1319,10 +1353,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/TrackingResponse" schema: $ref: "#/definitions/TrackingResponse" + responseSchema: + $ref: "#/definitions/TrackingResponse" /api/secure/monitoredtrip/track: post: tags: @@ -1340,10 +1374,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/TrackingResponse" schema: $ref: "#/definitions/TrackingResponse" + responseSchema: + $ref: "#/definitions/TrackingResponse" /api/secure/monitoredtrip/endtracking: post: tags: @@ -1361,10 +1395,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/EndTrackingResponse" schema: $ref: "#/definitions/EndTrackingResponse" + responseSchema: + $ref: "#/definitions/EndTrackingResponse" /api/secure/monitoredtrip/forciblyendtracking: post: tags: @@ -1382,10 +1416,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/EndTrackingResponse" schema: $ref: "#/definitions/EndTrackingResponse" + responseSchema: + $ref: "#/definitions/EndTrackingResponse" /api/secure/triprequests: get: tags: @@ -1430,10 +1464,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/TripRequest" schema: $ref: "#/definitions/TripRequest" + responseSchema: + $ref: "#/definitions/TripRequest" /api/secure/monitoredcomponent: get: tags: @@ -1462,10 +1496,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/ResponseList" schema: $ref: "#/definitions/ResponseList" + responseSchema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/monitoredcomponent" @@ -1485,10 +1519,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredComponent" schema: $ref: "#/definitions/MonitoredComponent" + responseSchema: + $ref: "#/definitions/MonitoredComponent" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1527,10 +1561,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredComponent" schema: $ref: "#/definitions/MonitoredComponent" + responseSchema: + $ref: "#/definitions/MonitoredComponent" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1576,10 +1610,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredComponent" schema: $ref: "#/definitions/MonitoredComponent" + responseSchema: + $ref: "#/definitions/MonitoredComponent" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1617,10 +1651,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredComponent" schema: $ref: "#/definitions/MonitoredComponent" + responseSchema: + $ref: "#/definitions/MonitoredComponent" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1660,10 +1694,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/VerificationResult" schema: $ref: "#/definitions/VerificationResult" + responseSchema: + $ref: "#/definitions/VerificationResult" /api/secure/user/{id}/verify_sms/{code}: post: tags: @@ -1683,10 +1717,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/VerificationResult" schema: $ref: "#/definitions/VerificationResult" + responseSchema: + $ref: "#/definitions/VerificationResult" /api/secure/user/fromtoken: get: tags: @@ -1700,10 +1734,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/OtpUser" schema: $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1734,10 +1768,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/OtpUser" schema: $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1767,10 +1801,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/Job" schema: $ref: "#/definitions/Job" + responseSchema: + $ref: "#/definitions/Job" /api/secure/user: get: tags: @@ -1799,10 +1833,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/ResponseList" schema: $ref: "#/definitions/ResponseList" + responseSchema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/user" @@ -1822,10 +1856,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/OtpUser" schema: $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1864,10 +1898,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/OtpUser" schema: $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1913,10 +1947,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/OtpUser" schema: $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1953,10 +1987,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/OtpUser" schema: $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -2010,11 +2044,11 @@ paths: responses: "200": description: "successful operation" - responseSchema: + schema: type: "array" items: $ref: "#/definitions/ApiUsageResult" - schema: + responseSchema: type: "array" items: $ref: "#/definitions/ApiUsageResult" @@ -2041,11 +2075,11 @@ paths: responses: "200": description: "successful operation" - responseSchema: + schema: type: "array" items: $ref: "#/definitions/BugsnagEvent" - schema: + responseSchema: type: "array" items: $ref: "#/definitions/BugsnagEvent" @@ -2072,11 +2106,11 @@ paths: responses: "200": description: "successful operation" - responseSchema: + schema: type: "array" items: $ref: "#/definitions/CDPFile" - schema: + responseSchema: type: "array" items: $ref: "#/definitions/CDPFile" @@ -2097,11 +2131,11 @@ paths: responses: "200": description: "successful operation" - responseSchema: + schema: type: "array" items: $ref: "#/definitions/URL" - schema: + responseSchema: type: "array" items: $ref: "#/definitions/URL" @@ -2269,6 +2303,15 @@ definitions: type: "string" S3DownloadTimes: $ref: "#/definitions/Map" + GuardianUser: + type: "object" + properties: + userId: + type: "string" + email: + type: "string" + status: + type: "string" TripMonitorNotification: type: "object" properties: @@ -2739,6 +2782,14 @@ definitions: $ref: "#/definitions/JourneyState" notifyAtLeadingInterval: type: "boolean" + primary: + $ref: "#/definitions/GuardianUser" + companion: + $ref: "#/definitions/GuardianUser" + observers: + type: "array" + items: + $ref: "#/definitions/GuardianUser" StartTrackingPayload: type: "object" properties: @@ -2962,6 +3013,14 @@ definitions: type: "boolean" applicationId: type: "string" + guardians: + type: "array" + items: + $ref: "#/definitions/GuardianUser" + dependents: + type: "array" + items: + type: "string" MobilityProfile: type: "object" properties: diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/GetMonitoredTripsTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/GetMonitoredTripsTest.java index e84f88766..deaf711eb 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/GetMonitoredTripsTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/GetMonitoredTripsTest.java @@ -1,6 +1,5 @@ package org.opentripplanner.middleware.controllers.api; -import com.auth0.exception.Auth0Exception; import com.auth0.json.mgmt.users.User; import com.mongodb.BasicDBObject; import org.eclipse.jetty.http.HttpMethod; @@ -12,6 +11,7 @@ import org.opentripplanner.middleware.auth.Auth0Users; import org.opentripplanner.middleware.controllers.response.ResponseList; import org.opentripplanner.middleware.models.AdminUser; +import org.opentripplanner.middleware.models.GuardianUser; import org.opentripplanner.middleware.models.MonitoredTrip; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.persistence.Persistence; @@ -22,14 +22,21 @@ import org.opentripplanner.middleware.utils.HttpResponseValues; import org.opentripplanner.middleware.utils.JsonUtils; +import java.util.HashMap; +import java.util.List; + import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; 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.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.mockAuthenticatedGet; import static org.opentripplanner.middleware.testutils.ApiTestUtils.mockAuthenticatedRequest; @@ -55,9 +62,16 @@ public class GetMonitoredTripsTest extends OtpMiddlewareTestEnvironment { private static AdminUser multiAdminUser; private static OtpUser soloOtpUser; private static OtpUser multiOtpUser; + private static OtpUser guardianUserOne; + private static OtpUser dependentUserOne; + private static OtpUser guardianUserTwo; + private static OtpUser dependentUserTwo; + private static OtpUser guardianUserThree; + private static OtpUser dependentUserThree; private static final String MONITORED_TRIP_PATH = "api/secure/monitoredtrip"; - + private static final String ACCEPT_DEPENDENT_PATH = MONITORED_TRIP_PATH + "/acceptdependent"; private static final String DUMMY_STRING = "ABCDxyz"; + private static HashMap guardianHeaders; /** * Create Otp and Admin user accounts. Create Auth0 account for just the Otp users. If @@ -68,29 +82,42 @@ public static void setUp() { assumeTrue(IS_END_TO_END); setAuthDisabled(false); - // Mock the OTP server TODO: Run a live OTP instance? OtpTestUtils.mockOtpServer(); String multiUserEmail = ApiTestUtils.generateEmailAddress("test-multiotpuser"); soloOtpUser = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-solootpuser")); multiOtpUser = PersistenceTestUtils.createUser(multiUserEmail); multiAdminUser = PersistenceTestUtils.createAdminUser(multiUserEmail); + guardianUserOne = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("guardian-one")); + dependentUserOne = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-one")); + guardianUserTwo = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("guardian-two")); + dependentUserTwo = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-two")); + guardianUserThree = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("guardian-three")); + dependentUserThree = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-three")); try { - // Should use Auth0User.createNewAuth0User but this generates a random password preventing the mock headers - // from being able to use TEMP_AUTH0_USER_PASSWORD. - User auth0User = Auth0Users.createAuth0UserForEmail(soloOtpUser.email, TEMP_AUTH0_USER_PASSWORD); - soloOtpUser.auth0UserId = auth0User.getId(); - Persistence.otpUsers.replace(soloOtpUser.id, soloOtpUser); - auth0User = Auth0Users.createAuth0UserForEmail(multiUserEmail, TEMP_AUTH0_USER_PASSWORD); + createAndAssignAuth0User(soloOtpUser); + guardianHeaders = createAndAssignAuth0User(guardianUserOne); + User auth0User = Auth0Users.createAuth0UserForEmail(multiUserEmail, TEMP_AUTH0_USER_PASSWORD); multiOtpUser.auth0UserId = auth0User.getId(); Persistence.otpUsers.replace(multiOtpUser.id, multiOtpUser); - // Use the same Auth0 user id as otpUser2 as the email address is the same. + // Use the same Auth0 user id as multiOtpUser as the email address is the same. multiAdminUser.auth0UserId = auth0User.getId(); Persistence.adminUsers.replace(multiAdminUser.id, multiAdminUser); - } catch (Auth0Exception e) { + } catch (Exception e) { throw new RuntimeException(e); } } + /** + * Should use Auth0User.createNewAuth0User but this generates a random password preventing the mock headers + * from being able to use TEMP_AUTH0_USER_PASSWORD. + */ + private static HashMap createAndAssignAuth0User(OtpUser user) throws Exception { + User auth0User = Auth0Users.createAuth0UserForEmail(user.email, TEMP_AUTH0_USER_PASSWORD); + user.auth0UserId = auth0User.getId(); + Persistence.otpUsers.replace(user.id, user); + return getMockHeaders(user); + } + /** * Delete the users if they were not already deleted during the test script. */ @@ -98,12 +125,25 @@ public static void setUp() { public static void tearDown() { assumeTrue(IS_END_TO_END); restoreDefaultAuthDisabled(); - soloOtpUser = Persistence.otpUsers.getById(soloOtpUser.id); - if (soloOtpUser != null) soloOtpUser.delete(false); - multiOtpUser = Persistence.otpUsers.getById(multiOtpUser.id); - if (multiOtpUser != null) multiOtpUser.delete(false); multiAdminUser = Persistence.adminUsers.getById(multiAdminUser.id); if (multiAdminUser != null) multiAdminUser.delete(); + deleteOtpUser( + soloOtpUser.id, + multiOtpUser.id, + dependentUserOne.id, + guardianUserOne.id, + dependentUserTwo.id, + guardianUserTwo.id, + dependentUserThree.id, + guardianUserThree.id + ); + } + + private static void deleteOtpUser(String... ids) { + for (String id : ids) { + OtpUser user = Persistence.otpUsers.getById(id); + if (user != null) user.delete(false); + } } @AfterEach @@ -194,6 +234,48 @@ void canPreserveTripFields() throws Exception { assertEquals(updatedTrip.to.name, originalTrip.to.name); } + @Test + void canAcceptDependentRequest() { + dependentUserOne.guardians.add(new GuardianUser(guardianUserOne.id, guardianUserOne.email, GuardianUser.GuardianUserStatus.PENDING)); + Persistence.otpUsers.replace(dependentUserOne.id, dependentUserOne); + + String path = String.format("%s?userId=%s", ACCEPT_DEPENDENT_PATH, dependentUserOne.id); + makeGetRequest(path, guardianHeaders); + + guardianUserOne = Persistence.otpUsers.getById(guardianUserOne.id); + assertTrue(guardianUserOne.dependents.contains(dependentUserOne.id)); + + dependentUserOne = Persistence.otpUsers.getById(dependentUserOne.id); + List guardians = dependentUserOne.guardians; + guardians + .stream() + .filter(user -> user.userId.equals(guardianUserOne.id)) + .forEach(user -> assertEquals(GuardianUser.GuardianUserStatus.CONFIRMED, user.status)); + } + + @Test + void canInvalidateDependent() { + guardianUserTwo.dependents.add(dependentUserTwo.id); + Persistence.otpUsers.replace(guardianUserTwo.id, guardianUserTwo); + dependentUserTwo.guardians.add(new GuardianUser(guardianUserTwo.id, guardianUserTwo.email, GuardianUser.GuardianUserStatus.CONFIRMED)); + Persistence.otpUsers.replace(dependentUserTwo.id, dependentUserTwo); + guardianUserTwo.delete(false); + dependentUserTwo = Persistence.otpUsers.getById(dependentUserTwo.id); + GuardianUser guardianUser = dependentUserTwo.guardians.get(0); + assertEquals(GuardianUser.GuardianUserStatus.INVALID, guardianUser.status); + } + + @Test + void canRemoveGuardian() { + guardianUserThree.dependents.add(dependentUserThree.id); + Persistence.otpUsers.replace(guardianUserThree.id, guardianUserThree); + dependentUserThree.guardians.add(new GuardianUser(guardianUserThree.id, guardianUserThree.email, GuardianUser.GuardianUserStatus.CONFIRMED)); + Persistence.otpUsers.replace(dependentUserThree.id, dependentUserThree); + dependentUserThree.delete(false); + guardianUserThree = Persistence.otpUsers.getById(guardianUserThree.id); + assertFalse(guardianUserThree.dependents.contains(dependentUserThree.id)); + } + /** * Helper method to get trips for user. */ From df4493d7ee4dda16e61b5b76cb96a6f05f7d1ce0 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 24 Sep 2024 15:14:39 +0100 Subject: [PATCH 02/17] refactor(Updated swagger output): --- src/main/resources/latest-spark-swagger-output.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index 50cdd5a1a..a3ee640aa 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -2312,6 +2312,10 @@ definitions: type: "string" status: type: "string" + enum: + - "PENDING" + - "CONFIRMED" + - "INVALID" TripMonitorNotification: type: "object" properties: From 650517476a905057e7dacafdfe034d547baec868 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 16 Oct 2024 14:55:05 +0100 Subject: [PATCH 03/17] refactor(Moved a few things arounds): accept dependant is now under otp user api --- .../api/MonitoredTripController.java | 57 ----------- .../controllers/api/OtpUserController.java | 61 +++++++++++- .../middleware/models/MonitoredTrip.java | 6 +- .../middleware/models/OtpUser.java | 12 +-- .../{GuardianUser.java => RelatedUser.java} | 12 +-- ....java => MonitoredTripControllerTest.java} | 97 ++----------------- .../api/OtpUserControllerTest.java | 95 ++++++++++++++++-- .../middleware/testutils/ApiTestUtils.java | 11 +++ .../testutils/PersistenceTestUtils.java | 9 ++ 9 files changed, 186 insertions(+), 174 deletions(-) rename src/main/java/org/opentripplanner/middleware/models/{GuardianUser.java => RelatedUser.java} (50%) rename src/test/java/org/opentripplanner/middleware/controllers/api/{GetMonitoredTripsTest.java => MonitoredTripControllerTest.java} (68%) diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java index 518d3a850..f22f60bbb 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java @@ -5,16 +5,11 @@ import com.mongodb.client.model.Filters; import org.bson.conversions.Bson; import org.eclipse.jetty.http.HttpStatus; -import org.opentripplanner.middleware.auth.Auth0Connection; -import org.opentripplanner.middleware.auth.RequestingUser; -import org.opentripplanner.middleware.models.GuardianUser; import org.opentripplanner.middleware.models.ItineraryExistence; import org.opentripplanner.middleware.models.MonitoredTrip; -import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip; import org.opentripplanner.middleware.tripmonitor.jobs.MonitoredTripLocks; -import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.InvalidItineraryReason; import org.opentripplanner.middleware.utils.JsonUtils; import org.opentripplanner.middleware.utils.SwaggerUtils; @@ -47,12 +42,6 @@ public MonitoredTripController(String apiPrefix) { protected void buildEndpoint(ApiEndpoint baseEndpoint) { // Add the api key route BEFORE the regular CRUD methods ApiEndpoint modifiedEndpoint = 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(), - MonitoredTripController::acceptDependent - ) .post(path("/checkitinerary") .withDescription("Returns the itinerary existence check results for a monitored trip.") .withRequestType(MonitoredTrip.class) @@ -186,52 +175,6 @@ boolean preDeleteHook(MonitoredTrip monitoredTrip, Request req) { return true; } - /** - * Accept a request from another user to be their dependent. This will include both companions and observers. - */ - private static OtpUser acceptDependent(Request request, Response response) { - RequestingUser requestingUser = Auth0Connection.getUserFromRequest(request); - OtpUser guardianUser = requestingUser.otpUser; - if (guardianUser == null) { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Otp user unknown."); - return null; - } - - String dependentUserId = HttpUtils.getQueryParamFromRequest(request, USER_ID_PARAM, false); - if (dependentUserId.isEmpty()) { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user id not provided."); - return null; - } - - OtpUser dependentUser = Persistence.otpUsers.getById(dependentUserId); - if (dependentUser == null) { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user unknown!"); - return null; - } - - boolean isGuardian = dependentUser.guardians - .stream() - .filter(guardian -> guardian.userId.equals(guardianUser.id)) - // Update guardian status. This assumes the guardian with "pending" status was previously added. - .peek(guardian -> guardian.status = GuardianUser.GuardianUserStatus.CONFIRMED) - .findFirst() - .isPresent(); - - if (isGuardian) { - // Maintain a list of dependents. - guardianUser.dependents.add(dependentUserId); - Persistence.otpUsers.replace(guardianUser.id, guardianUser); - // Update list of guardians. - Persistence.otpUsers.replace(dependentUser.id, dependentUser); - } else { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent did not request user to be a guardian!"); - return null; - } - - // TODO: Not sure what is required in the response. For now, returning the updated guardian user. - return guardianUser; - } - /** * Check itinerary existence by making OTP requests on all days of the week. * @return The results of the itinerary existence check. diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java index b91df942f..f0a0c3db5 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java @@ -7,11 +7,13 @@ import org.eclipse.jetty.http.HttpStatus; import org.opentripplanner.middleware.auth.Auth0Connection; import org.opentripplanner.middleware.auth.RequestingUser; -import org.opentripplanner.middleware.models.MobilityProfile; import org.opentripplanner.middleware.models.OtpUser; +import org.opentripplanner.middleware.models.RelatedUser; import org.opentripplanner.middleware.persistence.Persistence; +import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.JsonUtils; import org.opentripplanner.middleware.utils.NotificationUtils; +import org.opentripplanner.middleware.utils.SwaggerUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import spark.Request; @@ -73,6 +75,16 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) { // Add the api key route BEFORE the regular CRUD methods ApiEndpoint modifiedEndpoint = 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(), + OtpUserController::acceptDependent + ) .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.") .withPathParam().withName(ID_PARAM).withRequired(true).withDescription("The id of the OtpUser.").and() @@ -183,4 +195,51 @@ public static boolean isPhoneNumberValidE164(String phoneNumber) { Matcher m = PHONE_E164_PATTERN.matcher(phoneNumber); return m.matches(); } + + /** + * Accept a request from another user to be their dependent. This will include both companions and observers. + */ + private static OtpUser acceptDependent(Request request, Response response) { + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(request); + OtpUser relatedUser = requestingUser.otpUser; + if (relatedUser == null) { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Otp user unknown."); + return null; + } + + String dependentUserId = HttpUtils.getQueryParamFromRequest(request, USER_ID_PARAM, false); + if (dependentUserId.isEmpty()) { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user id not provided."); + return null; + } + + OtpUser dependentUser = Persistence.otpUsers.getById(dependentUserId); + if (dependentUser == null) { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user unknown!"); + return null; + } + + boolean isRelated = dependentUser.relatedUsers + .stream() + .filter(related -> related.userId.equals(relatedUser.id)) + // 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(dependentUserId); + Persistence.otpUsers.replace(relatedUser.id, relatedUser); + // Update list of related users. + Persistence.otpUsers.replace(dependentUser.id, dependentUser); + } else { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent did not request user to be related!"); + return null; + } + + // TODO: Not sure what is required in the response. For now, returning the updated related user. + return relatedUser; + } + } diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java index bdea0590b..b870d8974 100644 --- a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java @@ -174,9 +174,9 @@ public class MonitoredTrip extends Model { */ public boolean notifyAtLeadingInterval = true; - public GuardianUser primary; - public GuardianUser companion; - public List observers = new ArrayList<>(); + public RelatedUser primary; + public RelatedUser companion; + public List observers = new ArrayList<>(); public MonitoredTrip() { } diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index 556419027..2de06cf6c 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -86,7 +86,7 @@ public enum Notification { public String applicationId; /** Companions and observers of this user. */ - public List guardians = new ArrayList<>(); + public List relatedUsers = new ArrayList<>(); /** Users that are dependent on this user. */ public List dependents = new ArrayList<>(); @@ -128,9 +128,9 @@ public boolean delete(boolean deleteAuth0User) { for (String userId: dependents) { OtpUser dependent = Persistence.otpUsers.getById(userId); if (dependent != null) { - for (GuardianUser guardianUser : dependent.guardians) { - if (guardianUser.userId.equals(this.id)) { - guardianUser.status = GuardianUser.GuardianUserStatus.INVALID; + for (RelatedUser relatedUser : dependent.relatedUsers) { + if (relatedUser.userId.equals(this.id)) { + relatedUser.status = RelatedUser.RelatedUserStatus.INVALID; } } Persistence.otpUsers.replace(dependent.id, dependent); @@ -138,8 +138,8 @@ public boolean delete(boolean deleteAuth0User) { } // If a dependent, remove relationship with all guardians. - for (GuardianUser guardianUser : guardians) { - OtpUser guardian = Persistence.otpUsers.getById(guardianUser.userId); + for (RelatedUser relatedUser : relatedUsers) { + OtpUser guardian = Persistence.otpUsers.getById(relatedUser.userId); if (guardian != null) { guardian.dependents.remove(this.id); Persistence.otpUsers.replace(guardian.id, guardian); diff --git a/src/main/java/org/opentripplanner/middleware/models/GuardianUser.java b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java similarity index 50% rename from src/main/java/org/opentripplanner/middleware/models/GuardianUser.java rename to src/main/java/org/opentripplanner/middleware/models/RelatedUser.java index aa013949c..640a53d72 100644 --- a/src/main/java/org/opentripplanner/middleware/models/GuardianUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java @@ -1,20 +1,20 @@ package org.opentripplanner.middleware.models; -/** A guardian user is a companion or observer requested by a dependent. */ -public class GuardianUser { - public enum GuardianUserStatus { +/** A related user is a companion or observer requested by a dependent. */ +public class RelatedUser { + public enum RelatedUserStatus { PENDING, CONFIRMED, INVALID } public String userId; public String email; - public GuardianUserStatus status; + public RelatedUserStatus status; - public GuardianUser() { + public RelatedUser() { // Required for JSON deserialization. } - public GuardianUser(String userId, String email, GuardianUserStatus status) { + public RelatedUser(String userId, String email, RelatedUserStatus status) { this.userId = userId; this.email = email; this.status = status; diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/GetMonitoredTripsTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/MonitoredTripControllerTest.java similarity index 68% rename from src/test/java/org/opentripplanner/middleware/controllers/api/GetMonitoredTripsTest.java rename to src/test/java/org/opentripplanner/middleware/controllers/api/MonitoredTripControllerTest.java index 2e0eae890..6f32e75c9 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/GetMonitoredTripsTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/MonitoredTripControllerTest.java @@ -10,7 +10,6 @@ import org.opentripplanner.middleware.auth.Auth0Users; import org.opentripplanner.middleware.controllers.response.ResponseList; import org.opentripplanner.middleware.models.AdminUser; -import org.opentripplanner.middleware.models.GuardianUser; import org.opentripplanner.middleware.models.ItineraryExistence; import org.opentripplanner.middleware.models.MonitoredTrip; import org.opentripplanner.middleware.models.OtpUser; @@ -24,24 +23,21 @@ import org.opentripplanner.middleware.utils.HttpResponseValues; import org.opentripplanner.middleware.utils.JsonUtils; -import java.util.HashMap; -import java.util.List; import java.util.Date; +import java.util.HashMap; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; 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.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.createAndAssignAuth0User; import static org.opentripplanner.middleware.testutils.ApiTestUtils.mockAuthenticatedGet; import static org.opentripplanner.middleware.testutils.ApiTestUtils.mockAuthenticatedRequest; +import static org.opentripplanner.middleware.testutils.PersistenceTestUtils.deleteOtpUser; /** * Tests to simulate getting trips as an Otp user with enhanced admin credentials. The following config parameters must @@ -61,18 +57,11 @@ * * Auth0 must be correctly configured as described here: https://auth0.com/docs/flows/call-your-api-using-resource-owner-password-flow */ -public class GetMonitoredTripsTest extends OtpMiddlewareTestEnvironment { +public class MonitoredTripControllerTest extends OtpMiddlewareTestEnvironment { private static AdminUser multiAdminUser; private static OtpUser soloOtpUser; private static OtpUser multiOtpUser; - private static OtpUser guardianUserOne; - private static OtpUser dependentUserOne; - private static OtpUser guardianUserTwo; - private static OtpUser dependentUserTwo; - private static OtpUser guardianUserThree; - private static OtpUser dependentUserThree; private static final String MONITORED_TRIP_PATH = "api/secure/monitoredtrip"; - private static final String ACCEPT_DEPENDENT_PATH = MONITORED_TRIP_PATH + "/acceptdependent"; private static final String UI_QUERY_PARAMS = "?fromPlace=fromplace%3A%3A28.556631%2C-81.411781&toPlace=toplace%3A%3A28.545925%2C-81.348609&date=2020-11-13&time=14%3A21&arriveBy=false&mode=WALK%2CBUS%2CRAIL&numItineraries=3"; @@ -88,20 +77,12 @@ public static void setUp() { assumeTrue(IS_END_TO_END); setAuthDisabled(false); - OtpTestUtils.mockOtpServer(); String multiUserEmail = ApiTestUtils.generateEmailAddress("test-multiotpuser"); soloOtpUser = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-solootpuser")); multiOtpUser = PersistenceTestUtils.createUser(multiUserEmail); multiAdminUser = PersistenceTestUtils.createAdminUser(multiUserEmail); - guardianUserOne = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("guardian-one")); - dependentUserOne = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-one")); - guardianUserTwo = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("guardian-two")); - dependentUserTwo = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-two")); - guardianUserThree = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("guardian-three")); - dependentUserThree = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-three")); try { createAndAssignAuth0User(soloOtpUser); - guardianHeaders = createAndAssignAuth0User(guardianUserOne); User auth0User = Auth0Users.createAuth0UserForEmail(multiUserEmail, TEMP_AUTH0_USER_PASSWORD); multiOtpUser.auth0UserId = auth0User.getId(); Persistence.otpUsers.replace(multiOtpUser.id, multiOtpUser); @@ -113,17 +94,6 @@ public static void setUp() { } } - /** - * Should use Auth0User.createNewAuth0User but this generates a random password preventing the mock headers - * from being able to use TEMP_AUTH0_USER_PASSWORD. - */ - private static HashMap createAndAssignAuth0User(OtpUser user) throws Exception { - User auth0User = Auth0Users.createAuth0UserForEmail(user.email, TEMP_AUTH0_USER_PASSWORD); - user.auth0UserId = auth0User.getId(); - Persistence.otpUsers.replace(user.id, user); - return getMockHeaders(user); - } - /** * Delete the users if they were not already deleted during the test script. */ @@ -134,24 +104,11 @@ public static void tearDown() { multiAdminUser = Persistence.adminUsers.getById(multiAdminUser.id); if (multiAdminUser != null) multiAdminUser.delete(); deleteOtpUser( - soloOtpUser.id, - multiOtpUser.id, - dependentUserOne.id, - guardianUserOne.id, - dependentUserTwo.id, - guardianUserTwo.id, - dependentUserThree.id, - guardianUserThree.id + soloOtpUser, + multiOtpUser ); } - private static void deleteOtpUser(String... ids) { - for (String id : ids) { - OtpUser user = Persistence.otpUsers.getById(id); - if (user != null) user.delete(false); - } - } - @AfterEach public void tearDownAfterTest() { Persistence.monitoredTrips.removeFiltered(getUserFilter(soloOtpUser.id)); @@ -240,48 +197,6 @@ void canPreserveTripFields() throws Exception { assertEquals(updatedTrip.to.name, originalTrip.to.name); } - @Test - void canAcceptDependentRequest() { - dependentUserOne.guardians.add(new GuardianUser(guardianUserOne.id, guardianUserOne.email, GuardianUser.GuardianUserStatus.PENDING)); - Persistence.otpUsers.replace(dependentUserOne.id, dependentUserOne); - - String path = String.format("%s?userId=%s", ACCEPT_DEPENDENT_PATH, dependentUserOne.id); - makeGetRequest(path, guardianHeaders); - - guardianUserOne = Persistence.otpUsers.getById(guardianUserOne.id); - assertTrue(guardianUserOne.dependents.contains(dependentUserOne.id)); - - dependentUserOne = Persistence.otpUsers.getById(dependentUserOne.id); - List guardians = dependentUserOne.guardians; - guardians - .stream() - .filter(user -> user.userId.equals(guardianUserOne.id)) - .forEach(user -> assertEquals(GuardianUser.GuardianUserStatus.CONFIRMED, user.status)); - } - - @Test - void canInvalidateDependent() { - guardianUserTwo.dependents.add(dependentUserTwo.id); - Persistence.otpUsers.replace(guardianUserTwo.id, guardianUserTwo); - dependentUserTwo.guardians.add(new GuardianUser(guardianUserTwo.id, guardianUserTwo.email, GuardianUser.GuardianUserStatus.CONFIRMED)); - Persistence.otpUsers.replace(dependentUserTwo.id, dependentUserTwo); - guardianUserTwo.delete(false); - dependentUserTwo = Persistence.otpUsers.getById(dependentUserTwo.id); - GuardianUser guardianUser = dependentUserTwo.guardians.get(0); - assertEquals(GuardianUser.GuardianUserStatus.INVALID, guardianUser.status); - } - - @Test - void canRemoveGuardian() { - guardianUserThree.dependents.add(dependentUserThree.id); - Persistence.otpUsers.replace(guardianUserThree.id, guardianUserThree); - dependentUserThree.guardians.add(new GuardianUser(guardianUserThree.id, guardianUserThree.email, GuardianUser.GuardianUserStatus.CONFIRMED)); - Persistence.otpUsers.replace(dependentUserThree.id, dependentUserThree); - dependentUserThree.delete(false); - guardianUserThree = Persistence.otpUsers.getById(guardianUserThree.id); - assertFalse(guardianUserThree.dependents.contains(dependentUserThree.id)); - } - /** * Helper method to get trips for user. */ diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java index 9912ef7b6..deb1c6f27 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java @@ -10,32 +10,50 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.opentripplanner.middleware.models.OtpUser; +import org.opentripplanner.middleware.models.RelatedUser; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.testutils.ApiTestUtils; import org.opentripplanner.middleware.testutils.OtpMiddlewareTestEnvironment; +import org.opentripplanner.middleware.testutils.PersistenceTestUtils; import org.opentripplanner.middleware.utils.HttpResponseValues; import org.opentripplanner.middleware.utils.JsonUtils; -import java.io.IOException; import java.util.Date; +import java.util.HashMap; +import java.util.List; 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.testutils.ApiTestUtils.createAndAssignAuth0User; 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.auth.Auth0Connection.restoreDefaultAuthDisabled; import static org.opentripplanner.middleware.auth.Auth0Connection.setAuthDisabled; +import static org.opentripplanner.middleware.testutils.PersistenceTestUtils.deleteOtpUser; public class OtpUserControllerTest extends OtpMiddlewareTestEnvironment { private static final String INITIAL_PHONE_NUMBER = "+15555550222"; // Fake US 555 number. private static OtpUser otpUser; + private static OtpUser relatedUserOne; + private static OtpUser dependentUserOne; + private static OtpUser relatedUserTwo; + private static OtpUser dependentUserTwo; + private static OtpUser relatedUserThree; + private static OtpUser dependentUserThree; + private static HashMap relatedUserHeaders; + public static final String ACCEPT_DEPENDENT_PATH = "api/secure/user/acceptdependent"; @BeforeAll - public static void setUp() throws IOException { - // Ensure auth is disabled. - setAuthDisabled(true); + public static void setUp() throws Exception { + assumeTrue(IS_END_TO_END); + // Set the overall auth to disabled. + setAuthDisabled(false); + // Create a persisted OTP user. otpUser = new OtpUser(); otpUser.email = ApiTestUtils.generateEmailAddress("test-otpusercont"); @@ -44,14 +62,27 @@ public static void setUp() throws IOException { otpUser.isPhoneNumberVerified = true; otpUser.smsConsentDate = new Date(); Persistence.otpUsers.create(otpUser); + + relatedUserOne = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("related-user-one")); + dependentUserOne = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-one")); + relatedUserTwo = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("related-user-two")); + dependentUserTwo = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-two")); + relatedUserThree = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("related-user-three")); + dependentUserThree = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-three")); + relatedUserHeaders = createAndAssignAuth0User(relatedUserOne); } @AfterAll public static void tearDown() { - // Delete the users if they were not already deleted during the test script. - otpUser = Persistence.otpUsers.getById(otpUser.id); - // Delete OtpUser. No need to delete Auth0 user since one was never created above (auth is disabled). - if (otpUser != null) otpUser.delete(false); + deleteOtpUser( + otpUser, + relatedUserOne, + relatedUserTwo, + relatedUserThree, + dependentUserOne, + dependentUserTwo, + dependentUserThree + ); // Restore original isAuthDisabled state. restoreDefaultAuthDisabled(); @@ -63,7 +94,8 @@ public static void tearDown() { */ @ParameterizedTest @MethodSource("createBadPhoneNumbers") - public void invalidNumbersShouldProduceBadRequest(String badNumber, int statusCode) throws Exception { + void invalidNumbersShouldProduceBadRequest(String badNumber, int statusCode) throws Exception { + setAuthDisabled(true); // 1. Request verification SMS. // The invalid number should fail the call. HttpResponseValues response = mockAuthenticatedGet( @@ -86,6 +118,7 @@ public void invalidNumbersShouldProduceBadRequest(String badNumber, int statusCo OtpUser otpUserWithPhone = JsonUtils.getPOJOFromJSON(otpUserWithPhoneRequest.responseBody, OtpUser.class); assertEquals(INITIAL_PHONE_NUMBER, otpUserWithPhone.phoneNumber); assertTrue(otpUserWithPhone.isPhoneNumberVerified); + setAuthDisabled(false); } private static Stream createBadPhoneNumbers() { @@ -100,7 +133,7 @@ private static Stream createBadPhoneNumbers() { */ @ParameterizedTest @MethodSource("createPhoneNumberTestCases") - public void isPhoneNumberValidE164(String number, boolean isValid) { + void isPhoneNumberValidE164(String number, boolean isValid) { assertEquals(isValid, OtpUserController.isPhoneNumberValidE164(number)); } @@ -136,4 +169,46 @@ void canPreserveSmsConsentDate() throws Exception { OtpUser updatedUser = Persistence.otpUsers.getById(otpUser.id); Assertions.assertEquals(otpUser.smsConsentDate, updatedUser.smsConsentDate); } + + @Test + void canAcceptDependentRequest() { + dependentUserOne.relatedUsers.add(new RelatedUser(relatedUserOne.id, relatedUserOne.email, RelatedUser.RelatedUserStatus.PENDING)); + Persistence.otpUsers.replace(dependentUserOne.id, dependentUserOne); + + String path = String.format("%s?userId=%s", ACCEPT_DEPENDENT_PATH, dependentUserOne.id); + makeGetRequest(path, relatedUserHeaders); + + relatedUserOne = Persistence.otpUsers.getById(relatedUserOne.id); + assertTrue(relatedUserOne.dependents.contains(dependentUserOne.id)); + + dependentUserOne = Persistence.otpUsers.getById(dependentUserOne.id); + List relatedUsers = dependentUserOne.relatedUsers; + relatedUsers + .stream() + .filter(user -> user.userId.equals(relatedUserOne.id)) + .forEach(user -> assertEquals(RelatedUser.RelatedUserStatus.CONFIRMED, user.status)); + } + + @Test + void canInvalidateDependent() { + relatedUserTwo.dependents.add(dependentUserTwo.id); + Persistence.otpUsers.replace(relatedUserTwo.id, relatedUserTwo); + dependentUserTwo.relatedUsers.add(new RelatedUser(relatedUserTwo.id, relatedUserTwo.email, RelatedUser.RelatedUserStatus.CONFIRMED)); + Persistence.otpUsers.replace(dependentUserTwo.id, dependentUserTwo); + relatedUserTwo.delete(false); + dependentUserTwo = Persistence.otpUsers.getById(dependentUserTwo.id); + RelatedUser relatedUser = dependentUserTwo.relatedUsers.get(0); + assertEquals(RelatedUser.RelatedUserStatus.INVALID, relatedUser.status); + } + + @Test + void canRemoveRelatedUser() { + relatedUserThree.dependents.add(dependentUserThree.id); + Persistence.otpUsers.replace(relatedUserThree.id, relatedUserThree); + dependentUserThree.relatedUsers.add(new RelatedUser(relatedUserThree.id, relatedUserThree.email, RelatedUser.RelatedUserStatus.CONFIRMED)); + Persistence.otpUsers.replace(dependentUserThree.id, dependentUserThree); + dependentUserThree.delete(false); + relatedUserThree = Persistence.otpUsers.getById(relatedUserThree.id); + assertFalse(relatedUserThree.dependents.contains(dependentUserThree.id)); + } } diff --git a/src/test/java/org/opentripplanner/middleware/testutils/ApiTestUtils.java b/src/test/java/org/opentripplanner/middleware/testutils/ApiTestUtils.java index 163caa5ab..c8250d2bf 100644 --- a/src/test/java/org/opentripplanner/middleware/testutils/ApiTestUtils.java +++ b/src/test/java/org/opentripplanner/middleware/testutils/ApiTestUtils.java @@ -1,5 +1,6 @@ package org.opentripplanner.middleware.testutils; +import com.auth0.json.mgmt.users.User; import org.eclipse.jetty.http.HttpMethod; import com.auth0.json.auth.TokenHolder; import org.eclipse.jetty.http.HttpStatus; @@ -197,4 +198,14 @@ public static String generateEmailAddress(String prefix) { return String.format("%s-%s@example.com", prefix, UUID.randomUUID().toString()); } + /** + * Should use Auth0User.createNewAuth0User but this generates a random password preventing the mock headers + * from being able to use TEMP_AUTH0_USER_PASSWORD. + */ + public static HashMap createAndAssignAuth0User(OtpUser user) throws Exception { + User auth0User = Auth0Users.createAuth0UserForEmail(user.email, TEMP_AUTH0_USER_PASSWORD); + user.auth0UserId = auth0User.getId(); + Persistence.otpUsers.replace(user.id, user); + return getMockHeaders(user); + } } diff --git a/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java b/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java index e87a75a6f..12ddff947 100644 --- a/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java +++ b/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java @@ -267,4 +267,13 @@ static Itinerary createItinerary() { itinerary.legs = legs; return itinerary; } + + public static void deleteOtpUser(OtpUser... optUsers) { + for (OtpUser otpUser : optUsers) { + OtpUser user = Persistence.otpUsers.getById(otpUser.id); + if (user != null) { + user.delete(user.auth0UserId != null); + } + } + } } From 35fc49ab874994d40a7eae611c9465484af6dbcd Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 16 Oct 2024 14:58:51 +0100 Subject: [PATCH 04/17] refactor(Updated Swagger doc): --- .../latest-spark-swagger-output.yaml | 332 +++++++++--------- 1 file changed, 166 insertions(+), 166 deletions(-) diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index e489055c2..8cc87c2cc 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -56,10 +56,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/AdminUser" schema: $ref: "#/definitions/AdminUser" + responseSchema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -90,10 +90,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/AdminUser" schema: $ref: "#/definitions/AdminUser" + responseSchema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -123,10 +123,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/Job" schema: $ref: "#/definitions/Job" + responseSchema: + $ref: "#/definitions/Job" /api/admin/user: get: tags: @@ -155,10 +155,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/ResponseList" schema: $ref: "#/definitions/ResponseList" + responseSchema: + $ref: "#/definitions/ResponseList" post: tags: - "api/admin/user" @@ -178,10 +178,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/AdminUser" schema: $ref: "#/definitions/AdminUser" + responseSchema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -220,10 +220,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/AdminUser" schema: $ref: "#/definitions/AdminUser" + responseSchema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -269,10 +269,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/AdminUser" schema: $ref: "#/definitions/AdminUser" + responseSchema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -309,10 +309,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/AdminUser" schema: $ref: "#/definitions/AdminUser" + responseSchema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -356,10 +356,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -402,10 +402,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -447,10 +447,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/TokenHolder" schema: $ref: "#/definitions/TokenHolder" + responseSchema: + $ref: "#/definitions/TokenHolder" /api/secure/application/fromtoken: get: tags: @@ -464,10 +464,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -498,10 +498,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -531,10 +531,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/Job" schema: $ref: "#/definitions/Job" + responseSchema: + $ref: "#/definitions/Job" /api/secure/application: get: tags: @@ -563,10 +563,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/ResponseList" schema: $ref: "#/definitions/ResponseList" + responseSchema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/application" @@ -586,10 +586,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -628,10 +628,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -677,10 +677,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -717,10 +717,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ApiUser" schema: $ref: "#/definitions/ApiUser" + responseSchema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -754,10 +754,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/CDPUser" schema: $ref: "#/definitions/CDPUser" + responseSchema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -788,10 +788,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/CDPUser" schema: $ref: "#/definitions/CDPUser" + responseSchema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -821,10 +821,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/Job" schema: $ref: "#/definitions/Job" + responseSchema: + $ref: "#/definitions/Job" /api/secure/cdp: get: tags: @@ -853,10 +853,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/ResponseList" schema: $ref: "#/definitions/ResponseList" + responseSchema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/cdp" @@ -876,10 +876,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/CDPUser" schema: $ref: "#/definitions/CDPUser" + responseSchema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -918,10 +918,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/CDPUser" schema: $ref: "#/definitions/CDPUser" + responseSchema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -967,10 +967,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/CDPUser" schema: $ref: "#/definitions/CDPUser" + responseSchema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1007,44 +1007,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/CDPUser" schema: $ref: "#/definitions/CDPUser" - "400": - description: "The request was not formed properly (e.g., some required parameters\ - \ may be missing). See the details of the returned response to determine\ - \ the exact issue." - examples: {} - "401": - description: "The server was not able to authenticate the request. This\ - \ can happen if authentication headers are missing or malformed, or the\ - \ authentication server cannot be reached." - examples: {} - "403": - description: "The requesting user is not allowed to perform the request." - examples: {} - "404": - description: "The requested item was not found." - examples: {} - "500": - description: "An error occurred while performing the request. Contact an\ - \ API administrator for more information." - examples: {} - /api/secure/monitoredtrip/acceptdependent: - get: - tags: - - "api/secure/monitoredtrip" - description: "Accept a dependent request." - parameters: [] - responses: - "200": - description: "Successful operation" - examples: {} responseSchema: - $ref: "#/definitions/OtpUser" - schema: - $ref: "#/definitions/OtpUser" + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1084,10 +1050,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/ItineraryExistence" schema: $ref: "#/definitions/ItineraryExistence" + responseSchema: + $ref: "#/definitions/ItineraryExistence" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1136,10 +1102,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/ResponseList" schema: $ref: "#/definitions/ResponseList" + responseSchema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/monitoredtrip" @@ -1159,10 +1125,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredTrip" schema: $ref: "#/definitions/MonitoredTrip" + responseSchema: + $ref: "#/definitions/MonitoredTrip" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1201,10 +1167,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredTrip" schema: $ref: "#/definitions/MonitoredTrip" + responseSchema: + $ref: "#/definitions/MonitoredTrip" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1250,10 +1216,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredTrip" schema: $ref: "#/definitions/MonitoredTrip" + responseSchema: + $ref: "#/definitions/MonitoredTrip" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1291,10 +1257,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredTrip" schema: $ref: "#/definitions/MonitoredTrip" + responseSchema: + $ref: "#/definitions/MonitoredTrip" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1332,10 +1298,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/TrackingResponse" schema: $ref: "#/definitions/TrackingResponse" + responseSchema: + $ref: "#/definitions/TrackingResponse" /api/secure/monitoredtrip/updatetracking: post: tags: @@ -1353,10 +1319,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/TrackingResponse" schema: $ref: "#/definitions/TrackingResponse" + responseSchema: + $ref: "#/definitions/TrackingResponse" /api/secure/monitoredtrip/track: post: tags: @@ -1374,10 +1340,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/TrackingResponse" schema: $ref: "#/definitions/TrackingResponse" + responseSchema: + $ref: "#/definitions/TrackingResponse" /api/secure/monitoredtrip/endtracking: post: tags: @@ -1395,10 +1361,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/EndTrackingResponse" schema: $ref: "#/definitions/EndTrackingResponse" + responseSchema: + $ref: "#/definitions/EndTrackingResponse" /api/secure/monitoredtrip/forciblyendtracking: post: tags: @@ -1416,10 +1382,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/EndTrackingResponse" schema: $ref: "#/definitions/EndTrackingResponse" + responseSchema: + $ref: "#/definitions/EndTrackingResponse" /api/secure/triprequests: get: tags: @@ -1464,10 +1430,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/TripRequest" schema: $ref: "#/definitions/TripRequest" + responseSchema: + $ref: "#/definitions/TripRequest" /api/secure/monitoredcomponent: get: tags: @@ -1496,10 +1462,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/ResponseList" schema: $ref: "#/definitions/ResponseList" + responseSchema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/monitoredcomponent" @@ -1519,10 +1485,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredComponent" schema: $ref: "#/definitions/MonitoredComponent" + responseSchema: + $ref: "#/definitions/MonitoredComponent" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1561,10 +1527,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredComponent" schema: $ref: "#/definitions/MonitoredComponent" + responseSchema: + $ref: "#/definitions/MonitoredComponent" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1610,10 +1576,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/MonitoredComponent" schema: $ref: "#/definitions/MonitoredComponent" + responseSchema: + $ref: "#/definitions/MonitoredComponent" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1651,10 +1617,44 @@ paths: "200": description: "Successful operation" examples: {} + schema: + $ref: "#/definitions/MonitoredComponent" responseSchema: $ref: "#/definitions/MonitoredComponent" + "400": + description: "The request was not formed properly (e.g., some required parameters\ + \ may be missing). See the details of the returned response to determine\ + \ the exact issue." + examples: {} + "401": + description: "The server was not able to authenticate the request. This\ + \ can happen if authentication headers are missing or malformed, or the\ + \ authentication server cannot be reached." + examples: {} + "403": + description: "The requesting user is not allowed to perform the request." + examples: {} + "404": + description: "The requested item was not found." + examples: {} + "500": + description: "An error occurred while performing the request. Contact an\ + \ API administrator for more information." + examples: {} + /api/secure/user/acceptdependent: + get: + tags: + - "api/secure/user" + description: "Accept a dependent request." + parameters: [] + responses: + "200": + description: "Successful operation" + examples: {} schema: - $ref: "#/definitions/MonitoredComponent" + $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1694,10 +1694,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/VerificationResult" schema: $ref: "#/definitions/VerificationResult" + responseSchema: + $ref: "#/definitions/VerificationResult" /api/secure/user/{id}/verify_sms/{code}: post: tags: @@ -1717,10 +1717,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/VerificationResult" schema: $ref: "#/definitions/VerificationResult" + responseSchema: + $ref: "#/definitions/VerificationResult" /api/secure/user/fromtoken: get: tags: @@ -1734,10 +1734,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/OtpUser" schema: $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1768,10 +1768,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/OtpUser" schema: $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1801,10 +1801,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/Job" schema: $ref: "#/definitions/Job" + responseSchema: + $ref: "#/definitions/Job" /api/secure/user: get: tags: @@ -1833,10 +1833,10 @@ paths: responses: "200": description: "successful operation" - responseSchema: - $ref: "#/definitions/ResponseList" schema: $ref: "#/definitions/ResponseList" + responseSchema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/user" @@ -1856,10 +1856,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/OtpUser" schema: $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1898,10 +1898,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/OtpUser" schema: $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1947,10 +1947,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/OtpUser" schema: $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1987,10 +1987,10 @@ paths: "200": description: "Successful operation" examples: {} - responseSchema: - $ref: "#/definitions/OtpUser" schema: $ref: "#/definitions/OtpUser" + responseSchema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -2044,11 +2044,11 @@ paths: responses: "200": description: "successful operation" - responseSchema: + schema: type: "array" items: $ref: "#/definitions/ApiUsageResult" - schema: + responseSchema: type: "array" items: $ref: "#/definitions/ApiUsageResult" @@ -2075,11 +2075,11 @@ paths: responses: "200": description: "successful operation" - responseSchema: + schema: type: "array" items: $ref: "#/definitions/BugsnagEvent" - schema: + responseSchema: type: "array" items: $ref: "#/definitions/BugsnagEvent" @@ -2106,11 +2106,11 @@ paths: responses: "200": description: "successful operation" - responseSchema: + schema: type: "array" items: $ref: "#/definitions/CDPFile" - schema: + responseSchema: type: "array" items: $ref: "#/definitions/CDPFile" @@ -2131,11 +2131,11 @@ paths: responses: "200": description: "successful operation" - responseSchema: + schema: type: "array" items: $ref: "#/definitions/URL" - schema: + responseSchema: type: "array" items: $ref: "#/definitions/URL" @@ -2601,13 +2601,13 @@ definitions: notifyAtLeadingInterval: type: "boolean" primary: - $ref: "#/definitions/GuardianUser" + $ref: "#/definitions/RelatedUser" companion: - $ref: "#/definitions/GuardianUser" + $ref: "#/definitions/RelatedUser" observers: type: "array" items: - $ref: "#/definitions/GuardianUser" + $ref: "#/definitions/RelatedUser" StopTime: type: "object" properties: @@ -2640,19 +2640,6 @@ definitions: type: "string" name: type: "string" - GuardianUser: - type: "object" - properties: - userId: - type: "string" - email: - type: "string" - status: - type: "string" - enum: - - "PENDING" - - "CONFIRMED" - - "INVALID" OtpGraphQLRoutesAndTrips: type: "object" properties: @@ -2884,6 +2871,19 @@ definitions: - "PAST_TRIP" hasRealtimeData: type: "boolean" + RelatedUser: + type: "object" + properties: + userId: + type: "string" + email: + type: "string" + status: + type: "string" + enum: + - "PENDING" + - "CONFIRMED" + - "INVALID" FareComponent: type: "object" properties: @@ -3170,10 +3170,10 @@ definitions: type: "boolean" applicationId: type: "string" - guardians: + relatedUsers: type: "array" items: - $ref: "#/definitions/GuardianUser" + $ref: "#/definitions/RelatedUser" dependents: type: "array" items: From 4b2c56921144c20119860a0c397b376013cc03cc Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 16 Oct 2024 15:18:08 +0100 Subject: [PATCH 05/17] refactor(PersistenceTestUtils.java): Updated to guard against null OTP users when deleting --- .../middleware/testutils/PersistenceTestUtils.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java b/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java index 12ddff947..7ba62494a 100644 --- a/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java +++ b/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java @@ -270,9 +270,11 @@ static Itinerary createItinerary() { public static void deleteOtpUser(OtpUser... optUsers) { for (OtpUser otpUser : optUsers) { - OtpUser user = Persistence.otpUsers.getById(otpUser.id); - if (user != null) { - user.delete(user.auth0UserId != null); + if (otpUser != null) { + OtpUser user = Persistence.otpUsers.getById(otpUser.id); + if (user != null) { + user.delete(user.auth0UserId != null); + } } } } From bacb87e3ce664a39ffc25e8e7101e08cab00553e Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 23 Oct 2024 16:02:44 +0100 Subject: [PATCH 06/17] refactor(Update to send email to accpet trusted companion): --- .../controllers/api/OtpUserController.java | 68 ++------- .../middleware/i18n/Message.java | 5 + .../middleware/models/RelatedUser.java | 1 + .../tripmonitor/TrustedCompanion.java | 141 ++++++++++++++++++ .../tripmonitor/jobs/CheckMonitoredTrip.java | 4 +- src/main/resources/Message.properties | 5 + src/main/resources/Message_fr.properties | 5 + .../latest-spark-swagger-output.yaml | 2 + .../templates/AcceptDependentHtml.ftl | 15 ++ .../templates/AcceptDependentText.ftl | 11 ++ .../api/OtpUserControllerTest.java | 3 +- 11 files changed, 204 insertions(+), 56 deletions(-) create mode 100644 src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java create mode 100644 src/main/resources/templates/AcceptDependentHtml.ftl create mode 100644 src/main/resources/templates/AcceptDependentText.ftl diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java index f0a0c3db5..39bce3be6 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java @@ -1,16 +1,15 @@ package org.opentripplanner.middleware.controllers.api; -import io.github.manusant.ss.ApiEndpoint; import com.twilio.rest.verify.v2.service.Verification; import com.twilio.rest.verify.v2.service.VerificationCheck; +import io.github.manusant.ss.ApiEndpoint; import org.apache.commons.lang3.StringUtils; import org.eclipse.jetty.http.HttpStatus; import org.opentripplanner.middleware.auth.Auth0Connection; import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.models.OtpUser; -import org.opentripplanner.middleware.models.RelatedUser; import org.opentripplanner.middleware.persistence.Persistence; -import org.opentripplanner.middleware.utils.HttpUtils; +import org.opentripplanner.middleware.tripmonitor.TrustedCompanion; import org.opentripplanner.middleware.utils.JsonUtils; import org.opentripplanner.middleware.utils.NotificationUtils; import org.opentripplanner.middleware.utils.SwaggerUtils; @@ -25,6 +24,7 @@ import java.util.regex.Pattern; import static io.github.manusant.ss.descriptor.MethodDescriptor.path; +import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.manageAcceptDependentEmail; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; /** @@ -35,11 +35,18 @@ public class OtpUserController extends AbstractUserController { private static final Logger LOG = LoggerFactory.getLogger(OtpUserController.class); private static final String CODE_PARAM = "code"; + private static final String PHONE_PARAM = "phoneNumber"; + private static final String VERIFY_PATH = "verify_sms"; + public static final String OTP_USER_PATH = "secure/user"; + private static final String VERIFY_ROUTE_TEMPLATE = "/:%s/%s/:%s"; - /** Regex to check E.164 phone number format per https://www.twilio.com/docs/glossary/what-e164 */ + + /** + * Regex to check E.164 phone number format per https://www.twilio.com/docs/glossary/what-e164 + */ private static final Pattern PHONE_E164_PATTERN = Pattern.compile("^\\+[1-9]\\d{1,14}$"); public OtpUserController(String apiPrefix) { @@ -58,6 +65,7 @@ OtpUser preCreateHook(OtpUser user, Request req) { if (Objects.nonNull(user.mobilityProfile)) { user.mobilityProfile.updateMobilityMode(); } + manageAcceptDependentEmail(user); return super.preCreateHook(user, req); } @@ -66,6 +74,7 @@ OtpUser preUpdateHook(OtpUser user, OtpUser preExistingUser, Request req) { if (Objects.nonNull(user.mobilityProfile)) { user.mobilityProfile.updateMobilityMode(); } + manageAcceptDependentEmail(user); return super.preUpdateHook(user, preExistingUser, req); } @@ -83,7 +92,7 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) { .withRequired(true) .withDescription("The dependent user id.") .and(), - OtpUserController::acceptDependent + TrustedCompanion::acceptDependent ) .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.") @@ -195,51 +204,4 @@ public static boolean isPhoneNumberValidE164(String phoneNumber) { Matcher m = PHONE_E164_PATTERN.matcher(phoneNumber); return m.matches(); } - - /** - * Accept a request from another user to be their dependent. This will include both companions and observers. - */ - private static OtpUser acceptDependent(Request request, Response response) { - RequestingUser requestingUser = Auth0Connection.getUserFromRequest(request); - OtpUser relatedUser = requestingUser.otpUser; - if (relatedUser == null) { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Otp user unknown."); - return null; - } - - String dependentUserId = HttpUtils.getQueryParamFromRequest(request, USER_ID_PARAM, false); - if (dependentUserId.isEmpty()) { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user id not provided."); - return null; - } - - OtpUser dependentUser = Persistence.otpUsers.getById(dependentUserId); - if (dependentUser == null) { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user unknown!"); - return null; - } - - boolean isRelated = dependentUser.relatedUsers - .stream() - .filter(related -> related.userId.equals(relatedUser.id)) - // 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(dependentUserId); - Persistence.otpUsers.replace(relatedUser.id, relatedUser); - // Update list of related users. - Persistence.otpUsers.replace(dependentUser.id, dependentUser); - } else { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent did not request user to be related!"); - return null; - } - - // TODO: Not sure what is required in the response. For now, returning the updated related user. - return relatedUser; - } - -} +} \ No newline at end of file diff --git a/src/main/java/org/opentripplanner/middleware/i18n/Message.java b/src/main/java/org/opentripplanner/middleware/i18n/Message.java index d3946e31f..da82bca0e 100644 --- a/src/main/java/org/opentripplanner/middleware/i18n/Message.java +++ b/src/main/java/org/opentripplanner/middleware/i18n/Message.java @@ -13,6 +13,11 @@ * Message.properties. */ public enum Message { + ACCEPT_DEPENDENT_EMAIL_FOOTER, + ACCEPT_DEPENDENT_EMAIL_GREETING, + ACCEPT_DEPENDENT_EMAIL_LINK_TEXT, + ACCEPT_DEPENDENT_EMAIL_SUBJECT, + ACCEPT_DEPENDENT_EMAIL_MANAGE, LABEL_AND_CONTENT, SMS_STOP_NOTIFICATIONS, TRIP_EMAIL_SUBJECT, diff --git a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java index 640a53d72..e16473d4a 100644 --- a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java @@ -9,6 +9,7 @@ public enum RelatedUserStatus { public String userId; public String email; public RelatedUserStatus status; + public boolean acceptDependentEmailSent; public RelatedUser() { // Required for JSON deserialization. diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java new file mode 100644 index 000000000..c75f1a3d0 --- /dev/null +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java @@ -0,0 +1,141 @@ +package org.opentripplanner.middleware.tripmonitor; + +import org.eclipse.jetty.http.HttpStatus; +import org.opentripplanner.middleware.auth.Auth0Connection; +import org.opentripplanner.middleware.auth.RequestingUser; +import org.opentripplanner.middleware.i18n.Message; +import org.opentripplanner.middleware.models.OtpUser; +import org.opentripplanner.middleware.models.RelatedUser; +import org.opentripplanner.middleware.persistence.Persistence; +import org.opentripplanner.middleware.utils.ConfigUtils; +import org.opentripplanner.middleware.utils.HttpUtils; +import org.opentripplanner.middleware.utils.I18nUtils; +import org.opentripplanner.middleware.utils.NotificationUtils; +import spark.Request; +import spark.Response; + +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; + +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; + +public class TrustedCompanion { + + private TrustedCompanion() { + throw new IllegalStateException("Utility class"); + } + + private static final String OTP_UI_URL = ConfigUtils.getConfigPropertyAsText("OTP_UI_URL"); + + public static final String ACCEPT_DEPENDENT_PATH = "api/secure/user/acceptdependent"; + + /** + * Accept a request from another user to be their dependent. This will include both companions and observers. + */ + public static OtpUser acceptDependent(Request request, Response response) { + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(request); + OtpUser relatedUser = requestingUser.otpUser; + if (relatedUser == null) { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Otp user unknown."); + return null; + } + + String dependentUserId = HttpUtils.getQueryParamFromRequest(request, USER_ID_PARAM, false); + if (dependentUserId.isEmpty()) { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user id not provided."); + return null; + } + + OtpUser dependentUser = Persistence.otpUsers.getById(dependentUserId); + if (dependentUser == null) { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user unknown!"); + return null; + } + + boolean isRelated = dependentUser.relatedUsers + .stream() + .filter(related -> related.userId.equals(relatedUser.id)) + // 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(dependentUserId); + Persistence.otpUsers.replace(relatedUser.id, relatedUser); + // Update list of related users. + Persistence.otpUsers.replace(dependentUser.id, dependentUser); + } else { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent did not request user to be related!"); + return null; + } + + // TODO: Not sure what is required in the response. For now, returning the updated related user. + return relatedUser; + } + + /** + * When creating or updating an OTP user, extract a list of newly defined dependents and send an 'accept dependent' + * email to each. Then update which dependents have been sent an email so subsequent updates do not trigger + * additional emails. + */ + public static void manageAcceptDependentEmail(OtpUser dependentUser) { + if (dependentUser.relatedUsers.isEmpty()) { + // No related users defined by dependent. + return; + } + + dependentUser.relatedUsers + .stream() + .filter(relatedUser -> !relatedUser.acceptDependentEmailSent) + .forEach(relatedUser -> { + OtpUser user = Persistence.otpUsers.getById(relatedUser.userId); + if (user != null && sendAcceptDependentEmail(dependentUser, user)) { + relatedUser.acceptDependentEmailSent = true; + } + }); + + // Preserve email sent status. + Persistence.otpUsers.replace(dependentUser.id, dependentUser); + } + + /** + * Send 'accept dependent' email. + */ + private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser relatedUser) { + Locale locale = I18nUtils.getOtpUserLocale(relatedUser); + + String acceptDependentLinkLabel = Message.ACCEPT_DEPENDENT_EMAIL_LINK_TEXT.get(locale); + String acceptDependentUrl = getAcceptDependentUrl(dependentUser); + + // A HashMap is needed instead of a Map for template data to be serialized to the template renderer. + Map templateData = new HashMap<>(Map.of( + "acceptDependentLinkAnchorLabel", acceptDependentLinkLabel, + "acceptDependentLinkLabelAndUrl", label(acceptDependentLinkLabel, acceptDependentUrl, locale), + "acceptDependentUrl", getAcceptDependentUrl(dependentUser), + "emailFooter", Message.ACCEPT_DEPENDENT_EMAIL_FOOTER.get(locale), + "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 link to so providing link back to settings. + "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH), + "manageLinkText", Message.ACCEPT_DEPENDENT_EMAIL_MANAGE.get(locale) + )); + + return NotificationUtils.sendEmail( + relatedUser, + Message.ACCEPT_DEPENDENT_EMAIL_SUBJECT.get(locale), + "AcceptDependentText.ftl", + "AcceptDependentHtml.ftl", + templateData + ); + } + + private static String getAcceptDependentUrl(OtpUser dependentUser) { + // TODO: Is OTP_UI_URL the correct base URL to user here? If not, what?! + return String.format("%s%s?userId=%s", OTP_UI_URL, ACCEPT_DEPENDENT_PATH, dependentUser.id); + } +} diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index 081bbbc66..0ca1d2095 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -57,11 +57,11 @@ public class CheckMonitoredTrip implements Runnable { public static final int MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS = ConfigUtils.getConfigPropertyAsInt("MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS", 3); - private final String ACCOUNT_PATH = "/#/account"; + public static final String ACCOUNT_PATH = "/#/account"; private final String TRIPS_PATH = ACCOUNT_PATH + "/trips"; - private final String SETTINGS_PATH = ACCOUNT_PATH + "/settings"; + public static final String SETTINGS_PATH = ACCOUNT_PATH + "/settings"; public final MonitoredTrip trip; diff --git a/src/main/resources/Message.properties b/src/main/resources/Message.properties index 2f3a461b4..77a8b273b 100644 --- a/src/main/resources/Message.properties +++ b/src/main/resources/Message.properties @@ -1,3 +1,8 @@ +ACCEPT_DEPENDENT_EMAIL_FOOTER = You are receiving this email because you have been selected to be a trusted companion. +ACCEPT_DEPENDENT_EMAIL_GREETING = %s would like you to be their trusted companion. +ACCEPT_DEPENDENT_EMAIL_LINK_TEXT = Accept trusted companion +ACCEPT_DEPENDENT_EMAIL_SUBJECT = Trusted companion request +ACCEPT_DEPENDENT_EMAIL_MANAGE = Manage settings LABEL_AND_CONTENT = %s: %s SMS_STOP_NOTIFICATIONS = To stop receiving notifications, reply STOP. TRIP_EMAIL_SUBJECT = %s Notification diff --git a/src/main/resources/Message_fr.properties b/src/main/resources/Message_fr.properties index b64acce24..628bfd6d5 100644 --- a/src/main/resources/Message_fr.properties +++ b/src/main/resources/Message_fr.properties @@ -1,3 +1,8 @@ +ACCEPT_DEPENDENT_EMAIL_FOOTER = TODO +ACCEPT_DEPENDENT_EMAIL_GREETING = %s TODO. +ACCEPT_DEPENDENT_EMAIL_LINK_TEXT = TODO +ACCEPT_DEPENDENT_EMAIL_SUBJECT = TODO +ACCEPT_DEPENDENT_EMAIL_MANAGE = TODO LABEL_AND_CONTENT = %s\u00A0: %s SMS_STOP_NOTIFICATIONS = Pour arrêter ces notifications, envoyez STOP. TRIP_EMAIL_SUBJECT = Notification pour %s diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index 010aa1bd2..f6956e2f7 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -2887,6 +2887,8 @@ definitions: - "PENDING" - "CONFIRMED" - "INVALID" + acceptDependentEmailSent: + type: "boolean" FareComponent: type: "object" properties: diff --git a/src/main/resources/templates/AcceptDependentHtml.ftl b/src/main/resources/templates/AcceptDependentHtml.ftl new file mode 100644 index 000000000..583bd5549 --- /dev/null +++ b/src/main/resources/templates/AcceptDependentHtml.ftl @@ -0,0 +1,15 @@ +<#ftl auto_esc=false> +<#include "OtpUserContainer.ftl"> + +<#-- + This is a template for an HTML email that gets sent when a dependent user is requesting a trusted companion. +--> + +<#macro EmailMain> +
+

${emailGreeting}

+

${acceptDependentLinkAnchorLabel}

+
+ + +<@HtmlEmail/> \ No newline at end of file diff --git a/src/main/resources/templates/AcceptDependentText.ftl b/src/main/resources/templates/AcceptDependentText.ftl new file mode 100644 index 000000000..c87ed5a0c --- /dev/null +++ b/src/main/resources/templates/AcceptDependentText.ftl @@ -0,0 +1,11 @@ +<#-- + This is a template for a text email that gets sent when a dependent requests a trusted companion. + + Note: in plain text emails, all whitespace is preserved, + so the indentation of the notification content is intentionally not aligned + with the indentation of the macros. + +--> +${emailGreeting} + +${tripLinkLabelAndUrl} \ No newline at end of file diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java index deb1c6f27..70a1b3221 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java @@ -27,6 +27,7 @@ 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.testutils.ApiTestUtils.createAndAssignAuth0User; import static org.opentripplanner.middleware.testutils.ApiTestUtils.getMockHeaders; import static org.opentripplanner.middleware.testutils.ApiTestUtils.makeGetRequest; @@ -35,6 +36,7 @@ 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; public class OtpUserControllerTest extends OtpMiddlewareTestEnvironment { private static final String INITIAL_PHONE_NUMBER = "+15555550222"; // Fake US 555 number. @@ -46,7 +48,6 @@ public class OtpUserControllerTest extends OtpMiddlewareTestEnvironment { private static OtpUser relatedUserThree; private static OtpUser dependentUserThree; private static HashMap relatedUserHeaders; - public static final String ACCEPT_DEPENDENT_PATH = "api/secure/user/acceptdependent"; @BeforeAll public static void setUp() throws Exception { From c3214fa245a2cafbb807e70db60383309597eaff Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 24 Oct 2024 11:57:33 +0100 Subject: [PATCH 07/17] refactor(Update to send email): Sends accept depentent email to related user on create/update if app --- .../tripmonitor/TrustedCompanion.java | 25 +++++++---- src/main/resources/Message.properties | 2 +- .../api/MonitoredTripControllerTest.java | 1 + .../api/OtpUserControllerTest.java | 1 + .../testutils/PersistenceTestUtils.java | 4 +- .../tripmonitor/TrustedCompanionTest.java | 43 +++++++++++++++++++ 6 files changed, 64 insertions(+), 12 deletions(-) create mode 100644 src/test/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanionTest.java diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java index c75f1a3d0..4957393de 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java @@ -79,12 +79,16 @@ public static OtpUser acceptDependent(Request request, Response response) { return relatedUser; } + public static void manageAcceptDependentEmail(OtpUser dependentUser) { + manageAcceptDependentEmail(dependentUser, false); + } + /** * When creating or updating an OTP user, extract a list of newly defined dependents and send an 'accept dependent' * email to each. Then update which dependents have been sent an email so subsequent updates do not trigger * additional emails. */ - public static void manageAcceptDependentEmail(OtpUser dependentUser) { + public static void manageAcceptDependentEmail(OtpUser dependentUser, boolean isTest) { if (dependentUser.relatedUsers.isEmpty()) { // No related users defined by dependent. return; @@ -94,8 +98,8 @@ public static void manageAcceptDependentEmail(OtpUser dependentUser) { .stream() .filter(relatedUser -> !relatedUser.acceptDependentEmailSent) .forEach(relatedUser -> { - OtpUser user = Persistence.otpUsers.getById(relatedUser.userId); - if (user != null && sendAcceptDependentEmail(dependentUser, user)) { + OtpUser userToReceiveEmail = Persistence.otpUsers.getById(relatedUser.userId); + if (userToReceiveEmail != null && (isTest || sendAcceptDependentEmail(dependentUser, userToReceiveEmail))) { relatedUser.acceptDependentEmailSent = true; } }); @@ -114,16 +118,19 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r String acceptDependentUrl = getAcceptDependentUrl(dependentUser); // A HashMap is needed instead of a Map for template data to be serialized to the template renderer. - Map templateData = new HashMap<>(Map.of( + Map templateData = new HashMap<>( + Map.of( "acceptDependentLinkAnchorLabel", acceptDependentLinkLabel, "acceptDependentLinkLabelAndUrl", label(acceptDependentLinkLabel, acceptDependentUrl, locale), "acceptDependentUrl", getAcceptDependentUrl(dependentUser), "emailFooter", Message.ACCEPT_DEPENDENT_EMAIL_FOOTER.get(locale), - "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 link to so providing link back to settings. - "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH), + // 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? + "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH), "manageLinkText", Message.ACCEPT_DEPENDENT_EMAIL_MANAGE.get(locale) - )); + ) + ); return NotificationUtils.sendEmail( relatedUser, @@ -135,7 +142,7 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r } private static String getAcceptDependentUrl(OtpUser dependentUser) { - // TODO: Is OTP_UI_URL the correct base URL to user here? If not, what?! + // TODO: Is OTP_UI_URL the correct base URL to user here? I'm not sure. return String.format("%s%s?userId=%s", OTP_UI_URL, ACCEPT_DEPENDENT_PATH, dependentUser.id); } } diff --git a/src/main/resources/Message.properties b/src/main/resources/Message.properties index 77a8b273b..f7a0e99ee 100644 --- a/src/main/resources/Message.properties +++ b/src/main/resources/Message.properties @@ -1,5 +1,5 @@ ACCEPT_DEPENDENT_EMAIL_FOOTER = You are receiving this email because you have been selected to be a trusted companion. -ACCEPT_DEPENDENT_EMAIL_GREETING = %s would like you to be their trusted companion. +ACCEPT_DEPENDENT_EMAIL_GREETING = %s 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 diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/MonitoredTripControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/MonitoredTripControllerTest.java index 6f32e75c9..578270c74 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/MonitoredTripControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/MonitoredTripControllerTest.java @@ -104,6 +104,7 @@ public static void tearDown() { multiAdminUser = Persistence.adminUsers.getById(multiAdminUser.id); if (multiAdminUser != null) multiAdminUser.delete(); deleteOtpUser( + IS_END_TO_END, soloOtpUser, multiOtpUser ); diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java index 70a1b3221..0cf3c34dd 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java @@ -76,6 +76,7 @@ public static void setUp() throws Exception { @AfterAll public static void tearDown() { deleteOtpUser( + IS_END_TO_END, otpUser, relatedUserOne, relatedUserTwo, diff --git a/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java b/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java index 7ba62494a..420c5624a 100644 --- a/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java +++ b/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java @@ -268,12 +268,12 @@ static Itinerary createItinerary() { return itinerary; } - public static void deleteOtpUser(OtpUser... optUsers) { + public static void deleteOtpUser(boolean isEndToEnd, OtpUser... optUsers) { for (OtpUser otpUser : optUsers) { if (otpUser != null) { OtpUser user = Persistence.otpUsers.getById(otpUser.id); if (user != null) { - user.delete(user.auth0UserId != null); + user.delete(isEndToEnd); } } } diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanionTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanionTest.java new file mode 100644 index 000000000..2151eeb00 --- /dev/null +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanionTest.java @@ -0,0 +1,43 @@ +package org.opentripplanner.middleware.tripmonitor; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.opentripplanner.middleware.models.OtpUser; +import org.opentripplanner.middleware.models.RelatedUser; +import org.opentripplanner.middleware.persistence.Persistence; +import org.opentripplanner.middleware.testutils.ApiTestUtils; +import org.opentripplanner.middleware.testutils.OtpMiddlewareTestEnvironment; +import org.opentripplanner.middleware.testutils.PersistenceTestUtils; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.opentripplanner.middleware.auth.Auth0Connection.setAuthDisabled; +import static org.opentripplanner.middleware.testutils.PersistenceTestUtils.deleteOtpUser; + +public class TrustedCompanionTest extends OtpMiddlewareTestEnvironment { + private static OtpUser relatedUserOne; + private static OtpUser dependentUserOne; + + @BeforeAll + public static void setUp() { + setAuthDisabled(false); + relatedUserOne = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("related-user-one")); + dependentUserOne = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-one")); + } + + @AfterAll + public static void tearDown() { + deleteOtpUser(IS_END_TO_END, + relatedUserOne, + dependentUserOne + ); + } + + @Test + void canManageAcceptDependentEmail() { + dependentUserOne.relatedUsers.add(new RelatedUser(relatedUserOne.id, relatedUserOne.email, RelatedUser.RelatedUserStatus.PENDING)); + Persistence.otpUsers.replace(dependentUserOne.id, dependentUserOne); + TrustedCompanion.manageAcceptDependentEmail(dependentUserOne, true); + assertTrue(dependentUserOne.relatedUsers.get(0).acceptDependentEmailSent); + } +} \ No newline at end of file From 1d1f4fb949adbca6e88807e8381962c692754428 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 24 Oct 2024 12:03:27 +0100 Subject: [PATCH 08/17] refactor(TrustedCompanion.java): Corrected indentation --- .../middleware/tripmonitor/TrustedCompanion.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java index 4957393de..7b641b8cb 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java @@ -120,15 +120,15 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r // A HashMap is needed instead of a Map for template data to be serialized to the template renderer. Map templateData = new HashMap<>( Map.of( - "acceptDependentLinkAnchorLabel", acceptDependentLinkLabel, - "acceptDependentLinkLabelAndUrl", label(acceptDependentLinkLabel, acceptDependentUrl, locale), - "acceptDependentUrl", getAcceptDependentUrl(dependentUser), - "emailFooter", Message.ACCEPT_DEPENDENT_EMAIL_FOOTER.get(locale), + "acceptDependentLinkAnchorLabel", acceptDependentLinkLabel, + "acceptDependentLinkLabelAndUrl", label(acceptDependentLinkLabel, acceptDependentUrl, locale), + "acceptDependentUrl", getAcceptDependentUrl(dependentUser), + "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? "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH), - "manageLinkText", Message.ACCEPT_DEPENDENT_EMAIL_MANAGE.get(locale) + "manageLinkText", Message.ACCEPT_DEPENDENT_EMAIL_MANAGE.get(locale) ) ); From 0db59a3b2b555483e230107eb12153f15c5ac94b Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 25 Oct 2024 14:55:57 +0100 Subject: [PATCH 09/17] refactor(Addressed PR feedback): --- README.md | 2 + configurations/default/env.yml.tmp | 2 + .../middleware/OtpMiddlewareMain.java | 7 +- .../controllers/api/OtpUserController.java | 9 +- .../middleware/models/OtpUser.java | 18 ++-- .../middleware/models/RelatedUser.java | 8 +- .../tripmonitor/TrustedCompanion.java | 93 +++++++++++-------- src/main/resources/Message.properties | 2 +- src/main/resources/Message_fr.properties | 2 +- src/main/resources/env.schema.json | 10 ++ .../latest-spark-swagger-output.yaml | 4 +- .../templates/AcceptDependentText.ftl | 2 +- .../api/OtpUserControllerTest.java | 35 +++++-- .../testutils/PersistenceTestUtils.java | 4 +- .../tripmonitor/TrustedCompanionTest.java | 7 +- 15 files changed, 136 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index a605a6943..c07d0eadc 100644 --- a/README.md +++ b/README.md @@ -301,6 +301,8 @@ The special E2E client settings should be defined in `env.yml`: | TRIP_INSTRUCTION_IMMEDIATE_RADIUS | integer | Optional | 2 | The radius in meters under which an immediate instruction is given. | | TRIP_INSTRUCTION_UPCOMING_RADIUS | integer | Optional | 10 | The radius in meters under which an upcoming instruction is given. | | TWILIO_ACCOUNT_SID | string | Optional | your-account-sid | Twilio settings available at: https://twilio.com/user/account | +| TRUSTED_COMPANION_CONFIRMATION_PAGE_URL | string | Optional | https://otp-server.example.com/trusted/confirmation | URL to the trusted companion confirmation page. | +| TRUSTED_COMPANION_ERROR_PAGE_URL | string | Optional | https://otp-server.example.com/trusted/error | URL to the trusted companion error page. | | TWILIO_AUTH_TOKEN | string | Optional | your-auth-token | Twilio settings available at: https://twilio.com/user/account | | US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_API_URL | string | Optional | http://host.example.com | US RideGwinnett bus notifier API. | | US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_API_KEY | string | Optional | your-api-key | API key for the US RideGwinnett bus notifier API. | diff --git a/configurations/default/env.yml.tmp b/configurations/default/env.yml.tmp index 93730bf49..4724f1444 100644 --- a/configurations/default/env.yml.tmp +++ b/configurations/default/env.yml.tmp @@ -109,3 +109,5 @@ MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS: 3 # The location for an OTP plan query request. PLAN_QUERY_RESOURCE_URI: https://plan.resource.com +TRUSTED_COMPANION_CONFIRMATION_PAGE_URL: https://otp-server.example.com/trusted/confirmation +TRUSTED_COMPANION_ERROR_PAGE_URL: https://otp-server.example.com/trusted/error diff --git a/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java b/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java index 85c263b4d..72c0811f3 100644 --- a/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java +++ b/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java @@ -41,6 +41,7 @@ import static org.opentripplanner.middleware.bugsnag.BugsnagWebhook.processWebHookDelivery; import static org.opentripplanner.middleware.controllers.api.ApiUserController.API_USER_PATH; import static org.opentripplanner.middleware.controllers.api.ApiUserController.AUTHENTICATE_PATH; +import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.ACCEPT_DEPENDENT_PATH; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; /** @@ -172,7 +173,11 @@ private static void initializeHttpEndpoints() throws IOException, InterruptedExc // Security checks for admin and /secure/ endpoints. Excluding /authenticate so that API users can obtain a // bearer token to authenticate against all other /secure/ endpoints. spark.before(API_PREFIX + "/secure/*", ((request, response) -> { - if (!request.requestMethod().equals("OPTIONS") && !request.pathInfo().endsWith(API_USER_PATH + AUTHENTICATE_PATH)) { + if ( + !request.requestMethod().equals("OPTIONS") && + !request.pathInfo().endsWith(API_USER_PATH + AUTHENTICATE_PATH) && + !request.pathInfo().endsWith(ACCEPT_DEPENDENT_PATH) + ) { Auth0Connection.checkUser(request); } })); diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java index 39bce3be6..65f7acf4c 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java @@ -24,6 +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.manageAcceptDependentEmail; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; @@ -87,11 +88,9 @@ 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(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() + .withResponseType(OtpUser.class), TrustedCompanion::acceptDependent ) .get(path(ROOT_ROUTE + String.format(VERIFY_ROUTE_TEMPLATE, ID_PARAM, VERIFY_PATH, PHONE_PARAM)) diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index 2de06cf6c..3a4723c8e 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -18,6 +18,8 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.mongodb.client.model.Filters.eq; + /** * This represents a user of an OpenTripPlanner instance (typically of the standard OTP UI/otp-react-redux). * otp-middleware stores these users and associated information (e.g., home/work locations and other favorites). Users @@ -124,12 +126,12 @@ public boolean delete(boolean deleteAuth0User) { } } - // If a guardian, invalidate relationship with all dependents. - for (String userId: dependents) { + // If a related user, invalidate relationship with all dependents. + for (String userId : dependents) { OtpUser dependent = Persistence.otpUsers.getById(userId); if (dependent != null) { for (RelatedUser relatedUser : dependent.relatedUsers) { - if (relatedUser.userId.equals(this.id)) { + if (relatedUser.email.equals(this.email)) { relatedUser.status = RelatedUser.RelatedUserStatus.INVALID; } } @@ -137,12 +139,12 @@ public boolean delete(boolean deleteAuth0User) { } } - // If a dependent, remove relationship with all guardians. + // If a dependent, remove relationship with all related users. for (RelatedUser relatedUser : relatedUsers) { - OtpUser guardian = Persistence.otpUsers.getById(relatedUser.userId); - if (guardian != null) { - guardian.dependents.remove(this.id); - Persistence.otpUsers.replace(guardian.id, guardian); + OtpUser user = Persistence.otpUsers.getOneFiltered(eq("email", relatedUser.email)); + if (user != null) { + user.dependents.remove(this.id); + Persistence.otpUsers.replace(user.id, user); } } diff --git a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java index e16473d4a..ee440c8f8 100644 --- a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java @@ -6,19 +6,19 @@ public enum RelatedUserStatus { PENDING, CONFIRMED, INVALID } - public String userId; public String email; - public RelatedUserStatus status; + public RelatedUserStatus status = RelatedUserStatus.PENDING; public boolean acceptDependentEmailSent; + public String nickName; public RelatedUser() { // Required for JSON deserialization. } - public RelatedUser(String userId, String email, RelatedUserStatus status) { - this.userId = userId; + public RelatedUser(String email, RelatedUserStatus status, String nickName) { this.email = email; this.status = status; + this.nickName = nickName; } } diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java index 7b641b8cb..9b19b1e90 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java @@ -1,8 +1,7 @@ package org.opentripplanner.middleware.tripmonitor; import org.eclipse.jetty.http.HttpStatus; -import org.opentripplanner.middleware.auth.Auth0Connection; -import org.opentripplanner.middleware.auth.RequestingUser; +import org.opentripplanner.middleware.OtpMiddlewareMain; import org.opentripplanner.middleware.i18n.Message; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.models.RelatedUser; @@ -18,6 +17,7 @@ import java.util.Locale; import java.util.Map; +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; @@ -29,54 +29,74 @@ private TrustedCompanion() { throw new IllegalStateException("Utility class"); } + private static final String AWS_API_SERVER = ConfigUtils.getConfigPropertyAsText("AWS_API_SERVER"); + private static final String AWS_API_STAGE = ConfigUtils.getConfigPropertyAsText("AWS_API_STAGE"); 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"; + /** Note: This path is excluded from security checks, see {@link OtpMiddlewareMain#initializeHttpEndpoints()}. */ public static final String ACCEPT_DEPENDENT_PATH = "api/secure/user/acceptdependent"; /** - * Accept a request from another user to be their dependent. This will include both companions and observers. + * Accept a request from another user to be their dependent. This will include both companions and observers. If + * 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) { - RequestingUser requestingUser = Auth0Connection.getUserFromRequest(request); - OtpUser relatedUser = requestingUser.otpUser; - if (relatedUser == null) { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Otp user unknown."); - return null; + 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 + .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!"; } - String dependentUserId = HttpUtils.getQueryParamFromRequest(request, USER_ID_PARAM, false); - if (dependentUserId.isEmpty()) { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user id not provided."); + 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)); return null; } + } - OtpUser dependentUser = Persistence.otpUsers.getById(dependentUserId); - if (dependentUser == null) { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent user unknown!"); + /** + * Extract the user id from the request parameters and in-turn retrieve the matching user. + */ + 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."); return null; } - boolean isRelated = dependentUser.relatedUsers - .stream() - .filter(related -> related.userId.equals(relatedUser.id)) - // 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(dependentUserId); - Persistence.otpUsers.replace(relatedUser.id, relatedUser); - // Update list of related users. - Persistence.otpUsers.replace(dependentUser.id, dependentUser); - } else { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Dependent did not request user to be related!"); + OtpUser user = Persistence.otpUsers.getById(userId); + if (user == null) { + logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Otp user unknown."); return null; } - - // TODO: Not sure what is required in the response. For now, returning the updated related user. - return relatedUser; + return user; } public static void manageAcceptDependentEmail(OtpUser dependentUser) { @@ -98,7 +118,7 @@ public static void manageAcceptDependentEmail(OtpUser dependentUser, boolean isT .stream() .filter(relatedUser -> !relatedUser.acceptDependentEmailSent) .forEach(relatedUser -> { - OtpUser userToReceiveEmail = Persistence.otpUsers.getById(relatedUser.userId); + OtpUser userToReceiveEmail = Persistence.otpUsers.getOneFiltered(eq("email", relatedUser.email)); if (userToReceiveEmail != null && (isTest || sendAcceptDependentEmail(dependentUser, userToReceiveEmail))) { relatedUser.acceptDependentEmailSent = true; } @@ -125,7 +145,7 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r "acceptDependentUrl", getAcceptDependentUrl(dependentUser), "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)), + "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? "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH), "manageLinkText", Message.ACCEPT_DEPENDENT_EMAIL_MANAGE.get(locale) @@ -142,7 +162,6 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r } private static String getAcceptDependentUrl(OtpUser dependentUser) { - // TODO: Is OTP_UI_URL the correct base URL to user here? I'm not sure. - return String.format("%s%s?userId=%s", OTP_UI_URL, ACCEPT_DEPENDENT_PATH, dependentUser.id); + return String.format("%s/%s/%s?userId=%s", AWS_API_SERVER, AWS_API_STAGE, ACCEPT_DEPENDENT_PATH, dependentUser.id); } } diff --git a/src/main/resources/Message.properties b/src/main/resources/Message.properties index f7a0e99ee..3ffeef0a5 100644 --- a/src/main/resources/Message.properties +++ b/src/main/resources/Message.properties @@ -1,5 +1,5 @@ ACCEPT_DEPENDENT_EMAIL_FOOTER = You are receiving this email because you have been selected to be a trusted companion. -ACCEPT_DEPENDENT_EMAIL_GREETING = %s would like you to be a trusted companion. +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 diff --git a/src/main/resources/Message_fr.properties b/src/main/resources/Message_fr.properties index 628bfd6d5..ac0e936e8 100644 --- a/src/main/resources/Message_fr.properties +++ b/src/main/resources/Message_fr.properties @@ -1,5 +1,5 @@ ACCEPT_DEPENDENT_EMAIL_FOOTER = TODO -ACCEPT_DEPENDENT_EMAIL_GREETING = %s TODO. +ACCEPT_DEPENDENT_EMAIL_GREETING = TODO ACCEPT_DEPENDENT_EMAIL_LINK_TEXT = TODO ACCEPT_DEPENDENT_EMAIL_SUBJECT = TODO ACCEPT_DEPENDENT_EMAIL_MANAGE = TODO diff --git a/src/main/resources/env.schema.json b/src/main/resources/env.schema.json index fcc9f79f8..999a2e22e 100644 --- a/src/main/resources/env.schema.json +++ b/src/main/resources/env.schema.json @@ -324,6 +324,16 @@ "examples": ["your-account-sid"], "description": "Twilio settings available at: https://twilio.com/user/account" }, + "TRUSTED_COMPANION_CONFIRMATION_PAGE_URL": { + "type": "string", + "examples": ["https://otp-server.example.com/trusted/confirmation"], + "description": "URL to the trusted companion confirmation page." + }, + "TRUSTED_COMPANION_ERROR_PAGE_URL": { + "type": "string", + "examples": ["https://otp-server.example.com/trusted/error"], + "description": "URL to the trusted companion error page." + }, "TWILIO_AUTH_TOKEN": { "type": "string", "examples": ["your-auth-token"], diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index f6956e2f7..4b88ebbae 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -2877,8 +2877,6 @@ definitions: RelatedUser: type: "object" properties: - userId: - type: "string" email: type: "string" status: @@ -2889,6 +2887,8 @@ definitions: - "INVALID" acceptDependentEmailSent: type: "boolean" + nickName: + type: "string" FareComponent: type: "object" properties: diff --git a/src/main/resources/templates/AcceptDependentText.ftl b/src/main/resources/templates/AcceptDependentText.ftl index c87ed5a0c..248b91921 100644 --- a/src/main/resources/templates/AcceptDependentText.ftl +++ b/src/main/resources/templates/AcceptDependentText.ftl @@ -8,4 +8,4 @@ --> ${emailGreeting} -${tripLinkLabelAndUrl} \ No newline at end of file +${acceptDependentLinkLabelAndUrl} \ No newline at end of file diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java index 0cf3c34dd..a245885f8 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java @@ -28,6 +28,7 @@ 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,6 +38,7 @@ 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. @@ -48,6 +50,7 @@ public class OtpUserControllerTest extends OtpMiddlewareTestEnvironment { private static OtpUser relatedUserThree; private static OtpUser dependentUserThree; private static HashMap relatedUserHeaders; + private static final String nickName = "my-trusted-companion"; @BeforeAll public static void setUp() throws Exception { @@ -174,11 +177,23 @@ void canPreserveSmsConsentDate() throws Exception { @Test void canAcceptDependentRequest() { - dependentUserOne.relatedUsers.add(new RelatedUser(relatedUserOne.id, relatedUserOne.email, RelatedUser.RelatedUserStatus.PENDING)); + dependentUserOne.relatedUsers.add(new RelatedUser( + relatedUserOne.email, + RelatedUser.RelatedUserStatus.PENDING, + nickName + )); Persistence.otpUsers.replace(dependentUserOne.id, dependentUserOne); - String path = String.format("%s?userId=%s", ACCEPT_DEPENDENT_PATH, dependentUserOne.id); - makeGetRequest(path, relatedUserHeaders); + 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); relatedUserOne = Persistence.otpUsers.getById(relatedUserOne.id); assertTrue(relatedUserOne.dependents.contains(dependentUserOne.id)); @@ -187,7 +202,7 @@ void canAcceptDependentRequest() { List relatedUsers = dependentUserOne.relatedUsers; relatedUsers .stream() - .filter(user -> user.userId.equals(relatedUserOne.id)) + .filter(user -> user.email.equals(relatedUserOne.email)) .forEach(user -> assertEquals(RelatedUser.RelatedUserStatus.CONFIRMED, user.status)); } @@ -195,7 +210,11 @@ void canAcceptDependentRequest() { void canInvalidateDependent() { relatedUserTwo.dependents.add(dependentUserTwo.id); Persistence.otpUsers.replace(relatedUserTwo.id, relatedUserTwo); - dependentUserTwo.relatedUsers.add(new RelatedUser(relatedUserTwo.id, relatedUserTwo.email, RelatedUser.RelatedUserStatus.CONFIRMED)); + dependentUserTwo.relatedUsers.add(new RelatedUser( + relatedUserTwo.email, + RelatedUser.RelatedUserStatus.CONFIRMED, + nickName + )); Persistence.otpUsers.replace(dependentUserTwo.id, dependentUserTwo); relatedUserTwo.delete(false); dependentUserTwo = Persistence.otpUsers.getById(dependentUserTwo.id); @@ -207,7 +226,11 @@ void canInvalidateDependent() { void canRemoveRelatedUser() { relatedUserThree.dependents.add(dependentUserThree.id); Persistence.otpUsers.replace(relatedUserThree.id, relatedUserThree); - dependentUserThree.relatedUsers.add(new RelatedUser(relatedUserThree.id, relatedUserThree.email, RelatedUser.RelatedUserStatus.CONFIRMED)); + dependentUserThree.relatedUsers.add(new RelatedUser( + relatedUserThree.email, + RelatedUser.RelatedUserStatus.CONFIRMED, + nickName + )); Persistence.otpUsers.replace(dependentUserThree.id, dependentUserThree); dependentUserThree.delete(false); relatedUserThree = Persistence.otpUsers.getById(relatedUserThree.id); diff --git a/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java b/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java index 420c5624a..a221a7adf 100644 --- a/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java +++ b/src/test/java/org/opentripplanner/middleware/testutils/PersistenceTestUtils.java @@ -268,8 +268,8 @@ static Itinerary createItinerary() { return itinerary; } - public static void deleteOtpUser(boolean isEndToEnd, OtpUser... optUsers) { - for (OtpUser otpUser : optUsers) { + public static void deleteOtpUser(boolean isEndToEnd, OtpUser... otpUsers) { + for (OtpUser otpUser : otpUsers) { if (otpUser != null) { OtpUser user = Persistence.otpUsers.getById(otpUser.id); if (user != null) { diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanionTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanionTest.java index 2151eeb00..65bd7225e 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanionTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanionTest.java @@ -17,6 +17,7 @@ public class TrustedCompanionTest extends OtpMiddlewareTestEnvironment { private static OtpUser relatedUserOne; private static OtpUser dependentUserOne; + private static final String nickName = "my-trusted-companion"; @BeforeAll public static void setUp() { @@ -35,7 +36,11 @@ public static void tearDown() { @Test void canManageAcceptDependentEmail() { - dependentUserOne.relatedUsers.add(new RelatedUser(relatedUserOne.id, relatedUserOne.email, RelatedUser.RelatedUserStatus.PENDING)); + dependentUserOne.relatedUsers.add(new RelatedUser( + relatedUserOne.email, + RelatedUser.RelatedUserStatus.PENDING, + nickName + )); Persistence.otpUsers.replace(dependentUserOne.id, dependentUserOne); TrustedCompanion.manageAcceptDependentEmail(dependentUserOne, true); assertTrue(dependentUserOne.relatedUsers.get(0).acceptDependentEmailSent); From 581318cfc0b54048e886572b9f85ae291cf90533 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 30 Oct 2024 12:35:56 +0000 Subject: [PATCH 10/17] refactor(Addressed PR feedback): Update to introduce accept key --- .../controllers/api/OtpUserController.java | 19 +-- .../middleware/i18n/Message.java | 1 + .../middleware/models/OtpUser.java | 3 + .../middleware/models/RelatedUser.java | 7 +- .../tripmonitor/TrustedCompanion.java | 120 +++++++++++------- src/main/resources/Message.properties | 1 + src/main/resources/Message_fr.properties | 1 + .../latest-spark-swagger-output.yaml | 6 +- .../api/OtpUserControllerTest.java | 21 +-- .../tripmonitor/TrustedCompanionTest.java | 9 +- 10 files changed, 114 insertions(+), 74 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java index 65f7acf4c..6e08c6a50 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java +++ b/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 ) diff --git a/src/main/java/org/opentripplanner/middleware/i18n/Message.java b/src/main/java/org/opentripplanner/middleware/i18n/Message.java index da82bca0e..c5c3271fa 100644 --- a/src/main/java/org/opentripplanner/middleware/i18n/Message.java +++ b/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, diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index 3a4723c8e..0af252bc3 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/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 dependents = new ArrayList<>(); + /** This user's name */ + public String name; + @Override public boolean delete() { return delete(true); diff --git a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java index ee440c8f8..a85605f94 100644 --- a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java +++ b/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; + } } diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java index 9b19b1e90..6982db1f7 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java +++ b/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 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 = 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 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))); } } diff --git a/src/main/resources/Message.properties b/src/main/resources/Message.properties index 3ffeef0a5..6d7cb7de7 100644 --- a/src/main/resources/Message.properties +++ b/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 diff --git a/src/main/resources/Message_fr.properties b/src/main/resources/Message_fr.properties index ac0e936e8..60bd666ba 100644 --- a/src/main/resources/Message_fr.properties +++ b/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 diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index 4b88ebbae..5646b66e7 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/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: diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java index a245885f8..72d83f3bb 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java @@ -15,12 +15,14 @@ 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; @@ -28,7 +30,6 @@ 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)); diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanionTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanionTest.java index 65bd7225e..5c6d73517 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanionTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanionTest.java @@ -10,7 +10,9 @@ import org.opentripplanner.middleware.testutils.OtpMiddlewareTestEnvironment; import org.opentripplanner.middleware.testutils.PersistenceTestUtils; -import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.opentripplanner.middleware.auth.Auth0Connection.setAuthDisabled; import static org.opentripplanner.middleware.testutils.PersistenceTestUtils.deleteOtpUser; @@ -39,10 +41,11 @@ void canManageAcceptDependentEmail() { dependentUserOne.relatedUsers.add(new RelatedUser( relatedUserOne.email, RelatedUser.RelatedUserStatus.PENDING, - nickName + nickName, + UUID.randomUUID().toString() )); Persistence.otpUsers.replace(dependentUserOne.id, dependentUserOne); TrustedCompanion.manageAcceptDependentEmail(dependentUserOne, true); - assertTrue(dependentUserOne.relatedUsers.get(0).acceptDependentEmailSent); + assertNotNull(dependentUserOne.relatedUsers.get(0).acceptKey); } } \ No newline at end of file From 40405a25b783cb85ccc13e67de0a47f755412293 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 30 Oct 2024 12:39:22 +0000 Subject: [PATCH 11/17] refactor(Corrected swagger docs): --- src/main/resources/latest-spark-swagger-output.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index 5646b66e7..ca4029aac 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -3183,7 +3183,7 @@ definitions: type: "array" items: type: "string" - nickName: + name: type: "string" MobilityProfile: type: "object" From 4a00bfa58c2f747e1f645b4b2b0a4739ac136b6f Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 31 Oct 2024 11:29:56 +0000 Subject: [PATCH 12/17] refactor(Addressed PR feedback): Handle related user update on PUT --- README.md | 3 +- .../controllers/api/OtpUserController.java | 2 + .../middleware/models/RelatedUser.java | 10 +- .../tripmonitor/TrustedCompanion.java | 96 +++++++++++-------- src/main/resources/Message.properties | 2 +- src/main/resources/Message_fr.properties | 2 +- src/main/resources/env.schema.json | 7 +- .../api/OtpUserControllerTest.java | 66 ++++++++++--- 8 files changed, 123 insertions(+), 65 deletions(-) diff --git a/README.md b/README.md index c07d0eadc..e96122532 100644 --- a/README.md +++ b/README.md @@ -301,8 +301,7 @@ The special E2E client settings should be defined in `env.yml`: | TRIP_INSTRUCTION_IMMEDIATE_RADIUS | integer | Optional | 2 | The radius in meters under which an immediate instruction is given. | | TRIP_INSTRUCTION_UPCOMING_RADIUS | integer | Optional | 10 | The radius in meters under which an upcoming instruction is given. | | TWILIO_ACCOUNT_SID | string | Optional | your-account-sid | Twilio settings available at: https://twilio.com/user/account | -| TRUSTED_COMPANION_CONFIRMATION_PAGE_URL | string | Optional | https://otp-server.example.com/trusted/confirmation | URL to the trusted companion confirmation page. | -| TRUSTED_COMPANION_ERROR_PAGE_URL | string | Optional | https://otp-server.example.com/trusted/error | URL to the trusted companion error page. | +| TRUSTED_COMPANION_CONFIRMATION_PAGE_URL | string | Optional | https://otp-server.example.com/trusted/confirmation | URL to the trusted companion confirmation page. This page should support handling an error URL parameter. | | TWILIO_AUTH_TOKEN | string | Optional | your-auth-token | Twilio settings available at: https://twilio.com/user/account | | US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_API_URL | string | Optional | http://host.example.com | US RideGwinnett bus notifier API. | | US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_API_KEY | string | Optional | your-api-key | API key for the US RideGwinnett bus notifier API. | diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java index 6e08c6a50..75cb225b9 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java @@ -25,6 +25,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.ensureRelatedUserIntegrity; import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.manageAcceptDependentEmail; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; @@ -70,6 +71,7 @@ OtpUser preCreateHook(OtpUser user, Request req) { @Override OtpUser preUpdateHook(OtpUser user, OtpUser preExistingUser, Request req) { preliminaryTasks(user); + ensureRelatedUserIntegrity(user, preExistingUser); return super.preUpdateHook(user, preExistingUser, req); } diff --git a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java index a85605f94..920e94eda 100644 --- a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java @@ -9,20 +9,20 @@ public enum RelatedUserStatus { public String email; public RelatedUserStatus status = RelatedUserStatus.PENDING; public String acceptKey; - public String nickName; + public String nickname; public RelatedUser() { // Required for JSON deserialization. } - public RelatedUser(String email, RelatedUserStatus status, String nickName) { + public RelatedUser(String email, RelatedUserStatus status, String nickname) { this.email = email; this.status = status; - this.nickName = nickName; + this.nickname = nickname; } - public RelatedUser(String email, RelatedUserStatus status, String nickName, String acceptKey) { - this (email, status, nickName); + public RelatedUser(String email, RelatedUserStatus status, String nickname, String acceptKey) { + this (email, status, nickname); this.acceptKey = acceptKey; } } diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java index 6982db1f7..6937d5688 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java @@ -1,7 +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; import org.opentripplanner.middleware.models.OtpUser; @@ -15,15 +14,16 @@ import spark.Response; import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.UUID; +import java.util.stream.Collectors; import static com.mongodb.client.model.Filters.eq; 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; public class TrustedCompanion { @@ -35,8 +35,8 @@ private TrustedCompanion() { private static final String AWS_API_STAGE = ConfigUtils.getConfigPropertyAsText("AWS_API_STAGE"); 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 ACCEPT_KEY = "acceptKey"; + public static final String EMAIL_FIELD_NAME = "email"; /** Note: This path is excluded from security checks, see {@link OtpMiddlewareMain#initializeHttpEndpoints()}. */ public static final String ACCEPT_DEPENDENT_PATH = "api/secure/user/acceptdependent"; @@ -46,31 +46,33 @@ 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) { - String acceptKey = getAcceptKeyFromRequest(request); - OtpUser dependentUser = getUserFromAcceptKey(request, acceptKey); - OtpUser relatedUser = getRelatedUserFromEmail(dependentUser, acceptKey); - - if (dependentUser != null && relatedUser != null) { - Optional relatedUserToUpdate = dependentUser.relatedUsers - .stream() - .filter(related -> related.email.equals(relatedUser.email)) - .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); - - // Redirect to confirmation page and provide dependent user information. - response.redirect(TRUSTED_COMPANION_CONFIRMATION_PAGE_URL); - return dependentUser; + try { + String acceptKey = getAcceptKeyFromRequest(request); + OtpUser dependentUser = getUserFromAcceptKey(acceptKey); + OtpUser relatedUser = getRelatedUserFromEmail(dependentUser, acceptKey); + + if (relatedUser != null) { + Optional relatedUserToUpdate = dependentUser.relatedUsers + .stream() + .filter(related -> related.email.equals(relatedUser.email)) + .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); + + // Redirect to confirmation page and provide dependent user information. + response.redirect(TRUSTED_COMPANION_CONFIRMATION_PAGE_URL); + return dependentUser; + } + } catch (IllegalArgumentException e) { + response.redirect( + String.format("%s?error=%s", TRUSTED_COMPANION_CONFIRMATION_PAGE_URL, Message.ACCEPT_DEPENDENT_ERROR.get(null)) + ); } - - response.redirect( - String.format("%s?error=%s", TRUSTED_COMPANION_ERROR_PAGE_URL, Message.ACCEPT_DEPENDENT_ERROR.get(null)) - ); return null; } @@ -85,17 +87,17 @@ private static OtpUser getRelatedUserFromEmail(OtpUser dependentUser, String acc .stream() .filter(user -> user.acceptKey.equalsIgnoreCase(acceptKey)) .findFirst(); - return relatedUser.map(user -> Persistence.otpUsers.getOneFiltered(eq("email", user.email))).orElse(null); + return relatedUser.map(user -> Persistence.otpUsers.getOneFiltered(eq(EMAIL_FIELD_NAME, user.email))).orElse(null); } /** * Extract the accept key from the request parameters. */ - 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; + private static String getAcceptKeyFromRequest(Request request) throws IllegalArgumentException { + // Note: optional is true so a missing accept key will be handled here. + String acceptKey = HttpUtils.getQueryParamFromRequest(request, ACCEPT_KEY, true); + if (acceptKey == null || acceptKey.isEmpty()) { + throw new IllegalArgumentException("Accept key not provided."); } return acceptKey; } @@ -103,14 +105,13 @@ private static String getAcceptKeyFromRequest(Request request) { /** * Retrieve the dependent user matching the accept key. */ - private static OtpUser getUserFromAcceptKey(Request request, String acceptKey) { + private static OtpUser getUserFromAcceptKey(String acceptKey) throws IllegalArgumentException { if (acceptKey == null) { return null; } OtpUser user = getUserForAcceptKey(acceptKey); if (user == null) { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Otp user unknown."); - return null; + throw new IllegalArgumentException("Otp user unknown."); } return user; } @@ -135,13 +136,13 @@ public static void manageAcceptDependentEmail(OtpUser dependentUser, boolean isT .filter(relatedUser -> relatedUser.acceptKey == null) .forEach(relatedUser -> { String acceptKey = UUID.randomUUID().toString(); - OtpUser userToReceiveEmail = Persistence.otpUsers.getOneFiltered(eq("email", relatedUser.email)); + OtpUser userToReceiveEmail = Persistence.otpUsers.getOneFiltered(eq(EMAIL_FIELD_NAME, relatedUser.email)); if (userToReceiveEmail != null && (isTest || sendAcceptDependentEmail(dependentUser, userToReceiveEmail, acceptKey))) { relatedUser.acceptKey = acceptKey; } }); - // Preserve email sent status. + // Preserve email sent status (by storing the accept key). Persistence.otpUsers.replace(dependentUser.id, dependentUser); } @@ -162,7 +163,7 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r "acceptDependentLinkLabelAndUrl", label(acceptDependentLinkLabel, acceptDependentUrl, locale), "acceptDependentUrl", acceptDependentUrl, "emailFooter", Message.ACCEPT_DEPENDENT_EMAIL_FOOTER.get(locale), - "emailGreeting", String.format("%s %s", addressee, Message.ACCEPT_DEPENDENT_EMAIL_GREETING.get(locale)), + "emailGreeting", String.format(Message.ACCEPT_DEPENDENT_EMAIL_GREETING.get(locale), addressee), "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH), "manageLinkText", Message.ACCEPT_DEPENDENT_EMAIL_MANAGE.get(locale) ) @@ -192,4 +193,21 @@ public static String getAcceptDependentEndPoint(String acceptKey) { private static OtpUser getUserForAcceptKey(String acceptKey) { return Persistence.otpUsers.getOneFiltered(Filters.elemMatch("relatedUsers", Filters.eq(ACCEPT_KEY, acceptKey))); } + + /** + * If a dependent removes a related user, remove the dependent from the related user. + */ + public static void ensureRelatedUserIntegrity(OtpUser updatedUser, OtpUser preExistingUser) { + List difference = preExistingUser.relatedUsers + .stream() + .filter(relatedUser -> !updatedUser.relatedUsers.contains(relatedUser)) + .collect(Collectors.toList()); + for (RelatedUser relatedUser : difference) { + OtpUser user = Persistence.otpUsers.getOneFiltered(eq(EMAIL_FIELD_NAME, relatedUser.email)); + if (user != null) { + user.dependents.remove(updatedUser.id); + Persistence.otpUsers.replace(user.id, user); + } + } + } } diff --git a/src/main/resources/Message.properties b/src/main/resources/Message.properties index 6d7cb7de7..aae86669b 100644 --- a/src/main/resources/Message.properties +++ b/src/main/resources/Message.properties @@ -1,5 +1,5 @@ ACCEPT_DEPENDENT_EMAIL_FOOTER = You are receiving this email because you have been selected to be a trusted companion. -ACCEPT_DEPENDENT_EMAIL_GREETING = would like you to be a trusted companion. +ACCEPT_DEPENDENT_EMAIL_GREETING = %s 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 diff --git a/src/main/resources/Message_fr.properties b/src/main/resources/Message_fr.properties index 60bd666ba..c871d8ee0 100644 --- a/src/main/resources/Message_fr.properties +++ b/src/main/resources/Message_fr.properties @@ -1,5 +1,5 @@ ACCEPT_DEPENDENT_EMAIL_FOOTER = TODO -ACCEPT_DEPENDENT_EMAIL_GREETING = TODO +ACCEPT_DEPENDENT_EMAIL_GREETING = %s TODO ACCEPT_DEPENDENT_EMAIL_LINK_TEXT = TODO ACCEPT_DEPENDENT_EMAIL_SUBJECT = TODO ACCEPT_DEPENDENT_EMAIL_MANAGE = TODO diff --git a/src/main/resources/env.schema.json b/src/main/resources/env.schema.json index 999a2e22e..2c6828073 100644 --- a/src/main/resources/env.schema.json +++ b/src/main/resources/env.schema.json @@ -327,12 +327,7 @@ "TRUSTED_COMPANION_CONFIRMATION_PAGE_URL": { "type": "string", "examples": ["https://otp-server.example.com/trusted/confirmation"], - "description": "URL to the trusted companion confirmation page." - }, - "TRUSTED_COMPANION_ERROR_PAGE_URL": { - "type": "string", - "examples": ["https://otp-server.example.com/trusted/error"], - "description": "URL to the trusted companion error page." + "description": "URL to the trusted companion confirmation page. This page should support handling an error URL parameter." }, "TWILIO_AUTH_TOKEN": { "type": "string", diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java index 72d83f3bb..66892829b 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java @@ -20,7 +20,6 @@ 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; @@ -30,7 +29,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; -import static org.opentripplanner.middleware.testutils.ApiTestUtils.createAndAssignAuth0User; import static org.opentripplanner.middleware.testutils.ApiTestUtils.getMockHeaders; import static org.opentripplanner.middleware.testutils.ApiTestUtils.makeGetRequest; import static org.opentripplanner.middleware.testutils.ApiTestUtils.makeRequest; @@ -48,8 +46,9 @@ public class OtpUserControllerTest extends OtpMiddlewareTestEnvironment { private static OtpUser dependentUserTwo; private static OtpUser relatedUserThree; private static OtpUser dependentUserThree; - private static HashMap relatedUserHeaders; - private static final String nickName = "my-trusted-companion"; + private static OtpUser relatedUserFour; + private static OtpUser dependentUserFour; + private static final String nickname = "my-trusted-companion"; @BeforeAll public static void setUp() throws Exception { @@ -72,7 +71,8 @@ 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")); - relatedUserHeaders = createAndAssignAuth0User(relatedUserOne); + relatedUserFour = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("related-user-four")); + dependentUserFour = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("dependent-four")); } @AfterAll @@ -83,9 +83,11 @@ public static void tearDown() { relatedUserOne, relatedUserTwo, relatedUserThree, + relatedUserFour, dependentUserOne, dependentUserTwo, - dependentUserThree + dependentUserThree, + dependentUserFour ); // Restore original isAuthDisabled state. @@ -180,7 +182,7 @@ void canAcceptDependentRequest() { dependentUserOne.relatedUsers.add(new RelatedUser( relatedUserOne.email, RelatedUser.RelatedUserStatus.PENDING, - nickName, + nickname, acceptKey )); Persistence.otpUsers.replace(dependentUserOne.id, dependentUserOne); @@ -200,13 +202,13 @@ void canAcceptDependentRequest() { } @Test - void canInvalidateDependent() { + void canInvalidateDependentOnDelete() { relatedUserTwo.dependents.add(dependentUserTwo.id); Persistence.otpUsers.replace(relatedUserTwo.id, relatedUserTwo); dependentUserTwo.relatedUsers.add(new RelatedUser( relatedUserTwo.email, RelatedUser.RelatedUserStatus.CONFIRMED, - nickName + nickname )); Persistence.otpUsers.replace(dependentUserTwo.id, dependentUserTwo); relatedUserTwo.delete(false); @@ -216,17 +218,59 @@ void canInvalidateDependent() { } @Test - void canRemoveRelatedUser() { + void canRemoveRelatedUserOnDelete() { relatedUserThree.dependents.add(dependentUserThree.id); Persistence.otpUsers.replace(relatedUserThree.id, relatedUserThree); dependentUserThree.relatedUsers.add(new RelatedUser( relatedUserThree.email, RelatedUser.RelatedUserStatus.CONFIRMED, - nickName + nickname )); Persistence.otpUsers.replace(dependentUserThree.id, dependentUserThree); dependentUserThree.delete(false); relatedUserThree = Persistence.otpUsers.getById(relatedUserThree.id); assertFalse(relatedUserThree.dependents.contains(dependentUserThree.id)); } + + /** + * Confirm that a user can be removed from a related users list, and importantly, the related user no longer lists + * the removed dependent. + */ + @Test + void canRemoveUserFromRelatedUsersList() throws Exception { + setAuthDisabled(true); + relatedUserFour.dependents.add(dependentUserFour.id); + Persistence.otpUsers.replace(relatedUserFour.id, relatedUserFour); + dependentUserFour.relatedUsers.add(new RelatedUser( + relatedUserFour.email, + RelatedUser.RelatedUserStatus.CONFIRMED, + nickname + )); + Persistence.otpUsers.replace(dependentUserFour.id, dependentUserFour); + + // Remove the first related user. + dependentUserFour.relatedUsers.clear(); + + // Add a new related user that should not be considered for integrity update. + dependentUserFour.relatedUsers.add(new RelatedUser( + relatedUserThree.email, + RelatedUser.RelatedUserStatus.PENDING, + nickname + )); + + makeRequest( + String.format("api/secure/user/%s", dependentUserFour.id), + JsonUtils.toJson(dependentUserFour), + getMockHeaders(dependentUserFour), + HttpMethod.PUT + ); + + dependentUserFour = Persistence.otpUsers.getById(dependentUserFour.id); + assertFalse(dependentUserFour.relatedUsers.stream().anyMatch(u -> u.email.equalsIgnoreCase(relatedUserFour.email))); + + relatedUserFour = Persistence.otpUsers.getById(relatedUserFour.id); + assertFalse(relatedUserFour.dependents.contains(dependentUserFour.id)); + + setAuthDisabled(false); + } } From d2aecd01564a8278f32b382126ffc47da5442cf2 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 31 Oct 2024 11:36:12 +0000 Subject: [PATCH 13/17] refactor(Remove the trusted companion error page from the config file): --- configurations/default/env.yml.tmp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/configurations/default/env.yml.tmp b/configurations/default/env.yml.tmp index 4724f1444..6665b1522 100644 --- a/configurations/default/env.yml.tmp +++ b/configurations/default/env.yml.tmp @@ -109,5 +109,4 @@ MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS: 3 # The location for an OTP plan query request. PLAN_QUERY_RESOURCE_URI: https://plan.resource.com -TRUSTED_COMPANION_CONFIRMATION_PAGE_URL: https://otp-server.example.com/trusted/confirmation -TRUSTED_COMPANION_ERROR_PAGE_URL: https://otp-server.example.com/trusted/error +TRUSTED_COMPANION_CONFIRMATION_PAGE_URL: https://otp-server.example.com/trusted/confirmation \ No newline at end of file From 5d407546959ec4028ef1e6947c175f980310037a Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 31 Oct 2024 11:43:50 +0000 Subject: [PATCH 14/17] refactor(Updated Swagger docs): --- src/main/resources/latest-spark-swagger-output.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index ca4029aac..7a9098fb2 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -2887,7 +2887,7 @@ definitions: - "INVALID" acceptKey: type: "string" - nickName: + nickname: type: "string" FareComponent: type: "object" From b8070d38abb121a79f6d5ea72c0951c749560abd Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 31 Oct 2024 09:56:23 -0400 Subject: [PATCH 15/17] chore(Message): Add FR translations for companions. --- src/main/resources/Message_fr.properties | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/resources/Message_fr.properties b/src/main/resources/Message_fr.properties index c871d8ee0..9011607a2 100644 --- a/src/main/resources/Message_fr.properties +++ b/src/main/resources/Message_fr.properties @@ -1,9 +1,9 @@ -ACCEPT_DEPENDENT_EMAIL_FOOTER = TODO -ACCEPT_DEPENDENT_EMAIL_GREETING = %s TODO -ACCEPT_DEPENDENT_EMAIL_LINK_TEXT = TODO -ACCEPT_DEPENDENT_EMAIL_SUBJECT = TODO -ACCEPT_DEPENDENT_EMAIL_MANAGE = TODO -ACCEPT_DEPENDENT_ERROR = TODO +ACCEPT_DEPENDENT_EMAIL_FOOTER = Vous recevez cet email parce qu'une personne vous a designé comme accompagnateur·trice. +ACCEPT_DEPENDENT_EMAIL_GREETING = %s voudrait vous avoir comme accompagnateur·trice. +ACCEPT_DEPENDENT_EMAIL_LINK_TEXT = Accepter la demande +ACCEPT_DEPENDENT_EMAIL_SUBJECT = Demande d'accompagnateur +ACCEPT_DEPENDENT_EMAIL_MANAGE = Gérez vos préférences +ACCEPT_DEPENDENT_ERROR = La demande d'accompagnateur n'a pas été reçue. LABEL_AND_CONTENT = %s\u00A0: %s SMS_STOP_NOTIFICATIONS = Pour arrêter ces notifications, envoyez STOP. TRIP_EMAIL_SUBJECT = Notification pour %s From 99341cb2a25e6633d69fb424482c7936fb6adccc Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 31 Oct 2024 16:38:15 +0000 Subject: [PATCH 16/17] refactor(Addressed PR feedback and updated accept link to include locale): --- .../controllers/api/OtpUserController.java | 2 + .../middleware/models/OtpUser.java | 9 +-- .../tripmonitor/TrustedCompanion.java | 57 +++++++++++++------ .../middleware/utils/I18nUtils.java | 15 +++++ .../api/OtpUserControllerTest.java | 4 +- 5 files changed, 63 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java index 75cb225b9..096518d9e 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java @@ -25,6 +25,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.USER_LOCALE; import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.ensureRelatedUserIntegrity; import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.manageAcceptDependentEmail; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; @@ -95,6 +96,7 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) { .withDescription("Accept a dependent request.") .withResponses(SwaggerUtils.createStandardResponses(OtpUser.class)) .withPathParam().withName(ACCEPT_KEY).withRequired(true).withDescription("The accept dependent unique key.").and() + .withPathParam().withName(USER_LOCALE).withRequired(true).withDescription("The accepting user's locale.").and() .withResponseType(OtpUser.class), TrustedCompanion::acceptDependent ) diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index 0af252bc3..79de4104c 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -18,7 +18,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import static com.mongodb.client.model.Filters.eq; +import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.removeDependent; /** * This represents a user of an OpenTripPlanner instance (typically of the standard OTP UI/otp-react-redux). @@ -144,13 +144,8 @@ public boolean delete(boolean deleteAuth0User) { // If a dependent, remove relationship with all related users. for (RelatedUser relatedUser : relatedUsers) { - OtpUser user = Persistence.otpUsers.getOneFiltered(eq("email", relatedUser.email)); - if (user != null) { - user.dependents.remove(this.id); - Persistence.otpUsers.replace(user.id, user); - } + removeDependent(this, relatedUser); } - return Persistence.otpUsers.removeById(this.id); } diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java index 6937d5688..ea1d6cb3d 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java @@ -1,6 +1,7 @@ package org.opentripplanner.middleware.tripmonitor; import com.mongodb.client.model.Filters; +import org.apache.logging.log4j.util.Strings; import org.opentripplanner.middleware.OtpMiddlewareMain; import org.opentripplanner.middleware.i18n.Message; import org.opentripplanner.middleware.models.OtpUser; @@ -13,6 +14,7 @@ import spark.Request; import spark.Response; +import java.net.URLEncoder; import java.util.HashMap; import java.util.List; import java.util.Locale; @@ -22,7 +24,9 @@ import java.util.stream.Collectors; import static com.mongodb.client.model.Filters.eq; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.SETTINGS_PATH; +import static org.opentripplanner.middleware.utils.I18nUtils.getLocaleFromString; import static org.opentripplanner.middleware.utils.I18nUtils.label; public class TrustedCompanion { @@ -34,8 +38,10 @@ private TrustedCompanion() { private static final String AWS_API_SERVER = ConfigUtils.getConfigPropertyAsText("AWS_API_SERVER"); private static final String AWS_API_STAGE = ConfigUtils.getConfigPropertyAsText("AWS_API_STAGE"); 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_CONFIRMATION_PAGE_URL = + ConfigUtils.getConfigPropertyAsText("TRUSTED_COMPANION_CONFIRMATION_PAGE_URL"); public static final String ACCEPT_KEY = "acceptKey"; + public static final String USER_LOCALE = "userLocale"; public static final String EMAIL_FIELD_NAME = "email"; /** Note: This path is excluded from security checks, see {@link OtpMiddlewareMain#initializeHttpEndpoints()}. */ @@ -46,6 +52,7 @@ 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) { + Locale locale = getUserLocaleFromRequest(request); try { String acceptKey = getAcceptKeyFromRequest(request); OtpUser dependentUser = getUserFromAcceptKey(acceptKey); @@ -69,9 +76,11 @@ public static OtpUser acceptDependent(Request request, Response response) { return dependentUser; } } catch (IllegalArgumentException e) { - response.redirect( - String.format("%s?error=%s", TRUSTED_COMPANION_CONFIRMATION_PAGE_URL, Message.ACCEPT_DEPENDENT_ERROR.get(null)) - ); + response.redirect(String.format( + "%s?%s", + TRUSTED_COMPANION_CONFIRMATION_PAGE_URL, + URLEncoder.encode(String.format("error=%s", Message.ACCEPT_DEPENDENT_ERROR.get(locale)), UTF_8) + )); } return null; } @@ -102,6 +111,15 @@ private static String getAcceptKeyFromRequest(Request request) throws IllegalArg return acceptKey; } + /** + * Extract the user's language tag from the request and return the {@link Locale} from it. + */ + private static Locale getUserLocaleFromRequest(Request request) throws IllegalArgumentException { + // Note: optional is true so a missing locale will be handled here. + String languageTag = HttpUtils.getQueryParamFromRequest(request, USER_LOCALE, true); + return getLocaleFromString(languageTag); + } + /** * Retrieve the dependent user matching the accept key. */ @@ -111,7 +129,7 @@ private static OtpUser getUserFromAcceptKey(String acceptKey) throws IllegalArgu } OtpUser user = getUserForAcceptKey(acceptKey); if (user == null) { - throw new IllegalArgumentException("Otp user unknown."); + throw new IllegalArgumentException("OTP user unknown."); } return user; } @@ -153,8 +171,8 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r Locale locale = I18nUtils.getOtpUserLocale(relatedUser); String acceptDependentLinkLabel = Message.ACCEPT_DEPENDENT_EMAIL_LINK_TEXT.get(locale); - String acceptDependentUrl = getAcceptDependentUrl(acceptKey); - String addressee = (dependentUser.name != null) ? dependentUser.name : dependentUser.email; + String acceptDependentUrl = getAcceptDependentUrl(acceptKey, locale); + String addressee = (Strings.isBlank(dependentUser.name)) ? dependentUser.email : dependentUser.name; // A HashMap is needed instead of a Map for template data to be serialized to the template renderer. Map templateData = new HashMap<>( @@ -178,12 +196,12 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r ); } - private static String getAcceptDependentUrl(String acceptKey) { - return String.format("%s/%s%s", AWS_API_SERVER, AWS_API_STAGE, getAcceptDependentEndPoint(acceptKey)); + private static String getAcceptDependentUrl(String acceptKey, Locale locale) { + return String.format("%s/%s%s", AWS_API_SERVER, AWS_API_STAGE, getAcceptDependentEndPoint(acceptKey, locale)); } - public static String getAcceptDependentEndPoint(String acceptKey) { - return String.format("/%s?%s=%s", ACCEPT_DEPENDENT_PATH, ACCEPT_KEY, acceptKey); + public static String getAcceptDependentEndPoint(String acceptKey, Locale locale) { + return String.format("/%s?%s=%s&%s=%s", ACCEPT_DEPENDENT_PATH, ACCEPT_KEY, acceptKey, USER_LOCALE, locale.toLanguageTag()); } /** @@ -203,11 +221,18 @@ public static void ensureRelatedUserIntegrity(OtpUser updatedUser, OtpUser preEx .filter(relatedUser -> !updatedUser.relatedUsers.contains(relatedUser)) .collect(Collectors.toList()); for (RelatedUser relatedUser : difference) { - OtpUser user = Persistence.otpUsers.getOneFiltered(eq(EMAIL_FIELD_NAME, relatedUser.email)); - if (user != null) { - user.dependents.remove(updatedUser.id); - Persistence.otpUsers.replace(user.id, user); - } + removeDependent(updatedUser, relatedUser); + } + } + + /** + * Remove the dependent reference from the related user. + */ + public static void removeDependent(OtpUser dependent, RelatedUser relatedUser) { + OtpUser user = Persistence.otpUsers.getOneFiltered(eq(EMAIL_FIELD_NAME, relatedUser.email)); + if (user != null) { + user.dependents.remove(dependent.id); + Persistence.otpUsers.replace(user.id, user); } } } diff --git a/src/main/java/org/opentripplanner/middleware/utils/I18nUtils.java b/src/main/java/org/opentripplanner/middleware/utils/I18nUtils.java index c5c523844..087bce140 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/I18nUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/I18nUtils.java @@ -4,6 +4,7 @@ import org.opentripplanner.middleware.models.OtpUser; import java.util.Collection; +import java.util.IllformedLocaleException; import java.util.Locale; public class I18nUtils { @@ -31,4 +32,18 @@ public static Locale getOtpUserLocale(OtpUser user) { return Locale.forLanguageTag(user == null || user.preferredLocale == null ? "en-US" : user.preferredLocale); } + /** + * Attempt to create a {@link Locale} from the language tag. + */ + public static Locale getLocaleFromString(String languageTag) { + Locale locale = Locale.forLanguageTag("en-US"); + if (languageTag != null) { + try { + locale = Locale.forLanguageTag(languageTag); + } catch (NullPointerException | IllformedLocaleException e) { + // Give up and use the default! + } + } + return locale; + } } diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java index 66892829b..1f62260db 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java @@ -21,6 +21,7 @@ import java.util.Date; import java.util.List; +import java.util.Locale; import java.util.UUID; import java.util.stream.Stream; @@ -187,7 +188,8 @@ void canAcceptDependentRequest() { )); Persistence.otpUsers.replace(dependentUserOne.id, dependentUserOne); - String path = TrustedCompanion.getAcceptDependentEndPoint(acceptKey); + Locale locale = new Locale("en", "GB"); + String path = TrustedCompanion.getAcceptDependentEndPoint(acceptKey, locale); makeGetRequest(path, null); relatedUserOne = Persistence.otpUsers.getById(relatedUserOne.id); From fe06498ea5e49c59e9e68ada0bf2ecb52602e943 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 31 Oct 2024 16:44:44 +0000 Subject: [PATCH 17/17] refactor(TrustedCompanion.java): Update to use Strings.isblank --- .../middleware/tripmonitor/TrustedCompanion.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java index ea1d6cb3d..aeb23d611 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java @@ -105,7 +105,7 @@ private static OtpUser getRelatedUserFromEmail(OtpUser dependentUser, String acc private static String getAcceptKeyFromRequest(Request request) throws IllegalArgumentException { // Note: optional is true so a missing accept key will be handled here. String acceptKey = HttpUtils.getQueryParamFromRequest(request, ACCEPT_KEY, true); - if (acceptKey == null || acceptKey.isEmpty()) { + if (Strings.isBlank(acceptKey)) { throw new IllegalArgumentException("Accept key not provided."); } return acceptKey; @@ -124,7 +124,7 @@ private static Locale getUserLocaleFromRequest(Request request) throws IllegalAr * Retrieve the dependent user matching the accept key. */ private static OtpUser getUserFromAcceptKey(String acceptKey) throws IllegalArgumentException { - if (acceptKey == null) { + if (Strings.isBlank(acceptKey)) { return null; } OtpUser user = getUserForAcceptKey(acceptKey);