diff --git a/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java b/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java index 7c19889e6..2640e25d2 100644 --- a/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java +++ b/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java @@ -16,7 +16,6 @@ import org.opentripplanner.middleware.docs.PublicApiDocGenerator; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.tripMonitor.jobs.MonitorAllTripsJob; -import org.opentripplanner.middleware.utils.ConfigUtils; import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.Scheduler; import org.slf4j.Logger; @@ -30,6 +29,9 @@ import java.util.concurrent.TimeUnit; import static javax.ws.rs.core.MediaType.APPLICATION_JSON; +import static org.opentripplanner.middleware.controllers.api.ApiUserController.API_USER_PATH; +import static org.opentripplanner.middleware.controllers.api.ApiUserController.AUTHENTICATE_PATH; +import static org.opentripplanner.middleware.utils.ConfigUtils.loadConfig; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; /** @@ -43,7 +45,7 @@ public class OtpMiddlewareMain { public static void main(String[] args) throws IOException, InterruptedException { // Load configuration. - ConfigUtils.loadConfig(args); + loadConfig(args); // Connect to MongoDB. Persistence.initialize(); @@ -131,9 +133,12 @@ private static void initializeHttpEndpoints() throws IOException, InterruptedExc return "OK"; }); - // Security checks for admin and /secure/ endpoints. + // Security checks for admin and /secure/ endpoints. Excluding /authenticate so that API users can obtain a + // bearer token to authenticate against all other /secure/ endpoints. spark.before(API_PREFIX + "/secure/*", ((request, response) -> { - if (!request.requestMethod().equals("OPTIONS")) Auth0Connection.checkUser(request); + if (!request.requestMethod().equals("OPTIONS") && !request.pathInfo().endsWith(API_USER_PATH + AUTHENTICATE_PATH)) { + Auth0Connection.checkUser(request); + } })); spark.before(API_PREFIX + "admin/*", ((request, response) -> { if (!request.requestMethod().equals("OPTIONS")) { diff --git a/src/main/java/org/opentripplanner/middleware/auth/Auth0Connection.java b/src/main/java/org/opentripplanner/middleware/auth/Auth0Connection.java index 71b1c9eba..a46d56f6a 100644 --- a/src/main/java/org/opentripplanner/middleware/auth/Auth0Connection.java +++ b/src/main/java/org/opentripplanner/middleware/auth/Auth0Connection.java @@ -11,9 +11,13 @@ import com.auth0.jwt.interfaces.DecodedJWT; import com.fasterxml.jackson.core.JsonProcessingException; import org.eclipse.jetty.http.HttpStatus; +import org.opentripplanner.middleware.controllers.api.ApiUserController; +import org.opentripplanner.middleware.controllers.api.OtpUserController; import org.opentripplanner.middleware.models.AbstractUser; import org.opentripplanner.middleware.models.ApiUser; import org.opentripplanner.middleware.models.OtpUser; +import org.opentripplanner.middleware.persistence.Persistence; +import org.opentripplanner.middleware.utils.JsonUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import spark.HaltException; @@ -22,11 +26,8 @@ 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; /** @@ -54,9 +55,10 @@ public static void checkUser(Request req) { if (isAuthDisabled()) { // If in a development or testing environment, assign a mock profile of an admin user to the request // attribute and skip authentication. - addUserToRequest(req, Auth0UserProfile.createTestUser(req)); + addUserToRequest(req, RequestingUser.createTestUser(req)); return; } + // Admin and OTP users authenticated by Bearer token String token = getTokenFromRequest(req); // Handle getting the verifier outside of the below verification try/catch, which is intended to catch issues // with the client request. (getVerifier has its own exception/halt handling). @@ -65,7 +67,7 @@ public static void checkUser(Request req) { // for downstream controllers to check permissions. try { DecodedJWT jwt = verifier.verify(token); - Auth0UserProfile profile = new Auth0UserProfile(jwt); + RequestingUser profile = new RequestingUser(jwt); if (!isValidUser(profile)) { if (isCreatingSelf(req, profile)) { // If creating self, no user account is required (it does not exist yet!). Note: creating an @@ -74,7 +76,7 @@ public static void checkUser(Request req) { 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."); + logMessageAndHalt(req, HttpStatus.NOT_FOUND_404, "No user found in database associated with the provided auth token."); } } // The user attribute is used on the server side to check user permissions and does not have all of the @@ -94,22 +96,22 @@ public static void checkUser(Request req) { /** * Check for POST requests that are creating an {@link AbstractUser} (a proxy for OTP/API users). */ - private static boolean isCreatingSelf(Request req, Auth0UserProfile profile) { + private static boolean isCreatingSelf(Request req, RequestingUser 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); + boolean creatingOtpUser = uri.endsWith(OtpUserController.OTP_USER_PATH); + boolean creatingApiUser = uri.endsWith(ApiUserController.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); + AbstractUser user = JsonUtils.getPOJOFromRequestBody(req, userClass); return profile.auth0UserId.equals(user.auth0UserId); } catch (JsonProcessingException e) { LOG.warn("Could not parse user object from request.", e); @@ -124,7 +126,6 @@ public static boolean isAuthHeaderPresent(Request req) { return authHeader != null; } - /** * Assign user to request and check that the user is an admin. */ @@ -132,8 +133,8 @@ public static void checkUserIsAdmin(Request req, Response res) { // Check auth token in request (and add user object to request). checkUser(req); // Check that user object is present and is admin. - Auth0UserProfile user = Auth0Connection.getUserFromRequest(req); - if (!isUserAdmin(user)) { + RequestingUser user = Auth0Connection.getUserFromRequest(req); + if (!user.isAdmin()) { logMessageAndHalt( req, HttpStatus.UNAUTHORIZED_401, @@ -143,23 +144,36 @@ public static void checkUserIsAdmin(Request req, Response res) { } /** - * Check if the incoming user is an admin user + * Check that the API key used in the incoming request is associated with the matching {@link ApiUser} (which is + * determined from the Authorization header). */ - public static boolean isUserAdmin(Auth0UserProfile user) { - return user != null && user.adminUser != null; + //FIXME: Move this check into existing auth checks so it would be carried out automatically prior to any + // business logic. Consider edge cases where a user can be both an API user and OTP user. + public static void ensureApiUserHasApiKey(Request req) { + RequestingUser requestingUser = getUserFromRequest(req); + String apiKeyValueFromHeader = req.headers("x-api-key"); + if (requestingUser.apiUser == null || + apiKeyValueFromHeader == null || + !requestingUser.apiUser.hasApiKeyValue(apiKeyValueFromHeader)) { + // If API user not found, log message and halt. + logMessageAndHalt( + req, + HttpStatus.FORBIDDEN_403, + "API key not linked to an API user."); + } } /** * Add user profile to Spark Request object */ - public static void addUserToRequest(Request req, Auth0UserProfile user) { + public static void addUserToRequest(Request req, RequestingUser user) { req.attribute("user", user); } /** * Get user profile from Spark Request object */ - public static Auth0UserProfile getUserFromRequest(Request req) { + public static RequestingUser getUserFromRequest(Request req) { return req.attribute("user"); } @@ -216,7 +230,8 @@ private static JWTVerifier getVerifier(Request req, String token) { } public static boolean getDefaultAuthDisabled() { - return hasConfigProperty("DISABLE_AUTH") && "true".equals(getConfigPropertyAsText("DISABLE_AUTH")); + return hasConfigProperty("DISABLE_AUTH") && + "true".equals(getConfigPropertyAsText("DISABLE_AUTH")); } /** @@ -227,38 +242,56 @@ public static boolean isAuthDisabled() { } /** - * Override the current {@link #authDisabled} value. + * Override the current {@link #authDisabled} value. This is used principally for setting up test environments that + * require auth to be disabled. */ public static void setAuthDisabled(boolean authDisabled) { Auth0Connection.authDisabled = authDisabled; } + /** + * Restore default {@link #authDisabled} value. This is used principally for tearing down test environments that + * require auth to be disabled. + */ + public static void restoreDefaultAuthDisabled() { + setAuthDisabled(getDefaultAuthDisabled()); + } + /** * Confirm that the user exists in at least one of the MongoDB user collections. */ - private static boolean isValidUser(Auth0UserProfile profile) { + private static boolean isValidUser(RequestingUser 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. + * Confirm that the user's actions are on their items if not admin. In the case of an Api user confirm that the + * user's actions, on Otp users, are Otp users they created initially. */ public static void isAuthorized(String userId, Request request) { - Auth0UserProfile profile = getUserFromRequest(request); + RequestingUser requestingUser = getUserFromRequest(request); // let admin do anything - if (profile.adminUser != null) { + if (requestingUser.adminUser != null) { return; } - // If userId is defined, it must be set to a value associated with the user. + // If userId is defined, it must be set to a value associated with a user. if (userId != null) { - if (profile.otpUser != null && profile.otpUser.id.equals(userId)) { + if (requestingUser.otpUser != null && requestingUser.otpUser.id.equals(userId)) { + // Otp user requesting their item. return; } - if (profile.apiUser != null && profile.apiUser.id.equals(userId)) { + if (requestingUser.isThirdPartyUser() && requestingUser.apiUser.id.equals(userId)) { + // Api user requesting their item. return; } + if (requestingUser.isThirdPartyUser()) { + // Api user potentially requesting an item on behalf of an Otp user they created. + OtpUser otpUser = Persistence.otpUsers.getById(userId); + if (requestingUser.canManageEntity(otpUser)) { + 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 2cc9e6ab6..939ed36ac 100644 --- a/src/main/java/org/opentripplanner/middleware/auth/Auth0Users.java +++ b/src/main/java/org/opentripplanner/middleware/auth/Auth0Users.java @@ -3,16 +3,17 @@ import com.auth0.client.auth.AuthAPI; import com.auth0.client.mgmt.ManagementAPI; import com.auth0.exception.Auth0Exception; +import com.auth0.json.auth.TokenHolder; 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.opentripplanner.middleware.utils.JsonUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import spark.Request; @@ -26,8 +27,6 @@ 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; /** @@ -39,12 +38,11 @@ public class Auth0Users { // 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 DEFAULT_AUDIENCE = "https://otp-middleware"; private static final String MANAGEMENT_API_VERSION = "v2"; - private static final String SEARCH_API_VERSION = "v3"; public static final String API_PATH = "/api/" + MANAGEMENT_API_VERSION; + /** * Cached API token so that we do not have to request a new one each time a Management API request is made. */ @@ -53,8 +51,8 @@ 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, password (Defaulted to a random UUID) and connection type + * of {@link #DEFAULT_CONNECTION_TYPE}. */ public static User createAuth0UserForEmail(String email) throws Auth0Exception { return createAuth0UserForEmail(email, UUID.randomUUID().toString()); @@ -242,33 +240,39 @@ private static String getAuth0Url() { } /** - * 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. + * Get an Auth0 oauth token response 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 token response is returned to calling methods for evaluation. */ - public static String getAuth0Token(String username, String password) throws JsonProcessingException { + public static HttpResponse getCompleteAuth0TokenResponse(String username, String password) { if (Auth0Connection.isAuthDisabled()) return null; 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 + DEFAULT_AUDIENCE, // must match an API identifier + AUTH0_API_CLIENT, // Auth0 application client ID + AUTH0_API_SECRET // Auth0 application client secret ); - - HttpResponse response = httpRequestRawResponse( + return HttpUtils.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 ); + } + + /** + * Extract from a complete Auth0 token just the access token. If the token is not available, return null instead. + */ + public static String getAuth0AccessToken(String username, String password) { + HttpResponse response = getCompleteAuth0TokenResponse(username, password); 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()); + TokenHolder token = JsonUtils.getPOJOFromJSON(response.body(), TokenHolder.class); + return (token == null) ? null : token.getAccessToken(); } } diff --git a/src/main/java/org/opentripplanner/middleware/auth/Auth0UserProfile.java b/src/main/java/org/opentripplanner/middleware/auth/RequestingUser.java similarity index 54% rename from src/main/java/org/opentripplanner/middleware/auth/Auth0UserProfile.java rename to src/main/java/org/opentripplanner/middleware/auth/RequestingUser.java index 0afb55ec2..5a8a209d7 100644 --- a/src/main/java/org/opentripplanner/middleware/auth/Auth0UserProfile.java +++ b/src/main/java/org/opentripplanner/middleware/auth/RequestingUser.java @@ -2,31 +2,30 @@ import com.auth0.jwt.interfaces.DecodedJWT; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.mongodb.client.model.Filters; import org.bson.conversions.Bson; import org.opentripplanner.middleware.models.AdminUser; import org.opentripplanner.middleware.models.ApiUser; +import org.opentripplanner.middleware.models.Model; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.persistence.Persistence; import spark.Request; -import static com.mongodb.client.model.Filters.eq; -import static org.opentripplanner.middleware.auth.Auth0Connection.isAuthHeaderPresent; - /** * User profile that is attached to an HTTP request. */ @JsonIgnoreProperties(ignoreUnknown = true) -public class Auth0UserProfile { - public final OtpUser otpUser; - public final ApiUser apiUser; - public final AdminUser adminUser; - public final String auth0UserId; +public class RequestingUser { + public OtpUser otpUser; + public ApiUser apiUser; + public AdminUser adminUser; + public String auth0UserId; /** * Constructor is only used for creating a test user. If an Auth0 user id is provided check persistence for matching * user else create default user. */ - private Auth0UserProfile(String auth0UserId) { + private RequestingUser(String auth0UserId) { if (auth0UserId == null) { this.auth0UserId = "user_id:string"; otpUser = new OtpUser(); @@ -34,35 +33,63 @@ private Auth0UserProfile(String auth0UserId) { adminUser = new AdminUser(); } else { this.auth0UserId = auth0UserId; - Bson withAuth0UserId = eq("auth0UserId", auth0UserId); + Bson withAuth0UserId = Filters.eq("auth0UserId", auth0UserId); otpUser = Persistence.otpUsers.getOneFiltered(withAuth0UserId); - adminUser = Persistence.adminUsers.getOneFiltered(withAuth0UserId); apiUser = Persistence.apiUsers.getOneFiltered(withAuth0UserId); + adminUser = Persistence.adminUsers.getOneFiltered(withAuth0UserId); } } /** - * Create a user profile from the request's JSON web token. Check persistence for stored user + * Create a user profile from the request's JSON web token. Check persistence for stored user. */ - public Auth0UserProfile(DecodedJWT jwt) { + public RequestingUser(DecodedJWT jwt) { this.auth0UserId = jwt.getClaim("sub").asString(); - Bson withAuth0UserId = eq("auth0UserId", auth0UserId); + Bson withAuth0UserId = Filters.eq("auth0UserId", auth0UserId); otpUser = Persistence.otpUsers.getOneFiltered(withAuth0UserId); - adminUser = Persistence.adminUsers.getOneFiltered(withAuth0UserId); apiUser = Persistence.apiUsers.getOneFiltered(withAuth0UserId); + adminUser = Persistence.adminUsers.getOneFiltered(withAuth0UserId); } /** * Utility method for creating a test user. If a Auth0 user Id is defined within the Authorization header param * define test user based on this. */ - static Auth0UserProfile createTestUser(Request req) { + static RequestingUser createTestUser(Request req) { String auth0UserId = null; - if (isAuthHeaderPresent(req)) { + + if (Auth0Connection.isAuthHeaderPresent(req)) { // If the auth header has been provided get the Auth0 user id from it. This is different from normal // operation as the parameter will only contain the Auth0 user id and not "Bearer token". auth0UserId = req.headers("Authorization"); } - return new Auth0UserProfile(auth0UserId); + + return new RequestingUser(auth0UserId); + } + + /** + * Determine if the requesting user is a third party API user. A third party API user is classified as a user that has + * signed up for access to the otp-middleware API. These users are expected to make requests on behalf of the + * OtpUsers they sign up, via a server application that authenticates via otp-middleware's authenticate + * endpoint (/api/secure/application/authenticate). OtpUsers created for third party API users enjoy a more limited + * range of activities (e.g., they cannot receive email/SMS notifications from otp-middleware). + */ + public boolean isThirdPartyUser() { + // TODO: Look to enhance api user check. Perhaps define specific field to indicate this? + return apiUser != null; + } + + /** + * Check if the incoming user is an admin user. + */ + public boolean isAdmin() { + return adminUser != null; + } + + /** + * Check if this requesting user can manage the specified entity. + */ + public boolean canManageEntity(Model model) { + return model != null && model.canBeManagedBy(this); } } 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 30430275a..7ae61a4b6 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/AbstractUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/AbstractUserController.java @@ -5,10 +5,12 @@ import com.beerboy.ss.ApiEndpoint; import org.eclipse.jetty.http.HttpStatus; import org.opentripplanner.middleware.auth.Auth0Connection; -import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.auth.Auth0Users; import org.opentripplanner.middleware.models.AbstractUser; +import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.persistence.TypedPersistence; +import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.JsonUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -16,10 +18,6 @@ import spark.Response; import static com.beerboy.ss.descriptor.MethodDescriptor.path; -import static org.opentripplanner.middleware.auth.Auth0Users.createNewAuth0User; -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; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; /** @@ -50,7 +48,7 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) { // Get user from token. .get(path(ROOT_ROUTE + TOKEN_PATH) .withDescription("Retrieves an " + persistence.clazz.getSimpleName() + " entity using an Auth0 access token passed in an Authorization header.") - .withProduces(JSON_ONLY) + .withProduces(HttpUtils.JSON_ONLY) .withResponseType(persistence.clazz), this::getUserFromRequest, JsonUtils::toJson ) @@ -69,14 +67,14 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) { * Obtains the correct AbstractUser-derived object from the Auth0UserProfile object. * (Used in getUserForRequest.) */ - protected abstract U getUserProfile(Auth0UserProfile profile); + protected abstract U getUserProfile(RequestingUser profile); /** - * HTTP endpoint to get the {@link U} entity, if it exists, from an {@link Auth0UserProfile} attribute + * HTTP endpoint to get the {@link U} entity, if it exists, from an {@link RequestingUser} attribute * available from a {@link Request} (this is the case for '/api/secure/' endpoints). */ private U getUserFromRequest(Request req, Response res) { - Auth0UserProfile profile = Auth0Connection.getUserFromRequest(req); + RequestingUser profile = Auth0Connection.getUserFromRequest(req); U user = getUserProfile(profile); // If the user object is null, it is most likely because it was not created yet, @@ -84,14 +82,17 @@ private U getUserFromRequest(Request req, Response res) { // but have not completed the account setup form yet. // For those users, the user profile would be 404 not found (as opposed to 403 forbidden). if (user == null) { - logMessageAndHalt(req, HttpStatus.NOT_FOUND_404, String.format(NO_USER_WITH_AUTH0_ID_MESSAGE, profile.auth0UserId), null); + logMessageAndHalt(req, + HttpStatus.NOT_FOUND_404, + String.format(NO_USER_WITH_AUTH0_ID_MESSAGE, profile.auth0UserId), + null); } return user; } private Job resendVerificationEmail(Request req, Response res) { - Auth0UserProfile profile = Auth0Connection.getUserFromRequest(req); + RequestingUser profile = Auth0Connection.getUserFromRequest(req); return Auth0Users.resendVerificationEmail(profile.auth0UserId); } @@ -101,13 +102,22 @@ private Job resendVerificationEmail(Request req, Response res) { */ @Override U preCreateHook(U user, Request req) { - User auth0UserProfile = createNewAuth0User(user, req, this.persistence); - return updateAuthFieldsForUser(user, auth0UserProfile); + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(req); + // TODO: If MOD UI is to be an ApiUser, we may want to do an additional check here to determine if this is a + // first-party API user (MOD UI) or third party. + if (requestingUser.isThirdPartyUser() && user instanceof OtpUser) { + // Do not create Auth0 account for OtpUsers created on behalf of third party API users. + return user; + } else { + // For any other user account, create Auth0 account + User auth0UserProfile = Auth0Users.createNewAuth0User(user, req, this.persistence); + return Auth0Users.updateAuthFieldsForUser(user, auth0UserProfile); + } } @Override U preUpdateHook(U user, U preExistingUser, Request req) { - validateExistingUser(user, preExistingUser, req, this.persistence); + Auth0Users.validateExistingUser(user, preExistingUser, req, this.persistence); return user; } diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/AdminUserController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/AdminUserController.java index 36b2c9494..b96d1fb56 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/AdminUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/AdminUserController.java @@ -1,6 +1,6 @@ package org.opentripplanner.middleware.controllers.api; -import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.models.AdminUser; import org.opentripplanner.middleware.persistence.Persistence; @@ -18,7 +18,7 @@ public AdminUserController(String apiPrefix) { } @Override - protected AdminUser getUserProfile(Auth0UserProfile profile) { + protected AdminUser getUserProfile(RequestingUser profile) { return profile.adminUser; } } 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 015a99a3a..e2a7ca73f 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java @@ -6,12 +6,14 @@ 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; import org.opentripplanner.middleware.auth.Auth0Connection; -import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.controllers.response.ResponseList; import org.opentripplanner.middleware.models.Model; import org.opentripplanner.middleware.models.OtpUser; +import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.persistence.TypedPersistence; import org.opentripplanner.middleware.utils.DateTimeUtils; import org.opentripplanner.middleware.utils.HttpUtils; @@ -24,10 +26,7 @@ import static com.beerboy.ss.descriptor.EndpointDescriptor.endpointPath; import static com.beerboy.ss.descriptor.MethodDescriptor.path; -import static org.opentripplanner.middleware.auth.Auth0Connection.getUserFromRequest; -import static org.opentripplanner.middleware.auth.Auth0Connection.isUserAdmin; -import static org.opentripplanner.middleware.utils.HttpUtils.JSON_ONLY; -import static org.opentripplanner.middleware.utils.JsonUtils.getPOJOFromRequestBody; +import static org.opentripplanner.middleware.utils.HttpUtils.getRequiredParamFromRequest; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; /** @@ -53,6 +52,7 @@ public abstract class ApiController implements Endpoint { public static final int DEFAULT_LIMIT = 10; public static final int DEFAULT_OFFSET = 0; public static final String OFFSET_PARAM = "offset"; + public static final String USER_ID_PARAM = "userId"; public static final ParameterDescriptor LIMIT = ParameterDescriptor.newBuilder() .withName(LIMIT_PARAM) @@ -62,6 +62,10 @@ public abstract class ApiController implements Endpoint { .withName(OFFSET_PARAM) .withDefaultValue(String.valueOf(DEFAULT_OFFSET)) .withDescription("If specified, the number of records to skip/offset.").build(); + public static final ParameterDescriptor USER_ID = ParameterDescriptor.newBuilder() + .withName(USER_ID_PARAM) + .withRequired(false) + .withDescription("If specified, the required user id.").build(); /** * @param apiPrefix string prefix to use in determining the resource location @@ -91,7 +95,8 @@ public ApiController(String apiPrefix, TypedPersistence persistence, String r @Override public void bind(final SparkSwagger restApi) { ApiEndpoint apiEndpoint = restApi.endpoint( - endpointPath(ROOT_ROUTE).withDescription("Interface for querying and managing '" + className + "' entities."), + endpointPath(ROOT_ROUTE) + .withDescription("Interface for querying and managing '" + className + "' entities."), HttpUtils.NO_FILTER ); buildEndpoint(apiEndpoint); @@ -122,46 +127,43 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) { baseEndpoint // Get multiple entities. - .get( - path(ROOT_ROUTE) + .get(path(ROOT_ROUTE) .withDescription("Gets a paginated list of all '" + className + "' entities.") .withQueryParam(LIMIT) .withQueryParam(OFFSET) - .withProduces(JSON_ONLY) + .withQueryParam(USER_ID) + .withProduces(HttpUtils.JSON_ONLY) .withResponseType(ResponseList.class), this::getMany, JsonUtils::toJson ) // Get one entity. - .get( - path(ROOT_ROUTE + ID_PATH) + .get(path(ROOT_ROUTE + ID_PATH) .withDescription("Returns the '" + className + "' entity with the specified id, or 404 if not found.") .withPathParam().withName(ID_PARAM).withRequired(true).withDescription("The id of the entity to search.").and() // .withResponses(...) // FIXME: not implemented (requires source change). - .withProduces(JSON_ONLY) + .withProduces(HttpUtils.JSON_ONLY) .withResponseType(clazz), this::getEntityForId, JsonUtils::toJson ) // Create entity request - .post( - path("") + .post(path("") .withDescription("Creates a '" + className + "' entity.") - .withConsumes(JSON_ONLY) + .withConsumes(HttpUtils.JSON_ONLY) .withRequestType(clazz) - .withProduces(JSON_ONLY) + .withProduces(HttpUtils.JSON_ONLY) .withResponseType(clazz), this::createOrUpdate, JsonUtils::toJson ) // Update entity request - .put( - path(ID_PATH) + .put(path(ID_PATH) .withDescription("Updates and returns the '" + className + "' entity with the specified id, or 404 if not found.") .withPathParam().withName(ID_PARAM).withRequired(true).withDescription("The id of the entity to update.").and() - .withConsumes(JSON_ONLY) + .withConsumes(HttpUtils.JSON_ONLY) .withRequestType(clazz) - .withProduces(JSON_ONLY) + .withProduces(HttpUtils.JSON_ONLY) // FIXME: `withResponses` is supposed to document the expected HTTP responses (200, 403, 404)... // but that doesn't appear to be implemented in spark-swagger. // .withResponses(...) @@ -170,11 +172,10 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) { ) // Delete entity request - .delete( - path(ID_PATH) + .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() - .withProduces(JSON_ONLY) + .withProduces(HttpUtils.JSON_ONLY) .withResponseType(clazz), this::deleteOne, JsonUtils::toJson ); @@ -188,20 +189,42 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) { private ResponseList getMany(Request req, Response res) { int limit = HttpUtils.getQueryParamFromRequest(req, LIMIT_PARAM, 0, DEFAULT_LIMIT, 100); int offset = HttpUtils.getQueryParamFromRequest(req, OFFSET_PARAM, 0, DEFAULT_OFFSET); - Auth0UserProfile requestingUser = getUserFromRequest(req); - if (isUserAdmin(requestingUser)) { + String userId = HttpUtils.getQueryParamFromRequest(req, USER_ID_PARAM, true); + // Filter the response based on the user id, if provided. + // If the user id is not provided filter response based on requesting user. + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(req); + if (userId != null) { + OtpUser otpUser = Persistence.otpUsers.getById(userId); + if (requestingUser.canManageEntity(otpUser)) { + return persistence.getResponseList(Filters.eq(USER_ID_PARAM, userId), offset, limit); + } else { + res.status(HttpStatus.FORBIDDEN_403); + return null; + } + } + if (requestingUser.isAdmin()) { // If the user is admin, the context is presumed to be the admin dashboard, so we deliver all entities for // management or review without restriction. return persistence.getResponseList(offset, limit); } else if (persistence.clazz == OtpUser.class) { // If the required entity is of type 'OtpUser' the assumption is that a call is being made via the - // OtpUserController. Therefore, the request should be limited to return just the entity matching the - // requesting user. - return persistence.getResponseList(Filters.eq("_id", requestingUser.otpUser.id), offset, limit); + // OtpUserController. If the request is being made by an Api user the response will be limited to the Otp users + // created by this Api user. If not, the assumption is that an Otp user is making the request and the response + // will be limited to just the entity matching this Otp user. + Bson filter = (requestingUser.apiUser != null) + ? Filters.eq("applicationId", requestingUser.apiUser.id) + : Filters.eq("_id", requestingUser.otpUser.id); + return persistence.getResponseList(filter, offset, limit); + } else if (requestingUser.isThirdPartyUser()) { + // A user id must be provided if the request is being made by a third party user. + logMessageAndHalt(req, + HttpStatus.BAD_REQUEST_400, + String.format("The parameter name (%s) must be provided.", USER_ID_PARAM)); + return null; } else { // For all other cases the assumption is that the request is being made by an Otp user and the requested // entities have a 'userId' parameter. Only entities that match the requesting user id are returned. - return persistence.getResponseList(Filters.eq("userId", requestingUser.otpUser.id), offset, limit); + return persistence.getResponseList(Filters.eq(USER_ID_PARAM, requestingUser.otpUser.id), offset, limit); } } @@ -212,11 +235,11 @@ private ResponseList getMany(Request req, Response res) { * (must be admin) than is desired. */ protected T getEntityForId(Request req, Response res) { - Auth0UserProfile requestingUser = Auth0Connection.getUserFromRequest(req); + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(req); String id = getIdFromRequest(req); T object = getObjectForId(req, id); - if (!object.canBeManagedBy(requestingUser)) { + if (!requestingUser.canManageEntity(object)) { logMessageAndHalt(req, HttpStatus.FORBIDDEN_403, String.format("Requesting user not authorized to get %s.", className)); } @@ -229,11 +252,11 @@ protected T getEntityForId(Request req, Response res) { private T deleteOne(Request req, Response res) { long startTime = DateTimeUtils.currentTimeMillis(); String id = getIdFromRequest(req); - Auth0UserProfile requestingUser = Auth0Connection.getUserFromRequest(req); + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(req); try { T object = getObjectForId(req, id); // Check that requesting user can manage entity. - if (!object.canBeManagedBy(requestingUser)) { + if (!requestingUser.canManageEntity(object)) { logMessageAndHalt(req, HttpStatus.FORBIDDEN_403, String.format("Requesting user not authorized to delete %s.", className)); } // Run pre-delete hook. If return value is false, abort. @@ -309,16 +332,18 @@ private T createOrUpdate(Request req, Response res) { if (req.params(ID_PARAM) == null && req.requestMethod().equals("PUT")) { logMessageAndHalt(req, HttpStatus.BAD_REQUEST_400, "Must provide id"); } - Auth0UserProfile requestingUser = Auth0Connection.getUserFromRequest(req); + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(req); final boolean isCreating = req.params(ID_PARAM) == null; // Save or update to database try { // Validate fields by deserializing into POJO. - T object = getPOJOFromRequestBody(req, clazz); + T object = JsonUtils.getPOJOFromRequestBody(req, clazz); if (isCreating) { // Verify that the requesting user can create object. if (!object.canBeCreatedBy(requestingUser)) { - logMessageAndHalt(req, HttpStatus.FORBIDDEN_403, String.format("Requesting user not authorized to create %s.", className)); + logMessageAndHalt(req, + HttpStatus.FORBIDDEN_403, + String.format("Requesting user not authorized to create %s.", className)); } // Run pre-create hook and use updated object (with potentially modified values) in create operation. T updatedObject = preCreateHook(object, req); @@ -331,8 +356,10 @@ private T createOrUpdate(Request req, Response res) { return null; } // Check that requesting user can manage entity. - if (!preExistingObject.canBeManagedBy(requestingUser)) { - logMessageAndHalt(req, HttpStatus.FORBIDDEN_403, String.format("Requesting user not authorized to update %s.", className)); + if (!requestingUser.canManageEntity(preExistingObject)) { + logMessageAndHalt(req, + HttpStatus.FORBIDDEN_403, + String.format("Requesting user not authorized to update %s.", className)); } // Update last updated value. object.lastUpdated = DateTimeUtils.nowAsDate(); @@ -351,9 +378,13 @@ private T createOrUpdate(Request req, Response res) { } catch (HaltException e) { throw e; } catch (JsonProcessingException e) { - logMessageAndHalt(req, HttpStatus.BAD_REQUEST_400, "Error parsing JSON for " + clazz.getSimpleName(), 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); + logMessageAndHalt(req, + 500, + "An error was encountered while trying to save to the database", e); } finally { String operation = isCreating ? "Create" : "Update"; LOG.info("{} {} operation took {} msec", operation, className, DateTimeUtils.currentTimeMillis() - startTime); @@ -365,6 +396,6 @@ private T createOrUpdate(Request req, Response res) { * Get entity ID from request. */ private String getIdFromRequest(Request req) { - return HttpUtils.getRequiredParamFromRequest(req, "id"); + return getRequiredParamFromRequest(req, "id"); } } 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 16caffe68..ed5effcd2 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/ApiUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/ApiUserController.java @@ -1,9 +1,11 @@ package org.opentripplanner.middleware.controllers.api; +import com.auth0.json.auth.TokenHolder; import com.beerboy.ss.ApiEndpoint; import org.eclipse.jetty.http.HttpStatus; import org.opentripplanner.middleware.auth.Auth0Connection; -import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.auth.Auth0Users; +import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.models.ApiKey; import org.opentripplanner.middleware.models.ApiUser; import org.opentripplanner.middleware.persistence.Persistence; @@ -17,11 +19,10 @@ import spark.Request; import spark.Response; +import java.net.http.HttpResponse; + import static com.beerboy.ss.descriptor.MethodDescriptor.path; -import static org.opentripplanner.middleware.auth.Auth0Connection.isUserAdmin; -import static org.opentripplanner.middleware.utils.ApiGatewayUtils.deleteApiKey; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; -import static org.opentripplanner.middleware.utils.HttpUtils.JSON_ONLY; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; /** @@ -32,9 +33,12 @@ public class ApiUserController extends AbstractUserController { private static final Logger LOG = LoggerFactory.getLogger(ApiUserController.class); public static final String DEFAULT_USAGE_PLAN_ID = getConfigPropertyAsText("DEFAULT_USAGE_PLAN_ID"); private static final String API_KEY_PATH = "/apikey"; + public static final String AUTHENTICATE_PATH = "/authenticate"; 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"; + private static final String USERNAME_PARAM = "username"; + private static final String PASSWORD_PARAM = "password"; public ApiUserController(String apiPrefix) { super(apiPrefix, Persistence.apiUsers, API_USER_PATH); @@ -53,7 +57,7 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) { .and() .withQueryParam().withName("usagePlanId").withDescription("Optional AWS API Gateway usage plan ID.") .and() - .withProduces(JSON_ONLY) + .withProduces(HttpUtils.JSON_ONLY) .withResponseType(persistence.clazz), this::createApiKeyForApiUser, JsonUtils::toJson ) @@ -64,24 +68,49 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) { .and() .withPathParam().withName("apiKeyId").withDescription("The ID of the API key.") .and() - .withProduces(JSON_ONLY) + .withProduces(HttpUtils.JSON_ONLY) .withResponseType(persistence.clazz), - this::deleteApiKeyForApiUser, JsonUtils::toJson + this::deleteApiKeyForApiUser, JsonUtils::toJson) + // Authenticate user with Auth0 + .post(path(AUTHENTICATE_PATH) + .withDescription("Authenticates ApiUser with Auth0.") + .withQueryParam().withName(USERNAME_PARAM).withRequired(true) + .withDescription("Auth0 username (usually email address).").and() + .withQueryParam().withName(PASSWORD_PARAM).withRequired(true) + .withDescription("Auth0 password.").and() + .withProduces(HttpUtils.JSON_ONLY) + .withResponseType(TokenHolder.class), + this::authenticateAuth0User, JsonUtils::toJson ); - // Add the regular CRUD methods after defining the /apikey route. super.buildEndpoint(modifiedEndpoint); } /** - * Shorthand method to determine if an API user exists and has an API key. + * Authenticate user with Auth0 based on username (email) and password. If successful, return the complete Auth0 + * token else log message and halt. */ - private boolean userHasKey(ApiUser user, String apiKeyId) { - return user != null && - user.apiKeys - .stream() - .anyMatch(apiKey -> apiKeyId.equals(apiKey.keyId)); + private TokenHolder authenticateAuth0User(Request req, Response res) { + String username = HttpUtils.getQueryParamFromRequest(req, USERNAME_PARAM, false); + // FIXME: Should this be encrypted?! + String password = HttpUtils.getQueryParamFromRequest(req, PASSWORD_PARAM, false); + String apiKeyFromHeader = req.headers("x-api-key"); + ApiUser apiUser = ApiUser.userForApiKeyValue(apiKeyFromHeader); + if (apiKeyFromHeader == null || apiUser == null || !apiUser.email.equals(username)) { + logMessageAndHalt(req, + HttpStatus.BAD_REQUEST_400, + "An API key must be provided and match Api user." + ); + } + HttpResponse auth0TokenResponse = Auth0Users.getCompleteAuth0TokenResponse(username, password); + if (auth0TokenResponse == null || auth0TokenResponse.statusCode() != HttpStatus.OK_200) { + logMessageAndHalt(req, + auth0TokenResponse.statusCode(), + String.format("Cannot obtain Auth0 token for user %s", username) + ); + } + return JsonUtils.getPOJOFromJSON(auth0TokenResponse.body(), TokenHolder.class); } /** @@ -90,11 +119,11 @@ private boolean userHasKey(ApiUser user, String apiKeyId) { */ private ApiUser createApiKeyForApiUser(Request req, Response res) { ApiUser targetUser = getApiUser(req); - Auth0UserProfile requestingUser = Auth0Connection.getUserFromRequest(req); + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(req); String usagePlanId = req.queryParamOrDefault("usagePlanId", DEFAULT_USAGE_PLAN_ID); // If requester is not an admin user, force the usage plan ID to the default and enforce key limit. A non-admin // user should not be able to create an API key for any usage plan. - if (!isUserAdmin(requestingUser)) { + if (!requestingUser.isAdmin()) { usagePlanId = DEFAULT_USAGE_PLAN_ID; if (targetUser.apiKeys.size() >= API_KEY_LIMIT_PER_USER) { logMessageAndHalt(req, HttpStatus.BAD_REQUEST_400, "User has reached API key limit."); @@ -120,9 +149,9 @@ private ApiUser createApiKeyForApiUser(Request req, Response res) { * Delete an api key from a given user's list of api keys (if present) and from AWS api gateway. */ private ApiUser deleteApiKeyForApiUser(Request req, Response res) { - Auth0UserProfile requestingUser = Auth0Connection.getUserFromRequest(req); + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(req); // Do not permit key deletion unless user is an admin. - if (!isUserAdmin(requestingUser)) { + if (!requestingUser.isAdmin()) { logMessageAndHalt(req, HttpStatus.FORBIDDEN_403, "Must be an admin to delete an API key."); } ApiUser targetUser = getApiUser(req); @@ -133,7 +162,7 @@ private ApiUser deleteApiKeyForApiUser(Request req, Response res) { "An api key id is required", null); } - if (!userHasKey(targetUser, apiKeyId)) { + if (targetUser != null && !targetUser.hasApiKeyId(apiKeyId)) { logMessageAndHalt(req, HttpStatus.NOT_FOUND_404, String.format("User id (%s) does not have expected api key id (%s)", targetUser.id, apiKeyId), @@ -141,7 +170,7 @@ private ApiUser deleteApiKeyForApiUser(Request req, Response res) { } // Delete API key from AWS. - boolean success = deleteApiKey(new ApiKey(apiKeyId)); + boolean success = ApiGatewayUtils.deleteApiKey(new ApiKey(apiKeyId)); if (success) { // Delete api key from user and persist targetUser.apiKeys.removeIf(apiKey -> apiKeyId.equals(apiKey.keyId)); @@ -155,7 +184,7 @@ private ApiUser deleteApiKeyForApiUser(Request req, Response res) { } @Override - protected ApiUser getUserProfile(Auth0UserProfile profile) { + protected ApiUser getUserProfile(RequestingUser profile) { return profile.apiUser; } @@ -199,7 +228,7 @@ boolean preDeleteHook(ApiUser user, Request req) { * Get an Api user from Mongo DB based on the provided user id. Make sure user is admin or managing self. */ private static ApiUser getApiUser(Request req) { - Auth0UserProfile requestingUser = Auth0Connection.getUserFromRequest(req); + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(req); String userId = HttpUtils.getRequiredParamFromRequest(req, ID_PARAM); ApiUser apiUser = Persistence.apiUsers.getById(userId); if (apiUser == null) { @@ -211,10 +240,9 @@ private static ApiUser getApiUser(Request req) { ); } - if (!apiUser.canBeManagedBy(requestingUser)) { + if (!requestingUser.canManageEntity(apiUser)) { logMessageAndHalt(req, HttpStatus.FORBIDDEN_403, "Must be an admin to perform this operation."); } - return apiUser; } } diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/LogController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/LogController.java index 445da54f9..80523371d 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/LogController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/LogController.java @@ -2,10 +2,11 @@ import com.amazonaws.services.apigateway.model.GetUsageResult; import com.beerboy.ss.SparkSwagger; +import com.beerboy.ss.descriptor.EndpointDescriptor; import com.beerboy.ss.rest.Endpoint; import org.eclipse.jetty.http.HttpStatus; import org.opentripplanner.middleware.auth.Auth0Connection; -import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.models.ApiKey; import org.opentripplanner.middleware.models.ApiUsageResult; import org.opentripplanner.middleware.utils.ApiGatewayUtils; @@ -21,11 +22,8 @@ import java.util.List; import java.util.stream.Collectors; -import static com.beerboy.ss.descriptor.EndpointDescriptor.endpointPath; import static com.beerboy.ss.descriptor.MethodDescriptor.path; -import static org.opentripplanner.middleware.auth.Auth0Connection.isUserAdmin; import static org.opentripplanner.middleware.utils.DateTimeUtils.DEFAULT_DATE_FORMAT_PATTERN; -import static org.opentripplanner.middleware.utils.HttpUtils.JSON_ONLY; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; /** @@ -45,7 +43,7 @@ public LogController(String apiPrefix) { @Override public void bind(final SparkSwagger restApi) { restApi.endpoint( - endpointPath(ROOT_ROUTE).withDescription("Interface for retrieving API logs from AWS."), + EndpointDescriptor.endpointPath(ROOT_ROUTE).withDescription("Interface for retrieving API logs from AWS."), HttpUtils.NO_FILTER ).get(path(ROOT_ROUTE) .withDescription("Gets a list of all API usage logs.") @@ -68,7 +66,7 @@ public void bind(final SparkSwagger restApi) { "If specified, the latest date (format %s) for which usage logs are retrieved.", DEFAULT_DATE_FORMAT_PATTERN )).and() - .withProduces(JSON_ONLY) + .withProduces(HttpUtils.JSON_ONLY) // Note: unlike what the name suggests, withResponseAsCollection does not generate an array // as the return type for this method. (It does generate the type for that class nonetheless.) .withResponseAsCollection(ApiUsageResult.class), @@ -82,9 +80,9 @@ public void bind(final SparkSwagger restApi) { private static List getUsageLogs(Request req, Response res) { // Get list of API keys (if present) from request. List apiKeys = getApiKeyIdsFromRequest(req); - Auth0UserProfile requestingUser = Auth0Connection.getUserFromRequest(req); + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(req); // If the user is not an admin, the list of API keys is defaulted to their keys. - if (!isUserAdmin(requestingUser)) { + if (!requestingUser.isAdmin()) { if (requestingUser.apiUser == null) { logMessageAndHalt(req, HttpStatus.FORBIDDEN_403, "Action is not permitted for user."); return null; 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 ebeb429db..53e1b3aac 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpRequestProcessor.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpRequestProcessor.java @@ -1,10 +1,13 @@ package org.opentripplanner.middleware.controllers.api; import com.beerboy.ss.SparkSwagger; +import com.beerboy.ss.descriptor.EndpointDescriptor; +import com.beerboy.ss.descriptor.ParameterDescriptor; import com.beerboy.ss.rest.Endpoint; import org.eclipse.jetty.http.HttpStatus; import org.opentripplanner.middleware.auth.Auth0Connection; -import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.auth.RequestingUser; +import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.models.TripRequest; import org.opentripplanner.middleware.models.TripSummary; import org.opentripplanner.middleware.otp.OtpDispatcher; @@ -17,15 +20,14 @@ import org.slf4j.LoggerFactory; import spark.Request; +import javax.ws.rs.core.MediaType; import java.util.List; -import static com.beerboy.ss.descriptor.EndpointDescriptor.endpointPath; import static com.beerboy.ss.descriptor.MethodDescriptor.path; -import static javax.ws.rs.core.MediaType.APPLICATION_JSON; -import static javax.ws.rs.core.MediaType.APPLICATION_XML; +import static org.opentripplanner.middleware.auth.Auth0Connection.checkUser; +import static org.opentripplanner.middleware.auth.Auth0Connection.getUserFromRequest; import static org.opentripplanner.middleware.auth.Auth0Connection.isAuthHeaderPresent; -import static org.opentripplanner.middleware.otp.OtpDispatcher.OTP_API_ROOT; -import static org.opentripplanner.middleware.otp.OtpDispatcher.OTP_PLAN_ENDPOINT; +import static org.opentripplanner.middleware.controllers.api.ApiController.USER_ID_PARAM; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; /** @@ -58,13 +60,18 @@ public class OtpRequestProcessor implements Endpoint { */ @Override public void bind(final SparkSwagger restApi) { + ParameterDescriptor USER_ID = ParameterDescriptor.newBuilder() + .withName(USER_ID_PARAM) + .withRequired(false) + .withDescription("If a third-party application is making a trip plan request on behalf of an end user (OtpUser), the user id must be specified.") + .build(); restApi.endpoint( - endpointPath(OTP_PROXY_ENDPOINT).withDescription("Proxy interface for OTP endpoints. " + OTP_DOC_LINK), + EndpointDescriptor.endpointPath(OTP_PROXY_ENDPOINT).withDescription("Proxy interface for OTP endpoints. " + OTP_DOC_LINK), HttpUtils.NO_FILTER - ).get( - path("/*") + ).get(path("/*") .withDescription("Forwards any GET request to OTP. " + OTP_DOC_LINK) - .withProduces(List.of(APPLICATION_JSON, APPLICATION_XML)), + .withQueryParam(USER_ID) + .withProduces(List.of(MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML)), OtpRequestProcessor::proxy ); } @@ -76,25 +83,56 @@ public void bind(final SparkSwagger restApi) { * status) is passed back to the requester. */ private static String proxy(Request request, spark.Response response) { - if (OTP_API_ROOT == null) { + if (OtpDispatcher.OTP_API_ROOT == null) { logMessageAndHalt(request, HttpStatus.INTERNAL_SERVER_ERROR_500, "No OTP Server provided, check config."); return null; } + // If a user id is provided, the assumption is that an API user is making a plan request on behalf of an Otp user. + String userId = request.queryParams(USER_ID_PARAM); + String apiKeyValueFromHeader = request.headers("x-api-key"); + OtpUser otpUser = null; + // If the Auth header is present, this indicates that the request was made by a logged in user. + if (isAuthHeaderPresent(request)) { + checkUser(request); + RequestingUser requestingUser = getUserFromRequest(request); + if (requestingUser.otpUser != null && userId == null) { + // Otp user making a trip request for self. + otpUser = requestingUser.otpUser; + } else if (requestingUser.apiUser != null) { + Auth0Connection.ensureApiUserHasApiKey(request); + // Api user making a trip request on behalf of an Otp user. In this case, the Otp user id must be provided + // as a query parameter. + otpUser = Persistence.otpUsers.getById(userId); + if (otpUser == null && userId != null) { + logMessageAndHalt(request, HttpStatus.NOT_FOUND_404, "The specified user id was not found."); + } else if (!requestingUser.canManageEntity(otpUser)) { + logMessageAndHalt(request, + HttpStatus.FORBIDDEN_403, + String.format("User: %s not authorized to make trip requests for user: %s", + requestingUser.apiUser.email, + otpUser.email)); + } + } + } else if (userId != null && apiKeyValueFromHeader == null) { + // User id has been provided, but no means to authorize the requesting user. + logMessageAndHalt(request, + HttpStatus.UNAUTHORIZED_401, + "Unauthorized trip request, authorization required."); + } // Get request path intended for OTP API by removing the proxy endpoint (/otp). String otpRequestPath = request.uri().replaceFirst(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) { logMessageAndHalt(request, HttpStatus.INTERNAL_SERVER_ERROR_500, "No response from OTP server."); return null; } - // If the request path ends with the plan endpoint (e.g., '/plan' or '/default/plan'), process response. - if (otpRequestPath.endsWith(OTP_PLAN_ENDPOINT)) handlePlanTripResponse(request, otpDispatcherResponse); - + if (otpRequestPath.endsWith(OtpDispatcher.OTP_PLAN_ENDPOINT) && otpUser != null) { + handlePlanTripResponse(request, otpDispatcherResponse, otpUser); + } // provide response to requester as received from OTP server - response.type(APPLICATION_JSON); + response.type(MediaType.APPLICATION_JSON); response.status(otpDispatcherResponse.statusCode); return otpDispatcherResponse.responseBody; } @@ -103,37 +141,23 @@ private static String proxy(Request request, spark.Response response) { * Process plan response from OTP. Store the response if consent is given. Handle the process and all exceptions * seamlessly so as not to affect the response provided to the requester. */ - private static void handlePlanTripResponse(Request request, OtpDispatcherResponse otpDispatcherResponse) { - - // If the Auth header is present, this indicates that the request was made by a logged in user. This indicates - // that we should store trip history (but we verify this preference before doing so). - if (!isAuthHeaderPresent(request)) { - LOG.debug("Anonymous user, trip history not stored"); - return; - } + private static void handlePlanTripResponse(Request request, OtpDispatcherResponse otpDispatcherResponse, OtpUser otpUser) { String batchId = request.queryParams("batchId"); if (batchId == null) { //TODO place holder for now batchId = "-1"; } - - // Dispatch request to OTP and store request/response summary if user elected to store trip history. long tripStorageStartTime = DateTimeUtils.currentTimeMillis(); - - Auth0Connection.checkUser(request); - Auth0UserProfile profile = Auth0Connection.getUserFromRequest(request); - - final boolean storeTripHistory = profile != null && profile.otpUser != null && profile.otpUser.storeTripHistory; // only save trip details if the user has given consent and a response from OTP is provided - if (!storeTripHistory) { + if (!otpUser.storeTripHistory) { LOG.debug("User does not want trip history stored"); } else { OtpResponse otpResponse = otpDispatcherResponse.getResponse(); if (otpResponse == null) { LOG.warn("OTP response is null, cannot save trip history for user!"); } else { - TripRequest tripRequest = new TripRequest(profile.otpUser.id, batchId, request.queryParams("fromPlace"), + TripRequest tripRequest = new TripRequest(otpUser.id, batchId, request.queryParams("fromPlace"), request.queryParams("toPlace"), request.queryString()); // only save trip summary if the trip request was saved boolean tripRequestSaved = Persistence.tripRequests.create(tripRequest); 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 c52b3b10a..916dfe23a 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpUserController.java @@ -5,10 +5,8 @@ import com.twilio.rest.verify.v2.service.VerificationCheck; import org.apache.commons.lang3.StringUtils; import org.eclipse.jetty.http.HttpStatus; -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.auth.Auth0Connection; +import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.utils.JsonUtils; @@ -45,20 +43,12 @@ public OtpUserController(String apiPrefix) { @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.") - ); + RequestingUser requestingUser = Auth0Connection.getUserFromRequest(req); + if (requestingUser.apiUser != null) { + // 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. + Auth0Connection.ensureApiUserHasApiKey(req); + user.applicationId = requestingUser.apiUser.id; } return super.preCreateHook(user, req); } @@ -89,7 +79,7 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) { } @Override - protected OtpUser getUserProfile(Auth0UserProfile profile) { + protected OtpUser getUserProfile(RequestingUser profile) { return profile.otpUser; } diff --git a/src/main/java/org/opentripplanner/middleware/docs/PublicApiDocGenerator.java b/src/main/java/org/opentripplanner/middleware/docs/PublicApiDocGenerator.java index 1ecb6c98c..00213a20b 100644 --- a/src/main/java/org/opentripplanner/middleware/docs/PublicApiDocGenerator.java +++ b/src/main/java/org/opentripplanner/middleware/docs/PublicApiDocGenerator.java @@ -32,6 +32,7 @@ import java.util.function.Predicate; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; +import static org.opentripplanner.middleware.utils.ConfigUtils.getVersionFromJar; /** * Class that generates an enhanced public-facing documentation, in OpenAPI 2.0 (Swagger) format, @@ -116,7 +117,7 @@ public Path generatePublicApiDocs() throws IOException { // Overwrite top-level parameters. swaggerRoot.put("host", AWS_API_SERVER); swaggerRoot.put("basePath", "/" + AWS_API_STAGE); - ((ObjectNode) swaggerRoot.get("info")).put("version", ConfigUtils.getVersionFromJar()); + ((ObjectNode) swaggerRoot.get("info")).put("version", getVersionFromJar()); // Generate output file. diff --git a/src/main/java/org/opentripplanner/middleware/models/AbstractUser.java b/src/main/java/org/opentripplanner/middleware/models/AbstractUser.java index 5bf385c5a..7164e3491 100644 --- a/src/main/java/org/opentripplanner/middleware/models/AbstractUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/AbstractUser.java @@ -2,7 +2,7 @@ import com.auth0.exception.Auth0Exception; import org.opentripplanner.middleware.auth.Auth0Connection; -import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.auth.Permission; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -30,6 +30,7 @@ public abstract class AbstractUser extends Model { * value, so the stored user will contain the value from Auth0 (e.g., "auth0|abcd1234"). */ public String auth0UserId = UUID.randomUUID().toString(); + /** Whether a user is also a Data Tools user */ public boolean isDataToolsUser; /** @@ -44,7 +45,7 @@ public abstract class AbstractUser extends Model { * permissions) or if the requesting user has permission to manage the entity type. */ @Override - public boolean canBeManagedBy(Auth0UserProfile user) { + public boolean canBeManagedBy(RequestingUser user) { // If the user is attempting to update someone else's profile, they must be an admin. boolean isManagingSelf = this.auth0UserId.equals(user.auth0UserId); if (isManagingSelf) { diff --git a/src/main/java/org/opentripplanner/middleware/models/AdminUser.java b/src/main/java/org/opentripplanner/middleware/models/AdminUser.java index b23165dd4..f3d92692e 100644 --- a/src/main/java/org/opentripplanner/middleware/models/AdminUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/AdminUser.java @@ -1,13 +1,11 @@ package org.opentripplanner.middleware.models; -import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.auth.RequestingUser; 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; - /** * Represents an administrative user of the OTP Admin Dashboard (otp-admin-ui). */ @@ -31,8 +29,8 @@ public AdminUser() { * TODO: Change to application admin? */ @Override - public boolean canBeCreatedBy(Auth0UserProfile user) { - return isUserAdmin(user); + public boolean canBeCreatedBy(RequestingUser user) { + return user.isAdmin(); } @Override diff --git a/src/main/java/org/opentripplanner/middleware/models/ApiUser.java b/src/main/java/org/opentripplanner/middleware/models/ApiUser.java index a09cd7649..108346851 100644 --- a/src/main/java/org/opentripplanner/middleware/models/ApiUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/ApiUser.java @@ -87,4 +87,32 @@ public void createApiKey(String usagePlanId, boolean persist) throws CreateApiKe public static ApiUser userForApiKey(String apiKeyId) { return Persistence.apiUsers.getOneFiltered(Filters.elemMatch("apiKeys", Filters.eq("keyId", apiKeyId))); } + + /** + * @return the first {@link ApiUser} found with an {@link ApiKey#value} in {@link #apiKeys} that matches the + * provided apiKeyValue. + */ + public static ApiUser userForApiKeyValue(String apiKeyValue) { + return Persistence.apiUsers.getOneFiltered(Filters.elemMatch("apiKeys", Filters.eq("value", apiKeyValue))); + } + + /** + * Shorthand method to determine if an API user has an API key id. + */ + public boolean hasApiKeyId(String apiKeyId) { + return apiKeys + .stream() + .anyMatch(apiKey -> apiKeyId.equals(apiKey.keyId)); + } + + /** + * Shorthand method to determine if an API user has an API key value. + */ + public boolean hasApiKeyValue(String apiKeyValue) { + return apiKeys + .stream() + .anyMatch(apiKey -> apiKeyValue.equals(apiKey.value)); + } + + } diff --git a/src/main/java/org/opentripplanner/middleware/models/Model.java b/src/main/java/org/opentripplanner/middleware/models/Model.java index af1590f4e..38fb70447 100644 --- a/src/main/java/org/opentripplanner/middleware/models/Model.java +++ b/src/main/java/org/opentripplanner/middleware/models/Model.java @@ -1,6 +1,6 @@ package org.opentripplanner.middleware.models; -import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.utils.DateTimeUtils; import java.io.Serializable; @@ -8,8 +8,6 @@ import java.util.Objects; import java.util.UUID; -import static org.opentripplanner.middleware.auth.Auth0Connection.isUserAdmin; - public class Model implements Serializable { private static final long serialVersionUID = 1L; @@ -28,7 +26,7 @@ public Model () { * This is a basic authorization check for any entity to determine if a user can create the entity. By default any * user can create any entity. This method should be overridden if there are more restrictions needed. */ - public boolean canBeCreatedBy(Auth0UserProfile user) { + public boolean canBeCreatedBy(RequestingUser user) { return true; } @@ -36,9 +34,9 @@ public boolean canBeCreatedBy(Auth0UserProfile user) { * This is a basic authorization check for any entity to determine if a user can manage it. This method * should be overridden in subclasses in order to provide more fine-grained checks. */ - public boolean canBeManagedBy(Auth0UserProfile user) { + public boolean canBeManagedBy(RequestingUser user) { // TODO: Check if user has application administrator permission? - return isUserAdmin(user); + return user.isAdmin(); } @Override diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java index bde7f4fd9..25dbf16cd 100644 --- a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java @@ -7,7 +7,7 @@ import com.mongodb.client.FindIterable; import org.bson.codecs.pojo.annotations.BsonIgnore; import org.bson.conversions.Bson; -import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.auth.Permission; import org.opentripplanner.middleware.otp.OtpDispatcherResponse; import org.opentripplanner.middleware.otp.OtpRequest; @@ -33,7 +33,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.opentripplanner.middleware.utils.ItineraryUtils.DATE_PARAM; import static org.opentripplanner.middleware.utils.ItineraryUtils.TIME_PARAM; - /** * A monitored trip represents a trip a user would like to receive notification on if affected by a delay and/or route * change. @@ -283,9 +282,9 @@ public boolean isActiveOnDate(ZonedDateTime zonedDateTime) { } @Override - public boolean canBeCreatedBy(Auth0UserProfile profile) { - OtpUser otpUser = profile.otpUser; + public boolean canBeCreatedBy(RequestingUser requestingUser) { if (userId == null) { + OtpUser otpUser = requestingUser.otpUser; 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. @@ -295,35 +294,42 @@ public boolean canBeCreatedBy(Auth0UserProfile profile) { userId = otpUser.id; } else { // If userId was provided, follow authorization provided by canBeManagedBy - return canBeManagedBy(profile); + return canBeManagedBy(requestingUser); } - return super.canBeCreatedBy(profile); + return super.canBeCreatedBy(requestingUser); } /** * Confirm that the requesting user has the required permissions */ @Override - public boolean canBeManagedBy(Auth0UserProfile user) { + public boolean canBeManagedBy(RequestingUser requestingUser) { // 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. + // If the user is attempting to update someone else's monitored trip, they must be admin or an API user if the + // OTP user is assigned to that API. 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); + if (requestingUser.otpUser != null) { + belongsToUser = userId.equals(requestingUser.otpUser.id); } if (belongsToUser) { return true; - } else if (user.adminUser != null) { + } else if (requestingUser.apiUser != null) { + // get the required OTP user to confirm they are associated with the requesting API user. + OtpUser otpUser = Persistence.otpUsers.getById(userId); + if (requestingUser.canManageEntity(otpUser)) { + return true; + } + } else if (requestingUser.isAdmin()) { // If not managing self, user must have manage permission. - for (Permission permission : user.adminUser.permissions) { + for (Permission permission : requestingUser.adminUser.permissions) { if (permission.canManage(this.getClass())) return true; } } // Fallback to Model#userCanManage. - return super.canBeManagedBy(user); + return super.canBeManagedBy(requestingUser); } private Bson tripIdFilter() { @@ -391,16 +397,6 @@ public Map parseQueryParams() throws URISyntaxException { ).stream().collect(Collectors.toMap(NameValuePair::getName, NameValuePair::getValue)); } - /** - * Check if the trip is planned with the target time being an arriveBy or departAt query. - * - * @return true, if the trip's target time is for an arriveBy query - */ - public boolean isArriveBy() throws URISyntaxException { - // if arriveBy is not included in query params, OTP will default to false, so initialize to false - return parseQueryParams().getOrDefault("arriveBy", "false").equals("true"); - } - /** * Returns the target hour of the day that the trip is either departing at or arriving by */ diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index 1775c8678..189845d84 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -1,6 +1,8 @@ package org.opentripplanner.middleware.models; import com.fasterxml.jackson.annotation.JsonIgnore; +import org.opentripplanner.middleware.auth.Auth0Users; +import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.persistence.Persistence; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,6 +44,7 @@ public class OtpUser extends AbstractUser { public boolean storeTripHistory; @JsonIgnore + /** If this user was created by an {@link ApiUser}, this parameter will match the {@link ApiUser}'s id */ public String applicationId; @Override @@ -67,7 +70,8 @@ public boolean delete(boolean deleteAuth0User) { } } - if (deleteAuth0User) { + // Only attempt to delete Auth0 user if they exist within Auth0 tenant. + if (deleteAuth0User && Auth0Users.getUserByEmail(email, false) != null) { boolean auth0UserDeleted = super.delete(); if (!auth0UserDeleted) { LOG.warn("Aborting user deletion for {}", this.email); @@ -77,4 +81,18 @@ public boolean delete(boolean deleteAuth0User) { return Persistence.otpUsers.removeById(this.id); } + + /** + * Confirm that the requesting user has the required permissions + */ + @Override + public boolean canBeManagedBy(RequestingUser requestingUser) { + if (requestingUser.apiUser != null && requestingUser.apiUser.id.equals(applicationId)) { + // Otp user was created by this Api user (first or third party). + return true; + } + // Fallback to Model#userCanManage. + return super.canBeManagedBy(requestingUser); + } + } diff --git a/src/main/java/org/opentripplanner/middleware/tripMonitor/Main.java b/src/main/java/org/opentripplanner/middleware/tripMonitor/Main.java index 8772f768f..d2700a2e7 100644 --- a/src/main/java/org/opentripplanner/middleware/tripMonitor/Main.java +++ b/src/main/java/org/opentripplanner/middleware/tripMonitor/Main.java @@ -8,10 +8,12 @@ import java.io.IOException; import java.util.concurrent.TimeUnit; +import static org.opentripplanner.middleware.utils.ConfigUtils.loadConfig; + public class Main { public static void main(String[] args) throws IOException { // Load configuration. - ConfigUtils.loadConfig(args); + loadConfig(args); // Connect to MongoDB. Persistence.initialize(); diff --git a/src/main/java/org/opentripplanner/middleware/tripMonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripMonitor/jobs/CheckMonitoredTrip.java index 7c878adc0..ac1f9344e 100644 --- a/src/main/java/org/opentripplanner/middleware/tripMonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripMonitor/jobs/CheckMonitoredTrip.java @@ -14,7 +14,6 @@ import org.opentripplanner.middleware.otp.response.LocalizedAlert; import org.opentripplanner.middleware.otp.response.OtpResponse; import org.opentripplanner.middleware.persistence.Persistence; -import org.opentripplanner.middleware.utils.ConfigUtils; import org.opentripplanner.middleware.utils.DateTimeUtils; import org.opentripplanner.middleware.utils.NotificationUtils; import org.slf4j.Logger; diff --git a/src/main/java/org/opentripplanner/middleware/utils/DateTimeUtils.java b/src/main/java/org/opentripplanner/middleware/utils/DateTimeUtils.java index 665aff5d6..20be99a19 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/DateTimeUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/DateTimeUtils.java @@ -16,6 +16,8 @@ import java.time.temporal.ChronoField; import java.util.Date; +import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; + /** * Date and time specific utils. All timing in this application should be obtained by using this method in order to * ensure that the correct system clock is used. During testing, the internal clock is often set to a fixed instant to @@ -144,7 +146,7 @@ public static ZoneId getSystemZoneId() { * timezone identifier of the first agency that it finds. */ public static ZoneId getOtpZoneId() { - String otpTzId = ConfigUtils.getConfigPropertyAsText("OTP_TIMEZONE"); + String otpTzId = getConfigPropertyAsText("OTP_TIMEZONE"); if (otpTzId == null) { throw new RuntimeException("OTP_TIMEZONE is not defined in config!"); } diff --git a/src/main/java/org/opentripplanner/middleware/utils/JsonUtils.java b/src/main/java/org/opentripplanner/middleware/utils/JsonUtils.java index 488ae65e7..88f4b188c 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/JsonUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/JsonUtils.java @@ -1,11 +1,14 @@ package org.opentripplanner.middleware.utils; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.fasterxml.jackson.databind.type.CollectionType; import org.opentripplanner.middleware.bugsnag.BugsnagReporter; +import org.opentripplanner.middleware.controllers.response.ResponseList; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import spark.HaltException; @@ -69,12 +72,20 @@ public static T getPOJOFromJSON(String json, Class clazz) { return null; } + /** + * Utility method to parse a string representing a {@link ResponseList} correctly into its parameterized type. + */ + public static ResponseList getResponseListFromJSON(String json, Class contentClass) throws JsonProcessingException { + JavaType type = mapper.getTypeFactory().constructParametricType(ResponseList.class, contentClass); + return mapper.readValue(json, type); + } + /** * Utility method to parse generic objects from JSON String and return as list */ public static List getPOJOFromJSONAsList(String json, Class clazz) { try { - JavaType type = mapper.getTypeFactory().constructCollectionType(List.class, clazz); + CollectionType type = mapper.getTypeFactory().constructCollectionType(List.class, clazz); return mapper.readValue(json, type); } catch (JsonProcessingException e) { BugsnagReporter.reportErrorToBugsnag( @@ -147,11 +158,4 @@ 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 cf959268b..fbb1f8612 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -80,6 +80,11 @@ paths: required: false type: "string" default: "0" + - name: "userId" + in: "query" + description: "If specified, the required user id." + required: false + type: "string" responses: "200": description: "successful operation" @@ -212,6 +217,29 @@ paths: description: "successful operation" schema: $ref: "#/definitions/ApiUser" + /api/secure/application/authenticate: + post: + tags: + - "api/secure/application" + description: "Authenticates ApiUser with Auth0." + produces: + - "application/json" + parameters: + - name: "username" + in: "query" + description: "Auth0 username (usually email address)." + required: false + type: "string" + - name: "password" + in: "query" + description: "Auth0 password." + required: false + type: "string" + responses: + "200": + description: "successful operation" + schema: + $ref: "#/definitions/TokenHolder" /api/secure/application/fromtoken: get: tags: @@ -257,6 +285,11 @@ paths: required: false type: "string" default: "0" + - name: "userId" + in: "query" + description: "If specified, the required user id." + required: false + type: "string" responses: "200": description: "successful operation" @@ -381,6 +414,11 @@ paths: required: false type: "string" default: "0" + - name: "userId" + in: "query" + description: "If specified, the required user id." + required: false + type: "string" responses: "200": description: "successful operation" @@ -600,6 +638,11 @@ paths: required: false type: "string" default: "0" + - name: "userId" + in: "query" + description: "If specified, the required user id." + required: false + type: "string" responses: "200": description: "successful operation" @@ -754,7 +797,13 @@ paths: produces: - "application/json" - "application/xml" - parameters: [] + parameters: + - name: "userId" + in: "query" + description: "If a third-party application is making a trip plan request on\ + \ behalf of an end user (OtpUser), the user id must be specified." + required: false + type: "string" responses: "200": description: "successful operation" @@ -822,6 +871,20 @@ definitions: type: "boolean" name: type: "string" + TokenHolder: + type: "object" + properties: + accessToken: + type: "string" + idToken: + type: "string" + refreshToken: + type: "string" + tokenType: + type: "string" + expiresIn: + type: "integer" + format: "int64" EncodedPolyline: type: "object" properties: diff --git a/src/test/java/org/opentripplanner/middleware/ApiKeyManagementTest.java b/src/test/java/org/opentripplanner/middleware/ApiKeyManagementTest.java index 4525c0e62..37c1b9a71 100644 --- a/src/test/java/org/opentripplanner/middleware/ApiKeyManagementTest.java +++ b/src/test/java/org/opentripplanner/middleware/ApiKeyManagementTest.java @@ -22,7 +22,11 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; import static org.opentripplanner.middleware.TestUtils.isEndToEnd; +import static org.opentripplanner.middleware.TestUtils.mockAuthenticatedDelete; import static org.opentripplanner.middleware.TestUtils.mockAuthenticatedRequest; +import static org.opentripplanner.middleware.auth.Auth0Connection.getDefaultAuthDisabled; +import static org.opentripplanner.middleware.auth.Auth0Connection.isAuthDisabled; +import static org.opentripplanner.middleware.auth.Auth0Connection.restoreDefaultAuthDisabled; import static org.opentripplanner.middleware.auth.Auth0Connection.setAuthDisabled; import static org.opentripplanner.middleware.controllers.api.ApiUserController.DEFAULT_USAGE_PLAN_ID; @@ -63,8 +67,9 @@ public static void tearDown() { assumeTrue(isEndToEnd); // refresh API key(s) apiUser = Persistence.apiUsers.getById(apiUser.id); - apiUser.delete(false); + apiUser.delete(); Persistence.adminUsers.removeById(adminUser.id); + restoreDefaultAuthDisabled(); } /** @@ -157,7 +162,7 @@ private boolean ensureApiKeyExists() { */ private HttpResponse createApiKeyRequest(String targetUserId, AbstractUser requestingUser) { String path = String.format("api/secure/application/%s/apikey", targetUserId); - return mockAuthenticatedRequest(path, requestingUser, HttpUtils.REQUEST_METHOD.POST); + return mockAuthenticatedRequest(path, "", requestingUser, HttpUtils.REQUEST_METHOD.POST); } /** @@ -165,6 +170,6 @@ private HttpResponse createApiKeyRequest(String targetUserId, AbstractUs */ private static HttpResponse deleteApiKeyRequest(String targetUserId, String apiKeyId, AbstractUser requestingUser) { String path = String.format("api/secure/application/%s/apikey/%s", targetUserId, apiKeyId); - return mockAuthenticatedRequest(path, requestingUser, HttpUtils.REQUEST_METHOD.DELETE); + return mockAuthenticatedDelete(path, requestingUser); } } diff --git a/src/test/java/org/opentripplanner/middleware/ApiUserFlowTest.java b/src/test/java/org/opentripplanner/middleware/ApiUserFlowTest.java index 020cc741e..83b82f339 100644 --- a/src/test/java/org/opentripplanner/middleware/ApiUserFlowTest.java +++ b/src/test/java/org/opentripplanner/middleware/ApiUserFlowTest.java @@ -1,25 +1,21 @@ package org.opentripplanner.middleware; import com.auth0.exception.Auth0Exception; +import com.auth0.json.auth.TokenHolder; import com.auth0.json.mgmt.users.User; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.type.TypeReference; -import com.fasterxml.jackson.databind.ObjectMapper; 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.controllers.response.ResponseList; -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.otp.OtpDispatcherResponse; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.persistence.PersistenceUtil; import org.opentripplanner.middleware.utils.CreateApiKeyException; -import org.opentripplanner.middleware.utils.FileUtils; import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.JsonUtils; import org.slf4j.Logger; @@ -28,19 +24,23 @@ import java.io.IOException; import java.net.URISyntaxException; import java.net.http.HttpResponse; +import java.util.HashMap; 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.TEMP_AUTH0_USER_PASSWORD; +import static org.opentripplanner.middleware.TestUtils.makeDeleteRequest; +import static org.opentripplanner.middleware.TestUtils.makeGetRequest; +import static org.opentripplanner.middleware.TestUtils.makeRequest; import static org.opentripplanner.middleware.TestUtils.isEndToEnd; -import static org.opentripplanner.middleware.TestUtils.mockAuthenticatedPost; +import static org.opentripplanner.middleware.TestUtils.mockAuthenticatedGet; import static org.opentripplanner.middleware.TestUtils.mockAuthenticatedRequest; import static org.opentripplanner.middleware.auth.Auth0Connection.isAuthDisabled; 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_PROXY_ENDPOINT; -import static org.opentripplanner.middleware.otp.OtpDispatcherResponseTest.DEFAULT_PLAN_URI; import static org.opentripplanner.middleware.otp.OtpDispatcher.OTP_PLAN_ENDPOINT; /** @@ -70,6 +70,9 @@ public class ApiUserFlowTest { private static final Logger LOG = LoggerFactory.getLogger(ApiUserFlowTest.class); private static ApiUser apiUser; private static OtpUser otpUser; + private static OtpUser otpUserMatchingApiUser; + private static final String OTP_USER_PATH = "api/secure/user"; + private static final String MONITORED_TRIP_PATH = "api/secure/monitoredtrip"; /** * Whether tests for this class should run. End to End must be enabled and Auth must NOT be disabled. This should be @@ -81,7 +84,7 @@ private static boolean testsShouldRun() { } /** - * Create an {@link ApiUser} and an {@link AdminUser} prior to unit tests + * Create an {@link ApiUser} and an {@link OtpUser} prior to unit tests */ @BeforeAll public static void setUp() throws IOException, InterruptedException, CreateApiKeyException { @@ -98,16 +101,17 @@ public static void setUp() throws IOException, InterruptedException, CreateApiKe 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); + // create Auth0 user for apiUser. + User auth0User = createAuth0UserForEmail(apiUser.email, 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(); + // create Otp user matching Api user to aid with testing edge cases. + otpUserMatchingApiUser = new OtpUser(); + otpUserMatchingApiUser.email = apiUser.email; + otpUserMatchingApiUser.auth0UserId = apiUser.auth0UserId; + Persistence.otpUsers.create(otpUserMatchingApiUser); } catch (Auth0Exception e) { throw new RuntimeException(e); } @@ -122,68 +126,151 @@ public static void tearDown() { apiUser = Persistence.apiUsers.getById(apiUser.id); if (apiUser != null) apiUser.delete(); otpUser = Persistence.otpUsers.getById(otpUser.id); - if (otpUser != null) otpUser.delete(); + if (otpUser != null) otpUser.delete(false); + otpUserMatchingApiUser = Persistence.otpUsers.getById(otpUserMatchingApiUser.id); + if (otpUserMatchingApiUser != null) otpUserMatchingApiUser.delete(false); } /** * 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. + * records. This also includes Auth0 users if auth is enabled. The basic script for this test is as follows: + * 1. Start with existing API User that has a valid API key. + * 2. Use the API key to make a request to the authenticate endpoint to get Auth0 token for further requests. + * 3. Create OTP user. + * 4. Make subsequent requests on behalf of OTP users. + * 5. Delete user and verify that their associated objects are also deleted. */ @Test - public void canSimulateApiUserFlow() throws IOException, URISyntaxException { - // create otp user as api user - HttpResponse createUserResponse = mockAuthenticatedPost("api/secure/user", - apiUser, - JsonUtils.toJson(otpUser) + public void canSimulateApiUserFlow() throws URISyntaxException, JsonProcessingException { + + // Define the header values to be used in requests from this point forward. + HashMap apiUserHeaders = new HashMap<>(); + apiUserHeaders.put("x-api-key", apiUser.apiKeys.get(0).value); + // obtain Auth0 token for Api user. + String authenticateEndpoint = String.format("api/secure/application/authenticate?username=%s&password=%s", + apiUser.email, + TEMP_AUTH0_USER_PASSWORD); + HttpResponse getTokenResponse = makeRequest(authenticateEndpoint, + "", + apiUserHeaders, + HttpUtils.REQUEST_METHOD.POST ); - assertEquals(HttpStatus.OK_200, createUserResponse.statusCode()); - OtpUser otpUserResponse = JsonUtils.getPOJOFromJSON(createUserResponse.body(), OtpUser.class); + // Note: do not log the Auth0 token (could be a security risk). + LOG.info("Token response status: {}", getTokenResponse.statusCode()); + assertEquals(HttpStatus.OK_200, getTokenResponse.statusCode()); + TokenHolder tokenHolder = JsonUtils.getPOJOFromJSON(getTokenResponse.body(), TokenHolder.class); - // Generate mock response to create monitored trip - // FIXME: Refactor once the method to store monitored trip is refactored. - String mockResponse = FileUtils.getFileContents( - "src/test/resources/org/opentripplanner/middleware/persistence/planResponse.json" + // Define the bearer value to be used in requests from this point forward. + apiUserHeaders.put("Authorization", "Bearer " + tokenHolder.getAccessToken()); + + // create an Otp user authenticating as an Api user. + HttpResponse createUserResponse = makeRequest(OTP_USER_PATH, + JsonUtils.toJson(otpUser), + apiUserHeaders, + HttpUtils.REQUEST_METHOD.POST ); - OtpDispatcherResponse otpDispatcherResponse = new OtpDispatcherResponse(mockResponse, DEFAULT_PLAN_URI); + + assertEquals(HttpStatus.OK_200, createUserResponse.statusCode()); + + // Request all Otp users created by an Api user. This will work and return all Otp users. + HttpResponse getAllOtpUsersCreatedByApiUser = makeGetRequest(OTP_USER_PATH, apiUserHeaders); + assertEquals(HttpStatus.OK_200, getAllOtpUsersCreatedByApiUser.statusCode()); + ResponseList otpUsers = JsonUtils.getResponseListFromJSON(getAllOtpUsersCreatedByApiUser.body(), OtpUser.class); + assertEquals(1, otpUsers.total); + + // Attempt to create a monitored trip for an Otp user using mock authentication. This will fail because the user + // was created by an Api user and therefore does not have a Auth0 account. + OtpUser otpUserResponse = JsonUtils.getPOJOFromJSON(createUserResponse.body(), OtpUser.class); // Create a monitored trip for the Otp user (API users are prevented from doing this). MonitoredTrip monitoredTrip = new MonitoredTrip(TestUtils.sendSamplePlanRequest()); + monitoredTrip.updateAllDaysOfWeek(true); monitoredTrip.userId = otpUser.id; - HttpResponse createTripResponse = mockAuthenticatedPost("api/secure/monitoredtrip", + HttpResponse createTripResponseAsOtpUser = mockAuthenticatedRequest( + MONITORED_TRIP_PATH, + JsonUtils.toJson(monitoredTrip), otpUserResponse, - JsonUtils.toJson(monitoredTrip) + HttpUtils.REQUEST_METHOD.POST ); - assertEquals(HttpStatus.OK_200, createTripResponse.statusCode()); - MonitoredTrip monitoredTripResponse = JsonUtils.getPOJOFromJSON(createTripResponse.body(), MonitoredTrip.class); + assertEquals(HttpStatus.UNAUTHORIZED_401, createTripResponseAsOtpUser.statusCode()); - // Plan trip with OTP proxy. Mock plan response will be returned - String otpQuery = OTP_PROXY_ENDPOINT + OTP_PLAN_ENDPOINT + "?fromPlace=28.45119,-81.36818&toPlace=28.54834,-81.37745"; - HttpResponse planTripResponse = mockAuthenticatedRequest(otpQuery, - otpUserResponse, - HttpUtils.REQUEST_METHOD.GET + // Create a monitored trip for an Otp user authenticating as an Api user. An Api user can create a monitored + // trip for an Otp user they created. + HttpResponse createTripResponseAsApiUser = makeRequest( + MONITORED_TRIP_PATH, + JsonUtils.toJson(monitoredTrip), + apiUserHeaders, + HttpUtils.REQUEST_METHOD.POST + ); + assertEquals(HttpStatus.OK_200, createTripResponseAsApiUser.statusCode()); + MonitoredTrip monitoredTripResponse = JsonUtils.getPOJOFromJSON( + createTripResponseAsApiUser.body(), + MonitoredTrip.class ); - 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 + // Request all monitored trips for an Otp user authenticating as an Api user. This will work and return all trips + // matching the user id provided. + HttpResponse getAllMonitoredTripsForOtpUser = makeGetRequest( + String.format("api/secure/monitoredtrip?userId=%s", otpUserResponse.id), + apiUserHeaders ); - assertEquals(HttpStatus.OK_200, tripRequestResponse.statusCode()); - // Special handling of deserialization for ResponseList to avoid class cast issues encountered with JsonUtils - // methods. - ObjectMapper mapper = new ObjectMapper(); - ResponseList tripRequests = mapper.readValue(tripRequestResponse.body(), new TypeReference<>() {}); - - // Delete otp user. - HttpResponse deleteUserResponse = mockAuthenticatedRequest( - String.format("api/secure/user/%s", otpUserResponse.id), - otpUserResponse, - HttpUtils.REQUEST_METHOD.DELETE + assertEquals(HttpStatus.OK_200, getAllMonitoredTripsForOtpUser.statusCode()); + + // Request all monitored trips for an Otp user authenticating as an Api user, without defining the user id. This + // will fail because an Api user must provide a user id. + getAllMonitoredTripsForOtpUser = makeRequest(MONITORED_TRIP_PATH, + "", + apiUserHeaders, + HttpUtils.REQUEST_METHOD.GET ); - assertEquals(HttpStatus.OK_200, deleteUserResponse.statusCode()); + assertEquals(HttpStatus.BAD_REQUEST_400, getAllMonitoredTripsForOtpUser.statusCode()); + + + // Plan trip with OTP proxy authenticating as an OTP user. Mock plan response will be returned. This will work + // as an Otp user (created by MOD UI or an Api user) because the end point has no auth. A lack of auth also means + // the plan is not saved. + String otpQueryForOtpUserRequest = OTP_PROXY_ENDPOINT + + OTP_PLAN_ENDPOINT + + "?fromPlace=28.45119,-81.36818&toPlace=28.54834,-81.37745"; + HttpResponse planTripResponseAsOtpUser = mockAuthenticatedGet(otpQueryForOtpUserRequest, otpUserResponse); + LOG.info("OTP user: Plan trip response: {}\n....", planTripResponseAsOtpUser.body().substring(0, 300)); + assertEquals(HttpStatus.OK_200, planTripResponseAsOtpUser.statusCode()); + + + + // Plan trip with OTP proxy authenticating as an Api user. Mock plan response will be returned. This will work + // as an Api user because the end point has no auth. + String otpQueryForApiUserRequest = OTP_PROXY_ENDPOINT + + OTP_PLAN_ENDPOINT + + String.format("?fromPlace=28.45119,-81.36818&toPlace=28.54834,-81.37745&userId=%s",otpUserResponse.id); + HttpResponse planTripResponseAsApiUser = makeGetRequest(otpQueryForApiUserRequest, apiUserHeaders); + LOG.info("API user (on behalf of an Otp user): Plan trip response: {}\n....", planTripResponseAsApiUser.body().substring(0, 300)); + assertEquals(HttpStatus.OK_200, planTripResponseAsApiUser.statusCode()); + + // Get trip request history for user authenticating as an Otp user. This will fail because the user was created + // by an Api user and therefore does not have a Auth0 account. + String tripRequestsPath = String.format("api/secure/triprequests?userId=%s", otpUserResponse.id); + HttpResponse tripRequestResponseAsOtUser = mockAuthenticatedGet(tripRequestsPath, otpUserResponse); + + assertEquals(HttpStatus.UNAUTHORIZED_401, tripRequestResponseAsOtUser.statusCode()); + + // Get trip request history for user authenticating as an Api user. This will work because an Api user is able + // to get a trip on behalf of an Otp user they created. + HttpResponse tripRequestResponseAsApiUser = makeGetRequest(tripRequestsPath, apiUserHeaders); + assertEquals(HttpStatus.OK_200, tripRequestResponseAsApiUser.statusCode()); + + ResponseList tripRequests = JsonUtils.getResponseListFromJSON(tripRequestResponseAsApiUser.body(), TripRequest.class); + + // Delete Otp user authenticating as an Otp user. This will fail because the user was created by an Api user and + // therefore does not have a Auth0 account. + String otpUserPath = String.format("api/secure/user/%s", otpUserResponse.id); + HttpResponse deleteUserResponseAsOtpUser = mockAuthenticatedGet(otpUserPath, otpUserResponse); + assertEquals(HttpStatus.UNAUTHORIZED_401, deleteUserResponseAsOtpUser.statusCode()); + + // Delete Otp user authenticating as an Api user. This will work because an Api user can delete an Otp user they + // created. + HttpResponse deleteUserResponseAsApiUser = makeDeleteRequest(otpUserPath, apiUserHeaders); + assertEquals(HttpStatus.OK_200, deleteUserResponseAsApiUser.statusCode()); // Verify user no longer exists. OtpUser deletedOtpUser = Persistence.otpUsers.getById(otpUserResponse.id); @@ -194,14 +281,14 @@ public void canSimulateApiUserFlow() throws IOException, URISyntaxException { assertNull(deletedTrip); // Verify trip request no longer exists. - TripRequest tripRequest = Persistence.tripRequests.getById(tripRequests.data.get(0).id); - assertNull(tripRequest); + TripRequest tripRequestFromResponse = tripRequests.data.get(0); + TripRequest tripRequestFromDb = Persistence.tripRequests.getById(tripRequestFromResponse.id); + assertNull(tripRequestFromDb); // Delete API user (this would happen through the OTP Admin portal). - HttpResponse deleteApiUserResponse = mockAuthenticatedRequest( + HttpResponse deleteApiUserResponse = makeDeleteRequest( String.format("api/secure/application/%s", apiUser.id), - apiUser, - HttpUtils.REQUEST_METHOD.DELETE + apiUserHeaders ); assertEquals(HttpStatus.OK_200, deleteApiUserResponse.statusCode()); diff --git a/src/test/java/org/opentripplanner/middleware/GetMonitoredTripsTest.java b/src/test/java/org/opentripplanner/middleware/GetMonitoredTripsTest.java new file mode 100644 index 000000000..009240059 --- /dev/null +++ b/src/test/java/org/opentripplanner/middleware/GetMonitoredTripsTest.java @@ -0,0 +1,165 @@ +package org.opentripplanner.middleware; + +import com.auth0.exception.Auth0Exception; +import com.auth0.json.mgmt.users.User; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +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.auth.Auth0Users; +import org.opentripplanner.middleware.controllers.response.ResponseList; +import org.opentripplanner.middleware.models.AdminUser; +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.HttpUtils; +import org.opentripplanner.middleware.utils.JsonUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.net.URISyntaxException; +import java.net.http.HttpResponse; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assumptions.assumeTrue; +import static org.opentripplanner.middleware.TestUtils.TEMP_AUTH0_USER_PASSWORD; +import static org.opentripplanner.middleware.TestUtils.isEndToEnd; +import static org.opentripplanner.middleware.TestUtils.mockAuthenticatedGet; +import static org.opentripplanner.middleware.TestUtils.mockAuthenticatedRequest; +import static org.opentripplanner.middleware.auth.Auth0Connection.restoreDefaultAuthDisabled; +import static org.opentripplanner.middleware.auth.Auth0Connection.setAuthDisabled; + +/** + * Tests to simulate getting trips as an Otp user with enhanced admin credentials. 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. + * + * OTP_API_ROOT set to a live OTP instance (e.g. http://otp-server.example.com/otp). + * + * OTP_PLAN_ENDPOINT set to a live OTP plan endpoint (e.g. /routers/default/plan). + * + * 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 GetMonitoredTripsTest { + private static AdminUser multiAdminUser; + private static OtpUser soloOtpUser; + private static OtpUser multiOtpUser; + private static final String MONITORED_TRIP_PATH = "api/secure/monitoredtrip"; + private static final Logger LOG = LoggerFactory.getLogger(GetMonitoredTripsTest.class); + + /** + * Create Otp and Admin user accounts. Create Auth0 account for just the Otp users. If + * an Auth0 account is created for the admin user it will fail because the email address already exists. + */ + @BeforeAll + public static void setUp() throws IOException, InterruptedException { + // Load config before checking if tests should run. + OtpMiddlewareTest.setUp(); + assumeTrue(isEndToEnd); + setAuthDisabled(false); + // Mock the OTP server TODO: Run a live OTP instance? + TestUtils.mockOtpServer(); + String multiUserEmail = String.format("test-%s@example.com", UUID.randomUUID().toString()); + soloOtpUser = PersistenceUtil.createUser(String.format("test-%s@example.com", UUID.randomUUID().toString())); + multiOtpUser = PersistenceUtil.createUser(multiUserEmail); + multiAdminUser = PersistenceUtil.createAdminUser(multiUserEmail); + try { + // Should use Auth0User.createNewAuth0User but this generates a random password preventing the mock headers + // from being able to use TEMP_AUTH0_USER_PASSWORD. + User auth0User = Auth0Users.createAuth0UserForEmail(soloOtpUser.email, TEMP_AUTH0_USER_PASSWORD); + soloOtpUser.auth0UserId = auth0User.getId(); + Persistence.otpUsers.replace(soloOtpUser.id, soloOtpUser); + auth0User = Auth0Users.createAuth0UserForEmail(multiUserEmail, TEMP_AUTH0_USER_PASSWORD); + multiOtpUser.auth0UserId = auth0User.getId(); + Persistence.otpUsers.replace(multiOtpUser.id, multiOtpUser); + // Use the same Auth0 user id as otpUser2 as the email address is the same. + multiAdminUser.auth0UserId = auth0User.getId(); + Persistence.adminUsers.replace(multiAdminUser.id, multiAdminUser); + } 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(isEndToEnd); + restoreDefaultAuthDisabled(); + soloOtpUser = Persistence.otpUsers.getById(soloOtpUser.id); + if (soloOtpUser != null) soloOtpUser.delete(false); + multiOtpUser = Persistence.otpUsers.getById(multiOtpUser.id); + if (multiOtpUser != null) multiOtpUser.delete(false); + multiAdminUser = Persistence.adminUsers.getById(multiAdminUser.id); + if (multiAdminUser != null) multiAdminUser.delete(); + } + + /** + * Create trips for two different Otp users and attempt to get both trips with Otp user that has 'enhanced' admin + * credentials. + */ + @Test + public void canGetOwnMonitoredTrips() throws URISyntaxException, JsonProcessingException { + + // Create trip as Otp user 1. + MonitoredTrip monitoredTrip = new MonitoredTrip(TestUtils.sendSamplePlanRequest()); + monitoredTrip.updateAllDaysOfWeek(true); + monitoredTrip.userId = soloOtpUser.id; + HttpResponse createTrip1Response = mockAuthenticatedRequest(MONITORED_TRIP_PATH, + JsonUtils.toJson(monitoredTrip), + soloOtpUser, + HttpUtils.REQUEST_METHOD.POST + ); + assertEquals(HttpStatus.OK_200, createTrip1Response.statusCode()); + + // Create trip as Otp user 2. + monitoredTrip = new MonitoredTrip(TestUtils.sendSamplePlanRequest()); + monitoredTrip.updateAllDaysOfWeek(true); + monitoredTrip.userId = multiOtpUser.id; + HttpResponse createTripResponse2 = mockAuthenticatedRequest(MONITORED_TRIP_PATH, + JsonUtils.toJson(monitoredTrip), + multiOtpUser, + HttpUtils.REQUEST_METHOD.POST + ); + assertEquals(HttpStatus.OK_200, createTripResponse2.statusCode()); + + // Get trips for solo Otp user. + HttpResponse soloTripsResponse = mockAuthenticatedGet(MONITORED_TRIP_PATH, soloOtpUser); + ResponseList soloTrips = JsonUtils.getResponseListFromJSON(soloTripsResponse.body(), MonitoredTrip.class); + + // Expect only 1 trip for solo Otp user. + assertEquals(1, soloTrips.data.size()); + + // Get trips for multi Otp user/admin user. + HttpResponse multiTripsResponse = mockAuthenticatedGet(MONITORED_TRIP_PATH, multiOtpUser); + ResponseList multiTrips = JsonUtils.getResponseListFromJSON(multiTripsResponse.body(), MonitoredTrip.class); + + // Multi Otp user has 'enhanced' admin credentials both trips will be returned. The expectation here is that the UI + // will always provide the user id to prevent this (as with the next test). + // TODO: Determine if a separate admin endpoint should be maintained for getting all/combined trips. + assertEquals(2, multiTrips.data.size()); + + // Get trips for only the multi Otp user by specifying Otp user id. + HttpResponse tripsFilteredResponse = mockAuthenticatedGet( + String.format("api/secure/monitoredtrip?userId=%s", multiOtpUser.id), + multiOtpUser + ); + ResponseList tripsFiltered = JsonUtils.getResponseListFromJSON(tripsFilteredResponse.body(), MonitoredTrip.class); + // Just the trip for Otp user 2 will be returned. + assertEquals(1, tripsFiltered.data.size()); + } +} diff --git a/src/test/java/org/opentripplanner/middleware/TestUtils.java b/src/test/java/org/opentripplanner/middleware/TestUtils.java index 22fb92678..c3d85ff9c 100644 --- a/src/test/java/org/opentripplanner/middleware/TestUtils.java +++ b/src/test/java/org/opentripplanner/middleware/TestUtils.java @@ -1,12 +1,14 @@ package org.opentripplanner.middleware; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import org.opentripplanner.middleware.auth.Auth0UserProfile; +import org.opentripplanner.middleware.auth.Auth0Users; +import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.models.AbstractUser; import org.opentripplanner.middleware.models.ApiUser; +import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.otp.OtpDispatcher; import org.opentripplanner.middleware.otp.OtpDispatcherResponse; +import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.otp.response.OtpResponse; import org.opentripplanner.middleware.utils.FileUtils; import org.opentripplanner.middleware.utils.HttpUtils; @@ -25,7 +27,6 @@ import java.util.UUID; import static org.opentripplanner.middleware.auth.Auth0Connection.isAuthDisabled; -import static org.opentripplanner.middleware.auth.Auth0Users.getAuth0Token; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; import static org.opentripplanner.middleware.otp.OtpDispatcher.OTP_PLAN_ENDPOINT; import static spark.Service.ignite; @@ -35,11 +36,21 @@ public class TestUtils { private static final Logger LOG = LoggerFactory.getLogger(TestUtils.class); private static final ObjectMapper mapper = new ObjectMapper(); + /** + * Base URL for application running during testing. + */ + private static final String BASE_URL = "http://localhost:4567/"; + /** * Password used to create and validate temporary Auth0 users */ static final String TEMP_AUTH0_USER_PASSWORD = UUID.randomUUID().toString(); + /** + * x-api-key used when auth is disabled. + */ + static final String TEMP_X_API_KEY = UUID.randomUUID().toString(); + /** * Whether the end-to-end environment variable is enabled. */ @@ -75,37 +86,6 @@ public static T getResourceFileContentsAsJSON(String resourcePathName, Class ); } - /** - * 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") && isAuthDisabled(); - } - - /** - * 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, 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), - 1000, - requestMethod, - headers, - "" - ); - } - /** * 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. @@ -116,16 +96,10 @@ private static HashMap getMockHeaders(AbstractUser requestingUse // the request when received. if (isAuthDisabled()) { 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); + headers.put("x-api-key", TEMP_X_API_KEY); + return headers; } + // 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; @@ -133,25 +107,87 @@ private static HashMap getMockHeaders(AbstractUser requestingUse headers.put("x-api-key", apiUser.apiKeys.get(0).value); } } + + // If requester is an Otp user which was created by an Api user, return empty header because an Otp user created + // by an Api user can not directly access the middleware. + if (requestingUser instanceof OtpUser) { + OtpUser otpUserFromDB = Persistence.otpUsers.getById(requestingUser.id); + if (otpUserFromDB != null && otpUserFromDB.applicationId != null) { + return headers; + } + } + + // Otherwise, get a valid oauth token for the user + headers.put("Authorization", "Bearer " + Auth0Users.getAuth0AccessToken(requestingUser.email, TEMP_AUTH0_USER_PASSWORD)); 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. + * Send request to provided URL. */ - public static HttpResponse mockAuthenticatedPost(String path, AbstractUser requestingUser, String body) { - HashMap headers = getMockHeaders(requestingUser); - + static HttpResponse makeRequest(String path, String body, HashMap headers, + HttpUtils.REQUEST_METHOD requestMethod) { return HttpUtils.httpRequestRawResponse( - URI.create("http://localhost:4567/" + path), + URI.create(BASE_URL + path), 1000, - HttpUtils.REQUEST_METHOD.POST, + requestMethod, headers, body ); } + /** + * Send 'get' request to provided URL. + */ + static HttpResponse makeGetRequest(String path, HashMap headers) { + return HttpUtils.httpRequestRawResponse( + URI.create(BASE_URL + path), + 1000, + HttpUtils.REQUEST_METHOD.GET, + headers, + "" + ); + } + + /** + * Send 'delete' request to provided URL. + */ + static HttpResponse makeDeleteRequest(String path, HashMap headers) { + return HttpUtils.httpRequestRawResponse( + URI.create(BASE_URL + path), + 1000, + HttpUtils.REQUEST_METHOD.DELETE, + headers, + "" + ); + } + + /** + * Construct http headers according to caller request and then make an authenticated call by placing the Auth0 user + * id in the headers so that {@link RequestingUser} can check the database for a matching user. + */ + static HttpResponse mockAuthenticatedRequest(String path, String body, AbstractUser requestingUser, + HttpUtils.REQUEST_METHOD requestMethod) { + return makeRequest(path, body, getMockHeaders(requestingUser), requestMethod); + } + + /** + * Construct http headers according to caller request and then make an authenticated 'get' call. + */ + public static HttpResponse mockAuthenticatedGet(String path, AbstractUser requestingUser) { + return makeGetRequest(path, getMockHeaders(requestingUser)); + } + + /** + * Construct http headers according to caller request and then make an authenticated call by placing the Auth0 user + * id in the headers so that {@link RequestingUser} can check the database for a matching user. + */ + static HttpResponse mockAuthenticatedDelete(String path, AbstractUser requestingUser) { + return makeDeleteRequest(path, getMockHeaders(requestingUser)); + } + + /** * Configure a mock OTP server for providing mock OTP responses. Note: this expects the config value * OTP_API_ROOT=http://localhost:8080/otp diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java index fed99fa47..662f69465 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java @@ -9,7 +9,6 @@ import org.opentripplanner.middleware.OtpMiddlewareTest; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.persistence.Persistence; -import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.JsonUtils; import java.io.IOException; @@ -19,24 +18,20 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.opentripplanner.middleware.TestUtils.mockAuthenticatedRequest; -import static org.opentripplanner.middleware.auth.Auth0Connection.isAuthDisabled; +import static org.opentripplanner.middleware.TestUtils.mockAuthenticatedGet; +import static org.opentripplanner.middleware.auth.Auth0Connection.restoreDefaultAuthDisabled; import static org.opentripplanner.middleware.auth.Auth0Connection.setAuthDisabled; public class OtpUserControllerTest { private static final String INITIAL_PHONE_NUMBER = "+15555550222"; // Fake US 555 number. private static OtpUser otpUser; - private static boolean originalIsAuthDisabled; @BeforeAll public static void setUp() throws IOException, InterruptedException { // Load config. OtpMiddlewareTest.setUp(); - - // Save original isAuthDisabled state to restore it on tear down. - originalIsAuthDisabled = isAuthDisabled(); + // Ensure auth is disabled. setAuthDisabled(true); - // Create a persisted OTP user. otpUser = new OtpUser(); otpUser.email = String.format("test-%s@example.com", UUID.randomUUID().toString()); @@ -53,7 +48,7 @@ public static void tearDown() { if (otpUser != null) otpUser.delete(); // Restore original isAuthDisabled state. - setAuthDisabled(originalIsAuthDisabled); + restoreDefaultAuthDisabled(); } /** @@ -65,22 +60,20 @@ public static void tearDown() { public void invalidNumbersShouldProduceBadRequest(String badNumber, int statusCode) { // 1. Request verification SMS. // The invalid number should fail the call. - HttpResponse response = mockAuthenticatedRequest( + HttpResponse response = mockAuthenticatedGet( String.format("api/secure/user/%s/verify_sms/%s", otpUser.id, badNumber ), - otpUser, - HttpUtils.REQUEST_METHOD.GET + otpUser ); assertEquals(statusCode, response.statusCode()); // 2. Fetch the newly-created user. // The phone number should not be updated. - HttpResponse otpUserWithPhoneRequest = mockAuthenticatedRequest( + HttpResponse otpUserWithPhoneRequest = mockAuthenticatedGet( String.format("api/secure/user/%s", otpUser.id), - otpUser, - HttpUtils.REQUEST_METHOD.GET + otpUser ); assertEquals(HttpStatus.OK_200, otpUserWithPhoneRequest.statusCode()); diff --git a/src/test/java/org/opentripplanner/middleware/persistence/PersistenceUtil.java b/src/test/java/org/opentripplanner/middleware/persistence/PersistenceUtil.java index 884623d47..081d8485c 100644 --- a/src/test/java/org/opentripplanner/middleware/persistence/PersistenceUtil.java +++ b/src/test/java/org/opentripplanner/middleware/persistence/PersistenceUtil.java @@ -38,6 +38,8 @@ public static OtpUser createUser(String email, String phoneNumber) { user.email = email; user.phoneNumber = phoneNumber; user.notificationChannel = "email"; + user.hasConsentedToTerms = true; + user.storeTripHistory = true; Persistence.otpUsers.create(user); return user; } diff --git a/src/test/java/org/opentripplanner/middleware/persistence/TripHistoryPersistenceTest.java b/src/test/java/org/opentripplanner/middleware/persistence/TripHistoryPersistenceTest.java index d8dbf6c04..edc587adc 100644 --- a/src/test/java/org/opentripplanner/middleware/persistence/TripHistoryPersistenceTest.java +++ b/src/test/java/org/opentripplanner/middleware/persistence/TripHistoryPersistenceTest.java @@ -92,8 +92,6 @@ public void canDeleteTripSummary() { @Test public void canGetFilteredTripRequestsWithFromAndToDate() { - OtpUser user = createUser(TEST_EMAIL); - List tripRequests = createTripRequests(LIMIT, user.id); LocalDateTime fromStartOfDay = DateTimeUtils.nowAsLocalDate().atTime(LocalTime.MIN); LocalDateTime toEndOfDay = DateTimeUtils.nowAsLocalDate().atTime(LocalTime.MAX); Date fromDate = Date.from(fromStartOfDay @@ -102,7 +100,7 @@ public void canGetFilteredTripRequestsWithFromAndToDate() { Date toDate = Date.from(toEndOfDay .atZone(DateTimeUtils.getSystemZoneId()) .toInstant()); - Bson filter = filterByUserAndDateRange(user.id, fromDate, toDate); + Bson filter = filterByUserAndDateRange(otpUser.id, fromDate, toDate); ResponseList result = Persistence.tripRequests.getResponseList(filter, 0, LIMIT); assertEquals(result.data.size(), tripRequests.size()); }