Skip to content

Auditing plus fixes for terms-of-use acceptances #7991

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 1 commit into
base: develop
Choose a base branch
from
+52 −26
Open

Auditing plus fixes for terms-of-use acceptances #7991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
50 changes: 35 additions & 15 deletions api/src/org/labkey/api/security/WikiTermsOfUseProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public static boolean isTermsOfUseRequired(ViewContext ctx, @Nullable Project pr
// termsContainer is guaranteed to have a terms-of-use wiki
public static boolean isTermsOfUseApproved(ViewContext ctx, @NotNull Container termsContainer)
{
HttpSession session = ctx.getRequest().getSession(false);
HttpSession session = ctx.getRequestOrThrow().getSession(false);
if (null == session)
return false;
boolean approved;
Expand All @@ -114,6 +114,9 @@ public static boolean isTermsOfUseApproved(ViewContext ctx, @NotNull Container t
int frequencySeconds = AppProps.getInstance().getTermsOfUseFrequencySeconds();
if (frequencySeconds > 0)
{
// Look up the LAST_TERMS_ACCEPTANCE dates of the impersonator
if (user.isImpersonated())
user = user.getImpersonatingUser();
String isoDateString = PropertyManager.getProperties(user, termsContainer, LAST_TERMS_ACCEPTANCE).get(DATE);
if (isoDateString != null)
{
Expand All @@ -127,7 +130,7 @@ public static boolean isTermsOfUseApproved(ViewContext ctx, @NotNull Container t
{
try (var ignored = SpringActionController.ignoreSqlUpdates())
{
setTermsOfUseApprovedInSession(ctx, termsContainer);
setTermsOfUseApprovedInSession(ctx, termsContainer, ctx.getUser());
}
}
}
Expand Down Expand Up @@ -203,29 +206,46 @@ public static TermsOfUse getTermsOfUse(@Nullable Project project)

public static void setTermsOfUseApproved(ViewContext ctx, @NotNull Container termsContainer)
{
setTermsOfUseApprovedInSession(ctx, termsContainer);
User user = ctx.getUser();
if (!user.isGuest() && AppProps.getInstance().getTermsOfUseFrequencySeconds() > 0)
setTermsOfUseApproved(ctx, termsContainer, ctx.getUser());
}

// Callers mid-authentication (e.g., LoginApiAction) can't rely on ctx.getUser(), since the ViewContext's user
// isn't updated until after this request completes, and the fully-authenticated User may not exist yet if
// secondary authentication is pending. Resolve and pass the accepting user explicitly instead.
public static void setTermsOfUseApproved(ViewContext ctx, @NotNull Container termsContainer, @Nullable User user)
{
setTermsOfUseApprovedInSession(ctx, termsContainer, user);
if (null != user && !user.isGuest())
{
if (user.isImpersonated())
user = user.getImpersonatingUser();
WritablePropertyMap map = PropertyManager.getWritableProperties(user, termsContainer, LAST_TERMS_ACCEPTANCE, true);
map.put(DATE, Instant.now().toString());
map.save();
LOG.debug("Saving terms acceptance timestamp for {} in {}", user, termsContainer);
// If impersonating, terms-of-use audit event gets logged as accept by impersonated user while being
// impersonated by impersonator
UserManager.addAuditEvent(user, termsContainer, user, "Agreed to terms of use");
if (AppProps.getInstance().getTermsOfUseFrequencySeconds() > 0)
{
// Unlike the audit entry above (which will clearly show if the user was being impersonated), we want
// to attribute LAST_TERMS_ACCEPTANCE dates to the impersonator.
if (user.isImpersonated())
user = user.getImpersonatingUser();
WritablePropertyMap map = PropertyManager.getWritableProperties(user, termsContainer, LAST_TERMS_ACCEPTANCE, true);
map.put(DATE, Instant.now().toString());
map.save();
LOG.debug("Saving terms acceptance timestamp for {} in {}", user, termsContainer);
}
}
}

private static void setTermsOfUseApprovedInSession(ViewContext ctx, @NotNull Container termsContainer)
// Same ctx.getUser() caveat as setTermsOfUseApproved(): callers mid-authentication must pass the accepting user
// explicitly, since ctx.getUser() won't reflect this request's authentication and may be Guest or unavailable.
private static void setTermsOfUseApprovedInSession(ViewContext ctx, @NotNull Container termsContainer, @Nullable User user)
{
HttpSession session = ctx.getRequest().getSession(true);
HttpSession session = ctx.getRequestOrThrow().getSession(true);
synchronized (SessionHelper.getSessionLock(session))
{
Set<Container> termsApproved = getApprovedTerms(session);
termsApproved.add(termsContainer);
}
User user = ctx.getUser();
LOG.debug("Stashing terms acceptance in session for {} in {}", user + (user.isImpersonated() ? " (impersonated by " + user.getImpersonatingUser() + ")" : ""), termsContainer);
String userDescription = null != user ? user + (user.isImpersonated() ? " (impersonated by " + user.getImpersonatingUser() + ")" : "") : "<no user>";
LOG.debug("Stashing terms acceptance in session for {} in {}", userDescription, termsContainer);
}

public enum TermsOfUseType implements SafeToRenderEnum
Expand Down
9 changes: 2 additions & 7 deletions api/src/org/labkey/api/util/Rate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@

import java.util.concurrent.TimeUnit;

/**
* User: adam
* Date: 6/11/13
* Time: 10:05 AM
*/
public class Rate
{
private final double _rate;
Expand All @@ -45,9 +40,9 @@ public Rate(long count, long duration, TimeUnit unit)
_rate = (double)count / (double)unit.toMillis(duration);

if (duration == 1)
_toString = "" + count + "/" + StringUtils.stripEnd(unit.toString(), "S");
_toString = count + "/" + StringUtils.stripEnd(unit.toString(), "S");
else
_toString = "" + count + "/(" + duration + " " + unit + ")";
_toString = count + "/(" + duration + " " + unit + ")";
}

// Count per millisecond
Expand Down
19 changes: 15 additions & 4 deletions core/src/org/labkey/core/login/LoginController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,10 +712,21 @@ public Object execute(LoginForm form, BindException errors)

if (!form.isForceReauth() && form.isApprovedTermsOfUse())
{
// Pass in the user to setTermsOfUseApproved(), since the ViewContext doesn't have the authenticated
// user yet. Also, if MFA is configured, authResult user will be null; pull user from primary
// result in that case.
User acceptingUser = user;
if (null == acceptingUser)
{
PrimaryAuthenticationResult primaryResult = AuthenticationManager.getPrimaryAuthenticationResult(request.getSession(true));
if (primaryResult != null)
acceptingUser = primaryResult.getUser();
}

if (form.getTermsOfUseType() == TermsOfUseType.PROJECT_LEVEL)
WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), WikiTermsOfUseProvider.getTermsContainer(termsProject));
WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), WikiTermsOfUseProvider.getTermsContainer(termsProject), acceptingUser);
else if (form.getTermsOfUseType() == TermsOfUseType.SITE_WIDE)
WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), ContainerManager.getRoot());
WikiTermsOfUseProvider.setTermsOfUseApproved(getViewContext(), ContainerManager.getRoot(), acceptingUser);
response.put("approvedTermsOfUse", true);
}

Expand All @@ -732,8 +743,8 @@ else if (form.getTermsOfUseType() == TermsOfUseType.SITE_WIDE)
}
else
{
// AuthenticationResult returned by AuthenticationManager.handleAuthentication indicated that a secondary authentication is needed
// in the ajax response inform js handler to load page from secondary authenticator url
// AuthenticationResult indicated that a secondary authentication is needed. Inform js handler to
// load page from secondary authenticator url
response.put(ActionURL.Param.returnUrl.name(), redirectString);
}
}
Expand Down