Merge pull request #5064 from dan-r/openid-discovery

Updates to OpenID Implementation

6 files changed, 114 insertions, 62 deletions

diff --git a/src/main/java/org/traccar/MainModule.java b/src/main/java/org/traccar/MainModule.java
index 51097511a..6a2fe21c3 100644
--- a/src/main/java/org/traccar/MainModule.java
+++ b/src/main/java/org/traccar/MainModule.java
@@ -26,7 +26,6 @@ import com.google.inject.name.Names;
 import io.netty.util.HashedWheelTimer;
 import io.netty.util.Timer;
 import org.apache.velocity.app.VelocityEngine;
-import org.eclipse.jetty.util.URIUtil;
 import org.traccar.broadcast.BroadcastService;
 import org.traccar.broadcast.MulticastBroadcastService;
 import org.traccar.broadcast.NullBroadcastService;
@@ -75,6 +74,7 @@ import org.traccar.handler.GeolocationHandler;
 import org.traccar.handler.SpeedLimitHandler;
 import org.traccar.helper.ObjectMapperContextResolver;
 import org.traccar.helper.SanitizerModule;
+import org.traccar.helper.WebHelper;
 import org.traccar.mail.LogMailManager;
 import org.traccar.mail.MailManager;
 import org.traccar.mail.SmtpMailManager;
@@ -95,8 +95,8 @@ import javax.inject.Singleton;
 import javax.ws.rs.client.Client;
 import javax.ws.rs.client.ClientBuilder;
 import java.io.IOException;
