-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor api user check auth0 #84
Changes from 33 commits
e3f5ea9
385e20b
07b78c2
1f60f1e
cd7a668
c469e2c
0bf0e18
36855ca
ca3e5ce
b7758a8
777e218
4bd181a
7685977
d94f0e5
7e54d28
6e2df15
efad45f
d989ff9
befa0c1
ce30456
382c977
790e3fe
ae7bc47
3954147
86581bb
feef7f5
6b504c2
7729deb
bb80a15
1b57450
6edd37d
ec7d369
b16534a
31aea84
1c00d7a
9072f1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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<? extends AbstractUser> 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,16 +126,15 @@ public static boolean isAuthHeaderPresent(Request req) { | |
return authHeader != null; | ||
} | ||
|
||
|
||
/** | ||
* Assign user to request and check that the user is an admin. | ||
*/ | ||
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,35 @@ public static void checkUserIsAdmin(Request req, Response res) { | |
} | ||
|
||
/** | ||
* Check if the incoming user is an admin user | ||
* Check that an {@link ApiUser} is linked to an Api key. | ||
*/ | ||
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 linkApiKeyToApiUser(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."); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check appears to have been replaced with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is fine. All API calls (except to /auth) call |
||
|
||
/** | ||
* 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 +229,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 +241,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."); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -25,9 +26,8 @@ | |
import java.util.UUID; | ||
|
||
import static com.mongodb.client.model.Filters.eq; | ||
import static org.opentripplanner.middleware.utils.ConfigUtils.getBooleanEnvVar; | ||
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; | ||
|
||
/** | ||
|
@@ -42,9 +42,15 @@ public class Auth0Users { | |
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; | ||
|
||
/** | ||
* Whether the end-to-end environment variable is enabled. | ||
*/ | ||
private static final boolean isEndToEnd = getBooleanEnvVar("RUN_E2E"); | ||
|
||
/** | ||
* Cached API token so that we do not have to request a new one each time a Management API request is made. | ||
*/ | ||
|
@@ -53,8 +59,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 +248,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<String> getCompleteAuth0TokenResponse(String username, String password) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both downstream methods calling this method simply parse it into a TokenHolder. Why not just handle that within this method and change its return type? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because `ApiUserController.authenticateAuth0User' will provide the status code (produced by Auth0) to the user in the event of failure. I could update this to a fixed status code (403?) and then make your suggested update? |
||
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<String> 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<String> 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(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With respect to the method name, maybe just
ensureApiUserHasApiKey
? Has this logic changed from its previous iteration? I think, as it stands, this check will cause a halt under the following conditions:Perhaps the way to handle this is with token scopes that we provide in the
getCompleteAuth0TokenResponse
method. Let's discuss on our technical call tomorrow.Javadoc: