Skip to content

Admin option to customize frame-ancestors #7723

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

Open
labkey-adam wants to merge 4 commits into
base: develop
Choose a base branch
from
+91 −15
Open

Admin option to customize frame-ancestors #7723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7591ab7
Admin option to customize frame-ancestors
labkey-adam Jun 3, 2026
3ad8089
Add test. Fix double report-to directive. More reliable detection of …
labkey-adam Jun 3, 2026
6984afc
Correct JavaDoc
labkey-adam Jun 3, 2026
2b3a550
Merge remote-tracking branch 'origin/develop' into fb_frame_ancestors
labkey-adam Jun 3, 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
6 changes: 4 additions & 2 deletions api/src/org/labkey/api/security/Directive.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@
import org.labkey.api.util.SafeToRenderEnum;

/**
* All CSP directives that support substitutions. These constant names are persisted to the database, so be careful with
* any changes. If adding a Directive, make sure to add the corresponding substitutions in LabKeyServer baseCsp.
* All CSP directives that support substitutions. These constant names are persisted to the database, so be careful
* with any changes. If adding a Directive, make sure to add the corresponding substitutions to the appropriate CSP
* template(s) in LabKeyServer.
*/
public enum Directive implements StartupProperty, SafeToRenderEnum
{
Connection("connect-src", "Sources for fetch/XHR requests"),
Font("font-src", "Sources for fonts"),
Frame("frame-src", "Sources for iframes"),
FrameAncestors("frame-ancestors", "Parent hosts allowed to embed this site's resources in an <iframe>, etc."),
Image("image-src", "Sources for images"),
Object("object-src", "Sources for objects"), // Issue 53226
Script("script-src", "Sources for scripts"),
Expand Down
62 changes: 52 additions & 10 deletions api/src/org/labkey/filters/ContentSecurityPolicyFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public class ContentSecurityPolicyFilter implements Filter
// Per-filter-instance parameters that are set in init() and never changed
private ContentSecurityPolicyType _type = ContentSecurityPolicyType.Enforce;
private @NotNull String _cspVersion = "Unknown";
// These two are effectively @NotNull since they are set to non-null values in init() and never changed
// This is effectively @NotNull since it's set to non-null values in init() and never changed
private String _stashedTemplate = null;

// We can't set this statically because the class is referenced before URLProviders are available
Expand Down Expand Up @@ -208,18 +208,13 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
{
if (request instanceof HttpServletRequest req && response instanceof HttpServletResponse resp)
{
StringExpression expression = ensurePolicyExpression();
String csp = getSubstitutedCsp(req);

if (getType() != ContentSecurityPolicyType.Enforce || !OptionalFeatureService.get().isFeatureEnabled(FEATURE_FLAG_DISABLE_ENFORCE_CSP))
if (csp != null)
{
Map<String, String> map = Map.of(NONCE_SUBST, getScriptNonceHeader(req));
String csp = expression.eval(map);

if ("https".equals(req.getScheme()))
if ("https".equals(req.getScheme()) && resp.getHeader(REPORTING_ENDPOINTS_HEADER) == null)
{
if (resp.getHeader(REPORTING_ENDPOINTS_HEADER) == null)
resp.addHeader(REPORTING_ENDPOINTS_HEADER, _reportingEndpointsHeaderValue);
csp = csp + " report-to csp-report ;";
resp.addHeader(REPORTING_ENDPOINTS_HEADER, _reportingEndpointsHeaderValue);
}

resp.setHeader(getType().getHeaderName(), csp);
Expand All @@ -228,6 +223,26 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
chain.doFilter(request, response);
}

private String getSubstitutedCsp(HttpServletRequest req)
{
StringExpression expression = ensurePolicyExpression();

if (getType() != ContentSecurityPolicyType.Enforce || !OptionalFeatureService.get().isFeatureEnabled(FEATURE_FLAG_DISABLE_ENFORCE_CSP))
{
Map<String, String> map = Map.of(NONCE_SUBST, getScriptNonceHeader(req));
String csp = expression.eval(map);

if ("https".equals(req.getScheme()))
{
csp = csp + " report-to csp-report ;";
}

return csp;
}

return null;
}

public ContentSecurityPolicyType getType()
{
return _type;
Expand Down Expand Up @@ -347,6 +362,18 @@ public static boolean hasCsp(ContentSecurityPolicyType type)
return CSP_FILTERS.get(type) != null;
}

public static @Nullable String getStashedTemplate(ContentSecurityPolicyType type)
{
var filter = CSP_FILTERS.get(type);
return filter != null ? filter._stashedTemplate : null;
}

public static @Nullable String getSubstitutedCsp(ContentSecurityPolicyType type, HttpServletRequest req)
{
var filter = CSP_FILTERS.get(type);
return filter != null ? filter.getSubstitutedCsp(req) : null;
}

public static @NotNull String getCspVersion(@Nullable String disposition)
{
if (disposition != null)
Expand Down Expand Up @@ -565,6 +592,21 @@ public void testSubstitutionMap()
verifySubstitutionInPolicyExpressions("'none'", 0);
verifySubstitutionInPolicyExpressions("ObjectSource", 1);
verifySubstitutionInPolicyExpressions("BetterObjectStore", 1);

unregisterAllowedSources("frameancestors", Directive.FrameAncestors);
registerAllowedSources("frameancestors", Directive.FrameAncestors, "AncestorSource", "AnotherAncestor");
assertEquals(7, ALLOWED_SOURCES.size());
verifySubstitutionMapSize(5);
// frame-ancestors is enforce-only (absent from the report CSP template), so check the substitution map directly
String ancestorKey = Directive.FrameAncestors.getSubstitutionKey();
assertTrue(SUBSTITUTION_MAP.get(ancestorKey).contains("AncestorSource"));
assertTrue(SUBSTITUTION_MAP.get(ancestorKey).contains("AnotherAncestor"));
unregisterAllowedSources("frameancestors", Directive.FrameAncestors);
assertEquals(7, ALLOWED_SOURCES.size()); // Entry still exists but should be empty
assertTrue(ALLOWED_SOURCES.get(Directive.FrameAncestors).isEmpty());
verifySubstitutionMapSize(4); // Back to the way it was
verifySubstitutionInPolicyExpressions("AncestorSource", 0);
verifySubstitutionInPolicyExpressions("AnotherAncestor", 0);
}
finally
{
Expand Down
38 changes: 35 additions & 3 deletions core/src/org/labkey/core/admin/AdminController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@
import org.labkey.api.view.ViewContext;
import org.labkey.api.view.ViewServlet;
import org.labkey.api.view.WebPartView;
import org.labkey.api.view.template.EmptyView;
import org.labkey.api.view.template.ClientDependency;
import org.labkey.api.view.template.EmptyView;
import org.labkey.api.view.template.PageConfig;
import org.labkey.api.view.template.PageConfig.Template;
import org.labkey.api.wiki.WikiRendererType;
Expand All @@ -343,6 +343,7 @@
import org.labkey.core.security.SecurityController;
import org.labkey.data.xml.TablesDocument;
import org.labkey.filters.ContentSecurityPolicyFilter;
import org.labkey.filters.ContentSecurityPolicyFilter.ContentSecurityPolicyType;
import org.labkey.security.xml.GroupEnumType;
import org.labkey.vfs.FileLike;
import org.springframework.mock.web.MockHttpServletResponse;
Expand Down Expand Up @@ -427,9 +428,10 @@
import static org.labkey.api.util.DOM.IMG;
import static org.labkey.api.util.DOM.LI;
import static org.labkey.api.util.DOM.P;
import static org.labkey.api.util.DOM.PRE;
import static org.labkey.api.util.DOM.SPAN;
import static org.labkey.api.util.DOM.STYLE;
import static org.labkey.api.util.DOM.STRONG;
import static org.labkey.api.util.DOM.STYLE;
import static org.labkey.api.util.DOM.TABLE;
import static org.labkey.api.util.DOM.TD;
import static org.labkey.api.util.DOM.TH;
Expand Down Expand Up @@ -11587,14 +11589,44 @@ public ModelAndView getView(ExternalSourcesForm form, boolean reshow, BindExcept
{
boolean isTroubleshooter = !getContainer().hasPermission(getUser(), ApplicationAdminPermission.class);

String template = formatCsp(ContentSecurityPolicyFilter.getStashedTemplate(ContentSecurityPolicyType.Enforce), "No enforce CSP is present!");
String substituted = formatCsp(ContentSecurityPolicyFilter.getSubstitutedCsp(ContentSecurityPolicyType.Enforce, getViewContext().getRequest()), "Enforce CSP is disabled!");
HtmlView csps = new HtmlView("Current Enforce CSP",
TABLE(
TR(
TH(STRONG("With Substitution Placeholders")), TH(STRONG("With Substituted Values"))
),
TR(
TD(at(style, "padding-right: 20px;"), PRE(template)), TD(PRE(substituted))
)
)
);
JspView<ExternalSourcesForm> newView = new JspView<>("/org/labkey/core/admin/addNewExternalSource.jsp", form, errors);
newView.setTitle(isTroubleshooter ? "Overview" : "Register New External Resource Host");
newView.setFrame(WebPartView.FrameType.PORTAL);
JspView<ExternalSourcesForm> existingView = new JspView<>("/org/labkey/core/admin/existingExternalSources.jsp", form, errors);
existingView.setTitle("Existing External Resource Hosts");
existingView.setFrame(WebPartView.FrameType.PORTAL);

return new VBox(newView, existingView);
return new VBox(csps, newView, existingView);
}

private static final String UPGRADE_INSECURE_REQUESTS = "${UPGRADE.INSECURE.REQUESTS}";

private String formatCsp(@Nullable String csp, String defaultValue)
{
if (csp != null)
{
// There's no semicolon at the end of this substitution, but we want it to show it on its own line
csp = csp.replace(UPGRADE_INSECURE_REQUESTS + " ", UPGRADE_INSECURE_REQUESTS + "\n");
return Arrays.stream(csp.split(";"))
.map(String::trim)
.collect(Collectors.joining(" ;\n"));
}
else
{
return defaultValue;
}
}

private static final Object HOST_LOCK = new Object();
Expand Down