Skip to content

changes to fix stale profile issue #251

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.

Sign up for GitHub

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

Merged
mimi-tam merged 2 commits into from
Apr 30, 2026
+150 −4
Merged

changes to fix stale profile issue #251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f9f66d
changes to fix stale profile issue
gs-saurabh Apr 30, 2026
24921bb
Enhance LegendSecurityLogic to clear stale profiles from session base
gs-saurabh Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions legend-shared-pac4j/src/main/java/org/finos/legend/server/pac4j/LegendSecurityLogic.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import static org.finos.legend.server.pac4j.LegendRequestHandler.REDIRECT_PROTO_ATTRIBUTE;

Expand All @@ -41,21 +43,21 @@ public R perform(C context,
boolean isConstraintKerberosFlow = (boolean) context.getRequestAttribute(IS_CONSTRAINED_KERBEROS_FLOW).orElse(false);
if (!isConstraintKerberosFlow)
{
LOGGER.info("NonConstrained host, falling back to default handling");
LOGGER.debug("NonConstrained host, falling back to default handling");
return callParentPerform(context, config, securityGrantedAccessAdapter, httpActionAdapter, clients, CommonHelper.isBlank(authorizers) ? "none" : authorizers, matchers, false, parameters);
}
boolean multiProfile = inputMultiProfile != null && inputMultiProfile;
if (!multiProfile)
{
LOGGER.info("MultiProfile turned off falling back to default handling");
LOGGER.debug("MultiProfile turned off falling back to default handling");
return callParentPerform(context, config, securityGrantedAccessAdapter, httpActionAdapter, clients, authorizers, matchers, inputMultiProfile, parameters);
}
if (nonBrowserCall(context))
{
LOGGER.info("Non-browser call detected, falling back to default handling");
LOGGER.debug("Non-browser call detected, falling back to default handling");
return callParentPerform(context, config, securityGrantedAccessAdapter, httpActionAdapter, clients, CommonHelper.isBlank(authorizers) ? "none" : authorizers, matchers, false, parameters);
}
LOGGER.info("Browser call detected, using LegendSecurityLogic handling");
LOGGER.debug("Browser call detected, using LegendSecurityLogic handling");
LOGGER.debug("url: {}", context.getFullRequestURL());
LOGGER.debug("clients: {}", clients);
try
Expand Down Expand Up @@ -127,6 +129,18 @@ private boolean isValidProfilePresent(List<UserProfile> profiles, Client<? exten

R callParentPerform(C context, Config config, SecurityGrantedAccessAdapter<R, C> securityGrantedAccessAdapter, HttpActionAdapter<R, C> httpActionAdapter, String clients, String authorizers, String matchers, Boolean inputMultiProfile, Object[] parameters)
{
Set<String> requested = getClientFinder().find(config.getClients(), context, clients).stream()
.map(Client::getName)
.collect(Collectors.toSet());

ProfileManager<UserProfile> manager = getProfileManager(context);
boolean hasStaleProfile = manager.getAll(true).stream()
.anyMatch(p -> !requested.contains(p.getClientName()));
if (hasStaleProfile)
{
LOGGER.debug("Session contains profiles outside requested clients {} -> clearing session", requested);
manager.remove(true);
}
LOGGER.debug("Calling parent perform method");
return super.perform(context, config, securityGrantedAccessAdapter,
httpActionAdapter, clients, authorizers, matchers, inputMultiProfile, parameters);
Expand Down
132 changes: 132 additions & 0 deletions legend-shared-pac4j/src/test/java/org/finos/legend/server/pac4j/LegendSecurityLogicTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,138 @@ public void testPerform_WithIndirectClient_ExpiredProfile() throws Exception

}

@Test
public void testCallParentPerform_StaleSessionProfile_ClearsSession() throws Exception
{
UserProfile staleProfile = mock(UserProfile.class);
when(staleProfile.getClientName()).thenReturn("clientA");

ProfileManager profileManager = mock(ProfileManager.class);
when(profileManager.getAll(true)).thenReturn(Collections.singletonList(staleProfile));
legendSecurityLogic.setProfileManagerFactory((c) -> profileManager);

ClientFinder clientFinder = mock(ClientFinder.class);
IndirectClient clientB = mock(IndirectClient.class);
when(clientB.getName()).thenReturn("clientB");
when(clientFinder.find(any(), any(), anyString()))
.thenReturn(Collections.singletonList(clientB));
legendSecurityLogic.setClientFinder(clientFinder);

// Make super.perform short-circuit to the grant-access path.
legendSecurityLogic.setMatchingChecker(matchingChecker);
when(matchingChecker.matches(any(), any(), any(), anyList())).thenReturn(false);

Clients clients = mock(Clients.class);
when(config.getClients()).thenReturn(clients);
SecurityGrantedAccessAdapter adapter = mock(SecurityGrantedAccessAdapter.class);

legendSecurityLogic.callParentPerform(
webContext, config, adapter, httpActionAdapter,
"clientB", "", "matchers", true, new Object[0]);


verify(profileManager, times(1)).remove(true);
verify(clientFinder, times(2)).find(any(), any(), anyString());
}

@Test
public void testCallParentPerform_MatchingSessionProfile_KeepsSession() throws Exception
{
UserProfile matchingProfile = mock(UserProfile.class);
when(matchingProfile.getClientName()).thenReturn("clientB");

ProfileManager profileManager = mock(ProfileManager.class);
when(profileManager.getAll(true)).thenReturn(Collections.singletonList(matchingProfile));
legendSecurityLogic.setProfileManagerFactory((c) -> profileManager);

ClientFinder clientFinder = mock(ClientFinder.class);
IndirectClient clientB = mock(IndirectClient.class);
when(clientB.getName()).thenReturn("clientB");
when(clientFinder.find(any(), any(), anyString()))
.thenReturn(Collections.singletonList(clientB));
legendSecurityLogic.setClientFinder(clientFinder);

legendSecurityLogic.setMatchingChecker(matchingChecker);
when(matchingChecker.matches(any(), any(), any(), anyList())).thenReturn(false);

Clients clients = mock(Clients.class);
when(config.getClients()).thenReturn(clients);
SecurityGrantedAccessAdapter adapter = mock(SecurityGrantedAccessAdapter.class);

legendSecurityLogic.callParentPerform(
webContext, config, adapter, httpActionAdapter,
"clientB", "", "matchers", true, new Object[0]);

verify(profileManager, never()).remove(anyBoolean());
verify(clientFinder, times(2)).find(any(), any(), anyString());
}

@Test
public void testCallParentPerform_EmptySession_NoRemoval() throws Exception
{
ProfileManager profileManager = mock(ProfileManager.class);
when(profileManager.getAll(true)).thenReturn(Collections.emptyList());
legendSecurityLogic.setProfileManagerFactory((c) -> profileManager);

ClientFinder clientFinder = mock(ClientFinder.class);
IndirectClient clientB = mock(IndirectClient.class);
when(clientB.getName()).thenReturn("clientB");
when(clientFinder.find(any(), any(), anyString()))
.thenReturn(Collections.singletonList(clientB));
legendSecurityLogic.setClientFinder(clientFinder);

legendSecurityLogic.setMatchingChecker(matchingChecker);
when(matchingChecker.matches(any(), any(), any(), anyList())).thenReturn(false);

Clients clients = mock(Clients.class);
when(config.getClients()).thenReturn(clients);
SecurityGrantedAccessAdapter adapter = mock(SecurityGrantedAccessAdapter.class);

legendSecurityLogic.callParentPerform(
webContext, config, adapter, httpActionAdapter,
"clientB", "", "matchers", true, new Object[0]);

verify(profileManager, never()).remove(anyBoolean());
verify(clientFinder, times(2)).find(any(), any(), anyString());
}

@Test
public void testCallParentPerform_MixedProfiles_AnyStaleClearsWholeSession() throws Exception
{
UserProfile validProfile = mock(UserProfile.class);
when(validProfile.getClientName()).thenReturn("clientB");
UserProfile staleProfile = mock(UserProfile.class);
when(staleProfile.getClientName()).thenReturn("clientX");

ProfileManager profileManager = mock(ProfileManager.class);
when(profileManager.getAll(true))
.thenReturn(java.util.Arrays.asList(validProfile, staleProfile));
legendSecurityLogic.setProfileManagerFactory((c) -> profileManager);

ClientFinder clientFinder = mock(ClientFinder.class);
IndirectClient clientB = mock(IndirectClient.class);
when(clientB.getName()).thenReturn("clientB");
IndirectClient clientC = mock(IndirectClient.class);
when(clientC.getName()).thenReturn("clientC");
when(clientFinder.find(any(), any(), anyString()))
.thenReturn(java.util.Arrays.asList(clientB, clientC));
legendSecurityLogic.setClientFinder(clientFinder);

legendSecurityLogic.setMatchingChecker(matchingChecker);
when(matchingChecker.matches(any(), any(), any(), anyList())).thenReturn(false);

Clients clients = mock(Clients.class);
when(config.getClients()).thenReturn(clients);
SecurityGrantedAccessAdapter adapter = mock(SecurityGrantedAccessAdapter.class);

legendSecurityLogic.callParentPerform(
webContext, config, adapter, httpActionAdapter,
"clientB,clientC", "", "matchers", true, new Object[0]);

verify(profileManager, times(1)).remove(true);
verify(clientFinder, times(2)).find(any(), any(), anyString());
}

private static class TestableLegendSecurityLogic<R, C extends WebContext> extends LegendSecurityLogic<R, C> {
private HttpAction mockedRedirectResponse;

Expand Down
Loading