-import java.net.InetAddress;
-import java.net.UnknownHostException;
+import java.net.URISyntaxException;
+import java.net.http.HttpClient;
 import java.util.Properties;
 
 public class MainModule extends AbstractModule {
@@ -174,9 +174,11 @@ public class MainModule extends AbstractModule {
 
     @Singleton
     @Provides
-    public static OpenIdProvider provideOpenIDProvider(Config config, LoginService loginService) {
-        if (config.hasKey(Keys.OPENID_CLIENTID)) {
-            return new OpenIdProvider(config, loginService);
+    public static OpenIdProvider provideOpenIDProvider(
+        Config config, LoginService loginService, ObjectMapper objectMapper
+        ) throws InterruptedException, IOException, URISyntaxException {
+        if (config.hasKey(Keys.OPENID_CLIENT_ID)) {
+            return new OpenIdProvider(config, loginService, HttpClient.newHttpClient(), objectMapper);
         }
         return null;
     }
@@ -386,19 +388,7 @@ public class MainModule extends AbstractModule {
     public static VelocityEngine provideVelocityEngine(Config config) {
         Properties properties = new Properties();
         properties.setProperty("resource.loader.file.path", config.getString(Keys.TEMPLATES_ROOT) + "/");
-
-        if (config.hasKey(Keys.WEB_URL)) {
-            properties.setProperty("web.url", config.getString(Keys.WEB_URL).replaceAll("/$", ""));
-        } else {
-            String address;
-            try {
-                address = config.getString(Keys.WEB_ADDRESS, InetAddress.getLocalHost().getHostAddress());
-            } catch (UnknownHostException e) {
-                address = "localhost";
-            }
-            String url = URIUtil.newURI("http", address, config.getInteger(Keys.WEB_PORT), "", "");
-            properties.setProperty("web.url", url);
-        }
+        properties.setProperty("web.url", WebHelper.retrieveWebUrl(config));
 
         VelocityEngine velocityEngine = new VelocityEngine();
         velocityEngine.init(properties);
diff --git a/src/main/java/org/traccar/api/resource/SessionResource.java b/src/main/java/org/traccar/api/resource/SessionResource.java
index ac39fa449..9b6a74ddb 100644
--- a/src/main/java/org/traccar/api/resource/SessionResource.java
+++ b/src/main/java/org/traccar/api/resource/SessionResource.java
@@ -21,7 +21,7 @@ import org.traccar.api.signature.TokenManager;
 import org.traccar.database.OpenIdProvider;
 import org.traccar.helper.DataConverter;
 import org.traccar.helper.LogAction;
-import org.traccar.helper.ServletHelper;
+import org.traccar.helper.WebHelper;
 import org.traccar.model.User;
 import org.traccar.storage.StorageException;
 import org.traccar.storage.query.Columns;
@@ -84,7 +84,7 @@ public class SessionResource extends BaseResource {
             User user = loginService.login(token);
             if (user != null) {
                 request.getSession().setAttribute(USER_ID_KEY, user.getId());
-                LogAction.login(user.getId(), ServletHelper.retrieveRemoteAddress(request));
+                LogAction.login(user.getId(), WebHelper.retrieveRemoteAddress(request));
                 return user;
             }
         }
@@ -111,7 +111,7 @@ public class SessionResource extends BaseResource {
                 User user = loginService.login(email, password);
                 if (user != null) {
                     request.getSession().setAttribute(USER_ID_KEY, user.getId());
-                    LogAction.login(user.getId(), ServletHelper.retrieveRemoteAddress(request));
+                    LogAction.login(user.getId(), WebHelper.retrieveRemoteAddress(request));
                     return user;
                 }
             }
@@ -135,7 +135,7 @@ public class SessionResource extends BaseResource {
         User user = storage.getObject(User.class, new Request(
                 new Columns.All(), new Condition.Equals("id", userId)));
         request.getSession().setAttribute(USER_ID_KEY, user.getId());
-        LogAction.login(user.getId(), ServletHelper.retrieveRemoteAddress(request));
+        LogAction.login(user.getId(), WebHelper.retrieveRemoteAddress(request));
         return user;
     }
 
@@ -146,17 +146,17 @@ public class SessionResource extends BaseResource {
         User user = loginService.login(email, password);
         if (user != null) {
             request.getSession().setAttribute(USER_ID_KEY, user.getId());
-            LogAction.login(user.getId(), ServletHelper.retrieveRemoteAddress(request));
+            LogAction.login(user.getId(), WebHelper.retrieveRemoteAddress(request));
             return user;
         } else {
-            LogAction.failedLogin(ServletHelper.retrieveRemoteAddress(request));
+            LogAction.failedLogin(WebHelper.retrieveRemoteAddress(request));
             throw new WebApplicationException(Response.status(Response.Status.UNAUTHORIZED).build());
         }
     }
 
     @DELETE
     public Response remove() {
-        LogAction.logout(getUserId(), ServletHelper.retrieveRemoteAddress(request));
+        LogAction.logout(getUserId(), WebHelper.retrieveRemoteAddress(request));
         request.getSession().removeAttribute(USER_ID_KEY);
         return Response.noContent().build();
     }
diff --git a/src/main/java/org/traccar/config/Keys.java b/src/main/java/org/traccar/config/Keys.java
index 3ff423ad1..b97acfd66 100644
--- a/src/main/java/org/traccar/config/Keys.java
+++ b/src/main/java/org/traccar/config/Keys.java
@@ -623,7 +623,7 @@ public final class Keys {
      * This is a unique ID assigned to each application you register with your identity provider.
      * Required to enable SSO.
      */
-    public static final ConfigKey<String> OPENID_CLIENTID = new StringConfigKey(
+    public static final ConfigKey<String> OPENID_CLIENT_ID = new StringConfigKey(
             "openid.clientId",
             List.of(KeyType.CONFIG));
 
@@ -632,43 +632,60 @@ public final class Keys {
      * This is a secret assigned to each application you register with your identity provider.
      * Required to enable SSO.
      */
-    public static final ConfigKey<String> OPENID_CLIENTSECRET = new StringConfigKey(
+    public static final ConfigKey<String> OPENID_CLIENT_SECRET = new StringConfigKey(
             "openid.clientSecret",
             List.of(KeyType.CONFIG));
 
     /**
+     * OpenID Connect Issuer (Base) URL.
+     * This is used to automatically configure the authorization, token and user info URLs if provided.
+     */
+    public static final ConfigKey<String> OPENID_ISSUER_URL = new StringConfigKey(
+            "openid.issuerUrl",
+            List.of(KeyType.CONFIG));
+
+    /**
      * OpenID Connect Authorization URL.
      * This can usually be found in the documentation of your identity provider or by using the well-known
      * configuration endpoint, eg. https://auth.example.com//.well-known/openid-configuration
-     * Required to enable SSO.
+     * Required to enable SSO if openid.issuerUrl is not set.
      */
-    public static final ConfigKey<String> OPENID_AUTHURL = new StringConfigKey(
+    public static final ConfigKey<String> OPENID_AUTH_URL = new StringConfigKey(
             "openid.authUrl",
             List.of(KeyType.CONFIG));
     /**
      * OpenID Connect Token URL.
      * This can be found in the same ways at openid.authUrl.
-     * Required to enable SSO.
+     * Required to enable SSO if openid.issuerUrl is not set.
      */
-    public static final ConfigKey<String> OPENID_TOKENURL = new StringConfigKey(
+    public static final ConfigKey<String> OPENID_TOKEN_URL = new StringConfigKey(
             "openid.tokenUrl",
             List.of(KeyType.CONFIG));
 
     /**
      * OpenID Connect User Info URL.
      * This can be found in the same ways at openid.authUrl.
-     * Required to enable SSO.
+     * Required to enable SSO if openid.issuerUrl is not set.
      */
-    public static final ConfigKey<String> OPENID_USERINFOURL = new StringConfigKey(
+    public static final ConfigKey<String> OPENID_USERINFO_URL = new StringConfigKey(
             "openid.userInfoUrl",
             List.of(KeyType.CONFIG));
 
     /**
+     * OpenID Connect group to restrict access to.
+     * If this is not provided, all OpenID users will have access to Traccar.
+     * This option will only work if your OpenID provider supports the groups scope.
+     */
+    public static final ConfigKey<String> OPENID_ALLOW_GROUP = new StringConfigKey(
+        "openid.allowGroup",
+        List.of(KeyType.CONFIG));
+
+    /**
      * OpenID Connect group to grant admin access.
      * If this is not provided, no groups will be granted admin access.
      * This option will only work if your OpenID provider supports the groups scope.
      */
-    public static final ConfigKey<String> OPENID_ADMINGROUP = new StringConfigKey(
+    public static final ConfigKey<String> OPENID_ADMIN_GROUP = new StringConfigKey(
             "openid.adminGroup",
             List.of(KeyType.CONFIG));
 
diff --git a/src/main/java/org/traccar/database/OpenIdProvider.java b/src/main/java/org/traccar/database/OpenIdProvider.java
index 537319b31..d0ec4e98d 100644
--- a/src/main/java/org/traccar/database/OpenIdProvider.java
+++ b/src/main/java/org/traccar/database/OpenIdProvider.java
@@ -22,16 +22,22 @@ import org.traccar.api.security.LoginService;
 import org.traccar.model.User;
 import org.traccar.storage.StorageException;
 import org.traccar.helper.LogAction;
-import org.traccar.helper.ServletHelper;
+import org.traccar.helper.WebHelper;
 
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse.BodyHandlers;
 import java.security.GeneralSecurityException;
+import java.util.List;
+import java.util.Map;
 import java.io.IOException;
 import javax.servlet.http.HttpServletRequest;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
 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;
@@ -54,12 +60,9 @@ import com.nimbusds.openid.connect.sdk.OIDCTokenResponseParser;
 import com.nimbusds.openid.connect.sdk.UserInfoResponse;
 import com.nimbusds.openid.connect.sdk.UserInfoRequest;
 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);
-
     private final Boolean force;
     private final ClientID clientId;
     private final ClientAuthentication clientAuth;
@@ -69,28 +72,45 @@ public class OpenIdProvider {
     private URI userInfoUrl;
     private URI baseUrl;
     private final String adminGroup;
+    private final String allowGroup;
 
     private LoginService loginService;
 
     @Inject
-    public OpenIdProvider(Config config, LoginService loginService) {
+    public OpenIdProvider(
+        Config config, LoginService loginService, HttpClient httpClient, ObjectMapper objectMapper
+        ) throws InterruptedException, IOException, URISyntaxException {
         this.loginService = loginService;
 
         force = config.getBoolean(Keys.OPENID_FORCE);
-        clientId = new ClientID(config.getString(Keys.OPENID_CLIENTID));
-        clientAuth = new ClientSecretBasic(clientId, new Secret(config.getString(Keys.OPENID_CLIENTSECRET)));
-
-        try {
-            callbackUrl = new URI(config.getString(Keys.WEB_URL, "") + "/api/session/openid/callback");
-            authUrl = new URI(config.getString(Keys.OPENID_AUTHURL, ""));
-            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 error) {
-            LOGGER.error("Invalid URIs provided in OpenID configuration");
+        clientId = new ClientID(config.getString(Keys.OPENID_CLIENT_ID));
+        clientAuth = new ClientSecretBasic(clientId, new Secret(config.getString(Keys.OPENID_CLIENT_SECRET)));
+
+        baseUrl = new URI(WebHelper.retrieveWebUrl(config));
+        callbackUrl = new URI(WebHelper.retrieveWebUrl(config) + "/api/session/openid/callback");
+
+        if (config.hasKey(Keys.OPENID_ISSUER_URL)) {
+            HttpRequest httpRequest = HttpRequest.newBuilder(
+                URI.create(config.getString(Keys.OPENID_ISSUER_URL) + "/.well-known/openid-configuration"))
+                .header("Accept", "application/json")
+                .build();
+
+            String httpResponse = httpClient.send(httpRequest, BodyHandlers.ofString()).body();
+
+            Map<String, Object> discoveryMap = objectMapper.readValue(
+                httpResponse, new TypeReference<Map<String, Object>>() { });
+
+            authUrl = new URI((String) discoveryMap.get("authorization_endpoint"));
+            tokenUrl = new URI((String) discoveryMap.get("token_endpoint"));
+            userInfoUrl = new URI((String) discoveryMap.get("userinfo_endpoint"));
+        } else {
+            authUrl = new URI(config.getString(Keys.OPENID_AUTH_URL));
+            tokenUrl = new URI(config.getString(Keys.OPENID_TOKEN_URL));
+            userInfoUrl = new URI(config.getString(Keys.OPENID_USERINFO_URL));
         }
 
-        adminGroup = config.getString(Keys.OPENID_ADMINGROUP);
+        adminGroup = config.getString(Keys.OPENID_ADMIN_GROUP);
+        allowGroup = config.getString(Keys.OPENID_ALLOW_GROUP);
     }
 
     public URI createAuthUri() {
@@ -162,12 +182,17 @@ public class OpenIdProvider {
 
             UserInfo userInfo = getUserInfo(bearerToken);
 
-            Boolean administrator = adminGroup != null && userInfo.getStringListClaim("groups").contains(adminGroup);
+            List<String> userGroups = userInfo.getStringListClaim("groups");
+            Boolean administrator = adminGroup != null && userGroups.contains(adminGroup);
+
+            if (!(administrator || allowGroup == null || userGroups.contains(allowGroup))) {
+                throw new GeneralSecurityException("Your OpenID Groups do not permit access to Traccar.");
+            }
 
             User user = loginService.login(userInfo.getEmailAddress(), userInfo.getName(), administrator);
 
             request.getSession().setAttribute(SessionResource.USER_ID_KEY, user.getId());
-            LogAction.login(user.getId(), ServletHelper.retrieveRemoteAddress(request));
+            LogAction.login(user.getId(), WebHelper.retrieveRemoteAddress(request));
 
             return baseUrl;
     }
diff --git a/src/main/java/org/traccar/helper/ServletHelper.java b/src/main/java/org/traccar/helper/WebHelper.java
index b6c587ec3..e2844bc4d 100644
--- a/src/main/java/org/traccar/helper/ServletHelper.java
+++ b/src/main/java/org/traccar/helper/WebHelper.java
@@ -15,11 +15,18 @@
  */
 package org.traccar.helper;
 
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
 import javax.servlet.http.HttpServletRequest;
 
-public final class ServletHelper {
+import org.eclipse.jetty.util.URIUtil;
+import org.traccar.config.Config;
+import org.traccar.config.Keys;
+
+public final class WebHelper {
 
-    private ServletHelper() {
+    private WebHelper() {
     }
 
     public static String retrieveRemoteAddress(HttpServletRequest request) {
@@ -42,4 +49,17 @@ public final class ServletHelper {
         }
     }
 
+    public static String retrieveWebUrl(Config config) {
+        if (config.hasKey(Keys.WEB_URL)) {
+            return config.getString(Keys.WEB_URL).replaceAll("/$", "");
+        } else {
+            String address;
+            try {
+                address = config.getString(Keys.WEB_ADDRESS, InetAddress.getLocalHost().getHostAddress());
+            } catch (UnknownHostException e) {
+                address = "localhost";
+            }
+            return URIUtil.newURI("http", address, config.getInteger(Keys.WEB_PORT), "", "");
+        }
+    }
 }
diff --git a/src/test/java/org/traccar/helper/ServletHelperTest.java b/src/test/java/org/traccar/helper/WebHelperTest.java
index 3a645bc36..3a7329cb8 100644
--- a/src/test/java/org/traccar/helper/ServletHelperTest.java
+++ b/src/test/java/org/traccar/helper/WebHelperTest.java
@@ -8,7 +8,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-public class ServletHelperTest {
+public class WebHelperTest {
 
     @Test
     public void testRetrieveRemoteAddressProxyMultiple() {
@@ -16,7 +16,7 @@ public class ServletHelperTest {
         when(request.getRemoteAddr()).thenReturn("147.120.1.5");
         when(request.getHeader("X-FORWARDED-FOR")).thenReturn("231.23.45.65, 10.20.10.33, 10.20.20.34");
 
-        assertEquals("231.23.45.65", ServletHelper.retrieveRemoteAddress(request));
+        assertEquals("231.23.45.65", WebHelper.retrieveRemoteAddress(request));
     }
 
     @Test
@@ -25,7 +25,7 @@ public class ServletHelperTest {
         when(request.getRemoteAddr()).thenReturn("147.120.1.5");
         when(request.getHeader("X-FORWARDED-FOR")).thenReturn("231.23.45.65");
 
-        assertEquals("231.23.45.65", ServletHelper.retrieveRemoteAddress(request));
+        assertEquals("231.23.45.65", WebHelper.retrieveRemoteAddress(request));
     }
 
     @Test
@@ -33,7 +33,7 @@ public class ServletHelperTest {
         HttpServletRequest request = mock(HttpServletRequest.class);
         when(request.getRemoteAddr()).thenReturn("231.23.45.65");
 
-        assertEquals("231.23.45.65", ServletHelper.retrieveRemoteAddress(request));
+        assertEquals("231.23.45.65", WebHelper.retrieveRemoteAddress(request));
     }
 
 }