Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 16 additions & 1 deletion api/src/org/labkey/api/action/PermissionCheckableAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.labkey.api.action;

import jakarta.servlet.http.HttpServletResponse;
import org.apache.logging.log4j.Logger;
import org.jetbrains.annotations.Nullable;
import org.labkey.api.data.Container;
import org.labkey.api.module.IgnoresForbiddenProjectCheck;
Expand All @@ -39,6 +40,7 @@
import org.labkey.api.security.roles.RoleManager;
import org.labkey.api.util.ConfigurationException;
import org.labkey.api.util.HttpUtil;
import org.labkey.api.util.logging.LogHelper;
import org.labkey.api.view.BadRequestException;
import org.labkey.api.view.NotFoundException;
import org.labkey.api.view.RedirectException;
Expand All @@ -55,6 +57,7 @@

public abstract class PermissionCheckableAction implements Controller, PermissionCheckable, HasViewContext
{
private static final Logger LOG = LogHelper.getLogger(PermissionCheckableAction.class, "Permission checks for actions");
private static final HttpUtil.Method[] arrayGetPost = new HttpUtil.Method[] {Method.GET, Method.POST};
private ViewContext _context = null;
UnauthorizedException.Type _unauthorizedType = UnauthorizedException.Type.redirectToLogin;
Expand Down Expand Up @@ -148,6 +151,8 @@ private void _checkActionPermissions(Set<Role> contextualRoles) throws Unauthori
Container c = context.getContainer();
User user = context.getUser();
Class<? extends Controller> actionClass = getClass();
if (LOG.isDebugEnabled())
LOG.debug(actionClass.getName() + ": checking permissions for user " + (user == null ? "<null>" : user.getName() + " (impersonated=" + user.isImpersonated() + ")"));

if (!actionClass.isAnnotationPresent(IgnoresForbiddenProjectCheck.class))
c.throwIfForbiddenProject(user);
Expand All @@ -159,18 +164,22 @@ private void _checkActionPermissions(Set<Role> contextualRoles) throws Unauthori
methodsAllowed = methodsAllowedAnnotation.value();
if (Arrays.stream(methodsAllowed).noneMatch(s -> s.equals(method)))
{
throw new BadRequestException("Method Not Allowed: " + method, null, HttpServletResponse.SC_METHOD_NOT_ALLOWED);
String msg = "Method Not Allowed: " + method;
LOG.debug(msg);
throw new BadRequestException(msg, null, HttpServletResponse.SC_METHOD_NOT_ALLOWED);
}

boolean requiresSiteAdmin = actionClass.isAnnotationPresent(RequiresSiteAdmin.class);
if (requiresSiteAdmin && !user.hasSiteAdminPermission())
{
LOG.debug(actionClass.getName() + ": action requires site admin permissions");
throw new UnauthorizedException();
}

boolean requiresLogin = actionClass.isAnnotationPresent(RequiresLogin.class);
if (requiresLogin && user.isGuest())
{
LOG.debug(actionClass.getName() + ": action requires login (non-guest)");
throw new UnauthorizedException();
}

Expand Down Expand Up @@ -214,7 +223,10 @@ private void _checkActionPermissions(Set<Role> contextualRoles) throws Unauthori
// Must have all permissions in permissionsRequired
if (!SecurityManager.hasAllPermissions(this.getClass().getName()+"_checkActionPermissions",
c, user, permissionsRequired, contextualRoles))
{
LOG.debug(actionClass.getName() + ": action requires all permissions: " + permissionsRequired);
throw new UnauthorizedException();
}

CSRF.Method csrfCheck = actionClass.isAnnotationPresent(CSRF.class) ? actionClass.getAnnotation(CSRF.class).value() : CSRF.Method.POST;
csrfCheck.validate(context);
Expand All @@ -228,7 +240,10 @@ private void _checkActionPermissions(Set<Role> contextualRoles) throws Unauthori
Collections.addAll(permissionsAnyOf, requiresAnyOf.value());
if (!SecurityManager.hasAnyPermissions(this.getClass().getName() + "_checkActionPermissions",
c, user, permissionsAnyOf, contextualRoles))
{
LOG.debug(actionClass.getName() + ": action requires any permissions: " + permissionsAnyOf);
throw new UnauthorizedException();
}
}

