diff --git a/README.md b/README.md index df5e9f830..558d3bf81 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,13 @@ mvn package java -jar target/otp-middleware.jar configurations/default/env.yml ``` -## OTP Server Proxy Setup +## Configuration + +### Auth0 + +TODO: Add Auth0 setup instructions. + +### OTP Server Proxy Setup The follow parameters are used to interact with an OTP server. | Parameter | Description | Example | @@ -45,7 +51,12 @@ The follow parameters are used to interact with an OTP server. | OTP_API_ROOT | This is the address of the OTP server, including the root path to the OTP API, to which all OTP related requests will be sent to. | http://otp-server.example.com/otp | | OTP_PLAN_ENDPOINT | This defines the plan endpoint part of the requesting URL. If a request is made to this, the assumption is that a plan request has been made and that the response should be processed accordingly. | /plan | -## Bugsnag Configuration Parameters +### Bugsnag + +Bugsnag is used to report error events that occur within the otp-middleware application or +OpenTripPlanner components that it is monitoring. + +#### Bugsnag Configuration Parameters These values can used as defined here (were applicable), or commented out so the default values are used. Parameters that don't have default values (N/A) can be obtained my following the steps in the next section. @@ -61,8 +72,8 @@ that don't have default values (N/A) can be obtained my following the steps in t | BUGSNAG_REPORTING_WINDOW_IN_DAYS | 14 | The number of days in the past to start retrieving event information. | -## Bugsnag Setup -Where default parameters can not be used, these steps describe how to obtain each compulsory parameter. +#### Bugsnag Setup +Where default parameters cannot be used, these steps describe how to obtain each compulsory parameter. ##### BUGSNAG_API_KEY A bugsnag API key is a key that is unique to an individual Bugsnag user. This key can be obtained by logging into @@ -79,3 +90,26 @@ From here, click on the organization name and then copy the name from the pop-up A Bugsnag project identifier key is unique to a Bugsnag project and allows errors to be saved against it. This key can be obtained by logging into Bugsnag (https://app.bugsnag.com), clicking on Projects (left side menu) and selecting the required project. Once selected, the notifier API key is presented. + +## Testing + +### End-to-end (E2E) + +In order to run E2E tests, specific configuration and environment variables must be used. + +#### Auth0 +The standard Auth0 configuration can be used for the server settings. However, some additional settings must be applied +in order for the server to get an oath token from Auth0 for creating authenticated requests. A private application +should be created for use in Auth0 following these instructions: https://auth0.com/docs/flows/call-your-api-using-resource-owner-password-flow + +One special note is that the default directory must be set at the Account/tenant level. Otherwise, authorization will not +be successful. See this StackOverflow answer for more info: https://stackoverflow.com/a/43835563/915811 + +The special E2E client settings should be defined in `env.yml`: + +| Parameter | Default | Description | +| --- | --- | --- | +| AUTH0_CLIENT_ID | N/A | Special E2E application client ID. | +| AUTH0_CLIENT_SECRET | N/A | Special E2E application client secret. | + +**Note:** Just to reiterate, these are different from the server application settings and are only needed for E2E testing. \ No newline at end of file diff --git a/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java b/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java index a4dbe1f28..2c973a003 100644 --- a/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java +++ b/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java @@ -8,9 +8,9 @@ import org.opentripplanner.middleware.controllers.api.AdminUserController; import org.opentripplanner.middleware.controllers.api.ApiUserController; import org.opentripplanner.middleware.controllers.api.BugsnagController; -import org.opentripplanner.middleware.controllers.api.OtpUserController; import org.opentripplanner.middleware.controllers.api.LogController; import org.opentripplanner.middleware.controllers.api.MonitoredTripController; +import org.opentripplanner.middleware.controllers.api.OtpUserController; import org.opentripplanner.middleware.controllers.api.TripHistoryController; import org.opentripplanner.middleware.docs.PublicApiDocGenerator; import org.opentripplanner.middleware.controllers.api.OtpRequestProcessor; @@ -34,7 +34,7 @@ */ public class OtpMiddlewareMain { private static final Logger LOG = LoggerFactory.getLogger(OtpMiddlewareMain.class); - private static final String API_PREFIX = "/api/"; + public static final String API_PREFIX = "/api/"; public static boolean inTestEnvironment = false; public static void main(String[] args) throws IOException, InterruptedException { diff --git a/src/main/java/org/opentripplanner/middleware/auth/Auth0Connection.java b/src/main/java/org/opentripplanner/middleware/auth/Auth0Connection.java index e41a2118b..79e0cf22c 100644 --- a/src/main/java/org/opentripplanner/middleware/auth/Auth0Connection.java +++ b/src/main/java/org/opentripplanner/middleware/auth/Auth0Connection.java @@ -9,16 +9,24 @@ import com.auth0.jwt.algorithms.Algorithm; import com.auth0.jwt.exceptions.JWTVerificationException; import com.auth0.jwt.interfaces.DecodedJWT; +import com.fasterxml.jackson.core.JsonProcessingException; import org.eclipse.jetty.http.HttpStatus; +import org.opentripplanner.middleware.models.AbstractUser; +import org.opentripplanner.middleware.models.ApiUser; +import org.opentripplanner.middleware.models.OtpUser; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import spark.HaltException; import spark.Request; import spark.Response; import java.security.interfaces.RSAPublicKey; +import static org.opentripplanner.middleware.controllers.api.ApiUserController.API_USER_PATH; +import static org.opentripplanner.middleware.controllers.api.OtpUserController.OTP_USER_PATH; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; import static org.opentripplanner.middleware.utils.ConfigUtils.hasConfigProperty; +import static org.opentripplanner.middleware.utils.JsonUtils.getPOJOFromRequestBody; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; /** @@ -53,18 +61,59 @@ public static void checkUser(Request req) { try { DecodedJWT jwt = verifier.verify(token); Auth0UserProfile profile = new Auth0UserProfile(jwt); + if (!isValidUser(profile)) { + if (isCreatingSelf(req, profile)) { + // If creating self, no user account is required (it does not exist yet!). Note: creating an + // admin user requires that the requester is an admin (checkUserIsAdmin must be passed), so this + // is not a concern for that method/controller. + LOG.info("New user is creating self. OK to proceed without existing user object for auth0UserId"); + } else { + // Otherwise, if no valid user is found, halt the request. + logMessageAndHalt(req, HttpStatus.NOT_FOUND_404, "Unknown user."); + } + } // The user attribute is used on the server side to check user permissions and does not have all of the // fields that the raw Auth0 profile string does. addUserToRequest(req, profile); } catch (JWTVerificationException e) { // Invalid signature/claims logMessageAndHalt(req, 401, "Login failed to verify with our authorization provider.", e); + } catch (HaltException e) { + throw e; } catch (Exception e) { LOG.warn("Login failed to verify with our authorization provider.", e); logMessageAndHalt(req, 401, "Could not verify user's token"); } } + /** + * Check for POST requests that are creating an {@link AbstractUser} (a proxy for OTP/API users). + */ + private static boolean isCreatingSelf(Request req, Auth0UserProfile profile) { + String uri = req.uri(); + String method = req.requestMethod(); + // Check that this is a POST request. + if (method.equalsIgnoreCase("POST")) { + // Next, check that an OtpUser or ApiUser is being created (an admin must rely on another admin to create + // them). + boolean creatingOtpUser = uri.endsWith(OTP_USER_PATH); + boolean creatingApiUser = uri.endsWith(API_USER_PATH); + if (creatingApiUser || creatingOtpUser) { + // Get the correct user class depending on request path. + Class userClass = creatingApiUser ? ApiUser.class : OtpUser.class; + try { + // Next, get the user object from the request body, verifying that the Auth0UserId matches between + // requester and the new user object. + AbstractUser user = getPOJOFromRequestBody(req, userClass); + return profile.auth0UserId.equals(user.auth0UserId); + } catch (JsonProcessingException e) { + LOG.warn("Could not parse user object from request.", e); + } + } + } + return false; + } + public static boolean isAuthHeaderPresent(Request req) { final String authHeader = req.headers("Authorization"); return authHeader != null; @@ -106,7 +155,7 @@ public static void addUserToRequest(Request req, Auth0UserProfile user) { * Get user profile from Spark Request object */ public static Auth0UserProfile getUserFromRequest(Request req) { - return (Auth0UserProfile) req.attribute("user"); + return req.attribute("user"); } /** @@ -169,40 +218,30 @@ public static boolean authDisabled() { } /** - * Confirm that the user exists + * Confirm that the user exists in at least one of the MongoDB user collections. */ - private static Auth0UserProfile isValidUser(Request request) { - - Auth0UserProfile profile = getUserFromRequest(request); - if (profile == null || (profile.adminUser == null && profile.otpUser == null && profile.apiUser == null)) { - logMessageAndHalt(request, HttpStatus.NOT_FOUND_404, "Unknown user."); - } - - return profile; + private static boolean isValidUser(Auth0UserProfile profile) { + return profile != null && (profile.adminUser != null || profile.otpUser != null || profile.apiUser != null); } /** * Confirm that the user's actions are on their items if not admin. */ public static void isAuthorized(String userId, Request request) { - - Auth0UserProfile profile = isValidUser(request); - + Auth0UserProfile profile = getUserFromRequest(request); // let admin do anything if (profile.adminUser != null) { return; } - + // If userId is defined, it must be set to a value associated with the user. if (userId != null) { if (profile.otpUser != null && profile.otpUser.id.equals(userId)) { return; } - if (profile.apiUser != null && profile.apiUser.id.equals(userId)) { return; } } - logMessageAndHalt(request, HttpStatus.FORBIDDEN_403, "Unauthorized access."); } diff --git a/src/main/java/org/opentripplanner/middleware/auth/Auth0Users.java b/src/main/java/org/opentripplanner/middleware/auth/Auth0Users.java index e5d4dc68f..3bbe96755 100644 --- a/src/main/java/org/opentripplanner/middleware/auth/Auth0Users.java +++ b/src/main/java/org/opentripplanner/middleware/auth/Auth0Users.java @@ -6,21 +6,28 @@ import com.auth0.json.mgmt.jobs.Job; import com.auth0.json.mgmt.users.User; import com.auth0.net.AuthRequest; +import com.fasterxml.jackson.core.JsonProcessingException; import org.apache.commons.validator.routines.EmailValidator; import org.eclipse.jetty.http.HttpStatus; import org.opentripplanner.middleware.bugsnag.BugsnagReporter; import org.opentripplanner.middleware.models.AbstractUser; import org.opentripplanner.middleware.persistence.TypedPersistence; +import org.opentripplanner.middleware.utils.HttpUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import spark.Request; +import java.net.URI; +import java.net.http.HttpResponse; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.UUID; import static com.mongodb.client.model.Filters.eq; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; +import static org.opentripplanner.middleware.utils.HttpUtils.httpRequestRawResponse; +import static org.opentripplanner.middleware.utils.JsonUtils.getSingleNodeValueFromJSON; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; /** @@ -28,10 +35,12 @@ * searchable fields and query syntax are here: https://auth0.com/docs/api/management/v2/user-search */ public class Auth0Users { - private static final String AUTH0_DOMAIN = getConfigPropertyAsText("AUTH0_DOMAIN"); + public static final String AUTH0_DOMAIN = getConfigPropertyAsText("AUTH0_DOMAIN"); // This client/secret pair is for making requests for an API access token used with the Management API. private static final String AUTH0_API_CLIENT = getConfigPropertyAsText("AUTH0_API_CLIENT"); private static final String AUTH0_API_SECRET = getConfigPropertyAsText("AUTH0_API_SECRET"); + private static final String AUTH0_CLIENT_ID = getConfigPropertyAsText("AUTH0_CLIENT_ID"); + private static final String AUTH0_CLIENT_SECRET = getConfigPropertyAsText("AUTH0_CLIENT_SECRET"); private static final String DEFAULT_CONNECTION_TYPE = "Username-Password-Authentication"; private static final String MANAGEMENT_API_VERSION = "v2"; private static final String SEARCH_API_VERSION = "v3"; @@ -44,15 +53,19 @@ public class Auth0Users { private static final AuthAPI authAPI = new AuthAPI(AUTH0_DOMAIN, AUTH0_API_CLIENT, AUTH0_API_SECRET); /** - * Creates a standard user for the provided email address. Defaults to a random UUID password and connection type - * of {@link #DEFAULT_CONNECTION_TYPE}. + * Creates a standard user for the provided email address. Defaults to a random UUID password and connection type of + * {@link #DEFAULT_CONNECTION_TYPE}. */ public static User createAuth0UserForEmail(String email) throws Auth0Exception { + return createAuth0UserForEmail(email, UUID.randomUUID().toString()); + } + + public static User createAuth0UserForEmail(String email, String password) throws Auth0Exception { // Create user object and assign properties. User user = new User(); user.setEmail(email); // TODO set name? phone? other Auth0 properties? - user.setPassword(UUID.randomUUID().toString()); + user.setPassword(password); user.setConnection(DEFAULT_CONNECTION_TYPE); return getManagementAPI() .users() @@ -64,6 +77,7 @@ public static User createAuth0UserForEmail(String email) throws Auth0Exception { * Delete Auth0 user by Auth0 user ID using the Management API. */ public static void deleteAuth0User(String userId) throws Auth0Exception { + LOG.info("Deleting Auth0 user for {}", userId); getManagementAPI() .users() .delete(userId) @@ -79,8 +93,8 @@ public static TokenCache getCachedToken() { } /** - * Gets an Auth0 API access token for authenticating requests to the Auth0 Management API. This will either create - * a new token using the oauth token endpoint or grab a cached token that it has already created (if it has not + * Gets an Auth0 API access token for authenticating requests to the Auth0 Management API. This will either create a + * new token using the oauth token endpoint or grab a cached token that it has already created (if it has not * expired). More information on setting this up is here: https://auth0.com/docs/api/management/v2/get-access-tokens-for-production */ public static String getApiToken() { @@ -126,9 +140,9 @@ public static User getUserByEmail(String email, boolean createIfNotExists) { } /** - * Method to trigger an Auth0 job to resend a verification email. Returns an Auth0 {@link Job} which can be - * used to monitor the progress of the job (using job ID). Typically the verification email goes out pretty quickly - * so there shouldn't be too much of a need to monitor the result. + * Method to trigger an Auth0 job to resend a verification email. Returns an Auth0 {@link Job} which can be used to + * monitor the progress of the job (using job ID). Typically the verification email goes out pretty quickly so there + * shouldn't be too much of a need to monitor the result. */ public static Job resendVerificationEmail(String userId) { try { @@ -176,6 +190,7 @@ public static User createNewAuth0User(U user, Request r // Ensure no user with email exists in MongoDB. U userWithEmail = userStore.getOneFiltered(eq("email", user.email)); if (userWithEmail != null) { + // TODO: Does this need to change to allow multiple applications to create otpuser's with the same email? logMessageAndHalt(req, 400, "User with email already exists in database!"); } // Check for pre-existing user in Auth0 and create if not exists. @@ -183,6 +198,7 @@ public static User createNewAuth0User(U user, Request r if (auth0UserProfile == null) { logMessageAndHalt(req, HttpStatus.INTERNAL_SERVER_ERROR_500, "Error creating user for email " + user.email); } + LOG.info("Created new Auth0 user ({}) for user {}", auth0UserProfile.getId(), user.id); return auth0UserProfile; } @@ -224,4 +240,34 @@ private static String getAuth0Url() { ? "http://locahost:8089" : "https://" + AUTH0_DOMAIN; } + + /** + * Get an Auth0 oauth token for use in mocking user requests by using the Auth0 'Call Your API Using Resource Owner + * Password Flow' approach. Auth0 setup can be reviewed here: https://auth0.com/docs/flows/call-your-api-using-resource-owner-password-flow. + * If the user is successfully validated by Auth0 a bearer access token is returned, which is extracted and returned + * to the caller. In all other cases, null is returned. + */ + public static String getAuth0Token(String username, String password) throws JsonProcessingException { + String body = String.format( + "grant_type=password&username=%s&password=%s&audience=%s&scope=&client_id=%s&client_secret=%s", + username, + password, + "https://otp-middleware", // must match an API identifier + AUTH0_CLIENT_ID, // Auth0 application client ID + AUTH0_CLIENT_SECRET // Auth0 application client secret + ); + + HttpResponse response = httpRequestRawResponse( + URI.create(String.format("https://%s/oauth/token", AUTH0_DOMAIN)), + 1000, + HttpUtils.REQUEST_METHOD.POST, + Collections.singletonMap("content-type", "application/x-www-form-urlencoded"), + body + ); + if (response == null || response.statusCode() != HttpStatus.OK_200) { + LOG.error("Cannot obtain Auth0 token for user {}. response: {} - {}", username, response.statusCode(), response.body()); + return null; + } + return getSingleNodeValueFromJSON("access_token", response.body()); + } } diff --git a/src/main/java/org/opentripplanner/middleware/bugsnag/BugsnagReporter.java b/src/main/java/org/opentripplanner/middleware/bugsnag/BugsnagReporter.java index de456d26a..ff8caf404 100644 --- a/src/main/java/org/opentripplanner/middleware/bugsnag/BugsnagReporter.java +++ b/src/main/java/org/opentripplanner/middleware/bugsnag/BugsnagReporter.java @@ -39,10 +39,17 @@ public static boolean reportErrorToBugsnag(String message, Throwable throwable) } /** - * If Bugsnag has been configured, report error based on provided information. + * If Bugsnag has been configured, report error based on provided information. Note: throwable must be non-null. */ public static boolean reportErrorToBugsnag(String message, Object badEntity, Throwable throwable) { + // If no throwable provided, create a new UnknownError exception so that Bugsnag will accept the error report. + if (throwable == null) { + LOG.warn("No exception provided for this error report (message: {}). New UnknownError used instead.", message); + throwable = new UnknownError("Exception type is unknown! Please add exception where report method is called."); + } + // Log error to otp-middleware logs. LOG.error(message, throwable); + // If bugsnag is disabled, make sure to report full error to otp-middleware logs. if (bugsnag == null) { LOG.warn("Bugsnag error reporting is disabled. Unable to report to Bugsnag this message: {} for this bad entity: {}", message, @@ -50,14 +57,7 @@ public static boolean reportErrorToBugsnag(String message, Object badEntity, Thr throwable); return false; } - - if (throwable == null) { - LOG.warn("This error is not an exception and cannot be reported to Bugsnag. This message: {} for this bad entity: {}", - message, - badEntity); - return false; - } - + // Finally, construct report and send to bugsnag. Report report = bugsnag.buildReport(throwable); report.setContext(message); report.setAppInfo("entity", badEntity != null ? badEntity.toString() : "N/A"); diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/AbstractUserController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/AbstractUserController.java index 5cb6b0bba..30430275a 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/AbstractUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/AbstractUserController.java @@ -1,6 +1,5 @@ package org.opentripplanner.middleware.controllers.api; -import com.auth0.exception.Auth0Exception; import com.auth0.json.mgmt.jobs.Job; import com.auth0.json.mgmt.users.User; import com.beerboy.ss.ApiEndpoint; @@ -18,7 +17,6 @@ import static com.beerboy.ss.descriptor.MethodDescriptor.path; import static org.opentripplanner.middleware.auth.Auth0Users.createNewAuth0User; -import static org.opentripplanner.middleware.auth.Auth0Users.deleteAuth0User; import static org.opentripplanner.middleware.auth.Auth0Users.updateAuthFieldsForUser; import static org.opentripplanner.middleware.auth.Auth0Users.validateExistingUser; import static org.opentripplanner.middleware.utils.HttpUtils.JSON_ONLY; @@ -118,13 +116,6 @@ U preUpdateHook(U user, U preExistingUser, Request req) { */ @Override boolean preDeleteHook(U user, Request req) { - try { - deleteAuth0User(user.auth0UserId); - } catch (Auth0Exception e) { - // FIXME: Add Bugsnag error report. - logMessageAndHalt(req, 500, "Error deleting user.", e); - return false; - } return true; } } diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java index 8fe4a4f9d..b51517b02 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java @@ -3,6 +3,7 @@ import com.beerboy.ss.ApiEndpoint; import com.beerboy.ss.SparkSwagger; import com.beerboy.ss.rest.Endpoint; +import com.fasterxml.jackson.core.JsonProcessingException; import com.mongodb.client.model.Filters; import org.bson.conversions.Bson; import org.eclipse.jetty.http.HttpStatus; @@ -160,7 +161,8 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) { .delete(path(ID_PATH) .withDescription("Deletes the '" + className + "' entity with the specified id if it exists.") .withPathParam().withName(ID_PARAM).withRequired(true).withDescription("The id of the entity to delete.").and() - .withGenericResponse(), + .withProduces(JSON_ONLY) + .withResponseType(clazz), this::deleteOne, JsonUtils::toJson ); } @@ -218,7 +220,7 @@ private T getOne(Request req, Response res) { /** * HTTP endpoint to delete one entity specified by ID. */ - private String deleteOne(Request req, Response res) { + private T deleteOne(Request req, Response res) { long startTime = DateTimeUtils.currentTimeMillis(); String id = getIdFromRequest(req); Auth0UserProfile requestingUser = Auth0Connection.getUserFromRequest(req); @@ -232,12 +234,17 @@ private String deleteOne(Request req, Response res) { if (!preDeleteHook(object, req)) { logMessageAndHalt(req, 500, "Unknown error occurred during delete attempt."); } - boolean success = persistence.removeById(id); - int code = success ? HttpStatus.OK_200 : HttpStatus.INTERNAL_SERVER_ERROR_500; - String message = success - ? String.format("Successfully deleted %s.", className) - : String.format("Failed to delete %s", className); - logMessageAndHalt(req, code, message, null); + boolean success = object.delete(); + if (success) { + return object; + } else { + logMessageAndHalt( + req, + HttpStatus.INTERNAL_SERVER_ERROR_500, + String.format("Unknown error encountered. Failed to delete %s", className), + null + ); + } } catch (HaltException e) { throw e; } catch (Exception e) { @@ -337,6 +344,8 @@ private T createOrUpdate(Request req, Response res) { return persistence.getById(object.id); } catch (HaltException e) { throw e; + } catch (JsonProcessingException e) { + logMessageAndHalt(req, HttpStatus.BAD_REQUEST_400, "Error parsing JSON for " + clazz.getSimpleName(), e); } catch (Exception e) { logMessageAndHalt(req, 500, "An error was encountered while trying to save to the database", e); } finally { diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/ApiUserController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/ApiUserController.java index 1cddb5d3b..16caffe68 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/ApiUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/ApiUserController.java @@ -30,13 +30,14 @@ */ public class ApiUserController extends AbstractUserController { private static final Logger LOG = LoggerFactory.getLogger(ApiUserController.class); - private static final String DEFAULT_USAGE_PLAN_ID = getConfigPropertyAsText("DEFAULT_USAGE_PLAN_ID"); + public static final String DEFAULT_USAGE_PLAN_ID = getConfigPropertyAsText("DEFAULT_USAGE_PLAN_ID"); private static final String API_KEY_PATH = "/apikey"; private static final int API_KEY_LIMIT_PER_USER = 2; private static final String API_KEY_ID_PARAM = "/:apiKeyId"; + public static final String API_USER_PATH = "secure/application"; public ApiUserController(String apiPrefix) { - super(apiPrefix, Persistence.apiUsers, "secure/application"); + super(apiPrefix, Persistence.apiUsers, API_USER_PATH); } @Override @@ -164,9 +165,8 @@ protected ApiUser getUserProfile(Auth0UserProfile profile) { */ @Override ApiUser preCreateHook(ApiUser user, Request req) { - ApiKey apiKey; try { - apiKey = ApiGatewayUtils.createApiKey(user, DEFAULT_USAGE_PLAN_ID); + user.createApiKey(DEFAULT_USAGE_PLAN_ID, false); } catch (CreateApiKeyException e) { logMessageAndHalt( req, @@ -176,13 +176,11 @@ ApiUser preCreateHook(ApiUser user, Request req) { ); return null; } - // store api key id including the actual api key (value) - user.apiKeys.add(apiKey); // Call AbstractUserController#preCreateHook and delete api key in case something goes wrong. try { return super.preCreateHook(user, req); } catch (HaltException e) { - deleteApiKey(apiKey); + user.delete(); throw e; } } @@ -193,10 +191,7 @@ ApiUser preCreateHook(ApiUser user, Request req) { */ @Override boolean preDeleteHook(ApiUser user, Request req) { - // TODO: Create method for deleting user's API keys? - for (ApiKey apiKey : user.apiKeys) { - deleteApiKey(apiKey); - } + // Note: API keys deleted in ApiUser#delete return true; } 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 b55201b8c..65109d316 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java @@ -8,7 +8,6 @@ import spark.Request; import static com.mongodb.client.model.Filters.eq; -import static org.opentripplanner.middleware.auth.Auth0Connection.isAuthorized; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsInt; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; @@ -26,15 +25,12 @@ public MonitoredTripController(String apiPrefix) { @Override MonitoredTrip preCreateHook(MonitoredTrip monitoredTrip, Request req) { - isAuthorized(monitoredTrip.userId, req); verifyBelowMaxNumTrips(monitoredTrip.userId, req); - return monitoredTrip; } @Override MonitoredTrip preUpdateHook(MonitoredTrip monitoredTrip, MonitoredTrip preExisting, Request req) { - isAuthorized(monitoredTrip.userId, req); return monitoredTrip; } @@ -48,12 +44,15 @@ boolean preDeleteHook(MonitoredTrip monitoredTrip, Request req) { * Confirm that the maximum number of saved monitored trips has not been reached */ private void verifyBelowMaxNumTrips(String userId, Request request) { - // filter monitored trip on user id to find out how many have already been saved Bson filter = Filters.and(eq("userId", userId)); long count = this.persistence.getCountFiltered(filter); if (count >= MAXIMUM_PERMITTED_MONITORED_TRIPS) { - logMessageAndHalt(request, HttpStatus.BAD_REQUEST_400, "Maximum permitted saved monitored trips reached. Maximum = " + MAXIMUM_PERMITTED_MONITORED_TRIPS); + logMessageAndHalt( + request, + HttpStatus.BAD_REQUEST_400, + "Maximum permitted saved monitored trips reached. Maximum = " + MAXIMUM_PERMITTED_MONITORED_TRIPS + ); } } } diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpRequestProcessor.java b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpRequestProcessor.java index a9ee9d470..5c84646f9 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpRequestProcessor.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpRequestProcessor.java @@ -26,28 +26,26 @@ import static javax.ws.rs.core.MediaType.APPLICATION_XML; import static org.opentripplanner.middleware.auth.Auth0Connection.isAuthHeaderPresent; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; +import static org.opentripplanner.middleware.otp.OtpDispatcher.OTP_API_ROOT; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; /** * Responsible for getting a response from OTP based on the parameters provided by the requester. If the target service - * is of interest the response is intercepted and processed. In all cases, the response from OTP (content and HTTP status) - * is passed back to the requester. + * is of interest the response is intercepted and processed. In all cases, the response from OTP (content and HTTP + * status) is passed back to the requester. */ public class OtpRequestProcessor implements Endpoint { private static final Logger LOG = LoggerFactory.getLogger(OtpRequestProcessor.class); - /** - * URI location of the OpenTripPlanner API (e.g., https://otp-server.com/otp). Requests sent to this URI should - * return OTP version info. - */ - private static final String OTP_API_ROOT = getConfigPropertyAsText("OTP_API_ROOT"); + /** * Location of the plan endpoint for which all requests will be handled by {@link #handlePlanTripResponse} */ - private static final String OTP_PLAN_ENDPOINT = getConfigPropertyAsText("OTP_PLAN_ENDPOINT"); + public static final String OTP_PLAN_ENDPOINT = getConfigPropertyAsText("OTP_PLAN_ENDPOINT"); + /** * Endpoint for the OTP Middleware's OTP proxy */ - private static final String OTP_PROXY_ENDPOINT = "/otp"; + public static final String OTP_PROXY_ENDPOINT = "/otp"; /** * URL to OTP's documentation. */ @@ -90,6 +88,7 @@ private static String proxy(Request request, spark.Response response) { } // Get request path intended for OTP API by removing the proxy endpoint (/otp). String otpRequestPath = request.uri().replace(OTP_PROXY_ENDPOINT, ""); + // attempt to get response from OTP server based on requester's query parameters OtpDispatcherResponse otpDispatcherResponse = OtpDispatcher.sendOtpRequest(request.queryString(), otpRequestPath); if (otpDispatcherResponse == null || otpDispatcherResponse.responseBody == null) { 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 cf666c2ce..aa2fc4f65 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java @@ -1,15 +1,40 @@ package org.opentripplanner.middleware.controllers.api; +import com.mongodb.client.model.Filters; import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.bugsnag.BugsnagReporter; +import org.opentripplanner.middleware.models.ApiUser; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.persistence.Persistence; +import spark.Request; /** * Implementation of the {@link AbstractUserController} for {@link OtpUser}. */ public class OtpUserController extends AbstractUserController { + public static final String OTP_USER_PATH = "secure/user"; public OtpUserController(String apiPrefix) { - super(apiPrefix, Persistence.otpUsers, "secure/user"); + super(apiPrefix, Persistence.otpUsers, OTP_USER_PATH); + } + + @Override + OtpUser preCreateHook(OtpUser user, Request req) { + // Check API key and assign user to appropriate third-party application. Note: this is only relevant for + // instances of otp-middleware running behind API Gateway. + String apiKey = req.headers("x-api-key"); + ApiUser apiUser = Persistence.apiUsers.getOneFiltered(Filters.eq("apiKeys.value", apiKey)); + if (apiUser != null) { + // If API user found, assign to new OTP user. + user.applicationId = apiUser.id; + } else { + // If API user not found, report to Bugsnag for further investigation. + BugsnagReporter.reportErrorToBugsnag( + "OTP user created with API key that is not linked to any API user", + apiKey, + new IllegalArgumentException("API key not linked to API user.") + ); + } + return super.preCreateHook(user, req); } @Override diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/TripHistoryController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/TripHistoryController.java index 97498be43..7340251f5 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/TripHistoryController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/TripHistoryController.java @@ -2,12 +2,8 @@ import com.beerboy.ss.SparkSwagger; import com.beerboy.ss.rest.Endpoint; -import com.mongodb.client.model.Filters; -import org.bson.conversions.Bson; import org.eclipse.jetty.http.HttpStatus; import org.opentripplanner.middleware.models.TripRequest; -import org.opentripplanner.middleware.persistence.Persistence; -import org.opentripplanner.middleware.persistence.TypedPersistence; import org.opentripplanner.middleware.utils.DateTimeUtils; import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.JsonUtils; @@ -18,18 +14,12 @@ import java.time.LocalDate; import java.time.LocalTime; -import java.time.ZoneId; import java.time.format.DateTimeParseException; import java.util.Date; -import java.util.HashSet; import java.util.List; -import java.util.Set; import static com.beerboy.ss.descriptor.EndpointDescriptor.endpointPath; import static com.beerboy.ss.descriptor.MethodDescriptor.path; -import static com.mongodb.client.model.Filters.eq; -import static com.mongodb.client.model.Filters.gte; -import static com.mongodb.client.model.Filters.lte; import static org.opentripplanner.middleware.auth.Auth0Connection.isAuthorized; import static org.opentripplanner.middleware.utils.DateTimeUtils.YYYY_MM_DD; import static org.opentripplanner.middleware.utils.HttpUtils.JSON_ONLY; @@ -100,11 +90,8 @@ public void bind(final SparkSwagger restApi) { * An authorized user (Auth0) and user id are required. */ private static List getTripRequests(Request request, Response response) { - - TypedPersistence tripRequest = Persistence.tripRequests; - final String userId = HttpUtils.getRequiredQueryParamFromRequest(request, "userId", false); - + // Check that the user is authorized (otherwise a halt is thrown). isAuthorized(userId, request); int limit = DEFAULT_LIMIT; @@ -134,32 +121,7 @@ private static List getTripRequests(Request request, Response respo String.format("%s (%s) before %s (%s)", TO_DATE_PARAM_NAME, paramToDate, FROM_DATE_PARAM_NAME, paramFromDate)); } - - Bson filter = buildFilter(userId, fromDate, toDate); - return tripRequest.getFilteredWithLimit(filter, limit); - } - - /** - * Build the filter which is passed to Mongo based on available parameters - */ - private static Bson buildFilter(String userId, Date fromDate, Date toDate) { - - Set clauses = new HashSet<>(); - - // user id is required, so as a minimum return all trip requests for user - clauses.add(eq("userId", userId)); - - // Get all trip requests that occurred from supplied start date. - if (fromDate != null) { - clauses.add(gte("dateCreated", fromDate)); - } - - // Get all trip requests that occurred until the supplied end date. - if (toDate != null) { - clauses.add(lte("dateCreated", toDate)); - } - - return Filters.and(clauses); + return TripRequest.requestsForUser(userId, fromDate, toDate, limit); } /** diff --git a/src/main/java/org/opentripplanner/middleware/models/AbstractUser.java b/src/main/java/org/opentripplanner/middleware/models/AbstractUser.java index e15cba440..fe998c48c 100644 --- a/src/main/java/org/opentripplanner/middleware/models/AbstractUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/AbstractUser.java @@ -1,12 +1,17 @@ package org.opentripplanner.middleware.models; +import com.auth0.exception.Auth0Exception; import org.opentripplanner.middleware.auth.Auth0UserProfile; import org.opentripplanner.middleware.auth.Permission; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.HashSet; import java.util.Set; import java.util.UUID; +import static org.opentripplanner.middleware.auth.Auth0Users.deleteAuth0User; + /** * This is an abstract user class that {@link OtpUser}, {@link AdminUser}, and {@link ApiUser} extend. * @@ -14,6 +19,7 @@ * authorization check {@link #canBeManagedBy}. */ public abstract class AbstractUser extends Model { + private static final Logger LOG = LoggerFactory.getLogger(AbstractUser.class); private static final long serialVersionUID = 1L; /** Email address for contact. This must be unique in the collection. */ public String email; @@ -51,4 +57,17 @@ public boolean canBeManagedBy(Auth0UserProfile user) { // Fallback to Model#userCanManage. return super.canBeManagedBy(user); } + + @Override + public boolean delete() { + // FIXME: Check that the Auth0 user ID being deleted does not exist as a different child class? + try { + // Delete Auth0User. + deleteAuth0User(this.auth0UserId); + return true; + } catch (Auth0Exception e) { + LOG.error("Could not delete Auth0 user {}.", this.auth0UserId, e); + return false; + } + } } diff --git a/src/main/java/org/opentripplanner/middleware/models/AdminUser.java b/src/main/java/org/opentripplanner/middleware/models/AdminUser.java index e1df7a84d..b23165dd4 100644 --- a/src/main/java/org/opentripplanner/middleware/models/AdminUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/AdminUser.java @@ -2,6 +2,9 @@ import org.opentripplanner.middleware.auth.Auth0UserProfile; import org.opentripplanner.middleware.auth.Permission; +import org.opentripplanner.middleware.persistence.Persistence; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static org.opentripplanner.middleware.auth.Auth0Connection.isUserAdmin; @@ -9,6 +12,7 @@ * Represents an administrative user of the OTP Admin Dashboard (otp-admin-ui). */ public class AdminUser extends AbstractUser { + private static final Logger LOG = LoggerFactory.getLogger(AdminUser.class); // TODO: Add admin-specific fields /** @@ -30,4 +34,15 @@ public AdminUser() { public boolean canBeCreatedBy(Auth0UserProfile user) { return isUserAdmin(user); } + + @Override + public boolean delete() { + boolean auth0UserDeleted = super.delete(); + if (auth0UserDeleted) { + return Persistence.adminUsers.removeById(this.id); + } else { + LOG.warn("Aborting user deletion for {}", this.email); + return false; + } + } } diff --git a/src/main/java/org/opentripplanner/middleware/models/ApiUser.java b/src/main/java/org/opentripplanner/middleware/models/ApiUser.java index 1b7d4b662..c9e668307 100644 --- a/src/main/java/org/opentripplanner/middleware/models/ApiUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/ApiUser.java @@ -1,7 +1,11 @@ package org.opentripplanner.middleware.models; -import com.mongodb.client.model.Filters; import org.opentripplanner.middleware.persistence.Persistence; +import org.opentripplanner.middleware.utils.ApiGatewayUtils; +import org.opentripplanner.middleware.utils.CreateApiKeyException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import com.mongodb.client.model.Filters; import java.util.ArrayList; import java.util.List; @@ -11,6 +15,7 @@ * access otp-middleware's endpoints (as well as the geocoding and OTP endpoints). */ public class ApiUser extends AbstractUser { + private static final Logger LOG = LoggerFactory.getLogger(ApiUser.class); /** Holds the API keys assigned to the user. */ public List apiKeys = new ArrayList<>(); @@ -34,6 +39,37 @@ public class ApiUser extends AbstractUser { /** The name of this user */ public String name; + /** + * Delete API user's API keys (from AWS) and self (from Mongo). + */ + @Override + public boolean delete() { + for (ApiKey apiKey : apiKeys) { + if (!ApiGatewayUtils.deleteApiKey(apiKey)) { + LOG.error("Could not delete API key for user {}. Aborting delete user.", apiKey.keyId); + return false; + } + } + boolean auth0UserDeleted = super.delete(); + if (auth0UserDeleted) { + return Persistence.apiUsers.removeById(this.id); + } else { + LOG.warn("Aborting user deletion for {}", this.email); + return false; + } + } + + public void createApiKey(String usagePlanId, boolean persist) throws CreateApiKeyException { + try { + ApiKey apiKey = ApiGatewayUtils.createApiKey(this, usagePlanId); + apiKeys.add(apiKey); + if (persist) Persistence.apiUsers.replace(this.id, this); + } catch (CreateApiKeyException e) { + LOG.error("Could not create API key for user {}", email, e); + throw e; + } + } + /** * @return the first {@link ApiUser} found with an {@link ApiKey#keyId} in {@link #apiKeys} that matches the * provided apiKeyId. diff --git a/src/main/java/org/opentripplanner/middleware/models/Model.java b/src/main/java/org/opentripplanner/middleware/models/Model.java index ec7de0cd5..06a719cc4 100644 --- a/src/main/java/org/opentripplanner/middleware/models/Model.java +++ b/src/main/java/org/opentripplanner/middleware/models/Model.java @@ -38,4 +38,8 @@ public boolean canBeManagedBy(Auth0UserProfile user) { // TODO: Check if user has application administrator permission? return isUserAdmin(user); } + + public boolean delete() throws Exception { + return false; + } } diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java index 4611bc905..34218d4ff 100644 --- a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java @@ -3,12 +3,14 @@ import org.opentripplanner.middleware.auth.Auth0UserProfile; import org.opentripplanner.middleware.auth.Permission; import org.opentripplanner.middleware.otp.response.Itinerary; +import org.opentripplanner.middleware.persistence.Persistence; +import org.opentripplanner.middleware.persistence.TypedPersistence; -import java.util.Set; +import java.util.List; /** - * A monitored trip represents a trip a user would like to receive notification on if affected by a delay and/or - * route change. + * A monitored trip represents a trip a user would like to receive notification on if affected by a delay and/or route + * change. */ public class MonitoredTrip extends Model { @@ -23,9 +25,9 @@ public class MonitoredTrip extends Model { public String tripName; /** - * The time at which the trip takes place. This will be in the format HH:mm and is extracted (or provided separately) - * to the date and time within the query parameters. The reasoning is so that it doesn't have to be extracted every - * time the trip requires checking. + * The time at which the trip takes place. This will be in the format HH:mm and is extracted (or provided + * separately) to the date and time within the query parameters. The reasoning is so that it doesn't have to be + * extracted every time the trip requires checking. */ public String tripTime; @@ -91,29 +93,46 @@ public class MonitoredTrip extends Model { public Itinerary itinerary; //TODO, agree on and implement these parameters + /** - notificationThresholds - notifyRouteChange (true/false) - Instead of notificationThresholds - arrivalDelayMinutesThreshold (int minutes) - Instead of notificationThresholds - departureDelayMinutesThreshold (int minutes) - Instead of notificationThresholds - notifyDelayToTrip (true/false) - */ + * notificationThresholds notifyRouteChange (true/false) - Instead of notificationThresholds + * arrivalDelayMinutesThreshold (int minutes) - Instead of notificationThresholds departureDelayMinutesThreshold + * (int minutes) - Instead of notificationThresholds notifyDelayToTrip (true/false) + */ public MonitoredTrip() { } + @Override + public boolean canBeCreatedBy(Auth0UserProfile profile) { + OtpUser otpUser = profile.otpUser; + if (userId == null) { + if (otpUser == null) { + // The otpUser must exist (and be the requester) if the userId is null. Otherwise, there is nobody to + // assign the trip to. + return false; + } + // If userId on trip is null, auto-assign the otpUser's id to trip. + userId = otpUser.id; + } else { + // If userId was provided, follow authorization provided by canBeManagedBy + return canBeManagedBy(profile); + } + return super.canBeCreatedBy(profile); + } + /** * Confirm that the requesting user has the required permissions */ @Override public boolean canBeManagedBy(Auth0UserProfile user) { + // This should not be possible, but return false on a null userId just in case. + if (userId == null) return false; // If the user is attempting to update someone else's monitored trip, they must be admin. boolean belongsToUser = false; - + // Monitored trip can only be owned by an OtpUser (not an ApiUser or AdminUser). if (user.otpUser != null) { belongsToUser = userId.equals(user.otpUser.id); - } else if (user.apiUser != null) { - belongsToUser = userId.equals(user.apiUser.id); } if (belongsToUser) { @@ -128,5 +147,17 @@ public boolean canBeManagedBy(Auth0UserProfile user) { return super.canBeManagedBy(user); } + /** + * Get monitored trips for the specified {@link OtpUser} user Id. + */ + public static List tripsForUser(String userId) { + return Persistence.monitoredTrips.getFiltered(TypedPersistence.filterByUserId(userId)); + } + + @Override + public boolean delete() { + // TODO: Add journey state deletion. + return Persistence.monitoredTrips.removeById(this.id); + } } diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index 6a33d4cb8..9420b7457 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -1,5 +1,10 @@ package org.opentripplanner.middleware.models; +import com.fasterxml.jackson.annotation.JsonIgnore; +import org.opentripplanner.middleware.persistence.Persistence; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.util.ArrayList; import java.util.List; @@ -10,6 +15,7 @@ */ public class OtpUser extends AbstractUser { private static final long serialVersionUID = 1L; + private static final Logger LOG = LoggerFactory.getLogger(OtpUser.class); /** Whether the user has consented to terms of use. */ public boolean hasConsentedToTerms; @@ -32,4 +38,34 @@ public class OtpUser extends AbstractUser { /** Whether to store the user's trip history (user must opt in). */ public boolean storeTripHistory; + + @JsonIgnore + public String applicationId; + + @Override + public boolean delete() { + // Delete trip request history (related trip summaries are deleted in TripRequest#delete) + for (TripRequest request : TripRequest.requestsForUser(this.id)) { + boolean success = request.delete(); + if (!success) { + LOG.error("Error deleting user's ({}) trip request {}", this.id, request.id); + return false; + } + } + // Delete monitored trips. + for (MonitoredTrip trip : MonitoredTrip.tripsForUser(this.id)) { + boolean success = trip.delete(); + if (!success) { + LOG.error("Error deleting user's ({}) monitored trip {}", this.id, trip.id); + return false; + } + } + boolean auth0UserDeleted = super.delete(); + if (auth0UserDeleted) { + return Persistence.otpUsers.removeById(this.id); + } else { + LOG.warn("Aborting user deletion for {}", this.email); + return false; + } + } } diff --git a/src/main/java/org/opentripplanner/middleware/models/TripRequest.java b/src/main/java/org/opentripplanner/middleware/models/TripRequest.java index bb6dcafee..d634518b7 100644 --- a/src/main/java/org/opentripplanner/middleware/models/TripRequest.java +++ b/src/main/java/org/opentripplanner/middleware/models/TripRequest.java @@ -1,11 +1,23 @@ package org.opentripplanner.middleware.models; +import org.opentripplanner.middleware.persistence.Persistence; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.Date; +import java.util.List; + +import static com.mongodb.client.model.Filters.eq; +import static org.opentripplanner.middleware.persistence.TypedPersistence.filterByUserAndDateRange; +import static org.opentripplanner.middleware.persistence.TypedPersistence.filterByUserId; + /** * A trip request represents an OTP UI trip request (initiated by a user) destined for an OpenTripPlanner instance. * otp-middleware stores these trip requests for reporting purposes. */ public class TripRequest extends Model { private static final long serialVersionUID = 1L; + private static final Logger LOG = LoggerFactory.getLogger(TripRequest.class); /** * User Id. {@link OtpUser#id} of user making trip request. @@ -33,7 +45,9 @@ public class TripRequest extends Model { //TODO: This could be the request parameters returned as part of the plan response. Would be POJO based instead of just text. public String queryParams; - /** This no-arg constructor exists to make MongoDB happy. */ + /** + * This no-arg constructor exists to make MongoDB happy. + */ public TripRequest() { } @@ -58,4 +72,21 @@ public String toString() { ", dateCreated=" + dateCreated + '}'; } + + public static List requestsForUser(String userId) { + return Persistence.tripRequests.getFiltered(filterByUserId(userId)); + } + + public static List requestsForUser(String userId, Date fromDate, Date toDate, int limit) { + return Persistence.tripRequests.getFilteredWithLimit(filterByUserAndDateRange(userId, fromDate, toDate), limit); + } + + @Override + public boolean delete() { + boolean summariesDeleted = Persistence.tripSummaries.removeFiltered(eq("tripRequestId", this.id)); + if (!summariesDeleted) { + LOG.error("Could not delete linked trip summary for request ID {}", this.id); + } + return Persistence.tripRequests.removeById(this.id); + } } diff --git a/src/main/java/org/opentripplanner/middleware/otp/OtpDispatcher.java b/src/main/java/org/opentripplanner/middleware/otp/OtpDispatcher.java index 49322683b..2755da3d1 100644 --- a/src/main/java/org/opentripplanner/middleware/otp/OtpDispatcher.java +++ b/src/main/java/org/opentripplanner/middleware/otp/OtpDispatcher.java @@ -20,7 +20,11 @@ */ public class OtpDispatcher { private static final Logger LOG = LoggerFactory.getLogger(OtpDispatcher.class); - private static String OTP_API_ROOT = getConfigPropertyAsText("OTP_API_ROOT"); + /** + * URI location of the OpenTripPlanner API (e.g., https://otp-server.com/otp). Requests sent to this URI should + * return OTP version info. + */ + public static String OTP_API_ROOT = getConfigPropertyAsText("OTP_API_ROOT"); private static final int OTP_SERVER_REQUEST_TIMEOUT_IN_SECONDS = 10; /** diff --git a/src/main/java/org/opentripplanner/middleware/persistence/TypedPersistence.java b/src/main/java/org/opentripplanner/middleware/persistence/TypedPersistence.java index 8cb5aa2f0..a026368f6 100644 --- a/src/main/java/org/opentripplanner/middleware/persistence/TypedPersistence.java +++ b/src/main/java/org/opentripplanner/middleware/persistence/TypedPersistence.java @@ -2,6 +2,7 @@ import com.mongodb.client.MongoCollection; import com.mongodb.client.MongoDatabase; +import com.mongodb.client.model.Filters; import com.mongodb.client.model.FindOneAndUpdateOptions; import com.mongodb.client.model.ReturnDocument; import com.mongodb.client.result.DeleteResult; @@ -15,33 +16,32 @@ import java.lang.reflect.Constructor; import java.util.ArrayList; import java.util.Date; +import java.util.HashSet; import java.util.List; +import java.util.Set; import static com.mongodb.client.model.Filters.eq; +import static com.mongodb.client.model.Filters.gte; +import static com.mongodb.client.model.Filters.lte; /** * This provides some abstraction over the Mongo Java driver for storing a particular kind of POJO. * - * When performing an update (in our case with findOneAndUpdate) the Document of updates - * may contain extra fields beyond those in the Java model class, or values of a type that - * do not match the Java model class. The update will nonetheless add these extra fields - * and wrong-typed values to MongoDB, which is not shocking considering its schemaless - * nature. Of course a retrieved Java object will not contain these extra values - * because it simply doesn't have a field to hold the values. If a value of the wrong - * type has been stored in the database, deserialization will just fail with - * "org.bson.codecs.configuration.CodecConfigurationException: Failed to decode X." + * When performing an update (in our case with findOneAndUpdate) the Document of updates may contain extra fields beyond + * those in the Java model class, or values of a type that do not match the Java model class. The update will + * nonetheless add these extra fields and wrong-typed values to MongoDB, which is not shocking considering its + * schemaless nature. Of course a retrieved Java object will not contain these extra values because it simply doesn't + * have a field to hold the values. If a value of the wrong type has been stored in the database, deserialization will + * just fail with "org.bson.codecs.configuration.CodecConfigurationException: Failed to decode X." * - * This means clients have the potential to stuff any amount of garbage in our MongoDB - * and trigger deserialization errors during application execution unless we perform - * type checking and clean the incoming documents. There is probably a configuration - * option to force schema adherence, which would prevent long-term compatibility but - * would give us more safety in the short term. + * This means clients have the potential to stuff any amount of garbage in our MongoDB and trigger deserialization + * errors during application execution unless we perform type checking and clean the incoming documents. There is + * probably a configuration option to force schema adherence, which would prevent long-term compatibility but would give + * us more safety in the short term. * - * PojoCodecImpl does not seem to have any hooks to throw errors when unexpected fields - * are encountered (see else clause of - * org.bson.codecs.pojo.PojoCodecImpl#decodePropertyModel). We could make our own - * function to imitate the PropertyModel checking and fail early when unexpected fields - * are present in a document. + * PojoCodecImpl does not seem to have any hooks to throw errors when unexpected fields are encountered (see else clause + * of org.bson.codecs.pojo.PojoCodecImpl#decodePropertyModel). We could make our own function to imitate the + * PropertyModel checking and fail early when unexpected fields are present in a document. */ public class TypedPersistence { @@ -107,7 +107,8 @@ public void replace(String id, T replaceObject) { } /** - * Primary method to update Mongo object with provided document. This sets the lastUpdated field to the current time. + * Primary method to update Mongo object with provided document. This sets the lastUpdated field to the current + * time. */ public T update(String id, Document updateDocument) { // Set last updated. @@ -134,53 +135,76 @@ public T getById(String id) { } /** - * This is not memory efficient. - * TODO: Always use iterators / streams, always perform selection of subsets on the Mongo server side ("where clause"). + * This is not memory efficient. TODO: Always use iterators / streams, always perform selection of subsets on the + * Mongo server side ("where clause"). */ public List getAll() { return mongoCollection.find().into(new ArrayList<>()); } /** - * Get objects satisfying the supplied Mongo filter, limited to the specified maximum. - * This ties our persistence directly to Mongo for now but is expedient. - * We should really have a bit more abstraction here. + * Get objects satisfying the supplied Mongo filter, limited to the specified maximum. This ties our persistence + * directly to Mongo for now but is expedient. We should really have a bit more abstraction here. */ public List getFilteredWithLimit(Bson filter, int maximum) { return mongoCollection.find(filter).limit(maximum).into(new ArrayList<>()); } + /** + * Build a filter for querying Mongo based on userId and from/to dates. + */ + public static Bson filterByUserAndDateRange(String userId, Date fromDate, Date toDate) { + Set clauses = new HashSet<>(); + if (userId == null) { + throw new IllegalArgumentException("userId is required to be non-null."); + } + // user id is required, so as a minimum return all entities for user. + clauses.add(eq("userId", userId)); + // Get all entities created since the supplied "from date". + if (fromDate != null) { + clauses.add(gte("dateCreated", fromDate)); + } + // Get all entities created until the supplied "to date". + if (toDate != null) { + clauses.add(lte("dateCreated", toDate)); + } + return Filters.and(clauses); + } + + /** + * Build a filter for querying Mongo based on userId. + */ + public static Bson filterByUserId(String userId) { + return filterByUserAndDateRange(userId, null, null); + } + /** * Return the number of items based on the supplied Mongo filter - * */ public long getCountFiltered(Bson filter) { return mongoCollection.countDocuments(filter); } /** - * Get all objects satisfying the supplied Mongo filter. - * This ties our persistence directly to Mongo for now but is expedient. - * We should really have a bit more abstraction here. + * Get all objects satisfying the supplied Mongo filter. This ties our persistence directly to Mongo for now but is + * expedient. We should really have a bit more abstraction here. */ public List getFiltered(Bson filter) { return mongoCollection.find(filter).into(new ArrayList<>()); } /** - * Expose the internal MongoCollection to the caller. - * This ties our persistence directly to Mongo for now but is expedient. - * We will write all the queries we need in the calling methods, then make an abstraction here on TypedPersistence - * once we see everything we need to support. + * Expose the internal MongoCollection to the caller. This ties our persistence directly to Mongo for now but is + * expedient. We will write all the queries we need in the calling methods, then make an abstraction here on + * TypedPersistence once we see everything we need to support. */ public MongoCollection getMongoCollection() { return this.mongoCollection; } /** - * Get all objects satisfying the supplied Mongo filter. - * This ties our persistence directly to Mongo for now but is expedient. - * We should really have a bit more abstraction here. + * Get all objects satisfying the supplied Mongo filter. This ties our persistence directly to Mongo for now but is + * expedient. We should really have a bit more abstraction here. */ public T getOneFiltered(Bson filter, Bson sortBy) { if (sortBy != null) { @@ -190,7 +214,9 @@ public T getOneFiltered(Bson filter, Bson sortBy) { } } - /** Convenience wrapper for #getOneFiltered that supplies null for sortBy arg. */ + /** + * Convenience wrapper for #getOneFiltered that supplies null for sortBy arg. + */ public T getOneFiltered(Bson filter) { return getOneFiltered(filter, null); } diff --git a/src/main/java/org/opentripplanner/middleware/utils/ApiGatewayUtils.java b/src/main/java/org/opentripplanner/middleware/utils/ApiGatewayUtils.java index 2552ee24c..177907d21 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/ApiGatewayUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/ApiGatewayUtils.java @@ -8,6 +8,7 @@ import com.amazonaws.services.apigateway.model.CreateApiKeyResult; import com.amazonaws.services.apigateway.model.CreateUsagePlanKeyRequest; import com.amazonaws.services.apigateway.model.DeleteApiKeyRequest; +import com.amazonaws.services.apigateway.model.GetApiKeyRequest; import com.amazonaws.services.apigateway.model.GetUsagePlanRequest; import com.amazonaws.services.apigateway.model.GetUsagePlanResult; import com.amazonaws.services.apigateway.model.GetUsagePlansRequest; @@ -16,6 +17,7 @@ import com.amazonaws.services.apigateway.model.GetUsageResult; import com.amazonaws.services.apigateway.model.NotFoundException; import com.amazonaws.services.apigateway.model.UsagePlan; +import com.amazonaws.services.apigateway.model.transform.ApiKeyMarshaller; import org.opentripplanner.middleware.bugsnag.BugsnagReporter; import org.opentripplanner.middleware.models.ApiKey; import org.opentripplanner.middleware.models.ApiUser; @@ -58,8 +60,7 @@ private static AmazonApiGateway getAmazonApiGateway() { */ public static ApiKey createApiKey(ApiUser user, String usagePlanId) throws CreateApiKeyException { if (user == null || user.id == null || usagePlanId == null) { - LOG.error("All required input parameters must be provided."); - return null; + throw new CreateApiKeyException("All required input parameters must be provided."); } long startTime = DateTimeUtils.currentTimeMillis(); try { @@ -93,6 +94,7 @@ public static ApiKey createApiKey(ApiUser user, String usagePlanId) throws Creat .withKeyId(apiKeyResult.getId()) .withKeyType("API_KEY"); gateway.createUsagePlanKey(usagePlanKeyRequest); + LOG.info("Created new API key {}", apiKeyResult.getId()); return new ApiKey(apiKeyResult); } catch (Exception e) { CreateApiKeyException createApiKeyException = new CreateApiKeyException(user.id, usagePlanId, e); @@ -115,7 +117,7 @@ public static boolean deleteApiKey(ApiKey apiKey) { deleteApiKeyRequest.setSdkRequestTimeout(SDK_REQUEST_TIMEOUT); deleteApiKeyRequest.setApiKey(apiKey.keyId); gateway.deleteApiKey(deleteApiKeyRequest); - LOG.debug("Deleting Api keys took {} msec", DateTimeUtils.currentTimeMillis() - startTime); + LOG.info("Deleting Api key {} took {} msec", apiKey.keyId, DateTimeUtils.currentTimeMillis() - startTime); } catch (NotFoundException e) { LOG.warn("Api key ({}) not found, unable to delete", apiKey.keyId, e); } catch (Exception e) { diff --git a/src/main/java/org/opentripplanner/middleware/utils/CreateApiKeyException.java b/src/main/java/org/opentripplanner/middleware/utils/CreateApiKeyException.java index 90ff1359d..4de3316f4 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/CreateApiKeyException.java +++ b/src/main/java/org/opentripplanner/middleware/utils/CreateApiKeyException.java @@ -17,4 +17,8 @@ public CreateApiKeyException(String userId, String usagePlanId, Exception cause) cause ); } + + public CreateApiKeyException(String message) { + super(message); + } } diff --git a/src/main/java/org/opentripplanner/middleware/utils/HttpUtils.java b/src/main/java/org/opentripplanner/middleware/utils/HttpUtils.java index 589fc44de..edf7ac333 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/HttpUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/HttpUtils.java @@ -54,7 +54,7 @@ public static URI buildUri(String uri, String endpoint, String queryParams) { * Makes an http get/post request and returns the response body. The request is based on the provided params. */ public static String httpRequest(URI uri, int connectionTimeout, REQUEST_METHOD method, - HashMap headers, String bodyContent) { + Map headers, String bodyContent) { return httpRequestRawResponse(uri, connectionTimeout, method, headers, bodyContent).body(); } @@ -63,7 +63,7 @@ public static String httpRequest(URI uri, int connectionTimeout, REQUEST_METHOD * Makes an http get/post request and returns the response. The request is based on the provided params. */ public static HttpResponse httpRequestRawResponse(URI uri, int connectionTimeout, REQUEST_METHOD method, - HashMap headers, String bodyContent) { + Map headers, String bodyContent) { HttpClient client = HttpClient.newHttpClient(); diff --git a/src/main/java/org/opentripplanner/middleware/utils/JsonUtils.java b/src/main/java/org/opentripplanner/middleware/utils/JsonUtils.java index c87955f76..488ae65e7 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/JsonUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/JsonUtils.java @@ -5,13 +5,16 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; -import org.eclipse.jetty.http.HttpStatus; import org.opentripplanner.middleware.bugsnag.BugsnagReporter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import spark.HaltException; import spark.Request; + +import java.util.Arrays; import java.util.List; + +import static java.util.stream.Collectors.joining; import static spark.Spark.halt; public class JsonUtils { @@ -43,20 +46,16 @@ public static void logMessageAndHalt(Request request, int statusCode, String mes logMessageAndHalt(request, statusCode, message, null); } - /** Utility method to parse generic object from Spark request body. */ - public static T getPOJOFromRequestBody(Request req, Class clazz) { - try { - // This Gson call throws an error processing OTP itineraries sent from saving a trip. - // gson.fromJson(req.body(), clazz); - // Jackson seems to process those objects correctly. - return mapper.readValue(req.body(), clazz); - } catch (JsonProcessingException e) { - logMessageAndHalt(req, HttpStatus.BAD_REQUEST_400, "Error parsing JSON for " + clazz.getSimpleName(), e); - } - return null; + /** + * Utility method to parse generic object from Spark request body. + */ + public static T getPOJOFromRequestBody(Request req, Class clazz) throws JsonProcessingException { + return mapper.readValue(req.body(), clazz); } - /** Utility method to parse generic object from JSON String. */ + /** + * Utility method to parse generic object from JSON String. + */ public static T getPOJOFromJSON(String json, Class clazz) { try { return mapper.readValue(json, clazz); @@ -88,9 +87,8 @@ public static List getPOJOFromJSONAsList(String json, Class clazz) { } /** - * Wrapper around Spark halt method that formats message as JSON using {@link #formatJSON}. - * Extra logic occurs for when the status code is >= 500. A Bugsnag report is created if - * Bugsnag is configured. + * Wrapper around Spark halt method that formats message as JSON using {@link #formatJSON}. Extra logic occurs for + * when the status code is >= 500. A Bugsnag report is created if Bugsnag is configured. */ public static void logMessageAndHalt( Request request, @@ -98,9 +96,20 @@ public static void logMessageAndHalt( String message, Exception e ) throws HaltException { + int index = e == null ? 3 : 2; // Note that halting occurred, also print error stacktrace if applicable - if (e != null) e.printStackTrace(); - LOG.error("Halting {} with status code {}. Error message: {}", request.uri(), statusCode, message); + LOG.error( + "Halting {} with status code {}. Error message: {}\n halt originated at {}", + request != null ? request.uri() : "[unknown URI]", + statusCode, + message, + // Log a stack trace for the method calling logMessageAndHalt. + Arrays.stream(Thread.currentThread().getStackTrace()) + .map(StackTraceElement::toString) + .limit(8) + .collect(joining("\n")), + e + ); if (statusCode >= 500) { BugsnagReporter.reportErrorToBugsnag(message, e); @@ -120,7 +129,7 @@ public static String formatJSON(String message, int code, Exception e) { /** * Constructs a JSON string containing the provided key/value pair. */ - public static String formatJSON (String key, String value) { + public static String formatJSON(String key, String value) { return mapper.createObjectNode() .put(key, value) .toString(); @@ -138,4 +147,11 @@ public static ObjectNode getObjectNode(String message, int code, Exception e) { .put("code", code) .put("detail", detail); } + + /** + * Get a single node value from JSON if present, else return null + */ + public static String getSingleNodeValueFromJSON(String nodeName, String json) throws JsonProcessingException { + return mapper.readTree(json).get(nodeName).textValue(); + } } diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index e23bc98a2..ea05693d8 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -140,6 +140,8 @@ paths: tags: - "api/admin/user" description: "Deletes the 'AdminUser' entity with the specified id if it exists." + produces: + - "application/json" parameters: - name: "id" in: "path" @@ -150,7 +152,7 @@ paths: "200": description: "successful operation" schema: - $ref: "#/definitions/RestResponse" + $ref: "#/definitions/AdminUser" /api/secure/application/{id}/apikey: post: tags: @@ -303,6 +305,8 @@ paths: tags: - "api/secure/application" description: "Deletes the 'ApiUser' entity with the specified id if it exists." + produces: + - "application/json" parameters: - name: "id" in: "path" @@ -313,7 +317,7 @@ paths: "200": description: "successful operation" schema: - $ref: "#/definitions/RestResponse" + $ref: "#/definitions/ApiUser" /api/secure/monitoredtrip: get: tags: @@ -395,6 +399,8 @@ paths: - "api/secure/monitoredtrip" description: "Deletes the 'MonitoredTrip' entity with the specified id if it\ \ exists." + produces: + - "application/json" parameters: - name: "id" in: "path" @@ -405,7 +411,7 @@ paths: "200": description: "successful operation" schema: - $ref: "#/definitions/RestResponse" + $ref: "#/definitions/MonitoredTrip" /api/secure/triprequests: get: tags: @@ -552,6 +558,8 @@ paths: tags: - "api/secure/user" description: "Deletes the 'OtpUser' entity with the specified id if it exists." + produces: + - "application/json" parameters: - name: "id" in: "path" @@ -562,7 +570,7 @@ paths: "200": description: "successful operation" schema: - $ref: "#/definitions/RestResponse" + $ref: "#/definitions/OtpUser" /api/secure/logs: get: tags: @@ -641,14 +649,6 @@ definitions: type: "string" AdminUser[]: type: "object" - RestResponse: - type: "object" - properties: - statusCode: - type: "integer" - format: "int32" - body: - type: "string" ApiKey: type: "object" properties: @@ -1038,6 +1038,8 @@ definitions: $ref: "#/definitions/UserLocation" storeTripHistory: type: "boolean" + applicationId: + type: "string" OtpUser[]: type: "object" GetUsageResult: diff --git a/src/test/java/org/opentripplanner/middleware/ApiKeyManagementTest.java b/src/test/java/org/opentripplanner/middleware/ApiKeyManagementTest.java index 2b2d31e14..7544a0a80 100644 --- a/src/test/java/org/opentripplanner/middleware/ApiKeyManagementTest.java +++ b/src/test/java/org/opentripplanner/middleware/ApiKeyManagementTest.java @@ -4,12 +4,11 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.opentripplanner.middleware.models.AbstractUser; import org.opentripplanner.middleware.models.AdminUser; -import org.opentripplanner.middleware.models.ApiKey; import org.opentripplanner.middleware.models.ApiUser; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.persistence.PersistenceUtil; -import org.opentripplanner.middleware.utils.ApiGatewayUtils; import org.opentripplanner.middleware.utils.CreateApiKeyException; import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.JsonUtils; @@ -23,23 +22,21 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; import static org.opentripplanner.middleware.TestUtils.getBooleanEnvVar; +import static org.opentripplanner.middleware.TestUtils.isEndToEndAndAuthIsDisabled; import static org.opentripplanner.middleware.TestUtils.mockAuthenticatedRequest; -import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; +import static org.opentripplanner.middleware.controllers.api.ApiUserController.DEFAULT_USAGE_PLAN_ID; /** - * Tests for creating and deleting api keys. The following config parameters are must be set in + * Tests for creating and deleting api keys. The following config parameters must be set in * configurations/default/env.yml for these end-to-end tests to run: - * - RUN_E2E=true the end-to-end environment variable must be set (NOTE: this is not a config value) - * - An AWS_PROFILE is required, or AWS access has been configured for your operating environment e.g. - * C:\Users\\.aws\credentials in Windows or Mac OS equivalent. - * - DISABLE_AUTH set to true to bypass auth checks and use users defined here. DEFAULT_USAGE_PLAN_ID set to a valid usage - * plan id. AWS requires this to create an api key. - * - TODO: It might be useful to allow this to run without DISABLE_AUTH set to true (in an end-to-end environment using - * real tokens from Auth0. + * - RUN_E2E=true the end-to-end environment variable must be set (NOTE: this is not a config value) + * - An AWS_PROFILE is required, or AWS access has been configured for your operating environment. E.g., + * C:\Users\\.aws\credentials in Windows or Mac OS equivalent. + * - DISABLE_AUTH set to true to bypass auth checks and use users defined here. + * - DEFAULT_USAGE_PLAN_ID set to a valid usage plan id. AWS requires this to create an api key. */ public class ApiKeyManagementTest extends OtpMiddlewareTest { private static final Logger LOG = LoggerFactory.getLogger(ApiKeyManagementTest.class); - private static String DEFAULT_USAGE_PLAN_ID; private static ApiUser apiUser; private static AdminUser adminUser; @@ -48,8 +45,11 @@ public class ApiKeyManagementTest extends OtpMiddlewareTest { */ @BeforeAll public static void setUp() throws IOException, InterruptedException { + // Load config before checking if tests should run (otherwise authDisabled will always evaluate to false). OtpMiddlewareTest.setUp(); - DEFAULT_USAGE_PLAN_ID = getConfigPropertyAsText("DEFAULT_USAGE_PLAN_ID"); + // TODO: It might be useful to allow this to run without DISABLE_AUTH set to true (in an end-to-end environment + // using real tokens from Auth0. + assumeTrue(isEndToEndAndAuthIsDisabled()); apiUser = PersistenceUtil.createApiUser("test@example.com"); adminUser = PersistenceUtil.createAdminUser("test@example.com"); } @@ -59,15 +59,12 @@ public static void setUp() throws IOException, InterruptedException { */ @AfterAll public static void tearDown() { + assumeTrue(isEndToEndAndAuthIsDisabled()); // Delete admin user. Persistence.adminUsers.removeById(adminUser.id); - // Delete api keys for user. + // Refresh api keys for user. apiUser = Persistence.apiUsers.getById(apiUser.id); - for (ApiKey apiKey : apiUser.apiKeys) { - ApiGatewayUtils.deleteApiKey(apiKey); - } - // Delete api user. TODO: combine delete user/api keys into single method? - Persistence.apiUsers.removeById(apiUser.id); + apiUser.delete(); } /** @@ -76,7 +73,7 @@ public static void tearDown() { @Test public void canCreateApiKeyForSelf() { assumeTrue(getBooleanEnvVar("RUN_E2E")); - HttpResponse response = createApiKeyRequest(apiUser.id, apiUser.auth0UserId); + HttpResponse response = createApiKeyRequest(apiUser.id, apiUser); assertEquals(HttpStatus.OK_200, response.statusCode()); ApiUser userFromResponse = JsonUtils.getPOJOFromJSON(response.body(), ApiUser.class); // refresh API key @@ -91,7 +88,7 @@ public void canCreateApiKeyForSelf() { @Test public void adminCanCreateApiKeyForApiUser() { assumeTrue(getBooleanEnvVar("RUN_E2E")); - HttpResponse response = createApiKeyRequest(apiUser.id, adminUser.auth0UserId); + HttpResponse response = createApiKeyRequest(apiUser.id, adminUser); assertEquals(HttpStatus.OK_200, response.statusCode()); ApiUser userFromResponse = JsonUtils.getPOJOFromJSON(response.body(), ApiUser.class); // refresh API key @@ -111,7 +108,7 @@ public void cannotDeleteApiKeyForSelf() { int initialKeyCount = apiUser.apiKeys.size(); // delete key String keyId = apiUser.apiKeys.get(0).keyId; - HttpResponse response = deleteApiKeyRequest(apiUser.id, keyId, apiUser.auth0UserId); + HttpResponse response = deleteApiKeyRequest(apiUser.id, keyId, apiUser); int status = response.statusCode(); assertEquals(HttpStatus.FORBIDDEN_403, status); LOG.info("Delete key request status: {}", status); @@ -129,7 +126,7 @@ public void adminCanDeleteApiKeyForApiUser() { ensureApiKeyExists(); // delete key String keyId = apiUser.apiKeys.get(0).keyId; - HttpResponse response = deleteApiKeyRequest(apiUser.id, keyId, adminUser.auth0UserId); + HttpResponse response = deleteApiKeyRequest(apiUser.id, keyId, adminUser); assertEquals(HttpStatus.OK_200, response.statusCode()); ApiUser userFromResponse = JsonUtils.getPOJOFromJSON(response.body(), ApiUser.class); assertTrue(userFromResponse.apiKeys.isEmpty()); @@ -148,10 +145,7 @@ private boolean ensureApiKeyExists() { if (apiUser.apiKeys.isEmpty()) { // Create key if there are none. try { - ApiKey apiKey = ApiGatewayUtils.createApiKey(apiUser, DEFAULT_USAGE_PLAN_ID); - apiUser.apiKeys.add(apiKey); - // Save update so the API key delete endpoint is aware of the new API key. - Persistence.apiUsers.replace(apiUser.id, apiUser); + apiUser.createApiKey(DEFAULT_USAGE_PLAN_ID, true); LOG.info("Successfully created API key"); return true; } catch (CreateApiKeyException e) { @@ -165,16 +159,16 @@ private boolean ensureApiKeyExists() { /** * Create API key for target user based on authorization of requesting user */ - private HttpResponse createApiKeyRequest(String targetUserId, String requestingAuth0UserId) { + private HttpResponse createApiKeyRequest(String targetUserId, AbstractUser requestingUser) { String path = String.format("api/secure/application/%s/apikey", targetUserId); - return mockAuthenticatedRequest(path, HttpUtils.REQUEST_METHOD.POST, requestingAuth0UserId); + return mockAuthenticatedRequest(path, requestingUser, HttpUtils.REQUEST_METHOD.POST); } /** * Delete API key for target user based on authorization of requesting user */ - private HttpResponse deleteApiKeyRequest(String targetUserId, String apiKeyId, String requestingAuth0UserId) { + private HttpResponse deleteApiKeyRequest(String targetUserId, String apiKeyId, AbstractUser requestingUser) { String path = String.format("api/secure/application/%s/apikey/%s", targetUserId, apiKeyId); - return mockAuthenticatedRequest(path, HttpUtils.REQUEST_METHOD.DELETE, requestingAuth0UserId); + return mockAuthenticatedRequest(path, requestingUser, HttpUtils.REQUEST_METHOD.DELETE); } } diff --git a/src/test/java/org/opentripplanner/middleware/ApiUserFlowTest.java b/src/test/java/org/opentripplanner/middleware/ApiUserFlowTest.java new file mode 100644 index 000000000..7abdd2d36 --- /dev/null +++ b/src/test/java/org/opentripplanner/middleware/ApiUserFlowTest.java @@ -0,0 +1,178 @@ +package org.opentripplanner.middleware; + +import com.auth0.exception.Auth0Exception; +import com.auth0.json.mgmt.users.User; +import org.eclipse.jetty.http.HttpStatus; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.opentripplanner.middleware.models.AdminUser; +import org.opentripplanner.middleware.models.ApiUser; +import org.opentripplanner.middleware.models.MonitoredTrip; +import org.opentripplanner.middleware.models.OtpUser; +import org.opentripplanner.middleware.models.TripRequest; +import org.opentripplanner.middleware.persistence.Persistence; +import org.opentripplanner.middleware.persistence.PersistenceUtil; +import org.opentripplanner.middleware.utils.CreateApiKeyException; +import org.opentripplanner.middleware.utils.HttpUtils; +import org.opentripplanner.middleware.utils.JsonUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.net.http.HttpResponse; +import java.util.List; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assumptions.assumeTrue; +import static org.opentripplanner.middleware.TestUtils.getBooleanEnvVar; +import static org.opentripplanner.middleware.TestUtils.isEndToEndAndAuthIsDisabled; +import static org.opentripplanner.middleware.TestUtils.mockAuthenticatedPost; +import static org.opentripplanner.middleware.TestUtils.mockAuthenticatedRequest; +import static org.opentripplanner.middleware.auth.Auth0Users.createAuth0UserForEmail; +import static org.opentripplanner.middleware.controllers.api.ApiUserController.DEFAULT_USAGE_PLAN_ID; +import static org.opentripplanner.middleware.controllers.api.OtpRequestProcessor.OTP_PLAN_ENDPOINT; +import static org.opentripplanner.middleware.controllers.api.OtpRequestProcessor.OTP_PROXY_ENDPOINT; + +/** + * Tests to simulate API user flow. The following config parameters must be set in configurations/default/env.yml for + * these end-to-end tests to run: - AUTH0_DOMAIN set to a valid Auth0 domain - AUTH0_API_CLIENT set to a valid Auth0 + * application client id - AUTH0_API_SECRET set to a valid Auth0 application client secret - DEFAULT_USAGE_PLAN_ID set + * to a valid usage plan id (AWS requires this to create an api key) - OTP_API_ROOT set to http://localhost:8080/otp so + * the mock OTP server is used - OTP_PLAN_ENDPOINT set to /plan - An AWS_PROFILE is required, or AWS access has been + * configured for your operating environment e.g. C:\Users\\.aws\credentials in Windows or Mac OS equivalent. + * + * The following environment variable must be set for these tests to run: - RUN_E2E=true + * + * Auth0 must be correctly configured as described here: https://auth0.com/docs/flows/call-your-api-using-resource-owner-password-flow + */ +public class ApiUserFlowTest { + private static final Logger LOG = LoggerFactory.getLogger(ApiUserFlowTest.class); + private static ApiUser apiUser; + private static OtpUser otpUser; + + /** + * Create an {@link ApiUser} and an {@link AdminUser} prior to unit tests + */ + @BeforeAll + public static void setUp() throws IOException, InterruptedException, CreateApiKeyException { + // Load config before checking if tests should run (otherwise authDisabled will always evaluate to false). + OtpMiddlewareTest.setUp(); + assumeTrue(isEndToEndAndAuthIsDisabled()); + // Mock the OTP server TODO: Run a live OTP instance? + TestUtils.mockOtpServer(); + // As a pre-condition, create an API User with API key. + apiUser = PersistenceUtil.createApiUser(String.format("test-%s@example.com", UUID.randomUUID().toString())); + apiUser.createApiKey(DEFAULT_USAGE_PLAN_ID, true); + // Create, but do not persist, an OTP user. Also + otpUser = new OtpUser(); + otpUser.email = String.format("test-%s@example.com", UUID.randomUUID().toString()); + otpUser.hasConsentedToTerms = true; + otpUser.storeTripHistory = true; + // create Auth0 users for apiUser and optUser. + try { + User auth0User = createAuth0UserForEmail(apiUser.email, TestUtils.TEMP_AUTH0_USER_PASSWORD); + // update api user with valid auth0 user ID (so the Auth0 delete works) + apiUser.auth0UserId = auth0User.getId(); + Persistence.apiUsers.replace(apiUser.id, apiUser); + + auth0User = createAuth0UserForEmail(otpUser.email, TestUtils.TEMP_AUTH0_USER_PASSWORD); + // update otp user with valid auth0 user ID (so the Auth0 delete works) + otpUser.auth0UserId = auth0User.getId(); + } catch (Auth0Exception e) { + throw new RuntimeException(e); + } + } + + /** + * Delete the users if they were not already deleted during the test script. + */ + @AfterAll + public static void tearDown() { + assumeTrue(isEndToEndAndAuthIsDisabled()); + apiUser = Persistence.apiUsers.getById(apiUser.id); + if (apiUser != null) apiUser.delete(); + otpUser = Persistence.otpUsers.getById(otpUser.id); + if (otpUser != null) otpUser.delete(); + } + + /** + * Tests to confirm that an otp user, related monitored trip and plan can be created and deleted leaving no orphaned + * records. This also includes Auth0 users if auth is enabled. + */ + @Test + public void canSimulateApiUserFlow() { + assumeTrue(getBooleanEnvVar("RUN_E2E")); + + // create otp user as api user + HttpResponse createUserResponse = mockAuthenticatedPost("api/secure/user", + apiUser, + JsonUtils.toJson(otpUser) + ); + assertEquals(HttpStatus.OK_200, createUserResponse.statusCode()); + OtpUser otpUserResponse = JsonUtils.getPOJOFromJSON(createUserResponse.body(), OtpUser.class); + + // Create a monitored trip for the Otp user (API users are prevented from doing this). + // TODO use utils from trip monitoring PR. + MonitoredTrip trip = new MonitoredTrip(); + trip.monday = true; + HttpResponse createTripResponse = mockAuthenticatedPost("api/secure/monitoredtrip", + otpUserResponse, + JsonUtils.toJson(trip) + ); + assertEquals(HttpStatus.OK_200, createTripResponse.statusCode()); + MonitoredTrip monitoredTripResponse = JsonUtils.getPOJOFromJSON(createTripResponse.body(), MonitoredTrip.class); + + // Plan trip with OTP proxy. Mock plan response will be returned + String otpQuery = OTP_PROXY_ENDPOINT + OTP_PLAN_ENDPOINT + "?fromPlace=34,-87&toPlace=33.-87"; + HttpResponse planTripResponse = mockAuthenticatedRequest(otpQuery, + otpUserResponse, + HttpUtils.REQUEST_METHOD.GET + ); + LOG.info("Plan trip response: {}\n....", planTripResponse.body().substring(0, 300)); + assertEquals(HttpStatus.OK_200, planTripResponse.statusCode()); + + // Get trip for user, before it is deleted + HttpResponse tripRequestResponse = mockAuthenticatedRequest(String.format("api/secure/triprequests?userId=%s", + otpUserResponse.id), + otpUserResponse, + HttpUtils.REQUEST_METHOD.GET + ); + assertEquals(HttpStatus.OK_200, tripRequestResponse.statusCode()); + List tripRequests = JsonUtils.getPOJOFromJSONAsList(tripRequestResponse.body(), TripRequest.class); + + // Delete otp user. + HttpResponse deleteUserResponse = mockAuthenticatedRequest( + String.format("api/secure/user/%s", otpUserResponse.id), + otpUserResponse, + HttpUtils.REQUEST_METHOD.DELETE + ); + assertEquals(HttpStatus.OK_200, deleteUserResponse.statusCode()); + + // Verify user no longer exists. + OtpUser deletedOtpUser = Persistence.otpUsers.getById(otpUserResponse.id); + assertNull(deletedOtpUser); + + // Verify monitored trip no longer exists. + MonitoredTrip deletedTrip = Persistence.monitoredTrips.getById(monitoredTripResponse.id); + assertNull(deletedTrip); + + // Verify trip request no longer exists. + TripRequest tripRequest = Persistence.tripRequests.getById(tripRequests.get(0).id); + assertNull(tripRequest); + + // Delete API user (this would happen through the OTP Admin portal). + HttpResponse deleteApiUserResponse = mockAuthenticatedRequest( + String.format("api/secure/application/%s", apiUser.id), + apiUser, + HttpUtils.REQUEST_METHOD.DELETE + ); + assertEquals(HttpStatus.OK_200, deleteApiUserResponse.statusCode()); + + // Verify that API user is deleted. + ApiUser deletedApiUser = Persistence.apiUsers.getById(apiUser.id); + assertNull(deletedApiUser); + } +} diff --git a/src/test/java/org/opentripplanner/middleware/OtpMiddlewareTest.java b/src/test/java/org/opentripplanner/middleware/OtpMiddlewareTest.java index 0a28dae39..954f86643 100644 --- a/src/test/java/org/opentripplanner/middleware/OtpMiddlewareTest.java +++ b/src/test/java/org/opentripplanner/middleware/OtpMiddlewareTest.java @@ -10,8 +10,8 @@ import static org.opentripplanner.middleware.TestUtils.getBooleanEnvVar; /** - * This abstract class is used to start a test instance of otp-middleware that other tests can use to perform - * various tests. + * This abstract class is used to start a test instance of otp-middleware that other tests can use to perform various + * tests. */ public abstract class OtpMiddlewareTest { private static final Logger LOG = LoggerFactory.getLogger(OtpMiddlewareTest.class); @@ -35,8 +35,8 @@ public static void setUp() throws RuntimeException, IOException, InterruptedExce // If in the e2e environment, use the secret env.yml file to start the server. // TODO: When ran on Travis CI, this file will automatically be setup. String[] args = getBooleanEnvVar("RUN_E2E") - ? new String[] { "configurations/default/env.yml" } - : new String[] { "configurations/test/env.yml"}; + ? new String[]{"configurations/default/env.yml"} + : new String[]{"configurations/test/env.yml"}; // Fail this test and others if the above files do not exist. for (String arg : args) { File f = new File(arg); diff --git a/src/test/java/org/opentripplanner/middleware/TestUtils.java b/src/test/java/org/opentripplanner/middleware/TestUtils.java index 5b8c0aecc..9f6c30d04 100644 --- a/src/test/java/org/opentripplanner/middleware/TestUtils.java +++ b/src/test/java/org/opentripplanner/middleware/TestUtils.java @@ -1,13 +1,44 @@ package org.opentripplanner.middleware; +import com.fasterxml.jackson.core.JsonProcessingException; +import org.eclipse.jetty.http.HttpStatus; import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.models.AbstractUser; +import org.opentripplanner.middleware.models.ApiUser; +import org.opentripplanner.middleware.otp.OtpDispatcherResponse; +import org.opentripplanner.middleware.utils.FileUtils; import org.opentripplanner.middleware.utils.HttpUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import spark.Request; +import spark.Response; +import spark.Service; import java.net.URI; import java.net.http.HttpResponse; import java.util.HashMap; +import java.util.UUID; + +import static javax.ws.rs.core.MediaType.APPLICATION_JSON; +import static org.opentripplanner.middleware.auth.Auth0Connection.authDisabled; +import static org.opentripplanner.middleware.auth.Auth0Users.getAuth0Token; +import static org.opentripplanner.middleware.controllers.api.OtpRequestProcessor.OTP_PLAN_ENDPOINT; +import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; +import static spark.Service.ignite; + public class TestUtils { + private static final Logger LOG = LoggerFactory.getLogger(TestUtils.class); + + /** + * Prevents the mock OTP server being initialized more than once + */ + private static boolean mockOtpServerSetUpIsDone = false; + + /** + * Password used to create and validate temporary Auth0 users + */ + static final String TEMP_AUTH0_USER_PASSWORD = UUID.randomUUID().toString(); /** * Returns true only if an environment variable exists and is set to "true". @@ -17,13 +48,26 @@ public static boolean getBooleanEnvVar(String var) { return variable != null && variable.equals("true"); } + /** + * Helper method to determine if end to end is enabled and auth is disabled. (Used for checking if tests should run.) + */ + public static boolean isEndToEndAndAuthIsDisabled() { + return getBooleanEnvVar("RUN_E2E") && authDisabled(); + } + /** * Send request to provided URL placing the Auth0 user id in the headers so that {@link Auth0UserProfile} can check * the database for a matching user. Returns the response. */ - public static HttpResponse mockAuthenticatedRequest(String path, HttpUtils.REQUEST_METHOD requestMethod, String auth0UserId) { - HashMap headers = new HashMap<>(); - headers.put("Authorization", auth0UserId); + public static HttpResponse mockAuthenticatedRequest(String path, AbstractUser requestingUser, HttpUtils.REQUEST_METHOD requestMethod) { + HashMap headers = getMockHeaders(requestingUser); + // If requester is an API user, add API key value as x-api-key header to simulate request over API Gateway. + if (requestingUser instanceof ApiUser) { + ApiUser apiUser = (ApiUser) requestingUser; + if (!apiUser.apiKeys.isEmpty()) { + headers.put("x-api-key", apiUser.apiKeys.get(0).value); + } + } return HttpUtils.httpRequestRawResponse( URI.create("http://localhost:4567/" + path), @@ -34,4 +78,75 @@ public static HttpResponse mockAuthenticatedRequest(String path, HttpUti ); } + /** + * Construct http header values based on user type and status of DISABLE_AUTH config parameter. If authorization is + * disabled, use Auth0 user ID to authenticate else attempt to get a valid 0auth token from Auth0 and use this. + */ + private static HashMap getMockHeaders(AbstractUser requestingUser) { + HashMap headers = new HashMap<>(); + // If auth is disabled, simply place the Auth0 user ID in the authorization header, which will be extracted from + // the request when received. + if ("true".equals(getConfigPropertyAsText("DISABLE_AUTH"))) { + headers.put("Authorization", requestingUser.auth0UserId); + } else { + // Otherwise, get a valid oauth token for the user + String token = null; + try { + token = getAuth0Token(requestingUser.email, TEMP_AUTH0_USER_PASSWORD); + } catch (JsonProcessingException e) { + LOG.error("Cannot obtain Auth0 token for user {}", requestingUser.email, e); + } + headers.put("Authorization", "Bearer " + token); + } + // If requester is an API user, add API key value as x-api-key header to simulate request over API Gateway. + if (requestingUser instanceof ApiUser) { + ApiUser apiUser = (ApiUser) requestingUser; + if (!apiUser.apiKeys.isEmpty()) { + headers.put("x-api-key", apiUser.apiKeys.get(0).value); + } + } + return headers; + } + + /** + * Send request to provided URL placing the Auth0 user id in the headers so that {@link Auth0UserProfile} can check + * the database for a matching user. Returns the response. + */ + public static HttpResponse mockAuthenticatedPost(String path, AbstractUser requestingUser, String body) { + HashMap headers = getMockHeaders(requestingUser); + + return HttpUtils.httpRequestRawResponse( + URI.create("http://localhost:4567/" + path), + 1000, + HttpUtils.REQUEST_METHOD.POST, + headers, + body + ); + } + + /** + * Configure a mock OTP server for providing mock OTP responses. + * Note: this expects the config value OTP_API_ROOT=http://localhost:8080/otp + */ + static void mockOtpServer() { + if (mockOtpServerSetUpIsDone) { + return; + } + Service http = ignite().port(8080); + http.get("/otp" + OTP_PLAN_ENDPOINT, TestUtils::mockOtpPlanResponse); + } + + /** + * Mock an OTP server plan response by provide a static response from file. + */ + private static String mockOtpPlanResponse(Request request, Response response) { + final String filePath = "src/test/resources/org/opentripplanner/middleware/"; + OtpDispatcherResponse otpDispatcherResponse = new OtpDispatcherResponse(); + otpDispatcherResponse.statusCode = HttpStatus.OK_200; + otpDispatcherResponse.responseBody = FileUtils.getFileContents(filePath + "planResponse.json"); + + response.type(APPLICATION_JSON); + response.status(otpDispatcherResponse.statusCode); + return otpDispatcherResponse.responseBody; + } } diff --git a/src/test/java/org/opentripplanner/middleware/auth/Auth0UsersTest.java b/src/test/java/org/opentripplanner/middleware/auth/Auth0UsersTest.java index 89c532780..eb37e2798 100644 --- a/src/test/java/org/opentripplanner/middleware/auth/Auth0UsersTest.java +++ b/src/test/java/org/opentripplanner/middleware/auth/Auth0UsersTest.java @@ -3,6 +3,7 @@ import com.auth0.json.auth.TokenHolder; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.opentripplanner.middleware.OtpMiddlewareTest; @@ -31,6 +32,13 @@ public static void setUp() throws IOException, InterruptedException { Auth0Users.setCachedToken(new TokenCache(tokenHolder)); } + @AfterAll + public static void tearDown() throws IOException { + LOG.info("Clearing Auth0UsersTest fake token"); + // Set new cached token. + Auth0Users.setCachedToken(null); + } + /** * Checks that token is valid soon after creation and seconds until expiration does diminish. */ diff --git a/src/test/java/org/opentripplanner/middleware/docs/SwaggerTest.java b/src/test/java/org/opentripplanner/middleware/docs/SwaggerTest.java index 30f57f551..461d7f3bb 100644 --- a/src/test/java/org/opentripplanner/middleware/docs/SwaggerTest.java +++ b/src/test/java/org/opentripplanner/middleware/docs/SwaggerTest.java @@ -41,7 +41,7 @@ public void swaggerDocsAreUpToDate() throws IOException, InterruptedException { Path versionControlledSwaggerFile = new File(LATEST_SWAGGER_FILE).toPath(); String versionControlledSwagger = Files.readString(versionControlledSwaggerFile); - assertEquals(autoGeneratedSwagger, versionControlledSwagger, + assertEquals(versionControlledSwagger, autoGeneratedSwagger, String.format( "If you modified any API endpoint or configuration, please also run VersionControlledSwaggerUpdater#main and commit %s.", LATEST_SWAGGER_FILE diff --git a/src/test/java/org/opentripplanner/middleware/persistence/PersistenceTest.java b/src/test/java/org/opentripplanner/middleware/persistence/PersistenceTest.java index f38dcb7f1..4d0f70f39 100644 --- a/src/test/java/org/opentripplanner/middleware/persistence/PersistenceTest.java +++ b/src/test/java/org/opentripplanner/middleware/persistence/PersistenceTest.java @@ -4,6 +4,8 @@ import org.junit.jupiter.api.Test; import org.opentripplanner.middleware.OtpMiddlewareTest; import org.opentripplanner.middleware.models.OtpUser; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; @@ -16,6 +18,7 @@ */ public class PersistenceTest extends OtpMiddlewareTest { private static final String TEST_EMAIL = "john.doe@example.com"; + private static final Logger LOG = LoggerFactory.getLogger(PersistenceTest.class); OtpUser user = null; @@ -49,6 +52,7 @@ public void canDeleteUser() { @AfterEach public void remove() { if (user != null) { + LOG.info("Deleting user {}", user.id); Persistence.otpUsers.removeById(user.id); } } diff --git a/src/test/java/org/opentripplanner/middleware/persistence/PersistenceUtil.java b/src/test/java/org/opentripplanner/middleware/persistence/PersistenceUtil.java index d8caa3888..ce1965c79 100644 --- a/src/test/java/org/opentripplanner/middleware/persistence/PersistenceUtil.java +++ b/src/test/java/org/opentripplanner/middleware/persistence/PersistenceUtil.java @@ -1,5 +1,7 @@ package org.opentripplanner.middleware.persistence; +import com.auth0.exception.Auth0Exception; +import com.auth0.json.mgmt.users.User; import org.opentripplanner.middleware.models.AdminUser; import org.opentripplanner.middleware.models.ApiUser; import org.opentripplanner.middleware.models.MonitoredTrip; @@ -16,6 +18,9 @@ import java.util.Date; import java.util.List; +import static org.opentripplanner.middleware.auth.Auth0Users.createAuth0UserForEmail; +import static org.opentripplanner.middleware.auth.Auth0Users.createNewAuth0User; + /** * Utility class to aid with creating and storing objects in Mongo. */ diff --git a/src/test/java/org/opentripplanner/middleware/persistence/TripHistoryPersistenceTest.java b/src/test/java/org/opentripplanner/middleware/persistence/TripHistoryPersistenceTest.java index 7516a6790..b6a41562c 100644 --- a/src/test/java/org/opentripplanner/middleware/persistence/TripHistoryPersistenceTest.java +++ b/src/test/java/org/opentripplanner/middleware/persistence/TripHistoryPersistenceTest.java @@ -31,6 +31,7 @@ public class TripHistoryPersistenceTest extends OtpMiddlewareTest { private static final String TEST_EMAIL = "john.doe@example.com"; + OtpUser user = null; TripRequest tripRequest = null; TripSummary tripSummary = null; List tripRequests = null; @@ -86,7 +87,7 @@ public void canGetFilteredTripRequestsWithFromAndToDate() { String TRIP_REQUEST_DATE_CREATED_FIELD_NAME = "dateCreated"; String TRIP_REQUEST_USER_ID_FIELD_NAME = "userId"; - OtpUser user = createUser(TEST_EMAIL); + user = createUser(TEST_EMAIL); List tripRequests = createTripRequests(limit, user.id); @@ -114,7 +115,7 @@ public void canGetFilteredTripRequestsFromDate() { String TRIP_REQUEST_DATE_CREATED_FIELD_NAME = "dateCreated"; String TRIP_REQUEST_USER_ID_FIELD_NAME = "userId"; - OtpUser user = createUser(TEST_EMAIL); + user = createUser(TEST_EMAIL); List tripRequests = createTripRequests(limit, user.id); @@ -137,7 +138,7 @@ public void canGetFilteredTripRequestsToDate() { String TRIP_REQUEST_DATE_CREATED_FIELD_NAME = "dateCreated"; String TRIP_REQUEST_USER_ID_FIELD_NAME = "userId"; - OtpUser user = createUser(TEST_EMAIL); + user = createUser(TEST_EMAIL); tripRequests = createTripRequests(limit, user.id); @@ -159,7 +160,7 @@ public void canGetFilteredTripRequestsForUser() { int limit = 3; String TRIP_REQUEST_USER_ID_FIELD_NAME = "userId"; - OtpUser user = createUser(TEST_EMAIL); + user = createUser(TEST_EMAIL); tripRequests = createTripRequests(limit, user.id); @@ -175,7 +176,7 @@ public void canGetFilteredTripRequestsForUserWithMaxLimit() { int max = 5; String TRIP_REQUEST_USER_ID_FIELD_NAME = "userId"; - OtpUser user = createUser(TEST_EMAIL); + user = createUser(TEST_EMAIL); tripRequests = createTripRequests(limit, user.id); @@ -187,16 +188,9 @@ public void canGetFilteredTripRequestsForUserWithMaxLimit() { @AfterEach public void remove() { - if (tripRequest != null) { - Persistence.tripRequests.removeById(tripRequest.id); - } - - if (tripSummary != null) { - Persistence.tripSummaries.removeById(tripSummary.id); - } - - if (tripRequests != null) { - deleteTripRequests(tripRequests); - } + if (user != null) Persistence.otpUsers.removeById(user.id); + if (tripRequest != null) Persistence.tripRequests.removeById(tripRequest.id); + if (tripSummary != null) Persistence.tripSummaries.removeById(tripSummary.id); + if (tripRequests != null) deleteTripRequests(tripRequests); } }