From 0fc695a4c1a09ef9d33ea2fd0658f6dece989381 Mon Sep 17 00:00:00 2001 From: Dan Date: Mon, 3 Apr 2023 12:58:26 +0100 Subject: Second pass --- build.gradle | 1 - .../org/traccar/api/resource/SessionResource.java | 4 +- .../java/org/traccar/database/OpenIdProvider.java | 75 +++++++++------------- traccar-web | 2 +- 4 files changed, 34 insertions(+), 48 deletions(-) diff --git a/build.gradle b/build.gradle index ff5004c77..c29f3ba26 100644 --- a/build.gradle +++ b/build.gradle @@ -87,7 +87,6 @@ dependencies { implementation "redis.clients:jedis:4.3.1" implementation "com.google.firebase:firebase-admin:9.1.1" implementation "com.nimbusds:oauth2-oidc-sdk:10.7.1" - implementation "net.minidev:json-smart:2.4.10" testImplementation "org.junit.jupiter:junit-jupiter-api:$junitVersion" testImplementation "org.junit.jupiter:junit-jupiter-engine:$junitVersion" testImplementation "org.mockito:mockito-core:5.1.1" diff --git a/src/main/java/org/traccar/api/resource/SessionResource.java b/src/main/java/org/traccar/api/resource/SessionResource.java index 3de20b8c7..a20e5f100 100644 --- a/src/main/java/org/traccar/api/resource/SessionResource.java +++ b/src/main/java/org/traccar/api/resource/SessionResource.java @@ -176,11 +176,11 @@ public class SessionResource extends BaseResource { @PermitAll @Path("openid/callback") @GET - public Response requestToken() throws IOException, StorageException, ParseException { + public Response requestToken() throws IOException, StorageException, ParseException, GeneralSecurityException { StringBuilder requestUrl = new StringBuilder(request.getRequestURL().toString()); String queryString = request.getQueryString(); String requestUri = requestUrl.append('?').append(queryString).toString(); - return openIdProvider.handleCallback(URI.create(requestUri), request); + return Response.seeOther(openIdProvider.handleCallback(URI.create(requestUri), request)).build(); } } diff --git a/src/main/java/org/traccar/database/OpenIdProvider.java b/src/main/java/org/traccar/database/OpenIdProvider.java index fc1409d14..6f44d0e80 100644 --- a/src/main/java/org/traccar/database/OpenIdProvider.java +++ b/src/main/java/org/traccar/database/OpenIdProvider.java @@ -26,11 +26,12 @@ import org.traccar.helper.ServletHelper; import java.net.URI; import java.net.URISyntaxException; +import java.security.GeneralSecurityException; import java.io.IOException; import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.WebApplicationException; -import javax.ws.rs.core.Response; import com.google.inject.Inject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.nimbusds.oauth2.sdk.http.HTTPResponse; import com.nimbusds.oauth2.sdk.AuthorizationCode; @@ -40,7 +41,6 @@ import com.nimbusds.oauth2.sdk.AuthorizationGrant; import com.nimbusds.oauth2.sdk.TokenRequest; import com.nimbusds.oauth2.sdk.TokenResponse; import com.nimbusds.oauth2.sdk.AuthorizationCodeGrant; -import com.nimbusds.oauth2.sdk.AuthorizationErrorResponse; import com.nimbusds.oauth2.sdk.ParseException; import com.nimbusds.oauth2.sdk.AuthorizationResponse; import com.nimbusds.oauth2.sdk.auth.Secret; @@ -59,9 +59,11 @@ import com.nimbusds.openid.connect.sdk.AuthenticationRequest; import com.nimbusds.openid.connect.sdk.claims.UserInfo; public class OpenIdProvider { + private static final Logger LOGGER = LoggerFactory.getLogger(OpenIdProvider.class); + public final Boolean force; private final ClientID clientId; - private final Secret clientSecret; + private final ClientAuthentication clientAuth; private URI callbackUrl; private URI authUrl; private URI tokenUrl; @@ -72,12 +74,12 @@ public class OpenIdProvider { private LoginService loginService; @Inject - public OpenIdProvider(Config config, LoginService loginService) { + public OpenIdProvider(Config config, LoginService loginService) { + this.loginService = loginService; + force = config.getBoolean(Keys.OPENID_FORCE); clientId = new ClientID(config.getString(Keys.OPENID_CLIENTID)); - clientSecret = new Secret(config.getString(Keys.OPENID_CLIENTSECRET)); - - this.loginService = loginService; + clientAuth = new ClientSecretBasic(clientId, new Secret(config.getString(Keys.OPENID_CLIENTSECRET))); try { callbackUrl = new URI(config.getString(Keys.WEB_URL, "") + "/api/session/openid/callback"); @@ -85,7 +87,8 @@ public class OpenIdProvider { tokenUrl = new URI(config.getString(Keys.OPENID_TOKENURL, "")); userInfoUrl = new URI(config.getString(Keys.OPENID_USERINFOURL, "")); baseUrl = new URI(config.getString(Keys.WEB_URL, "")); - } catch (URISyntaxException e) { + } catch(URISyntaxException error) { + LOGGER.error("Invalid URIs provided in OpenID configuration"); } adminGroup = config.getString(Keys.OPENID_ADMINGROUP); @@ -105,30 +108,21 @@ public class OpenIdProvider { return request.toURI(); } - private OIDCTokenResponse getToken(AuthorizationCode code) { - // Credentials to authenticate us to the token endpoint - ClientAuthentication clientAuth = new ClientSecretBasic(clientId, clientSecret); + private OIDCTokenResponse getToken(AuthorizationCode code) throws IOException, ParseException, GeneralSecurityException { AuthorizationGrant codeGrant = new AuthorizationCodeGrant(code, callbackUrl); + TokenRequest tokenRequest = new TokenRequest(tokenUrl, clientAuth, codeGrant); - TokenRequest request = new TokenRequest(tokenUrl, clientAuth, codeGrant); - TokenResponse tokenResponse; - - try { - HTTPResponse tokenReq = request.toHTTPRequest().send(); - tokenResponse = OIDCTokenResponseParser.parse(tokenReq); - if (!tokenResponse.indicatesSuccess()) { - return null; - } - - return (OIDCTokenResponse) tokenResponse.toSuccessResponse(); - } catch (IOException e) { - return null; - } catch (ParseException e) { - return null; + HTTPResponse tokenReq = tokenRequest.toHTTPRequest().send(); + TokenResponse tokenResponse = OIDCTokenResponseParser.parse(tokenReq); + if (!tokenResponse.indicatesSuccess()) { + LOGGER.warn("Invalid authorization code provided to OpenID callback"); + throw new GeneralSecurityException("Unable to authenticate with the OpenID Connect provider."); } + + return (OIDCTokenResponse) tokenResponse.toSuccessResponse(); } - private UserInfo getUserInfo(BearerAccessToken token) throws IOException, ParseException { + private UserInfo getUserInfo(BearerAccessToken token) throws IOException, ParseException, GeneralSecurityException { UserInfoResponse userInfoResponse; HTTPResponse httpResponse = new UserInfoRequest(userInfoUrl, token) @@ -138,46 +132,39 @@ public class OpenIdProvider { userInfoResponse = UserInfoResponse.parse(httpResponse); if (!userInfoResponse.indicatesSuccess()) { - // User info request failed - usually from expiring - return null; + LOGGER.error("Failed to access OpenID user info endpoint"); + throw new GeneralSecurityException("Failed to access OpenID Connect user info endpoint. Please contact your administrator."); } return userInfoResponse.toSuccessResponse().getUserInfo(); } - public Response handleCallback(URI requestUri, HttpServletRequest request) throws StorageException, ParseException, IOException, WebApplicationException { + public URI handleCallback(URI requestUri, HttpServletRequest request) throws StorageException, ParseException, IOException, GeneralSecurityException { AuthorizationResponse response = AuthorizationResponse.parse(requestUri); if (!response.indicatesSuccess()) { - AuthorizationErrorResponse error = response.toErrorResponse(); - throw new WebApplicationException(Response.status(403).entity(error.getErrorObject().getDescription()).build()); + LOGGER.warn("Callback received error response from OpenID identity provider"); + throw new GeneralSecurityException(response.toErrorResponse().getErrorObject().getDescription()); } AuthorizationCode authCode = response.toSuccessResponse().getAuthorizationCode(); if (authCode == null) { - return Response.status(403).entity( "Invalid OpenID Connect callback.").build(); + LOGGER.warn("Malformed OpenID callback"); + throw new GeneralSecurityException( "Malformed OpenID callback."); } OIDCTokenResponse tokens = getToken(authCode); - if (tokens == null) { - return Response.status(403).entity("Unable to authenticate with the OpenID Connect provider. Please try again.").build(); - } - BearerAccessToken bearerToken = tokens.getOIDCTokens().getBearerAccessToken(); UserInfo userInfo = getUserInfo(bearerToken); - if (userInfo == null) { - return Response.status(500).entity("Failed to access OpenID Connect user info endpoint. Please contact your administrator.").build(); - } - User user = loginService.login(userInfo.getEmailAddress(), userInfo.getName(), userInfo.getStringListClaim("groups").contains(adminGroup)); request.getSession().setAttribute(SessionResource.USER_ID_KEY, user.getId()); LogAction.login(user.getId(), ServletHelper.retrieveRemoteAddress(request)); - return Response.seeOther( - baseUrl).build(); + + return baseUrl; } } diff --git a/traccar-web b/traccar-web index 091d10531..506dd66b7 160000 --- a/traccar-web +++ b/traccar-web @@ -1 +1 @@ -Subproject commit 091d10531a59216c5f0a812609742e097c68ff2c +Subproject commit 506dd66b793803a24a2872e242482f263087df52 -- cgit v1.2.3