boolean requiresNoPermission = actionClass.isAnnotationPresent(RequiresNoPermission.class);
Expand Down
12 changes: 9 additions & 3 deletions api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,8 @@ private DataIterator checkData(
DomainProperty wellLocationPropFinder = null;
DomainProperty wellLsidPropFinder = null;

RemapCache cache = new RemapCache();
RemapCache cacheWithoutPkLookup = new RemapCache();
RemapCache cacheWithPkLookup = new RemapCache();
Map<DomainProperty, TableInfo> remappableLookup = new HashMap<>();
Map<Long, ExpMaterial> materialCache = new LongHashMap<>();
Map<Long, Map<String, Long>> plateWellCache = new LongHashMap<>();
Expand Down Expand Up @@ -879,7 +880,12 @@ else if (entry.getKey().equalsIgnoreCase(ProvenanceService.PROVENANCE_INPUT_PROP
{
String s = o instanceof String ? (String) o : o.toString();
TableInfo lookupTable = remappableLookup.get(pd);
Object remapped = cache.remap(lookupTable, s, true);

// GitHub Issue #443: similar to LookupResolutionType.alternateThenPrimaryKey, we want to check if the string value remaps using alternate keys (titleColumn) first
Object remapped = cacheWithoutPkLookup.remap(lookupTable, s, false);
if (remapped == null)
remapped = cacheWithPkLookup.remap(lookupTable, s, true);

if (remapped == null)
{
if (SAMPLE_CONCEPT_URI.equals(pd.getConceptURI()))
Expand Down Expand Up @@ -1026,7 +1032,7 @@ else if (o instanceof MvFieldWrapper mvWrapper)
try
{
if (material == null)
material = exp.findExpMaterial(lookupContainer, user, materialName, byNameSS, cache, materialCache);
material = exp.findExpMaterial(lookupContainer, user, materialName, byNameSS, cacheWithoutPkLookup, materialCache);
}
catch (ValidationException ve)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ private LookAndFeelResourceType()
@Override
public void addWhereSql(SQLFragment sql, String parentColumn, String documentNameColumn)
{
// Keep in sync with CoreMigrationSchemaHandler.copyAttachments()
sql.append(parentColumn).append(" IN (SELECT EntityId FROM ").append(CoreSchema.getInstance().getTableInfoContainers(), "c").append(") AND (");
sql.append(documentNameColumn).append(" IN (?, ?) OR ");
sql.add(AttachmentCache.FAVICON_FILE_NAME);
Expand Down
13 changes: 11 additions & 2 deletions api/src/org/labkey/api/data/Container.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.labkey.api.query.QueryService;
import org.labkey.api.security.HasPermission;
import org.labkey.api.security.SecurableResource;
import org.labkey.api.security.SecurityLogger;
import org.labkey.api.security.SecurityManager;
import org.labkey.api.security.SecurityPolicy;
import org.labkey.api.security.SecurityPolicyManager;
Expand Down Expand Up @@ -550,7 +551,11 @@ private boolean handleForbiddenProject(User user, Set<Role> contextualRoles, boo
if (null != impersonationProject && !impersonationProject.equals(currentProject))
{
if (shouldThrow)
throw new ForbiddenProjectException("You are not allowed to access this folder while impersonating within a different project.");
{
String msg = "You are not allowed to access this folder while impersonating within a different project.";
SecurityLogger.log(msg, user, null, null);
throw new ForbiddenProjectException(msg);
}

return true;
}
Expand All @@ -563,7 +568,11 @@ private boolean handleForbiddenProject(User user, Set<Role> contextualRoles, boo
if (lockState.isLocked() && ContainerManager.LOCKED_PROJECT_HANDLER.isForbidden(currentProject, user, contextualRoles, lockState))
{
if (shouldThrow)
throw new ForbiddenProjectException("You are not allowed to access this folder; it is " + lockState.getDescription() + ".");
{
String msg = "You are not allowed to access this folder; it is " + lockState.getDescription() + ".";
SecurityLogger.log(msg, user, null, null);
throw new ForbiddenProjectException(msg);
}

return true;
}
Expand Down
29 changes: 29 additions & 0 deletions api/src/org/labkey/api/data/SQLParameterException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright (c) 2011 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.api.data;

import org.labkey.api.util.SkipMothershipLogging;

/**
* Signals there was a problem with a SQL parameter, such as a conversion problem.
*/
public class SQLParameterException extends SQLGenerationException implements SkipMothershipLogging
{
public SQLParameterException(String message)
{
super(message);
}
}
35 changes: 35 additions & 0 deletions api/src/org/labkey/api/data/SimpleFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -1865,6 +1865,41 @@ private int howManyLessThan(List<Integer> userIdsDesc, int max)
}
return howMany;
}

/**
* This used to be a PostgreSQL-specific test, but it should run and pass on SQL Server as well. It's largely
* redundant with testLargeInClause() above, but causes no harm.
*/
@Test
public void testTempTableInClause()
{
DbSchema core = CoreSchema.getInstance().getSchema();
SqlDialect d = core.getSqlDialect();

Collection<Integer> allUserIds = new TableSelector(CoreSchema.getInstance().getTableInfoUsersData(), Collections.singleton("UserId")).getCollection(Integer.class);
SQLFragment shortSql = new SQLFragment("SELECT * FROM core.UsersData WHERE UserId");
d.appendInClauseSql(shortSql, allUserIds);
assertEquals(allUserIds.size(), new SqlSelector(core, shortSql).getRowCount());

ArrayList<Object> l = new ArrayList<>(allUserIds);
while (l.size() < SqlDialect.TEMP_TABLE_GENERATOR_MIN_SIZE)
l.addAll(allUserIds);
SQLFragment longSql = new SQLFragment("SELECT * FROM core.UsersData WHERE UserId");
d.appendInClauseSql(longSql, l);
assertEquals(allUserIds.size(), new SqlSelector(core, longSql).getRowCount());

Collection<String> allDisplayNames = new TableSelector(CoreSchema.getInstance().getTableInfoUsersData(), Collections.singleton("DisplayName")).getCollection(String.class);
SQLFragment shortSqlStr = new SQLFragment("SELECT * FROM core.UsersData WHERE DisplayName");
d.appendInClauseSql(shortSqlStr, allDisplayNames);
assertEquals(allDisplayNames.size(), new SqlSelector(core, shortSqlStr).getRowCount());

l = new ArrayList<>(allDisplayNames);
while (l.size() < SqlDialect.TEMP_TABLE_GENERATOR_MIN_SIZE)
l.addAll(allDisplayNames);
SQLFragment longSqlStr = new SQLFragment("SELECT * FROM core.UsersData WHERE DisplayName");
d.appendInClauseSql(longSqlStr, l);
assertEquals(allDisplayNames.size(), new SqlSelector(core, longSqlStr).getRowCount());
}
}

public static class BetweenClauseTestCase extends ClauseTestCase
Expand Down
4 changes: 3 additions & 1 deletion api/src/org/labkey/api/data/SqlExecutingSelector.java
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,9 @@ public void handleSqlException(SQLException e, @Nullable Connection conn)
if (null != _sql)
ExceptionUtil.decorateException(e, ExceptionUtil.ExceptionInfo.DialectSQL, _sql.toDebugString(), false);

throw getExceptionFramework().translate(getScope(), "ExecutingSelector", e);
RuntimeException translated = getExceptionFramework().translate(getScope(), "ExecutingSelector", e);
ExceptionUtil.copyDecorations(e, translated);
throw translated;
}
}
}
10 changes: 6 additions & 4 deletions api/src/org/labkey/api/data/Table.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.apache.commons.beanutils.BeanUtils;
import org.apache.commons.beanutils.PropertyUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.Strings;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Logger;
import org.jetbrains.annotations.NotNull;
Expand Down Expand Up @@ -50,6 +51,7 @@
import org.labkey.api.query.RuntimeValidationException;
import org.labkey.api.security.User;
import org.labkey.api.util.BaseScanner;
import org.labkey.api.util.ExceptionUtil;
import org.labkey.api.util.GUID;
import org.labkey.api.util.JunitUtil;
import org.labkey.api.util.MemTracker;
Expand Down Expand Up @@ -571,7 +573,7 @@ Object getObject(ResultSet rs) throws SQLException
// Standard SQLException catch block: log exception, query SQL, and params
static void logException(@Nullable SQLFragment sql, @Nullable Connection conn, SQLException e, Level logLevel)
{
if (SqlDialect.isCancelException(e))
if (SqlDialect.isCancelException(e) || ExceptionUtil.isIgnorable(e))
{
return;
}
Expand All @@ -585,7 +587,7 @@ static void logException(@Nullable SQLFragment sql, @Nullable Connection conn, S
String trim = sql.getSQL().trim();

// Treat a ConstraintException during INSERT/UPDATE as a WARNING. Treat all other SQLExceptions as an ERROR.
if (RuntimeSQLException.isConstraintException(e) && (StringUtils.startsWithIgnoreCase(trim, "INSERT") || StringUtils.startsWithIgnoreCase(trim, "UPDATE")))
if (RuntimeSQLException.isConstraintException(e) && (Strings.CI.startsWith(trim, "INSERT") || Strings.CI.startsWith(trim, "UPDATE")))
{
// Log this ConstraintException if log Level is WARN (the default) or lower. Skip logging for callers that request just ERRORs.
if (Level.WARN.isMoreSpecificThan(logLevel))
Expand Down Expand Up @@ -1409,7 +1411,7 @@ public void testSelect() throws SQLException
//noinspection EmptyTryBlock,UnusedDeclaration
try (ResultSet rs = new TableSelector(tinfo).getResultSet()){}

Map[] maps = new TableSelector(tinfo).getMapArray();
Map<?, ?>[] maps = new TableSelector(tinfo).getMapArray();
assertNotNull(maps);

Principal[] principals = new TableSelector(tinfo).getArray(Principal.class);
Expand Down Expand Up @@ -1739,7 +1741,7 @@ public static ParameterMapStatement deleteStatement(Connection conn, TableInfo t
//

Domain domain = tableDelete.getDomain();
DomainKind domainKind = tableDelete.getDomainKind();
DomainKind<?> domainKind = tableDelete.getDomainKind();
if (null != domain && null != domainKind && StringUtils.isEmpty(domainKind.getStorageSchemaName()))
{
if (!d.isPostgreSQL() && !d.isSqlServer())
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/data/TableSelector.java
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ public Results getResultsAsync(final boolean cache, final boolean scrollable, Ht
}

/**
* Setting this options asks the TableSelector to add additional display columns to the generated SQL, as well
* Setting this option asks the TableSelector to add additional display columns to the generated SQL, as well

* as forcing the results to be sorted.
* @return this
Expand Down
2 changes: 2 additions & 0 deletions api/src/org/labkey/api/data/TempTableInClauseGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,9 @@ else if (jdbcType == JdbcType.INTEGER)
try (var ignored = SpringActionController.ignoreSqlUpdates())
{
new SqlExecutor(tempSchema).execute(indexSql);
tempSchema.getSqlDialect().updateStatistics(tempTableInfo); // Immediately analyze the newly populated & indexed table
}

TempTableInfo cacheEntry = tempTableInfo;

// Don't bother caching if we're in a transaction
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/data/URLDisplayColumn.java
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public Renderable createThumbnailImage()
String thumbnailUrl = _fileIconUrl != null ? ensureAbsoluteUrl(_ctx, _fileIconUrl) : ensureAbsoluteUrl(_ctx, _url);

return IMG(at(
style, "display:block; vertical-align:middle;" + (renderSize ? (_thumbnailWidth != null ? " width:" + _thumbnailWidth : " max-width:32px" + "; ") + (_thumbnailHeight != null ? " height:" + _thumbnailHeight : " height:auto" + ";") : ""),
style, "display:block; vertical-align:middle;" + (renderSize ? (_thumbnailWidth != null ? " width:" + _thumbnailWidth : " max-width:32px") + "; " + (_thumbnailHeight != null ? " height:" + _thumbnailHeight : " height:auto") + ";" : ""),
src, thumbnailUrl, title, _displayName
));
}
Expand Down
Loading