From e2e930dad18d577db581c17d0994f36bc2ca73c8 Mon Sep 17 00:00:00 2001 From: Harm Brugge Date: Mon, 28 Oct 2024 13:58:06 +0100 Subject: [PATCH 1/4] When OIDC setting is available use it --- .../java/org/molgenis/emx2/sql/SqlDatabase.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlDatabase.java b/backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlDatabase.java index 1e859d2f80..8de73cd887 100644 --- a/backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlDatabase.java +++ b/backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlDatabase.java @@ -52,8 +52,7 @@ public class SqlDatabase extends HasSettings implements Database { private String initialAdminPassword = (String) EnvironmentProperty.getParameter(Constants.MOLGENIS_ADMIN_PW, ADMIN_PW_DEFAULT, STRING); - private final Boolean isOidcEnabled = - EnvironmentProperty.getParameter(Constants.MOLGENIS_OIDC_CLIENT_ID, null, STRING) != null; + private Boolean isOidcEnabled = false; private static String postgresUser = (String) EnvironmentProperty.getParameter( @@ -179,9 +178,16 @@ public void init() { // setup default stuff // get the settings clearCache(); - if (getSetting(Constants.IS_OIDC_ENABLED) == null) { - // use environment property unless overridden in settings - this.setSetting(Constants.IS_OIDC_ENABLED, String.valueOf(isOidcEnabled)); + String isOidcEnabledSetting = getSetting(Constants.IS_OIDC_ENABLED); + if (isOidcEnabledSetting != null) { + this.isOidcEnabled = Boolean.parseBoolean(isOidcEnabledSetting); + } else { + Object envSetting = + EnvironmentProperty.getParameter(Constants.MOLGENIS_OIDC_CLIENT_ID, null, STRING); + if (envSetting != null) { + this.setSetting(Constants.IS_OIDC_ENABLED, "true"); + this.isOidcEnabled = true; + } } String instanceId = getSetting(Constants.MOLGENIS_INSTANCE_ID); From 6e4515bf95efaa33f5c95716c9f808ac768df8d4 Mon Sep 17 00:00:00 2001 From: mswertz Date: Wed, 6 Nov 2024 22:19:44 +0100 Subject: [PATCH 2/4] provide an error state if parameters are missing --- .../org/molgenis/emx2/sql/SqlDatabase.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlDatabase.java b/backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlDatabase.java index 8de73cd887..0f29498a14 100644 --- a/backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlDatabase.java +++ b/backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlDatabase.java @@ -178,6 +178,7 @@ public void init() { // setup default stuff // get the settings clearCache(); + // oidc settings takes priority over env variables String isOidcEnabledSetting = getSetting(Constants.IS_OIDC_ENABLED); if (isOidcEnabledSetting != null) { this.isOidcEnabled = Boolean.parseBoolean(isOidcEnabledSetting); @@ -189,6 +190,25 @@ public void init() { // setup default stuff this.isOidcEnabled = true; } } + // check if OIDC settings are complete otherwise log error and set to false + if (this.isOidcEnabled) { + if (EnvironmentProperty.getParameter(Constants.MOLGENIS_OIDC_CLIENT_SECRET, null, STRING) + == null) { + setSetting( + Constants.IS_OIDC_ENABLED, + "error: " + + Constants.MOLGENIS_OIDC_CLIENT_SECRET + + " is missing. Fix and then set again to true"); + } + if (EnvironmentProperty.getParameter(Constants.MOLGENIS_OIDC_CLIENT_ID, null, STRING) + == null) { + setSetting( + Constants.IS_OIDC_ENABLED, + "error: " + + Constants.MOLGENIS_OIDC_CLIENT_ID + + " is missing. Fix and set again to true"); + } + } String instanceId = getSetting(Constants.MOLGENIS_INSTANCE_ID); if (instanceId == null) { From 52c0342b4d259a2fa251aedaa38673085bb8b6e9 Mon Sep 17 00:00:00 2001 From: Harm Brugge Date: Mon, 18 Nov 2024 14:54:04 +0100 Subject: [PATCH 3/4] Load db settings in security config --- .../org/molgenis/emx2/sql/SqlDatabase.java | 75 +++++++++++-------- .../emx2/web/MolgenisSessionManager.java | 3 + .../molgenis/emx2/web/MolgenisWebservice.java | 6 +- .../emx2/web/SecurityConfigFactory.java | 7 +- .../emx2/web/controllers/OIDCController.java | 16 ++-- 5 files changed, 63 insertions(+), 44 deletions(-) diff --git a/backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlDatabase.java b/backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlDatabase.java index 0f29498a14..97cbf1402b 100644 --- a/backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlDatabase.java +++ b/backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlDatabase.java @@ -178,37 +178,7 @@ public void init() { // setup default stuff // get the settings clearCache(); - // oidc settings takes priority over env variables - String isOidcEnabledSetting = getSetting(Constants.IS_OIDC_ENABLED); - if (isOidcEnabledSetting != null) { - this.isOidcEnabled = Boolean.parseBoolean(isOidcEnabledSetting); - } else { - Object envSetting = - EnvironmentProperty.getParameter(Constants.MOLGENIS_OIDC_CLIENT_ID, null, STRING); - if (envSetting != null) { - this.setSetting(Constants.IS_OIDC_ENABLED, "true"); - this.isOidcEnabled = true; - } - } - // check if OIDC settings are complete otherwise log error and set to false - if (this.isOidcEnabled) { - if (EnvironmentProperty.getParameter(Constants.MOLGENIS_OIDC_CLIENT_SECRET, null, STRING) - == null) { - setSetting( - Constants.IS_OIDC_ENABLED, - "error: " - + Constants.MOLGENIS_OIDC_CLIENT_SECRET - + " is missing. Fix and then set again to true"); - } - if (EnvironmentProperty.getParameter(Constants.MOLGENIS_OIDC_CLIENT_ID, null, STRING) - == null) { - setSetting( - Constants.IS_OIDC_ENABLED, - "error: " - + Constants.MOLGENIS_OIDC_CLIENT_ID - + " is missing. Fix and set again to true"); - } - } + initOidc(); String instanceId = getSetting(Constants.MOLGENIS_INSTANCE_ID); if (instanceId == null) { @@ -256,6 +226,49 @@ public void init() { // setup default stuff } } + private void initOidc() { + // oidc settings takes priority over env variables + String isOidcEnabledSetting = getSetting(Constants.IS_OIDC_ENABLED); + if (isOidcEnabledSetting != null) { + this.isOidcEnabled = Boolean.parseBoolean(isOidcEnabledSetting); + } else { + Object envSetting = + EnvironmentProperty.getParameter(Constants.MOLGENIS_OIDC_CLIENT_ID, null, STRING); + if (envSetting != null) { + this.setSetting(Constants.IS_OIDC_ENABLED, "true"); + this.isOidcEnabled = true; + } + } + if (!this.isOidcEnabled) return; + + // check if OIDC settings are complete otherwise log error and set to false + String oidcClientId = getDbOrEnvSetting(Constants.MOLGENIS_OIDC_CLIENT_ID); + String callBackUrl = getDbOrEnvSetting(Constants.MOLGENIS_OIDC_CALLBACK_URL); + Object clientSecret = + EnvironmentProperty.getParameter(Constants.MOLGENIS_OIDC_CLIENT_SECRET, null, STRING); + + if (clientSecret == null) { + setSetting( + Constants.IS_OIDC_ENABLED, + "error: " + + Constants.MOLGENIS_OIDC_CLIENT_SECRET + + " is missing. Fix and then set again to true"); + } + if (oidcClientId == null) { + setSetting( + Constants.IS_OIDC_ENABLED, + "error: " + Constants.MOLGENIS_OIDC_CLIENT_ID + " is missing. Fix and set again to true"); + } + } + + private String getDbOrEnvSetting(String settingId) { + String setting = getSetting(settingId); + if (setting == null) { + setting = (String) EnvironmentProperty.getParameter(settingId, null, STRING); + } + return setting; + } + @Override public void setListener(DatabaseListener listener) { this.listener = listener; diff --git a/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/MolgenisSessionManager.java b/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/MolgenisSessionManager.java index 4120fea14d..b3fa17b866 100644 --- a/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/MolgenisSessionManager.java +++ b/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/MolgenisSessionManager.java @@ -1,5 +1,7 @@ package org.molgenis.emx2.web; +import static org.molgenis.emx2.web.MolgenisWebservice.oidcController; + import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpSessionEvent; import jakarta.servlet.http.HttpSessionListener; @@ -77,6 +79,7 @@ public String findUsedAuthTokenKey(HttpServletRequest request) { * this method is used to reset cache of all sessions, necessary when for example metadata changes */ public void clearAllCaches() { + oidcController.reloadConfig(); for (MolgenisSession session : sessions.values()) { session.clearCache(); } diff --git a/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/MolgenisWebservice.java b/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/MolgenisWebservice.java index e6e9b2a1b7..dc3b184ebe 100644 --- a/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/MolgenisWebservice.java +++ b/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/MolgenisWebservice.java @@ -29,8 +29,8 @@ public class MolgenisWebservice { static final Logger logger = LoggerFactory.getLogger(MolgenisWebservice.class); private static final String ROBOTS_TXT = "robots.txt"; private static final String USER_AGENT_ALLOW = "User-agent: *\nAllow: /"; - static MolgenisSessionManager sessionManager; - static OIDCController oidcController; + public static MolgenisSessionManager sessionManager; + public static OIDCController oidcController; private MolgenisWebservice() { // hide constructor @@ -39,7 +39,7 @@ private MolgenisWebservice() { public static void start(int port) { sessionManager = new MolgenisSessionManager(); - oidcController = new OIDCController(sessionManager, new SecurityConfigFactory().build()); + oidcController = new OIDCController(); Javalin app = Javalin.create( config -> { diff --git a/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/SecurityConfigFactory.java b/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/SecurityConfigFactory.java index 7bf3c734da..84f8bbd0b3 100644 --- a/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/SecurityConfigFactory.java +++ b/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/SecurityConfigFactory.java @@ -13,9 +13,8 @@ public class SecurityConfigFactory { - private String oidcClientId = - (String) EnvironmentProperty.getParameter(Constants.MOLGENIS_OIDC_CLIENT_ID, null, STRING); - private String oidcClientSecret = + private String oidcClientId; + private final String oidcClientSecret = (String) EnvironmentProperty.getParameter(Constants.MOLGENIS_OIDC_CLIENT_SECRET, null, STRING); public static String OIDC_CLIENT_NAME = @@ -39,6 +38,8 @@ public class SecurityConfigFactory { public Config build() { final OidcConfiguration oidcConfiguration = new OidcConfiguration(); + oidcClientId = + (String) EnvironmentProperty.getParameter(Constants.MOLGENIS_OIDC_CLIENT_ID, null, STRING); oidcConfiguration.setClientId(oidcClientId); oidcConfiguration.setSecret(oidcClientSecret); oidcConfiguration.setDiscoveryURI(oidcDiscoveryURI); diff --git a/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/controllers/OIDCController.java b/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/controllers/OIDCController.java index 0f6146af05..ca5279e9dd 100644 --- a/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/controllers/OIDCController.java +++ b/backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/controllers/OIDCController.java @@ -1,6 +1,6 @@ package org.molgenis.emx2.web.controllers; -import static java.util.Objects.requireNonNull; +import static org.molgenis.emx2.web.MolgenisWebservice.sessionManager; import static org.molgenis.emx2.web.SecurityConfigFactory.OIDC_CLIENT_NAME; import io.javalin.http.Context; @@ -8,7 +8,7 @@ import org.molgenis.emx2.Database; import org.molgenis.emx2.MolgenisException; import org.molgenis.emx2.web.JavalinCustomHttpActionAdapter; -import org.molgenis.emx2.web.MolgenisSessionManager; +import org.molgenis.emx2.web.SecurityConfigFactory; import org.pac4j.core.config.Config; import org.pac4j.core.context.session.SessionStore; import org.pac4j.core.engine.CallbackLogic; @@ -30,16 +30,18 @@ public class OIDCController { private static final Logger logger = LoggerFactory.getLogger(OIDCController.class); - private final MolgenisSessionManager sessionManager; - private final Config securityConfig; + private Config securityConfig; private final SessionStore sessionStore; - public OIDCController(MolgenisSessionManager sessionManager, Config securityConfig) { - this.sessionManager = requireNonNull(sessionManager); - this.securityConfig = requireNonNull(securityConfig); + public OIDCController() { + this.securityConfig = new SecurityConfigFactory().build(); this.sessionStore = FindBest.sessionStore(null, securityConfig, JEESessionStore.INSTANCE); } + public void reloadConfig() { + this.securityConfig = new SecurityConfigFactory().build(); + } + public void handleLoginRequest(Context ctx) { final JavalinWebContext context = new JavalinWebContext(ctx); sessionStore.set(context, Pac4jConstants.REQUESTED_URL, ctx.queryParams("redirect")); From d0c89ff6b380ef2544388010aac6b433d904959f Mon Sep 17 00:00:00 2001 From: Harm Brugge Date: Tue, 19 Nov 2024 13:18:40 +0100 Subject: [PATCH 4/4] Show error message when setting oidc to true when settings are incomplete --- .../src/components/admin/ManageSettings.vue | 17 +++++-- .../org/molgenis/emx2/sql/SqlDatabase.java | 51 +++++++++---------- 2 files changed, 35 insertions(+), 33 deletions(-) diff --git a/apps/central/src/components/admin/ManageSettings.vue b/apps/central/src/components/admin/ManageSettings.vue index 2270da2f9b..45e5b4aea2 100644 --- a/apps/central/src/components/admin/ManageSettings.vue +++ b/apps/central/src/components/admin/ManageSettings.vue @@ -1,5 +1,6 @@