diff --git a/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java b/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java index 16f9895bab1..a1f153740f3 100644 --- a/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java +++ b/api/src/org/labkey/api/security/AuthenticationConfigurationCache.java @@ -82,8 +82,6 @@ protected Set> createCollection() private AuthenticationConfigurationCollections() { - boolean acceptOnlyFicamProviders = AuthenticationManager.isAcceptOnlyFicamProviders(); - // Select the configurations stored in the core.AuthenticationConfigurations table, add the database // authentication configuration, map each to the appropriate AuthenticationConfiguration, and add to the maps. @@ -99,13 +97,13 @@ private AuthenticationConfigurationCollections() configs .map(this::getAuthenticationConfiguration) .filter(Objects::nonNull) - .filter(c->!acceptOnlyFicamProviders || c.getAuthenticationProvider().isFicamApproved()) .forEach(this::addConfiguration); // MultiValuedMap of domains to AuthenticationConfigurations that claim them _activeDomainMap = getActive(PrimaryAuthenticationConfiguration.class).stream() .filter(config -> null != config.getDomain()) .filter(config -> !AuthenticationManager.ALL_DOMAINS.equals(config.getDomain())) + .map(config -> (AuthenticationConfiguration) config) .collect(LabKeyCollectors.toMultiValuedMap(AuthenticationConfiguration::getDomain, config -> config)); List activeDomains = new ArrayList<>(_activeDomainMap.keySet()); @@ -113,15 +111,18 @@ private AuthenticationConfigurationCollections() _activeDomains = Collections.unmodifiableCollection(activeDomains); } - // Little helper method simplifies the stream handling above + // Little helper method simplifies the stream handling above. Filters out configurations based on FICAM-only setting. private @Nullable AuthenticationConfiguration getAuthenticationConfiguration(Map map) { String providerName = (String)map.get("Provider"); AuthenticationProvider provider = AuthenticationProviderCache.getProvider(AuthenticationProvider.class, providerName); if (null == provider) { - String description = (String)map.get("Description"); - LOG.warn("A saved authentication configuration requires the \"{}\" authentication provider, but that provider is not present in this deployment. Authentication via {} will not be available.", providerName, null != description ? "\"" + description + "\"" : "this mechanism"); + if (!AuthenticationManager.isAcceptOnlyFicamProviders()) // Don't warn if FICAM-only is checked + { + String description = (String)map.get("Description"); + LOG.warn("A saved authentication configuration requires the \"{}\" authentication provider, but that provider is not present in this deployment. Authentication via {} will not be available.", providerName, null != description ? "\"" + description + "\"" : "this mechanism"); + } return null; } @@ -162,7 +163,7 @@ private void addToMap(SetValuedMap, return null != configurations ? configurations : Collections.emptyList(); } - private @NotNull Collection getActiveConfigurationsForDomain(String domain) + private @NotNull Collection> getActiveConfigurationsForDomain(String domain) { return new ArrayList<>(_activeDomainMap.get(domain)); } @@ -233,7 +234,7 @@ public static void clear() /** * Return a collection of authentication configurations that claim the specified domain */ - public static @NotNull Collection getActiveConfigurationsForDomain(String domain) + public static @NotNull Collection> getActiveConfigurationsForDomain(String domain) { return CACHE.get(CACHE_KEY).getActiveConfigurationsForDomain(domain); } diff --git a/api/src/org/labkey/api/security/AuthenticationManager.java b/api/src/org/labkey/api/security/AuthenticationManager.java index 9f35e3b36a1..f35db4daff3 100644 --- a/api/src/org/labkey/api/security/AuthenticationManager.java +++ b/api/src/org/labkey/api/security/AuthenticationManager.java @@ -662,6 +662,12 @@ public static void deleteConfiguration(User user, int rowId) { // Delete any logos attached to the configuration AuthenticationConfiguration configuration = AuthenticationConfigurationCache.getConfiguration(AuthenticationConfiguration.class, rowId); + + if (null == configuration) + { + throw new NotFoundException("Unable to delete authentication configuration"); + } + AttachmentService.get().deleteAttachments(configuration); // Delete configuration @@ -746,6 +752,7 @@ public static void setAcceptOnlyFicamProviders(User user, boolean enable) if (isAcceptOnlyFicamProviders() != enable) { saveAuthSetting(user, ACCEPT_ONLY_FICAM_PROVIDERS_KEY, enable); + AuthenticationProviderCache.clear(); AuthenticationConfigurationCache.clear(); } } diff --git a/api/src/org/labkey/api/security/AuthenticationProvider.java b/api/src/org/labkey/api/security/AuthenticationProvider.java index b293a5ddf02..07df797aaef 100644 --- a/api/src/org/labkey/api/security/AuthenticationProvider.java +++ b/api/src/org/labkey/api/security/AuthenticationProvider.java @@ -234,6 +234,12 @@ interface ResetPasswordProvider extends AuthenticationProvider * @param isAdminCopy true for sending admin a copy of reset password email */ @Nullable SecurityMessage getAPIResetPasswordMessage(User user, boolean isAdminCopy); + + @Override + default boolean isFicamApproved() + { + return true; + } } interface SecondaryAuthenticationProvider> extends ConfigurableAuthenticationProvider @@ -286,11 +292,23 @@ interface DisableLoginProvider extends AuthenticationProvider void addUserDelay(HttpServletRequest request, String id, int addCount); void resetUserDelay(String id); + + @Override + default boolean isFicamApproved() + { + return true; + } } interface ExpireAccountProvider extends AuthenticationProvider { boolean isEnabled(); + + @Override + default boolean isFicamApproved() + { + return true; + } } class AuthenticationResponse diff --git a/api/src/org/labkey/api/security/AuthenticationProviderCache.java b/api/src/org/labkey/api/security/AuthenticationProviderCache.java index 974898bd7eb..fac6f6ed008 100644 --- a/api/src/org/labkey/api/security/AuthenticationProviderCache.java +++ b/api/src/org/labkey/api/security/AuthenticationProviderCache.java @@ -27,9 +27,6 @@ import java.util.LinkedHashSet; import java.util.Set; -/** - * Created by adam on 5/20/2016. - */ public class AuthenticationProviderCache { // We have just a single object to cache (a global AuthenticationProviderCollection), but use standard cache (blocking cache wrapping the @@ -56,13 +53,15 @@ protected Set createCollection() private AuthenticationProviderCollections() { - for (AuthenticationProvider provider : AuthenticationManager.getAllProviders()) - { - AuthenticationProvider.ALL_PROVIDER_INTERFACES + boolean acceptOnlyFicamProviders = AuthenticationManager.isAcceptOnlyFicamProviders(); + + AuthenticationManager.getAllProviders().stream() + .filter(provider -> !acceptOnlyFicamProviders || provider.isFicamApproved()) + .forEach(provider -> AuthenticationProvider.ALL_PROVIDER_INTERFACES .stream() .filter(providerClass -> providerClass.isInstance(provider)) - .forEach(providerClass -> _map.put(providerClass, provider)); - } + .forEach(providerClass -> _map.put(providerClass, provider)) + ); } private Collection get(Class clazz) diff --git a/api/src/org/labkey/api/security/SaveConfigurationAction.java b/api/src/org/labkey/api/security/SaveConfigurationAction.java index 3f14a00bcef..0d0c7f55b66 100644 --- a/api/src/org/labkey/api/security/SaveConfigurationAction.java +++ b/api/src/org/labkey/api/security/SaveConfigurationAction.java @@ -127,7 +127,8 @@ public static AuthenticationConfiguration s return StringUtilsLabKey.getMapDifference( null != oldConfiguration ? oldConfiguration.getLoggingProperties() : null, null != newConfiguration ? newConfiguration.getLoggingProperties() : null, - 50); + 50 + ); } protected AC getFromCache(int rowId) @@ -136,9 +137,20 @@ protected AC getFromCache(int rowId) return (AC)AuthenticationConfigurationCache.getConfiguration(AuthenticationConfiguration.class, rowId); } - protected Map getConfigurationMap(int rowId) + protected final Map getConfigurationMap(int rowId) { AC configuration = getFromCache(rowId); + + if (null == configuration) + { + throw new NotFoundException("Unable to save configuration"); + } + + return getConfigurationMap(configuration); + } + + protected Map getConfigurationMap(@NotNull AC configuration) + { return AuthenticationManager.getConfigurationMap(configuration); } } diff --git a/api/src/org/labkey/api/security/SsoSaveConfigurationAction.java b/api/src/org/labkey/api/security/SsoSaveConfigurationAction.java index cd3a798bedf..c3b5c27f941 100644 --- a/api/src/org/labkey/api/security/SsoSaveConfigurationAction.java +++ b/api/src/org/labkey/api/security/SsoSaveConfigurationAction.java @@ -120,9 +120,8 @@ public static void logLogoAction(User user, SSOAuthenticationConfiguration co } @Override - protected Map getConfigurationMap(int rowId) + protected Map getConfigurationMap(@NotNull AC configuration) { - AC configuration = getFromCache(rowId); return AuthenticationManager.getSsoConfigurationMap(configuration); } diff --git a/core/src/client/components/AuthRow.tsx b/core/src/client/components/AuthRow.tsx index 1362a830f22..988fb0500ff 100644 --- a/core/src/client/components/AuthRow.tsx +++ b/core/src/client/components/AuthRow.tsx @@ -44,6 +44,11 @@ export default class AuthRow extends PureComponent> { })); }; + closeDeleteModal = (): void => { + this.onToggleModal('deleteModalOpen', this.state.deleteModalOpen); + this.props.toggleModalOpen(false); + }; + render() { const { authConfig, @@ -141,11 +146,15 @@ export default class AuthRow extends PureComponent> { { - this.onToggleModal('deleteModalOpen', this.state.deleteModalOpen); - toggleModalOpen(false); + onCancel={this.closeDeleteModal} + onConfirm={() => { + // Close the confirmation modal as soon as the user confirms, regardless of whether the + // delete succeeds. On success the row unmounts anyway; on failure (e.g. the configuration + // was already removed, or FICAM-only mode was enabled in another tab and the provider is no + // longer available) the error is surfaced in the main window and the modal should not linger. + this.closeDeleteModal(); + onDelete?.(); }} - onConfirm={onDelete} title={`Permanently delete ${authConfig.provider} configuration?`} >
diff --git a/core/src/org/labkey/core/login/LoginController.java b/core/src/org/labkey/core/login/LoginController.java index 2a0d3ac1adc..b94f313e60d 100644 --- a/core/src/org/labkey/core/login/LoginController.java +++ b/core/src/org/labkey/core/login/LoginController.java @@ -1652,7 +1652,11 @@ public Object execute(ReturnUrlForm form, BindException errors) PrimaryAuthenticationConfiguration configuration = AuthenticationManager.getConfiguration(getViewContext().getSession()); if (configuration == null) { - throw new NotFoundException("No configuration found"); + throw new NotFoundException("Configuration was not found" + CONFIGURATION_ADVICE); + } + if (!configuration.isEnabled()) + { + throw new NotFoundException("Configuration is no longer active" + CONFIGURATION_ADVICE); } JSONObject resp = new JSONObject(); resp.put("description", configuration.getDescription()); @@ -1665,6 +1669,8 @@ public Object execute(ReturnUrlForm form, BindException errors) } } + private static final String CONFIGURATION_ADVICE = ". Please Sign Out and Sign In again."; + public static final String PASSWORD1_TEXT_FIELD_NAME = "password"; public static final String PASSWORD2_TEXT_FIELD_NAME = "password2"; diff --git a/devtools/src/org/labkey/devtools/view/testReauth.jsp b/devtools/src/org/labkey/devtools/view/testReauth.jsp index ecc40c615e4..58019bb6d12 100644 --- a/devtools/src/org/labkey/devtools/view/testReauth.jsp +++ b/devtools/src/org/labkey/devtools/view/testReauth.jsp @@ -34,19 +34,21 @@ success: function(response) { const needReauth = <%=form.reauthToken() == null%>; const data = JSON.parse(response.responseText).data; - document.getElementById("description").textContent = data.description; + document.getElementById("description").textContent = data.description; // Setting textContent HTML encodes the value if (needReauth) { document.getElementById("link").href = data.reauthUrl; } }, - failure: function() { - alert('Failed to retrieve configuration!'); - } + failure: LABKEY.Utils.getCallbackWrapper(function(errorInfo) { + document.getElementById("content").innerHTML = '' + LABKEY.Utils.encodeHtml(errorInfo.exception ?? 'Failed to retrieve configuration') + ''; + }, this, true) }); }); -You authenticated with:
+
+ + You authenticated with:
<% if (form.reauthToken() != null) @@ -79,3 +81,5 @@ You need to re-authenticate. <% } %> + +
diff --git a/devtools/test/src/org/labkey/test/pages/devtools/TestSecondaryPage.java b/devtools/test/src/org/labkey/test/pages/devtools/TestSecondaryPage.java new file mode 100644 index 00000000000..d21783470f6 --- /dev/null +++ b/devtools/test/src/org/labkey/test/pages/devtools/TestSecondaryPage.java @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2018-2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.test.pages.devtools; + +import org.junit.Assert; +import org.labkey.test.Locator; +import org.labkey.test.WebDriverWrapper; +import org.labkey.test.WebTestHelper; +import org.labkey.test.components.html.RadioButton; +import org.labkey.test.pages.LabKeyPage; +import org.openqa.selenium.WebDriver; +import org.openqa.selenium.WebElement; + +/** + * Wraps 'testSecondary.jsp' + */ +public class TestSecondaryPage extends LabKeyPage +{ + public TestSecondaryPage(WebDriver driver) + { + super(driver); + } + + public void denyIdentity() + { + elementCache().noButton.check(); + clickAndWait(elementCache().submitButton); + clearCache(); // Stays on same page + } + + public void confirmIdentity() + { + elementCache().yesButton.check(); + clickAndWait(elementCache().submitButton); + } + + @Override + protected ElementCache newElementCache() + { + return new ElementCache(); + } + + protected class ElementCache extends LabKeyPage.ElementCache + { + final RadioButton yesButton = RadioButton.RadioButton(Locator.radioButtonByNameAndValue("valid", "1")).findWhenNeeded(this); + final RadioButton noButton = RadioButton.RadioButton(Locator.radioButtonByNameAndValue("valid", "0")).findWhenNeeded(this); + final WebElement submitButton = Locator.name("TestSecondary").findWhenNeeded(this); + } +} diff --git a/devtools/test/src/org/labkey/test/pages/devtools/TestSsoPage.java b/devtools/test/src/org/labkey/test/pages/devtools/TestSsoPage.java new file mode 100644 index 00000000000..f4633890c8c --- /dev/null +++ b/devtools/test/src/org/labkey/test/pages/devtools/TestSsoPage.java @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.test.pages.devtools; + +import org.junit.Assert; +import org.labkey.test.Locator; +import org.labkey.test.WebDriverWrapper; +import org.labkey.test.WebTestHelper; +import org.labkey.test.components.html.Input; +import org.labkey.test.pages.LabKeyPage; +import org.openqa.selenium.WebDriver; +import org.openqa.selenium.WebElement; + +/** + * Wraps testSso.jsp + */ +public class TestSsoPage extends LabKeyPage +{ + public TestSsoPage(WebDriver driver) + { + super(driver); + } + + public void authenticate(String email) + { + attemptToAuthenticate(email); + Assert.assertEquals("Logged in as", email, getCurrentUser()); + } + + public void authenticateExpectingError(String email) + { + attemptToAuthenticate(email); + Assert.assertEquals("Logged in as", "guest", getCurrentUserName()); + } + + public void attemptToAuthenticate(String email) + { + elementCache().emailInput.set(email); + clickAndWait(elementCache().authenticateButton); + } + + @Override + protected ElementCache newElementCache() + { + return new ElementCache(); + } + + protected class ElementCache extends LabKeyPage.ElementCache + { + final Input emailInput = Input.Input(Locator.name("email"), getDriver()).findWhenNeeded(this); + // Might be "Authenticate" or "Reauthenticate" + final WebElement authenticateButton = Locator.lkButton().withClass("primary").findWhenNeeded(this); + } +} diff --git a/devtools/test/src/org/labkey/test/params/devtools/TestSsoProvider.java b/devtools/test/src/org/labkey/test/params/devtools/TestSsoProvider.java index 82189134588..e3010cdd36e 100644 --- a/devtools/test/src/org/labkey/test/params/devtools/TestSsoProvider.java +++ b/devtools/test/src/org/labkey/test/params/devtools/TestSsoProvider.java @@ -15,13 +15,19 @@ */ package org.labkey.test.params.devtools; +import org.labkey.test.TestFileUtils; import org.labkey.test.pages.core.login.LoginConfigRow; import org.labkey.test.pages.core.login.SsoAuthDialogBase; import org.labkey.test.params.login.AuthenticationProvider; import org.openqa.selenium.WebDriver; +import java.io.File; + public class TestSsoProvider extends AuthenticationProvider { + public static final File THUMBNAIL_COOL = TestFileUtils.getSampleData("thumbnails/Super Cool R Report/Thumbnail.png"); + public static final File THUMBNAIL_UNCOOL = TestFileUtils.getSampleData("thumbnails/Want To Be Cool/Thumbnail.png"); + @Override public String getProviderName() { diff --git a/devtools/test/src/org/labkey/test/tests/devtools/AuthenticationProviderReorderTest.java b/devtools/test/src/org/labkey/test/tests/devtools/AuthenticationProviderReorderTest.java index c80d8d92888..3f4703221a7 100644 --- a/devtools/test/src/org/labkey/test/tests/devtools/AuthenticationProviderReorderTest.java +++ b/devtools/test/src/org/labkey/test/tests/devtools/AuthenticationProviderReorderTest.java @@ -21,23 +21,22 @@ import org.labkey.test.BaseWebDriverTest; import org.labkey.test.BootstrapLocators; import org.labkey.test.Locator; -import org.labkey.test.TestFileUtils; import org.labkey.test.categories.Git; import org.labkey.test.pages.core.login.LoginConfigurePage; import org.labkey.test.params.devtools.TestSsoProvider; import org.labkey.test.util.login.AuthenticationAPIUtils; import org.openqa.selenium.WebElement; -import java.io.File; import java.util.Arrays; import java.util.List; +import static org.labkey.test.params.devtools.TestSsoProvider.THUMBNAIL_COOL; +import static org.labkey.test.params.devtools.TestSsoProvider.THUMBNAIL_UNCOOL; + @Category({Git.class}) public class AuthenticationProviderReorderTest extends BaseWebDriverTest { private static final TestSsoProvider PROVIDER = new TestSsoProvider(); - private static final File THUMBNAIL_COOL = TestFileUtils.getSampleData("thumbnails/Super Cool R Report/Thumbnail.png"); - private static final File THUMBNAIL_UNCOOL = TestFileUtils.getSampleData("thumbnails/Want To Be Cool/Thumbnail.png"); @Override protected void doCleanup(boolean afterTest) diff --git a/devtools/test/src/org/labkey/test/tests/devtools/SecondaryAuthenticationTest.java b/devtools/test/src/org/labkey/test/tests/devtools/SecondaryAuthenticationTest.java index 72ba09a00e0..c6608ea69fc 100644 --- a/devtools/test/src/org/labkey/test/tests/devtools/SecondaryAuthenticationTest.java +++ b/devtools/test/src/org/labkey/test/tests/devtools/SecondaryAuthenticationTest.java @@ -16,6 +16,7 @@ package org.labkey.test.tests.devtools; import org.apache.commons.lang3.time.DateUtils; +import org.junit.Assert; import org.junit.Test; import org.junit.experimental.categories.Category; import org.labkey.remoteapi.CommandException; @@ -29,6 +30,7 @@ import org.labkey.test.categories.Git; import org.labkey.test.pages.core.login.LoginConfigRow; import org.labkey.test.pages.core.login.LoginConfigurePage; +import org.labkey.test.pages.devtools.TestSecondaryPage; import org.labkey.test.params.devtools.SecondaryAuthenticationProvider; import org.labkey.test.util.PasswordUtil; import org.labkey.test.util.login.AuthenticationAPIUtils; @@ -119,21 +121,19 @@ public void testSecondaryAuthentication() //'Sign In' link shouldn't be present waitForElementToDisappear(Locator.linkContainingText("Sign In")); + TestSecondaryPage secondaryAuthPage = new TestSecondaryPage(getDriver()); + //User should be still recognized as guest until secondary authentication is successful. assertTextPresent("Is " + PasswordUtil.getUsername() +" really you?"); //Select Radio button No - checkRadioButton(Locator.radioButtonByNameAndValue("valid", "0")); - click(Locator.input("TestSecondary")); + secondaryAuthPage.denyIdentity(); //should stay on Secondary Authentication page until user selects Yes radio assertTextPresent("Secondary Authentication"); + Assert.assertEquals("Logged in as", "guest", getCurrentUserName()); - //Select radio Yes - checkRadioButton(Locator.radioButtonByNameAndValue("valid", "1")); - - //Click on button 'TestSecondary' - clickAndWait(Locator.input("TestSecondary")); + secondaryAuthPage.confirmIdentity(); //get current relative URL after Sign In String relativeURLAfterSignIn = getCurrentRelativeURL();